Browse Source

fix(issue-details): Fix 'Open in Discover' button when on the All Events tab (#54912)

Fixes https://github.com/getsentry/sentry/issues/54787

Previously, clicking the "Open in Discover" button (only when on the All
Events tab) would take you to the discover page with an empty query.
This is because the `useCleanQueryParamsOnRouteLeave` was being applied
on all navigations. The hook only needs to be applied when navigating
between issue details tabs.
Malachi Willey 1 year ago
parent
commit
3f2a21338f

+ 24 - 0
static/app/utils/useCleanQueryParamsOnRouteLeave.spec.tsx

@@ -42,6 +42,30 @@ describe('useCleanQueryParamsOnRouteLeave', () => {
     expect(unsubscriber).toHaveBeenCalled();
   });
 
+  it('should not update the history if shouldLeave returns false', () => {
+    MockBrowserHistoryListen.mockImplementation(onRouteLeave => {
+      onRouteLeave(
+        TestStubs.location({
+          pathname: '/next',
+          query: {
+            cursor: '0:1:0',
+            limit: '5',
+          },
+        })
+      );
+      return () => {};
+    });
+
+    reactHooks.renderHook(useCleanQueryParamsOnRouteLeave, {
+      initialProps: {
+        fieldsToClean: ['cursor'],
+        shouldClean: () => false,
+      },
+    });
+
+    expect(MockBrowserHistoryReplace).not.toHaveBeenCalled();
+  });
+
   it('should not update the history if the pathname is unchanged', () => {
     handleRouteLeave({
       fieldsToClean: ['cursor'],

+ 11 - 8
static/app/utils/useCleanQueryParamsOnRouteLeave.tsx

@@ -4,8 +4,9 @@ import type {Location} from 'history';
 
 import {useLocation} from 'sentry/utils/useLocation';
 
-type Opts = {
+type Opts<Q> = {
   fieldsToClean: string[];
+  shouldClean?: (newLocation: Location<Q>) => boolean;
 };
 
 export function handleRouteLeave<Q extends object>({
@@ -41,18 +42,20 @@ export function handleRouteLeave<Q extends object>({
   });
 }
 
-function useCleanQueryParamsOnRouteLeave({fieldsToClean}: Opts) {
+function useCleanQueryParamsOnRouteLeave<Q>({fieldsToClean, shouldClean}: Opts<Q>) {
   const location = useLocation();
 
   const onRouteLeave = useCallback(
     newLocation => {
-      handleRouteLeave({
-        fieldsToClean,
-        newLocation,
-        oldPathname: location.pathname,
-      });
+      if (!shouldClean || shouldClean(newLocation)) {
+        handleRouteLeave({
+          fieldsToClean,
+          newLocation,
+          oldPathname: location.pathname,
+        });
+      }
     },
-    [location.pathname, fieldsToClean]
+    [shouldClean, fieldsToClean, location.pathname]
   );
 
   useEffect(() => {

+ 4 - 1
static/app/views/issueDetails/groupEvents.tsx

@@ -30,7 +30,10 @@ function GroupEvents({params, location, group}: Props) {
 
   const {groupId} = params;
 
-  useCleanQueryParamsOnRouteLeave({fieldsToClean: ['cursor', 'query']});
+  useCleanQueryParamsOnRouteLeave({
+    fieldsToClean: ['cursor', 'query'],
+    shouldClean: newLocation => newLocation.pathname.includes(`/issues/${group.id}/`),
+  });
 
   const handleSearch = useCallback(
     (query: string) =>

+ 4 - 1
static/app/views/issueDetails/groupReplays/useReplaysFromIssue.tsx

@@ -62,7 +62,10 @@ function useReplayFromIssue({
     });
   }, [location.query.sort, replayIds]);
 
-  useCleanQueryParamsOnRouteLeave({fieldsToClean: ['cursor']});
+  useCleanQueryParamsOnRouteLeave({
+    fieldsToClean: ['cursor'],
+    shouldClean: newLocation => newLocation.pathname.includes(`/issues/${group.id}/`),
+  });
   useEffect(() => {
     fetchReplayIds();
   }, [fetchReplayIds]);