Skip to content

Commit 5407932

Browse files
sadpandajoeclaude
andcommitted
refactor(table-chart): add comments and edge case handling for sanitizeHeaderId
Addresses additional code review feedback from PR #35968: - Add explanatory comments documenting why semantic replacements come first (preserves meaning in IDs: 'percentpct_nice' vs '_pct_nice') - Add leading/trailing underscore trimming to handle edge cases (e.g., '@col' → 'col' instead of '_col') - Add comprehensive edge case tests (4 new test blocks, 19 additional assertions) covering leading/trailing underscores and special-character-only inputs Tests: 45 total (13 unit + 2 integration + 30 existing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f680611 commit 5407932

2 files changed

Lines changed: 47 additions & 7 deletions

File tree

superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,19 @@ function cellWidth({
151151
* Exported for testing.
152152
*/
153153
export function sanitizeHeaderId(columnId: string): string {
154-
return columnId
155-
.replace(/%/g, 'percent')
156-
.replace(/#/g, 'hash')
157-
.replace(//g, 'delta')
158-
.replace(/\s+/g, '_')
159-
.replace(/[^a-zA-Z0-9_-]/g, '_')
160-
.replace(/_+/g, '_'); // Collapse consecutive underscores
154+
return (
155+
columnId
156+
// Semantic replacements first: preserve meaning in IDs for readability
157+
// (e.g., 'percentpct_nice' instead of '_pct_nice')
158+
.replace(/%/g, 'percent')
159+
.replace(/#/g, 'hash')
160+
.replace(//g, 'delta')
161+
// Generic sanitization for remaining special characters
162+
.replace(/\s+/g, '_')
163+
.replace(/[^a-zA-Z0-9_-]/g, '_')
164+
.replace(/_+/g, '_') // Collapse consecutive underscores
165+
.replace(/^_+|_+$/g, '') // Trim leading/trailing underscores
166+
);
161167
}
162168

163169
/**

superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,40 @@ test('sanitizeHeaderId should collapse consecutive underscores', () => {
6868
expect(sanitizeHeaderId('test@@name')).toBe('test_name');
6969
});
7070

71+
test('sanitizeHeaderId should remove leading underscores', () => {
72+
expect(sanitizeHeaderId('@col')).toBe('col');
73+
expect(sanitizeHeaderId('!revenue')).toBe('revenue');
74+
expect(sanitizeHeaderId('@@test')).toBe('test');
75+
expect(sanitizeHeaderId(' leading_spaces')).toBe('leading_spaces');
76+
});
77+
78+
test('sanitizeHeaderId should remove trailing underscores', () => {
79+
expect(sanitizeHeaderId('col@')).toBe('col');
80+
expect(sanitizeHeaderId('revenue!')).toBe('revenue');
81+
expect(sanitizeHeaderId('test@@')).toBe('test');
82+
expect(sanitizeHeaderId('trailing_spaces ')).toBe('trailing_spaces');
83+
});
84+
85+
test('sanitizeHeaderId should remove leading and trailing underscores', () => {
86+
expect(sanitizeHeaderId('@col@')).toBe('col');
87+
expect(sanitizeHeaderId('!test!')).toBe('test');
88+
expect(sanitizeHeaderId(' spaced ')).toBe('spaced');
89+
expect(sanitizeHeaderId('@@multiple@@')).toBe('multiple');
90+
});
91+
92+
test('sanitizeHeaderId should handle inputs with only special characters', () => {
93+
expect(sanitizeHeaderId('@')).toBe('');
94+
expect(sanitizeHeaderId('@@')).toBe('');
95+
expect(sanitizeHeaderId(' ')).toBe('');
96+
expect(sanitizeHeaderId('!@$')).toBe('');
97+
expect(sanitizeHeaderId('!@#$')).toBe('hash'); // # → 'hash' is preserved (semantic replacement)
98+
// Semantic replacements should be preserved even when alone
99+
expect(sanitizeHeaderId('%')).toBe('percent');
100+
expect(sanitizeHeaderId('#')).toBe('hash');
101+
expect(sanitizeHeaderId('△')).toBe('delta');
102+
expect(sanitizeHeaderId('% # △')).toBe('percent_hash_delta');
103+
});
104+
71105
describe('plugin-chart-table', () => {
72106
describe('transformProps', () => {
73107
it('should parse pageLength to pageSize', () => {

0 commit comments

Comments
 (0)