Browse Source

feat(issues): Disable route analytics until the event is loaded (#51508)

Scott Cooper 1 year ago
parent
commit
9cbc4407b7

+ 44 - 15
static/app/utils/routeAnalytics/useDisableRouteAnalytics.spec.tsx

@@ -4,35 +4,64 @@ import {RouteAnalyticsContext} from 'sentry/views/routeAnalyticsContextProvider'
 
 import useDisableRouteAnalytics from './useDisableRouteAnalytics';
 
-function TestComponent() {
-  useDisableRouteAnalytics();
-  return <div>hi</div>;
-}
-
 describe('useDisableRouteAnalytics', function () {
+  const setDisableRouteAnalytics = jest.fn();
+  const otherFns = {
+    setRouteAnalyticsParams: jest.fn(),
+    setOrganization: jest.fn(),
+    setEventNames: jest.fn(),
+  };
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
   it('disables analytics', function () {
-    const setDisableRouteAnalytics = jest.fn();
-    const getComponent = (extraContext?: Record<string, any>) => (
+    function TestComponent() {
+      useDisableRouteAnalytics();
+      return <div>hi</div>;
+    }
+    const getComponent = ({previousUrl = ''}: {previousUrl?: string} = {}) => (
       <RouteAnalyticsContext.Provider
         value={{
+          ...otherFns,
           setDisableRouteAnalytics,
-          setRouteAnalyticsParams: jest.fn(),
-          setOrganization: jest.fn(),
-          setEventNames: jest.fn(),
-          previousUrl: '',
-          ...extraContext,
+          previousUrl,
         }}
       >
         <TestComponent />
       </RouteAnalyticsContext.Provider>
     );
-    const {rerender} = render(getComponent());
-    expect(setDisableRouteAnalytics).toHaveBeenCalledWith();
+    const {rerender} = render(getComponent({}));
+    expect(setDisableRouteAnalytics).toHaveBeenCalledWith(true);
     setDisableRouteAnalytics.mockClear();
     rerender(getComponent());
     // should still be called 0 times because previousURL the same
     expect(setDisableRouteAnalytics).toHaveBeenCalledTimes(0);
     rerender(getComponent({previousUrl: 'something-else'}));
-    expect(setDisableRouteAnalytics).toHaveBeenCalledWith();
+    expect(setDisableRouteAnalytics).toHaveBeenCalledWith(true);
+  });
+
+  it('re-enables analytics', function () {
+    function TestComponent({disabled}: {disabled: boolean}) {
+      useDisableRouteAnalytics(disabled);
+      return <div>hi</div>;
+    }
+    const getComponent = ({disabled}: {disabled: boolean}) => (
+      <RouteAnalyticsContext.Provider
+        value={{
+          ...otherFns,
+          setDisableRouteAnalytics,
+          previousUrl: '',
+        }}
+      >
+        <TestComponent disabled={disabled} />
+      </RouteAnalyticsContext.Provider>
+    );
+    const {rerender} = render(getComponent({disabled: true}));
+    expect(setDisableRouteAnalytics).toHaveBeenCalledWith(true);
+    setDisableRouteAnalytics.mockClear();
+    rerender(getComponent({disabled: false}));
+    expect(setDisableRouteAnalytics).toHaveBeenCalledWith(false);
   });
 });

+ 3 - 3
static/app/utils/routeAnalytics/useDisableRouteAnalytics.tsx

@@ -6,9 +6,9 @@ import {RouteAnalyticsContext} from 'sentry/views/routeAnalyticsContextProvider'
  * Disables route analytics when called in a component.
  * Must be called within 4s after the organization context is loaded.
  */
-export default function useDisableRouteAnalytics() {
+export default function useDisableRouteAnalytics(disabled = true) {
   const {setDisableRouteAnalytics, previousUrl} = useContext(RouteAnalyticsContext);
   useEffect(() => {
-    setDisableRouteAnalytics();
-  }, [setDisableRouteAnalytics, previousUrl]);
+    setDisableRouteAnalytics(disabled);
+  }, [setDisableRouteAnalytics, previousUrl, disabled]);
 }

+ 2 - 0
static/app/views/issueDetails/groupDetails.tsx

@@ -41,6 +41,7 @@ import {
 } from 'sentry/utils/queryClient';
 import recreateRoute from 'sentry/utils/recreateRoute';
 import RequestError from 'sentry/utils/requestError/requestError';
+import useDisableRouteAnalytics from 'sentry/utils/routeAnalytics/useDisableRouteAnalytics';
 import useRouteAnalyticsEventNames from 'sentry/utils/routeAnalytics/useRouteAnalyticsEventNames';
 import useRouteAnalyticsParams from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams';
 import useApi from 'sentry/utils/useApi';
@@ -524,6 +525,7 @@ function useTrackView({
     // Will be updated in GroupDetailsHeader if there are replays
     group_has_replay: false,
   });
+  useDisableRouteAnalytics(!group || !event || !project);
 }
 
 const trackTabChanged = ({

+ 5 - 1
static/app/views/routeAnalyticsContextProvider.tsx

@@ -19,7 +19,11 @@ const DEFAULT_CONTEXT = {
  */
 export const RouteAnalyticsContext = createContext<{
   previousUrl: string;
-  setDisableRouteAnalytics: () => void;
+  /**
+   * Enable/disable route analytics manually
+   * @param disabled - defaults to true
+   */
+  setDisableRouteAnalytics: (disabled?: boolean) => void;
   setEventNames: (evetKey: string, eventName: string) => void;
   setOrganization: (organization: Organization) => void;
   setRouteAnalyticsParams: (params: Record<string, any>) => void;