Skip to content

Commit f01a21c

Browse files
authored
fix(discover): Refresh discover saved queries on delete/duplicate (#96685)
just refreshes the current query this acceptance test below is testing this case, however it currently only passes because it attempts to navigate to the saved query directly after deleting (event propagation) therefore the card disappears and it passes. https://github.com/getsentry/sentry/blob/195d6990b8a694ee733a1483e0b3f7486b53a5f4/tests/acceptance/test_organization_events_v2.py#L447-L472
1 parent 47382c2 commit f01a21c

File tree

3 files changed

+29
-2
lines changed

3 files changed

+29
-2
lines changed

static/app/views/discover/landing.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ function DiscoverLanding() {
143143
error,
144144
data: savedQueries = [],
145145
getResponseHeader,
146+
refetch: refreshSavedQueries,
146147
} = useDiscoverLandingQuery(renderPrebuilt);
147148

148149
const savedQueriesPageLinks = getResponseHeader?.('Link');
@@ -254,6 +255,7 @@ function DiscoverLanding() {
254255
location={location}
255256
organization={organization}
256257
router={router}
258+
refetchSavedQueries={refreshSavedQueries}
257259
/>
258260
)}
259261
</Layout.Main>

static/app/views/discover/queryList.spec.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe('Discover > QueryList', function () {
2626
let duplicateMock: jest.Mock;
2727
let updateHomepageMock: jest.Mock;
2828
let eventsStatsMock: jest.Mock;
29+
const refetchSavedQueries = jest.fn();
2930

3031
const {router} = initializeOrg();
3132

@@ -34,6 +35,7 @@ describe('Discover > QueryList', function () {
3435
});
3536

3637
beforeEach(function () {
38+
jest.resetAllMocks();
3739
organization = OrganizationFixture({
3840
features: ['discover-basic', 'discover-query'],
3941
});
@@ -91,6 +93,7 @@ describe('Discover > QueryList', function () {
9193
pageLinks=""
9294
renderPrebuilt={false}
9395
location={location}
96+
refetchSavedQueries={refetchSavedQueries}
9497
/>,
9598
{
9699
deprecatedRouterMocks: true,
@@ -110,6 +113,7 @@ describe('Discover > QueryList', function () {
110113
renderPrebuilt
111114
pageLinks=""
112115
location={location}
116+
refetchSavedQueries={refetchSavedQueries}
113117
/>,
114118
{
115119
deprecatedRouterMocks: true,
@@ -151,6 +155,7 @@ describe('Discover > QueryList', function () {
151155
renderPrebuilt
152156
pageLinks=""
153157
location={location}
158+
refetchSavedQueries={refetchSavedQueries}
154159
/>,
155160
{
156161
router,
@@ -210,6 +215,7 @@ describe('Discover > QueryList', function () {
210215
renderPrebuilt
211216
pageLinks=""
212217
location={location}
218+
refetchSavedQueries={refetchSavedQueries}
213219
/>,
214220
{
215221
deprecatedRouterMocks: true,
@@ -248,6 +254,7 @@ describe('Discover > QueryList', function () {
248254
pageLinks=""
249255
renderPrebuilt={false}
250256
location={location}
257+
refetchSavedQueries={refetchSavedQueries}
251258
/>,
252259
{
253260
router,
@@ -282,6 +289,7 @@ describe('Discover > QueryList', function () {
282289
savedQueries={savedQueries}
283290
pageLinks=""
284291
location={location}
292+
refetchSavedQueries={refetchSavedQueries}
285293
/>,
286294
{
287295
deprecatedRouterMocks: true,
@@ -295,6 +303,7 @@ describe('Discover > QueryList', function () {
295303
await userEvent.click(withinCard.getByText('Delete Query'));
296304

297305
expect(deleteMock).toHaveBeenCalled();
306+
expect(refetchSavedQueries).toHaveBeenCalled();
298307
});
299308

300309
it('redirects to Discover on card click', async function () {
@@ -307,6 +316,7 @@ describe('Discover > QueryList', function () {
307316
pageLinks=""
308317
renderPrebuilt={false}
309318
location={location}
319+
refetchSavedQueries={refetchSavedQueries}
310320
/>,
311321
{
312322
router,
@@ -331,6 +341,7 @@ describe('Discover > QueryList', function () {
331341
renderPrebuilt={false}
332342
pageLinks=""
333343
location={location}
344+
refetchSavedQueries={refetchSavedQueries}
334345
/>,
335346
{
336347
router,
@@ -368,6 +379,7 @@ describe('Discover > QueryList', function () {
368379
pageLinks=""
369380
renderPrebuilt={false}
370381
location={location}
382+
refetchSavedQueries={refetchSavedQueries}
371383
/>,
372384
{
373385
deprecatedRouterMocks: true,
@@ -401,6 +413,7 @@ describe('Discover > QueryList', function () {
401413
pageLinks=""
402414
renderPrebuilt={false}
403415
location={location}
416+
refetchSavedQueries={refetchSavedQueries}
404417
/>,
405418
{
406419
deprecatedRouterMocks: true,
@@ -443,6 +456,7 @@ describe('Discover > QueryList', function () {
443456
pageLinks=""
444457
renderPrebuilt={false}
445458
location={location}
459+
refetchSavedQueries={refetchSavedQueries}
446460
/>,
447461
{
448462
deprecatedRouterMocks: true,
@@ -471,6 +485,7 @@ describe('Discover > QueryList', function () {
471485
renderPrebuilt={false}
472486
pageLinks=""
473487
location={location}
488+
refetchSavedQueries={refetchSavedQueries}
474489
/>,
475490
{
476491
deprecatedRouterMocks: true,
@@ -509,6 +524,7 @@ describe('Discover > QueryList', function () {
509524
pageLinks=""
510525
renderPrebuilt={false}
511526
location={location}
527+
refetchSavedQueries={refetchSavedQueries}
512528
/>
513529
);
514530

@@ -545,6 +561,7 @@ describe('Discover > QueryList', function () {
545561
pageLinks=""
546562
renderPrebuilt={false}
547563
location={location}
564+
refetchSavedQueries={refetchSavedQueries}
548565
/>
549566
);
550567

@@ -580,6 +597,7 @@ describe('Discover > QueryList', function () {
580597
]}
581598
pageLinks=""
582599
location={location}
600+
refetchSavedQueries={refetchSavedQueries}
583601
/>,
584602
{
585603
deprecatedRouterMocks: true,
@@ -647,6 +665,7 @@ describe('Discover > QueryList', function () {
647665
]}
648666
pageLinks=""
649667
location={location}
668+
refetchSavedQueries={refetchSavedQueries}
650669
/>,
651670
{
652671
deprecatedRouterMocks: true,
@@ -714,6 +733,7 @@ describe('Discover > QueryList', function () {
714733
]}
715734
pageLinks=""
716735
location={location}
736+
refetchSavedQueries={refetchSavedQueries}
717737
/>
718738
);
719739

@@ -750,6 +770,7 @@ describe('Discover > QueryList', function () {
750770
]}
751771
pageLinks=""
752772
location={location}
773+
refetchSavedQueries={refetchSavedQueries}
753774
/>
754775
);
755776

@@ -787,6 +808,7 @@ describe('Discover > QueryList', function () {
787808
]}
788809
pageLinks=""
789810
location={location}
811+
refetchSavedQueries={refetchSavedQueries}
790812
/>,
791813
{
792814
deprecatedRouterMocks: true,

static/app/views/discover/queryList.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ type Props = {
4747
location: Location;
4848
organization: Organization;
4949
pageLinks: string;
50+
refetchSavedQueries: () => void;
5051
renderPrebuilt: boolean;
5152
router: InjectedRouter;
5253
savedQueries: SavedQuery[];
@@ -63,9 +64,10 @@ class QueryList extends Component<Props> {
6364
}
6465

6566
handleDeleteQuery = (eventView: EventView) => {
66-
const {api, organization, location, savedQueries} = this.props;
67+
const {api, organization, location, savedQueries, refetchSavedQueries} = this.props;
6768

6869
handleDeleteQuery(api, organization, eventView).then(() => {
70+
refetchSavedQueries();
6971
if (savedQueries.length === 1 && location.query.cursor) {
7072
browserHistory.push({
7173
pathname: location.pathname,
@@ -76,12 +78,13 @@ class QueryList extends Component<Props> {
7678
};
7779

7880
handleDuplicateQuery = (eventView: EventView, yAxis: string[]) => {
79-
const {api, location, organization} = this.props;
81+
const {api, location, organization, refetchSavedQueries} = this.props;
8082

8183
eventView = eventView.clone();
8284
eventView.name = `${eventView.name} copy`;
8385

8486
handleCreateQuery(api, organization, eventView, yAxis).then(() => {
87+
refetchSavedQueries();
8588
browserHistory.push({
8689
pathname: location.pathname,
8790
query: {},

0 commit comments

Comments
 (0)