Skip to content

Commit 3b6d7d1

Browse files
leeandherandrewshie-sentry
authored andcommitted
ref(issue-details): Remove all query params from legacy UI when no event is found. (#83441)
Adds back a portion removed in [this PR](https://github.com/getsentry/sentry/pull/82476/files#diff-a95c753ccf3abebbfb06333be0780c03f31de6cb0b8d178665b8ea24a02f00feL255-L275) that clears the query when it doesn't find an event, but only applies it when the UI is not streamlined. Now though, instead of just removing `query`, we remove everything except `project`, since if we kept any `date` params, the endpoint would still return empty for recommend/latest events. It also searches for any of those qparams to be set before triggering so when they are all removed, it won't infinitely call `navigate` -- may add a test for this just in case though.
1 parent e14712e commit 3b6d7d1

File tree

2 files changed

+170
-0
lines changed

2 files changed

+170
-0
lines changed

static/app/views/issueDetails/groupDetails.spec.tsx

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {ConfigFixture} from 'sentry-fixture/config';
22
import {EnvironmentsFixture} from 'sentry-fixture/environments';
33
import {EventFixture} from 'sentry-fixture/event';
4+
import {EventsStatsFixture} from 'sentry-fixture/events';
45
import {GroupFixture} from 'sentry-fixture/group';
56
import {LocationFixture} from 'sentry-fixture/locationFixture';
67
import {ProjectFixture} from 'sentry-fixture/project';
@@ -16,12 +17,18 @@ import OrganizationStore from 'sentry/stores/organizationStore';
1617
import PageFiltersStore from 'sentry/stores/pageFiltersStore';
1718
import ProjectsStore from 'sentry/stores/projectsStore';
1819
import {IssueCategory} from 'sentry/types/group';
20+
import {useNavigate} from 'sentry/utils/useNavigate';
1921
import GroupDetails from 'sentry/views/issueDetails/groupDetails';
2022

2123
const SAMPLE_EVENT_ALERT_TEXT =
2224
'You are viewing a sample error. Configure Sentry to start viewing real errors.';
2325

26+
jest.mock('sentry/utils/useNavigate', () => ({
27+
useNavigate: jest.fn(),
28+
}));
29+
2430
describe('groupDetails', () => {
31+
let mockNavigate: jest.Mock;
2532
const group = GroupFixture({issueCategory: IssueCategory.ERROR});
2633
const event = EventFixture();
2734
const project = ProjectFixture({teams: [TeamFixture()]});
@@ -86,9 +93,11 @@ describe('groupDetails', () => {
8693
};
8794

8895
beforeEach(() => {
96+
mockNavigate = jest.fn();
8997
MockApiClient.clearMockResponses();
9098
OrganizationStore.onUpdate(defaultInit.organization);
9199
act(() => ProjectsStore.loadInitialData(defaultInit.projects));
100+
jest.mocked(useNavigate).mockReturnValue(mockNavigate);
92101

93102
MockApiClient.addMockResponse({
94103
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/`,
@@ -297,6 +306,118 @@ describe('groupDetails', () => {
297306
await waitFor(() => expect(recommendedMock).toHaveBeenCalledTimes(1));
298307
});
299308

309+
it("refires request when recommended endpoint doesn't return an event", async function () {
310+
const recommendedWithSearchMock = MockApiClient.addMockResponse({
311+
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/events/recommended/`,
312+
query: {
313+
query: 'foo:bar',
314+
statsPeriod: '14d',
315+
},
316+
statusCode: 404,
317+
body: {
318+
detail: 'No matching event',
319+
},
320+
});
321+
322+
createWrapper({
323+
...defaultInit,
324+
router: {
325+
...defaultInit.router,
326+
location: LocationFixture({
327+
query: {
328+
query: 'foo:bar',
329+
statsPeriod: '14d',
330+
},
331+
}),
332+
},
333+
});
334+
335+
await waitFor(() => expect(recommendedWithSearchMock).toHaveBeenCalledTimes(1));
336+
337+
await waitFor(() =>
338+
expect(mockNavigate).toHaveBeenCalledWith(
339+
expect.objectContaining(
340+
// query and period should be removed
341+
{query: {}}
342+
),
343+
{
344+
replace: true,
345+
}
346+
)
347+
);
348+
});
349+
350+
it('does not refire for request with streamlined UI', async function () {
351+
// Bunch of mocks to load streamlined UI
352+
MockApiClient.addMockResponse({
353+
url: '/organizations/org-slug/flags/logs/',
354+
body: {data: []},
355+
});
356+
MockApiClient.addMockResponse({
357+
url: `/organizations/${defaultInit.organization.slug}/users/`,
358+
body: [],
359+
});
360+
MockApiClient.addMockResponse({
361+
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/attachments/`,
362+
body: [],
363+
});
364+
MockApiClient.addMockResponse({
365+
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/tags/`,
366+
body: [],
367+
});
368+
MockApiClient.addMockResponse({
369+
url: `/organizations/${defaultInit.organization.slug}/releases/stats/`,
370+
body: [],
371+
});
372+
MockApiClient.addMockResponse({
373+
url: `/organizations/${defaultInit.organization.slug}/events-stats/`,
374+
body: {'count()': EventsStatsFixture(), 'count_unique(user)': EventsStatsFixture()},
375+
method: 'GET',
376+
});
377+
MockApiClient.addMockResponse({
378+
url: `/organizations/${defaultInit.organization.slug}/events/`,
379+
body: {data: [{'count_unique(user)': 21}]},
380+
});
381+
MockApiClient.addMockResponse({
382+
url: `/organizations/${defaultInit.organization.slug}/sentry-app-installations/`,
383+
body: [],
384+
});
385+
MockApiClient.addMockResponse({
386+
url: `/organizations/${defaultInit.organization.slug}/sentry-app-components/`,
387+
body: [],
388+
});
389+
390+
const recommendedWithSearchMock = MockApiClient.addMockResponse({
391+
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/events/recommended/`,
392+
query: {
393+
query: 'foo:bar',
394+
statsPeriod: '14d',
395+
},
396+
statusCode: 404,
397+
body: {
398+
detail: 'No matching event',
399+
},
400+
});
401+
402+
createWrapper({
403+
...defaultInit,
404+
router: {
405+
...defaultInit.router,
406+
location: LocationFixture({
407+
query: {
408+
query: 'foo:bar',
409+
statsPeriod: '14d',
410+
streamline: '1',
411+
},
412+
}),
413+
},
414+
});
415+
416+
await waitFor(() => expect(recommendedWithSearchMock).toHaveBeenCalledTimes(1));
417+
418+
await waitFor(() => expect(mockNavigate).not.toHaveBeenCalled());
419+
});
420+
300421
it('uses /latest endpoint when default is set to latest', async function () {
301422
ConfigStore.loadInitialData(ConfigFixture({user: latestUser}));
302423
const latestMock = MockApiClient.addMockResponse({

static/app/views/issueDetails/groupDetails.tsx

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import useApi from 'sentry/utils/useApi';
4242
import {useDetailedProject} from 'sentry/utils/useDetailedProject';
4343
import {useLocation} from 'sentry/utils/useLocation';
4444
import {useMemoWithPrevious} from 'sentry/utils/useMemoWithPrevious';
45+
import {useNavigate} from 'sentry/utils/useNavigate';
4546
import useOrganization from 'sentry/utils/useOrganization';
4647
import {useParams} from 'sentry/utils/useParams';
4748
import useProjects from 'sentry/utils/useProjects';
@@ -223,6 +224,9 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
223224
const organization = useOrganization();
224225
const router = useRouter();
225226
const params = router.params;
227+
const navigate = useNavigate();
228+
const defaultIssueEvent = useDefaultIssueEvent();
229+
const hasStreamlinedUI = useHasStreamlinedUI();
226230

227231
const [allProjectChanged, setAllProjectChanged] = useState<boolean>(false);
228232

@@ -234,6 +238,7 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
234238
const {
235239
data: event,
236240
isPending: loadingEvent,
241+
isError: isEventError,
237242
refetch: refetchEvent,
238243
} = useGroupEvent({
239244
groupId,
@@ -248,6 +253,50 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
248253
refetch: refetchGroupCall,
249254
} = useGroup({groupId});
250255

256+
/**
257+
* TODO(streamline-ui): Remove this whole hook once the legacy UI is removed. The streamlined UI exposes the
258+
* filters on the page so the user is expected to clear it themselves, and the empty state is actually expected.
259+
*/
260+
useEffect(() => {
261+
if (hasStreamlinedUI) {
262+
return;
263+
}
264+
265+
const eventIdUrl = params.eventId ?? defaultIssueEvent;
266+
const isLatestOrRecommendedEvent =
267+
eventIdUrl === 'latest' || eventIdUrl === 'recommended';
268+
269+
if (
270+
isLatestOrRecommendedEvent &&
271+
isEventError &&
272+
// Expanding this list to ensure invalid date ranges get removed as well as queries
273+
(router.location.query.query ||
274+
router.location.query.start ||
275+
router.location.query.end ||
276+
router.location.query.statsPeriod)
277+
) {
278+
// If we get an error from the helpful event endpoint, it probably means
279+
// the query failed validation. We should remove the query to try again if
280+
// we are not using streamlined UI.
281+
navigate(
282+
{
283+
...router.location,
284+
query: {
285+
project: router.location.query.project,
286+
},
287+
},
288+
{replace: true}
289+
);
290+
}
291+
}, [
292+
defaultIssueEvent,
293+
isEventError,
294+
navigate,
295+
router.location,
296+
params.eventId,
297+
hasStreamlinedUI,
298+
]);
299+
251300
/**
252301
* Allows the GroupEventHeader to display the previous event while the new event is loading.
253302
* This is not closer to the GroupEventHeader because it is unmounted

0 commit comments

Comments
 (0)