Skip to content

Commit c05a0c6

Browse files
ZylphrexZylphrex
authored
ref(explore): Visualize should only have 1 y axis (#93938)
Previously, explore supported multiple y axis per chart, so each visualize supported multiple y axis. That functionality has since been removed for simplicity so update the types here to match. Keep in mind that saved queries still store them as an array so when serializing/deserializing, we still need to treat it as an array.
1 parent 786861a commit c05a0c6

21 files changed

+170
-225
lines changed

‎static/app/views/explore/charts/index.tsx‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {t} from 'sentry/locale';
1010
import {space} from 'sentry/styles/space';
1111
import type {Confidence} from 'sentry/types/organization';
1212
import {defined} from 'sentry/utils';
13-
import {dedupeArray} from 'sentry/utils/dedupeArray';
1413
import {parseFunction, prettifyParsedFunction} from 'sentry/utils/discover/fields';
1514
import type {QueryError} from 'sentry/utils/discover/genericDiscoverQuery';
1615
import {isTimeSeriesOther} from 'sentry/utils/timeSeries/isTimeSeriesOther';
@@ -151,8 +150,8 @@ export function ExploreCharts({
151150
);
152151

153152
const getChartInfo = useCallback(
154-
(yAxes: readonly string[]) => {
155-
const dedupedYAxes = dedupeArray(yAxes);
153+
(yAxis: string) => {
154+
const dedupedYAxes = [yAxis];
156155

157156
const formattedYAxes = dedupedYAxes.map(yaxis => {
158157
const func = parseFunction(yaxis);
@@ -197,7 +196,7 @@ export function ExploreCharts({
197196
sampleCount,
198197
isSampled,
199198
dataScanned,
200-
} = getChartInfo(visualize.yAxes);
199+
} = getChartInfo(visualize.yAxis);
201200

202201
let overrideSampleCount = undefined;
203202
let overrideIsSampled = undefined;
@@ -208,7 +207,7 @@ export function ExploreCharts({
208207
// When this happens, we override it with the sampling meta
209208
// data from the DEFAULT_VISUALIZATION.
210209
if (sampleCount === 0 && !defined(isSampled)) {
211-
const chartInfo = getChartInfo([DEFAULT_VISUALIZATION]);
210+
const chartInfo = getChartInfo(DEFAULT_VISUALIZATION);
212211
overrideSampleCount = chartInfo.sampleCount;
213212
overrideIsSampled = chartInfo.isSampled;
214213
overrideDataScanned = chartInfo.dataScanned;
@@ -224,7 +223,7 @@ export function ExploreCharts({
224223
chartType: visualize.chartType,
225224
stack: visualize.stack,
226225
label: shouldRenderLabel ? visualize.label : undefined,
227-
yAxes: visualize.yAxes,
226+
yAxes: [visualize.yAxis],
228227
formattedYAxes,
229228
data,
230229
error,

‎static/app/views/explore/contexts/pageParamsContext/aggregateFields.tsx‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export interface GroupBy {
1818
groupBy: string;
1919
}
2020

21-
function isBaseVisualize(value: any): value is BaseVisualize {
21+
export function isBaseVisualize(value: any): value is BaseVisualize {
2222
return (
2323
typeof value === 'object' &&
2424
Array.isArray(value.yAxes) &&
@@ -32,7 +32,7 @@ export function isGroupBy(value: any): value is GroupBy {
3232
}
3333

3434
export function isVisualize(value: any): value is Visualize {
35-
return typeof value === 'object' && 'yAxes' in value && Array.isArray(value.yAxes);
35+
return typeof value === 'object' && 'yAxis' in value && typeof value.yAxis === 'string';
3636
}
3737

3838
export type BaseAggregateField = GroupBy | BaseVisualize;
@@ -80,7 +80,7 @@ export function getAggregateFieldsFromLocation(
8080
} else if (isBaseVisualize(groupByOrBaseVisualize)) {
8181
for (const yAxis of groupByOrBaseVisualize.yAxes) {
8282
aggregateFields.push(
83-
new Visualize([yAxis], {
83+
new Visualize(yAxis, {
8484
label: String.fromCharCode(65 + i), // starts from 'A',
8585
chartType: groupByOrBaseVisualize.chartType,
8686
})
@@ -111,11 +111,12 @@ export function updateLocationWithAggregateFields(
111111
aggregateFields: Array<GroupBy | BaseVisualize> | null | undefined
112112
) {
113113
if (defined(aggregateFields)) {
114-
location.query.aggregateField = aggregateFields.map(aggregateField => {
114+
location.query.aggregateField = aggregateFields.flatMap(aggregateField => {
115115
if (isBaseVisualize(aggregateField)) {
116-
return JSON.stringify(Visualize.fromJSON(aggregateField).toJSON());
116+
const visualizes = Visualize.fromJSON(aggregateField);
117+
return visualizes.map(visualize => JSON.stringify(visualize.toJSON()));
117118
}
118-
return JSON.stringify(aggregateField);
119+
return [JSON.stringify(aggregateField)];
119120
});
120121
} else if (aggregateFields === null) {
121122
delete location.query.aggregateField;

‎static/app/views/explore/contexts/pageParamsContext/index.spec.tsx‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ describe('PageParamsProvider', function () {
115115
sortBys: [{field: 'timestamp', kind: 'desc'}],
116116
aggregateFields: [
117117
{groupBy: ''},
118-
new Visualize(['count(span.duration)'], {label: 'A'}),
118+
new Visualize('count(span.duration)', {label: 'A'}),
119119
],
120120
})
121121
);
@@ -135,7 +135,7 @@ describe('PageParamsProvider', function () {
135135
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
136136
aggregateFields: [
137137
{groupBy: 'span.op'},
138-
new Visualize(['count(span.self_time)'], {
138+
new Visualize('count(span.self_time)', {
139139
label: 'A',
140140
chartType: ChartType.AREA,
141141
}),
@@ -158,7 +158,7 @@ describe('PageParamsProvider', function () {
158158
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
159159
aggregateFields: [
160160
{groupBy: 'browser.name'},
161-
new Visualize(['count(span.self_time)'], {
161+
new Visualize('count(span.self_time)', {
162162
label: 'A',
163163
chartType: ChartType.AREA,
164164
}),
@@ -181,7 +181,7 @@ describe('PageParamsProvider', function () {
181181
query: '',
182182
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
183183
aggregateFields: [
184-
new Visualize(['count(span.self_time)'], {
184+
new Visualize('count(span.self_time)', {
185185
label: 'A',
186186
chartType: ChartType.AREA,
187187
}),
@@ -205,7 +205,7 @@ describe('PageParamsProvider', function () {
205205
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
206206
aggregateFields: [
207207
{groupBy: ''},
208-
new Visualize(['count(span.self_time)'], {
208+
new Visualize('count(span.self_time)', {
209209
label: 'A',
210210
chartType: ChartType.AREA,
211211
}),
@@ -228,7 +228,7 @@ describe('PageParamsProvider', function () {
228228
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
229229
aggregateFields: [
230230
{groupBy: 'span.op'},
231-
new Visualize(['count(span.self_time)'], {
231+
new Visualize('count(span.self_time)', {
232232
label: 'A',
233233
chartType: ChartType.AREA,
234234
}),
@@ -262,7 +262,7 @@ describe('PageParamsProvider', function () {
262262
sortBys: [{field: 'timestamp', kind: 'desc'}],
263263
aggregateFields: [
264264
{groupBy: ''},
265-
new Visualize(['count(span.self_time)'], {
265+
new Visualize('count(span.self_time)', {
266266
label: 'A',
267267
chartType: ChartType.AREA,
268268
}),
@@ -310,7 +310,7 @@ describe('PageParamsProvider', function () {
310310
{groupBy: 'sdk.version'},
311311
{groupBy: 'span.op'},
312312
{groupBy: ''},
313-
new Visualize(['count(span.self_time)'], {
313+
new Visualize('count(span.self_time)', {
314314
label: 'A',
315315
chartType: ChartType.AREA,
316316
}),
@@ -333,7 +333,7 @@ describe('PageParamsProvider', function () {
333333
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
334334
aggregateFields: [
335335
{groupBy: 'span.op'},
336-
new Visualize(['count(span.self_time)'], {
336+
new Visualize('count(span.self_time)', {
337337
label: 'A',
338338
chartType: ChartType.AREA,
339339
}),
@@ -356,7 +356,7 @@ describe('PageParamsProvider', function () {
356356
sortBys: [{field: 'id', kind: 'desc'}],
357357
aggregateFields: [
358358
{groupBy: 'span.op'},
359-
new Visualize(['count(span.self_time)'], {
359+
new Visualize('count(span.self_time)', {
360360
label: 'A',
361361
chartType: ChartType.AREA,
362362
}),
@@ -379,7 +379,7 @@ describe('PageParamsProvider', function () {
379379
sortBys: [{field: 'timestamp', kind: 'desc'}],
380380
aggregateFields: [
381381
{groupBy: 'span.op'},
382-
new Visualize(['count(span.self_time)'], {
382+
new Visualize('count(span.self_time)', {
383383
label: 'A',
384384
chartType: ChartType.AREA,
385385
}),
@@ -412,11 +412,11 @@ describe('PageParamsProvider', function () {
412412
sortBys: [{field: 'max(span.duration)', kind: 'desc'}],
413413
aggregateFields: [
414414
{groupBy: 'span.op'},
415-
new Visualize(['min(span.self_time)'], {
415+
new Visualize('min(span.self_time)', {
416416
label: 'A',
417417
chartType: ChartType.AREA,
418418
}),
419-
new Visualize(['max(span.duration)'], {
419+
new Visualize('max(span.duration)', {
420420
label: 'B',
421421
chartType: ChartType.AREA,
422422
}),
@@ -449,11 +449,11 @@ describe('PageParamsProvider', function () {
449449
sortBys: [{field: 'min(span.self_time)', kind: 'desc'}],
450450
aggregateFields: [
451451
{groupBy: 'span.op'},
452-
new Visualize(['min(span.self_time)'], {
452+
new Visualize('min(span.self_time)', {
453453
label: 'A',
454454
chartType: ChartType.AREA,
455455
}),
456-
new Visualize(['max(span.duration)'], {
456+
new Visualize('max(span.duration)', {
457457
label: 'B',
458458
chartType: ChartType.AREA,
459459
}),
@@ -486,7 +486,7 @@ describe('PageParamsProvider', function () {
486486
sortBys: [{field: 'sdk.name', kind: 'desc'}],
487487
aggregateFields: [
488488
{groupBy: 'sdk.name'},
489-
new Visualize(['count(span.self_time)'], {
489+
new Visualize('count(span.self_time)', {
490490
label: 'A',
491491
chartType: ChartType.AREA,
492492
}),
@@ -519,7 +519,7 @@ describe('PageParamsProvider', function () {
519519
sortBys: [{field: 'count(span.self_time)', kind: 'desc'}],
520520
aggregateFields: [
521521
{groupBy: 'sdk.name'},
522-
new Visualize(['count(span.self_time)'], {
522+
new Visualize('count(span.self_time)', {
523523
label: 'A',
524524
chartType: ChartType.AREA,
525525
}),
@@ -542,7 +542,7 @@ describe('PageParamsProvider', function () {
542542
sortBys: [{field: 'count(span.duration)', kind: 'desc'}],
543543
aggregateFields: [
544544
{groupBy: 'span.op'},
545-
new Visualize(['count(span.duration)'], {label: 'A'}),
545+
new Visualize('count(span.duration)', {label: 'A'}),
546546
],
547547
})
548548
);
@@ -573,15 +573,15 @@ describe('PageParamsProvider', function () {
573573
sortBys: [{field: 'count(span.self_time)', kind: 'asc'}],
574574
aggregateFields: [
575575
{groupBy: 'span.op'},
576-
new Visualize(['count(span.self_time)'], {
576+
new Visualize('count(span.self_time)', {
577577
label: 'A',
578578
chartType: ChartType.AREA,
579579
}),
580-
new Visualize(['avg(span.duration)'], {
580+
new Visualize('avg(span.duration)', {
581581
label: 'B',
582582
chartType: ChartType.LINE,
583583
}),
584-
new Visualize(['avg(span.self_time)'], {
584+
new Visualize('avg(span.self_time)', {
585585
label: 'C',
586586
chartType: ChartType.LINE,
587587
}),

‎static/app/views/explore/contexts/pageParamsContext/index.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import type {AggregateField, BaseAggregateField, GroupBy} from './aggregateField
2626
import {
2727
defaultAggregateFields,
2828
getAggregateFieldsFromLocation,
29+
isBaseVisualize,
2930
isGroupBy,
3031
isVisualize,
3132
updateLocationWithAggregateFields,
@@ -121,7 +122,7 @@ function defaultPageParams(): ReadablePageParams {
121122
const sortBys = defaultSortBys(
122123
mode,
123124
fields,
124-
aggregateFields.filter(isVisualize).flatMap(visualize => visualize.yAxes)
125+
aggregateFields.filter(isVisualize).map(visualize => visualize.yAxis)
125126
);
126127

127128
return new ReadablePageParams({
@@ -357,7 +358,7 @@ function findAllFieldRefs(
357358

358359
const readableVisualizeFields = readablePageParams.aggregateFields
359360
.filter<Visualize>(isVisualize)
360-
.flatMap(visualize => visualize.yAxes)
361+
.map(visualize => visualize.yAxis)
361362
.map(yAxis => parseFunction(yAxis)?.arguments?.[0])
362363
.filter<string>(defined);
363364

@@ -371,7 +372,7 @@ function findAllFieldRefs(
371372
writablePageParams.aggregateFields === null
372373
? []
373374
: writablePageParams.aggregateFields
374-
?.filter<Visualize>(isVisualize)
375+
?.filter<BaseVisualize>(isBaseVisualize)
375376
?.flatMap(visualize => visualize.yAxes)
376377
?.map(yAxis => parseFunction(yAxis)?.arguments?.[0])
377378
?.filter<string>(defined);

‎static/app/views/explore/contexts/pageParamsContext/sortBys.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export function getSortBysFromLocation(
6262
sortBys.every(
6363
sortBy =>
6464
groupBys.includes(sortBy.field) ||
65-
visualizes.some(visualize => visualize.yAxes.includes(sortBy.field))
65+
visualizes.some(visualize => visualize.yAxis === sortBy.field)
6666
)
6767
) {
6868
return sortBys;
@@ -72,7 +72,7 @@ export function getSortBysFromLocation(
7272
return defaultSortBys(
7373
mode,
7474
fields,
75-
visualizes.flatMap(visualize => visualize.yAxes)
75+
visualizes.map(visualize => visualize.yAxis)
7676
);
7777
}
7878

0 commit comments

Comments
 (0)