Skip to content

Commit a7e8608

Browse files
committed
fix(dashboards): Make toPythonString match Python's str() output for null and arrays
- Fix null handling: return 'None' instead of 'null' to match Python's str(None) - Fix array handling: add spaces after commas, wrap strings in quotes, convert null to 'None' to match Python's str([...]) - Add comprehensive tests for null values and array values with strings, numbers, and null elements These fixes ensure matchTimeSeriesToTableRow correctly matches table rows with time series groupBy values when null or array values are present, preventing breakdown legend values from showing '—' instead of the correct aggregate.
1 parent f9b8052 commit a7e8608

File tree

2 files changed

+85
-1
lines changed

2 files changed

+85
-1
lines changed

static/app/views/dashboards/widgetCard/matchTimeSeriesToTableRow.spec.tsx

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,64 @@ describe('matchTimeSeriesToTableRow', () => {
9090

9191
expect(result).toBe(42);
9292
});
93+
94+
it('matches null groupBy values correctly', () => {
95+
const result = matchTimeSeriesToTableRow({
96+
tableDataRows: [
97+
{id: '1', project: 'None', 'count()': 15},
98+
{id: '2', project: 'my-project', 'count()': 25},
99+
],
100+
timeSeries: TimeSeriesFixture({
101+
yAxis: 'count()',
102+
groupBy: [{key: 'project', value: null}],
103+
}),
104+
});
105+
106+
expect(result).toBe(15);
107+
});
108+
109+
it('matches array groupBy values with strings correctly', () => {
110+
const result = matchTimeSeriesToTableRow({
111+
tableDataRows: [
112+
{id: '1', 'error.type': "['ValueError', 'TypeError']", 'count()': 10},
113+
{id: '2', 'error.type': "['Exception']", 'count()': 5},
114+
],
115+
timeSeries: TimeSeriesFixture({
116+
yAxis: 'count()',
117+
groupBy: [{key: 'error.type', value: ['ValueError', 'TypeError']}],
118+
}),
119+
});
120+
121+
expect(result).toBe(10);
122+
});
123+
124+
it('matches array groupBy values with null elements correctly', () => {
125+
const result = matchTimeSeriesToTableRow({
126+
tableDataRows: [
127+
{id: '1', 'error.type': "['Exception', None, 'TypeError']", 'count()': 20},
128+
{id: '2', 'error.type': "['ValueError']", 'count()': 8},
129+
],
130+
timeSeries: TimeSeriesFixture({
131+
yAxis: 'count()',
132+
groupBy: [{key: 'error.type', value: ['Exception', null, 'TypeError']}],
133+
}),
134+
});
135+
136+
expect(result).toBe(20);
137+
});
138+
139+
it('matches array groupBy values with numbers correctly', () => {
140+
const result = matchTimeSeriesToTableRow({
141+
tableDataRows: [
142+
{id: '1', 'status_codes': '[200, 404]', 'count()': 30},
143+
{id: '2', 'status_codes': '[500]', 'count()': 2},
144+
],
145+
timeSeries: TimeSeriesFixture({
146+
yAxis: 'count()',
147+
groupBy: [{key: 'status_codes', value: [200, 404]}],
148+
}),
149+
});
150+
151+
expect(result).toBe(30);
152+
});
93153
});

static/app/views/dashboards/widgetCard/matchTimeSeriestoTableRow.tsx

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,36 @@ import type {TimeSeries} from 'sentry/views/dashboards/widgets/common/types';
77
* match that behavior for comparisons.
88
*/
99
function toPythonString(value: unknown): string {
10+
if (value === null) {
11+
return 'None';
12+
}
1013
if (typeof value === 'boolean') {
1114
return value ? 'True' : 'False';
1215
}
1316
if (Array.isArray(value)) {
14-
return `[${value.join(',')}]`;
17+
const elements = value.map(item => {
18+
if (item === null) {
19+
return 'None';
20+
}
21+
if (typeof item === 'string') {
22+
return `'${item}'`;
23+
}
24+
if (typeof item === 'number') {
25+
return item.toString();
26+
}
27+
// Fallback for unexpected types in array
28+
return String(item);
29+
});
30+
return `[${elements.join(', ')}]`;
31+
}
32+
if (typeof value === 'string') {
33+
return value;
34+
}
35+
if (typeof value === 'number') {
36+
return value.toString();
1537
}
38+
// Fallback for unexpected types (should never be reached with valid GroupBy values)
39+
// eslint-disable-next-line @typescript-eslint/no-base-to-string
1640
return String(value);
1741
}
1842

0 commit comments

Comments
 (0)