Skip to content

ref(issue-details): Remove all query params from legacy UI when no event is found. #83441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions static/app/views/issueDetails/groupDetails.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {ConfigFixture} from 'sentry-fixture/config';
import {EnvironmentsFixture} from 'sentry-fixture/environments';
import {EventFixture} from 'sentry-fixture/event';
import {EventsStatsFixture} from 'sentry-fixture/events';
import {GroupFixture} from 'sentry-fixture/group';
import {LocationFixture} from 'sentry-fixture/locationFixture';
import {ProjectFixture} from 'sentry-fixture/project';
Expand All @@ -16,12 +17,18 @@ import OrganizationStore from 'sentry/stores/organizationStore';
import PageFiltersStore from 'sentry/stores/pageFiltersStore';
import ProjectsStore from 'sentry/stores/projectsStore';
import {IssueCategory} from 'sentry/types/group';
import {useNavigate} from 'sentry/utils/useNavigate';
import GroupDetails from 'sentry/views/issueDetails/groupDetails';

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

jest.mock('sentry/utils/useNavigate', () => ({
useNavigate: jest.fn(),
}));

describe('groupDetails', () => {
let mockNavigate: jest.Mock;
const group = GroupFixture({issueCategory: IssueCategory.ERROR});
const event = EventFixture();
const project = ProjectFixture({teams: [TeamFixture()]});
Expand Down Expand Up @@ -86,9 +93,11 @@ describe('groupDetails', () => {
};

beforeEach(() => {
mockNavigate = jest.fn();
MockApiClient.clearMockResponses();
OrganizationStore.onUpdate(defaultInit.organization);
act(() => ProjectsStore.loadInitialData(defaultInit.projects));
jest.mocked(useNavigate).mockReturnValue(mockNavigate);

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

it("refires request when recommended endpoint doesn't return an event", async function () {
const recommendedWithSearchMock = MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/events/recommended/`,
query: {
query: 'foo:bar',
statsPeriod: '14d',
},
statusCode: 404,
body: {
detail: 'No matching event',
},
});

createWrapper({
...defaultInit,
router: {
...defaultInit.router,
location: LocationFixture({
query: {
query: 'foo:bar',
statsPeriod: '14d',
},
}),
},
});

await waitFor(() => expect(recommendedWithSearchMock).toHaveBeenCalledTimes(1));

await waitFor(() =>
expect(mockNavigate).toHaveBeenCalledWith(
expect.objectContaining(
// query and period should be removed
{query: {}}
),
{
replace: true,
}
)
);
});

it('does not refire for request with streamlined UI', async function () {
// Bunch of mocks to load streamlined UI
MockApiClient.addMockResponse({
url: '/organizations/org-slug/flags/logs/',
body: {data: []},
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/users/`,
body: [],
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/attachments/`,
body: [],
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/tags/`,
body: [],
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/releases/stats/`,
body: [],
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/events-stats/`,
body: {'count()': EventsStatsFixture(), 'count_unique(user)': EventsStatsFixture()},
method: 'GET',
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/events/`,
body: {data: [{'count_unique(user)': 21}]},
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/sentry-app-installations/`,
body: [],
});
MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/sentry-app-components/`,
body: [],
});

const recommendedWithSearchMock = MockApiClient.addMockResponse({
url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/events/recommended/`,
query: {
query: 'foo:bar',
statsPeriod: '14d',
},
statusCode: 404,
body: {
detail: 'No matching event',
},
});

createWrapper({
...defaultInit,
router: {
...defaultInit.router,
location: LocationFixture({
query: {
query: 'foo:bar',
statsPeriod: '14d',
streamline: '1',
},
}),
},
});

await waitFor(() => expect(recommendedWithSearchMock).toHaveBeenCalledTimes(1));

await waitFor(() => expect(mockNavigate).not.toHaveBeenCalled());
});

it('uses /latest endpoint when default is set to latest', async function () {
ConfigStore.loadInitialData(ConfigFixture({user: latestUser}));
const latestMock = MockApiClient.addMockResponse({
Expand Down
49 changes: 49 additions & 0 deletions static/app/views/issueDetails/groupDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import useApi from 'sentry/utils/useApi';
import {useDetailedProject} from 'sentry/utils/useDetailedProject';
import {useLocation} from 'sentry/utils/useLocation';
import {useMemoWithPrevious} from 'sentry/utils/useMemoWithPrevious';
import {useNavigate} from 'sentry/utils/useNavigate';
import useOrganization from 'sentry/utils/useOrganization';
import {useParams} from 'sentry/utils/useParams';
import useProjects from 'sentry/utils/useProjects';
Expand Down Expand Up @@ -223,6 +224,9 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
const organization = useOrganization();
const router = useRouter();
const params = router.params;
const navigate = useNavigate();
const defaultIssueEvent = useDefaultIssueEvent();
const hasStreamlinedUI = useHasStreamlinedUI();

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

Expand All @@ -234,6 +238,7 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
const {
data: event,
isPending: loadingEvent,
isError: isEventError,
refetch: refetchEvent,
} = useGroupEvent({
groupId,
Expand All @@ -248,6 +253,50 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
refetch: refetchGroupCall,
} = useGroup({groupId});

/**
* TODO(streamline-ui): Remove this whole hook once the legacy UI is removed. The streamlined UI exposes the
* filters on the page so the user is expected to clear it themselves, and the empty state is actually expected.
*/
useEffect(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we put a TODO to delete this entire useEffect when cleaning up streamline ui

if (hasStreamlinedUI) {
return;
}

const eventIdUrl = params.eventId ?? defaultIssueEvent;
const isLatestOrRecommendedEvent =
eventIdUrl === 'latest' || eventIdUrl === 'recommended';

if (
isLatestOrRecommendedEvent &&
isEventError &&
// Expanding this list to ensure invalid date ranges get removed as well as queries
(router.location.query.query ||
router.location.query.start ||
router.location.query.end ||
router.location.query.statsPeriod)
) {
// If we get an error from the helpful event endpoint, it probably means
// the query failed validation. We should remove the query to try again if
// we are not using streamlined UI.
navigate(
{
...router.location,
query: {
project: router.location.query.project,
},
},
{replace: true}
);
}
}, [
defaultIssueEvent,
isEventError,
navigate,
router.location,
params.eventId,
hasStreamlinedUI,
]);

/**
* Allows the GroupEventHeader to display the previous event while the new event is loading.
* This is not closer to the GroupEventHeader because it is unmounted
Expand Down
Loading