Skip to content

Commit 39d9da9

Browse files
shruthilayajpriscilawebdev
authored andcommitted
feat(discover): Show banner for split decision (#73740)
1. Show banner for split decision 2. Use saved query dataset to decide which actual dataset to query 3. Switching datasets preserves old state and only switches dataset instead of resetting the whole query when already in a saved query - it's a better experience if we guess the wrong dataset, the state remains intact as the switch between datasets 4. Removes references to queryDataset in EventView to keep event dataset selector agnostic
1 parent 3da0795 commit 39d9da9

File tree

13 files changed

+662
-74
lines changed

13 files changed

+662
-74
lines changed

static/app/utils/discover/eventView.spec.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
DiscoverDatasets,
1919
DISPLAY_MODE_OPTIONS,
2020
DisplayModes,
21+
SavedQueryDatasets,
2122
} from 'sentry/utils/discover/types';
2223
import {AggregationKey, WebVital} from 'sentry/utils/fields';
2324
import {SpanOperationBreakdownFilter} from 'sentry/views/performance/transactionSummary/filter';
@@ -1694,6 +1695,7 @@ describe('EventView.toNewQuery()', function () {
16941695
environment: ['staging'],
16951696
display: 'releases',
16961697
dataset: DiscoverDatasets.DISCOVER,
1698+
queryDataset: SavedQueryDatasets.DISCOVER,
16971699
};
16981700

16991701
expect(output).toEqual(expected);
@@ -1724,6 +1726,7 @@ describe('EventView.toNewQuery()', function () {
17241726
environment: ['staging'],
17251727
display: 'releases',
17261728
dataset: DiscoverDatasets.DISCOVER,
1729+
queryDataset: SavedQueryDatasets.DISCOVER,
17271730
};
17281731

17291732
expect(output).toEqual(expected);
@@ -1754,6 +1757,7 @@ describe('EventView.toNewQuery()', function () {
17541757
environment: ['staging'],
17551758
display: 'releases',
17561759
dataset: DiscoverDatasets.DISCOVER,
1760+
queryDataset: SavedQueryDatasets.DISCOVER,
17571761
};
17581762

17591763
expect(output).toEqual(expected);

static/app/utils/discover/eventView.tsx

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ import {
3838
DISPLAY_MODE_FALLBACK_OPTIONS,
3939
DISPLAY_MODE_OPTIONS,
4040
DisplayModes,
41-
SavedQueryDatasets,
4241
TOP_N,
4342
} from 'sentry/utils/discover/types';
4443
import {statsPeriodToDays} from 'sentry/utils/duration/statsPeriodToDays';
4544
import {decodeList, decodeScalar, decodeSorts} from 'sentry/utils/queryString';
4645
import {normalizeUrl} from 'sentry/utils/withDomainRequired';
46+
import {getSavedQueryDatasetFromLocationOrDataset} from 'sentry/views/discover/savedQuery/utils';
4747
import type {TableColumn, TableColumnSort} from 'sentry/views/discover/table/types';
4848
import {FieldValueKind} from 'sentry/views/discover/table/types';
4949
import {decodeColumnOrder} from 'sentry/views/discover/utils';
@@ -258,7 +258,6 @@ export type EventViewOptions = {
258258
dataset?: DiscoverDatasets;
259259
expired?: boolean;
260260
interval?: string;
261-
queryDataset?: SavedQueryDatasets;
262261
utc?: string | boolean | undefined;
263262
yAxis?: string | string[] | undefined;
264263
};
@@ -284,7 +283,6 @@ class EventView {
284283
createdBy: User | undefined;
285284
additionalConditions: MutableSearch; // This allows views to always add additional conditions to the query to get specific data. It should not show up in the UI unless explicitly called.
286285
dataset?: DiscoverDatasets;
287-
queryDataset?: SavedQueryDatasets;
288286

289287
constructor(props: EventViewOptions) {
290288
const fields: Field[] = Array.isArray(props.fields) ? props.fields : [];
@@ -329,7 +327,6 @@ class EventView {
329327
this.environment = environment;
330328
this.yAxis = props.yAxis;
331329
this.dataset = props.dataset;
332-
this.queryDataset = props.queryDataset;
333330
this.display = props.display;
334331
this.topEvents = props.topEvents;
335332
this.interval = props.interval;
@@ -362,7 +359,6 @@ class EventView {
362359
createdBy: undefined,
363360
additionalConditions: new MutableSearch([]),
364361
dataset: decodeScalar(location.query.dataset) as DiscoverDatasets,
365-
queryDataset: decodeScalar(location.query.queryDataset) as SavedQueryDatasets,
366362
});
367363
}
368364

@@ -454,7 +450,6 @@ class EventView {
454450
expired: saved.expired,
455451
additionalConditions: new MutableSearch([]),
456452
dataset: saved.dataset,
457-
queryDataset: saved.queryDataset,
458453
});
459454
}
460455

@@ -523,9 +518,6 @@ class EventView {
523518
utc,
524519
dataset:
525520
(decodeScalar(location.query.dataset) as DiscoverDatasets) ?? saved.dataset,
526-
queryDataset:
527-
(decodeScalar(location.query.queryDataset) as SavedQueryDatasets) ??
528-
saved.queryDataset,
529521
});
530522
}
531523
return EventView.fromLocation(location);
@@ -546,7 +538,6 @@ class EventView {
546538
display: DisplayModes.DEFAULT,
547539
topEvents: '5',
548540
dataset: DiscoverDatasets.DISCOVER,
549-
queryDataset: SavedQueryDatasets.DISCOVER,
550541
};
551542
const keys = Object.keys(defaults).filter(key => !omitList.includes(key));
552543
for (const key of keys) {
@@ -597,7 +588,6 @@ class EventView {
597588
environment: this.environment,
598589
yAxis: typeof this.yAxis === 'string' ? [this.yAxis] : this.yAxis,
599590
dataset: this.dataset,
600-
queryDataset: this.queryDataset,
601591
display: this.display,
602592
topEvents: this.topEvents,
603593
interval: this.interval,
@@ -609,6 +599,13 @@ class EventView {
609599
delete newQuery.query;
610600
}
611601

602+
if (this.dataset) {
603+
newQuery.queryDataset = getSavedQueryDatasetFromLocationOrDataset(
604+
undefined,
605+
this.dataset
606+
);
607+
}
608+
612609
return newQuery;
613610
}
614611

@@ -684,7 +681,6 @@ class EventView {
684681
query: this.query,
685682
yAxis: this.yAxis || this.getYAxis(),
686683
dataset: this.dataset,
687-
queryDataset: this.queryDataset,
688684
display: this.display,
689685
topEvents: this.topEvents,
690686
interval: this.interval,
@@ -776,7 +772,6 @@ class EventView {
776772
environment: this.environment,
777773
yAxis: this.yAxis,
778774
dataset: this.dataset,
779-
queryDataset: this.queryDataset,
780775
display: this.display,
781776
topEvents: this.topEvents,
782777
interval: this.interval,
@@ -793,6 +788,12 @@ class EventView {
793788

794789
return newEventView;
795790
}
791+
withDataset(dataset?: DiscoverDatasets): EventView {
792+
const newEventView = this.clone();
793+
newEventView.dataset = dataset;
794+
795+
return newEventView;
796+
}
796797

797798
withColumns(columns: Column[]): EventView {
798799
const newEventView = this.clone();

static/app/views/discover/eventDetails/index.spec.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -212,19 +212,19 @@ describe('Discover > EventDetails', function () {
212212
expect(await screen.findByText('Firefox')).toBeInTheDocument();
213213
expect(screen.getByRole('link', {name: 'Firefox'})).toHaveAttribute(
214214
'href',
215-
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=browser%3AFirefox%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
215+
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=browser%3AFirefox%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
216216
);
217217

218218
// Get the second link
219219
expect(screen.getByRole('link', {name: 'test-uuid'})).toHaveAttribute(
220220
'href',
221-
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=tags%5Bdevice.uuid%5D%3Atest-uuid%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
221+
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=tags%5Bdevice.uuid%5D%3Atest-uuid%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
222222
);
223223

224224
// Get the third link
225225
expect(screen.getByRole('link', {name: '82ebf297206a'})).toHaveAttribute(
226226
'href',
227-
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=release%3A82ebf297206a%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
227+
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=release%3A82ebf297206a%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
228228
);
229229
});
230230

@@ -252,19 +252,19 @@ describe('Discover > EventDetails', function () {
252252
expect(await screen.findByText('Firefox')).toBeInTheDocument();
253253
expect(screen.getByRole('link', {name: 'Firefox'})).toHaveAttribute(
254254
'href',
255-
'/organizations/org-slug/discover/homepage/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=browser%3AFirefox%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
255+
'/organizations/org-slug/discover/homepage/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=browser%3AFirefox%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
256256
);
257257

258258
// Get the second link
259259
expect(screen.getByRole('link', {name: 'test-uuid'})).toHaveAttribute(
260260
'href',
261-
'/organizations/org-slug/discover/homepage/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=tags%5Bdevice.uuid%5D%3Atest-uuid%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
261+
'/organizations/org-slug/discover/homepage/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=tags%5Bdevice.uuid%5D%3Atest-uuid%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
262262
);
263263

264264
// Get the third link
265265
expect(screen.getByRole('link', {name: '82ebf297206a'})).toHaveAttribute(
266266
'href',
267-
'/organizations/org-slug/discover/homepage/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=release%3A82ebf297206a%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
267+
'/organizations/org-slug/discover/homepage/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=release%3A82ebf297206a%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
268268
);
269269
});
270270

@@ -295,18 +295,18 @@ describe('Discover > EventDetails', function () {
295295
expect(await screen.findByText('Firefox')).toBeInTheDocument();
296296
expect(screen.getByRole('link', {name: 'Firefox'})).toHaveAttribute(
297297
'href',
298-
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=Dumpster%20browser%3AFirefox%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
298+
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=Dumpster%20browser%3AFirefox%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
299299
);
300300

301301
// Get the second link
302302
expect(screen.getByRole('link', {name: 'test-uuid'})).toHaveAttribute(
303303
'href',
304-
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=Dumpster%20tags%5Bdevice.uuid%5D%3Atest-uuid%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
304+
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=Dumpster%20tags%5Bdevice.uuid%5D%3Atest-uuid%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
305305
);
306306
// Get the third link
307307
expect(screen.getByRole('link', {name: '82ebf297206a'})).toHaveAttribute(
308308
'href',
309-
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=Dumpster%20release%3A82ebf297206a%20title%3A%22Oh%20no%20something%20bad%22&queryDataset=discover&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
309+
'/organizations/org-slug/discover/results/?field=title&field=event.type&field=project&field=user.display&field=timestamp&name=All%20Events&query=Dumpster%20release%3A82ebf297206a%20title%3A%22Oh%20no%20something%20bad%22&sort=-timestamp&statsPeriod=24h&yAxis=count%28%29'
310310
);
311311
});
312312

0 commit comments

Comments
 (0)