Просмотр исходного кода

Revert "fix(replay): Don't emit an EventView from useReplaysFromIssue when no replayIds are in the fetched list (#62119)"

This reverts commit 53ee4e454fd8c2f9fda8b1fffcb3d8235e50770b.

Co-authored-by: ryan953 <187460+ryan953@users.noreply.github.com>
getsentry-bot 1 год назад
Родитель
Сommit
3738a2cada

+ 2 - 2
static/app/utils/replays/fetchReplayList.tsx

@@ -24,8 +24,8 @@ type Props = {
   eventView: EventView;
   location: Location;
   organization: Organization;
-  queryReferrer: string;
   perPage?: number;
+  queryReferrer?: 'issueReplays';
 };
 
 async function fetchReplayList({
@@ -59,7 +59,7 @@ async function fetchReplayList({
         // when queryReferrer === 'issueReplays' we override the global view check on the backend
         // we also require a project param otherwise we won't yield results
         queryReferrer,
-        project: ALL_ACCESS_PROJECTS,
+        project: queryReferrer === 'issueReplays' ? ALL_ACCESS_PROJECTS : payload.project,
       },
     });
 

+ 1 - 1
static/app/utils/replays/hooks/useReplayList.tsx

@@ -11,8 +11,8 @@ type Options = {
   eventView: EventView;
   location: Location<ReplayListLocationQuery>;
   organization: Organization;
-  queryReferrer: string;
   perPage?: number;
+  queryReferrer?: 'issueReplays';
 };
 
 type State = Awaited<ReturnType<typeof fetchReplayList>> & {isFetching: boolean};

+ 42 - 44
static/app/views/issueDetails/groupReplays/groupReplays.spec.tsx

@@ -121,38 +121,37 @@ describe('GroupReplays', () => {
             },
           })
         );
+        // Expect api path to have the correct query params
+        expect(mockReplayApi).toHaveBeenCalledWith(
+          mockReplayUrl,
+          expect.objectContaining({
+            query: expect.objectContaining({
+              environment: [],
+              field: [
+                'activity',
+                'browser',
+                'count_dead_clicks',
+                'count_errors',
+                'count_rage_clicks',
+                'duration',
+                'finished_at',
+                'id',
+                'is_archived',
+                'os',
+                'project_id',
+                'started_at',
+                'user',
+              ],
+              per_page: 50,
+              project: -1,
+              queryReferrer: 'issueReplays',
+              query: `id:[${REPLAY_ID_1},${REPLAY_ID_2}]`,
+              sort: '-started_at',
+              statsPeriod: '14d',
+            }),
+          })
+        );
       });
-
-      // Expect api path to have the correct query params
-      expect(mockReplayApi).toHaveBeenCalledWith(
-        mockReplayUrl,
-        expect.objectContaining({
-          query: expect.objectContaining({
-            environment: [],
-            field: [
-              'activity',
-              'browser',
-              'count_dead_clicks',
-              'count_errors',
-              'count_rage_clicks',
-              'duration',
-              'finished_at',
-              'id',
-              'is_archived',
-              'os',
-              'project_id',
-              'started_at',
-              'user',
-            ],
-            per_page: 50,
-            project: -1,
-            queryReferrer: 'groupReplays',
-            query: `id:[${REPLAY_ID_1},${REPLAY_ID_2}]`,
-            sort: '-started_at',
-            statsPeriod: '14d',
-          }),
-        })
-      );
     });
 
     it('should show empty message when no replays are found', async () => {
@@ -211,12 +210,11 @@ describe('GroupReplays', () => {
 
       await waitFor(() => {
         expect(mockReplayCountApi).toHaveBeenCalledTimes(1);
+        expect(mockReplayApi).toHaveBeenCalledTimes(1);
+        expect(
+          screen.getByText('Invalid number: asdf. Expected number.')
+        ).toBeInTheDocument();
       });
-
-      expect(mockReplayApi).toHaveBeenCalledTimes(1);
-      expect(
-        screen.getByText('Invalid number: asdf. Expected number.')
-      ).toBeInTheDocument();
     });
 
     it('should display default error message when api call fails without a body', async () => {
@@ -243,13 +241,13 @@ describe('GroupReplays', () => {
 
       await waitFor(() => {
         expect(mockReplayCountApi).toHaveBeenCalledTimes(1);
+        expect(mockReplayApi).toHaveBeenCalledTimes(1);
+        expect(
+          screen.getByText(
+            'Sorry, the list of replays could not be loaded. This could be due to invalid search parameters or an internal systems error.'
+          )
+        ).toBeInTheDocument();
       });
-      expect(mockReplayApi).toHaveBeenCalledTimes(1);
-      expect(
-        screen.getByText(
-          'Sorry, the list of replays could not be loaded. This could be due to invalid search parameters or an internal systems error.'
-        )
-      ).toBeInTheDocument();
     });
 
     it('should show loading indicator when loading replays', async () => {
@@ -279,8 +277,8 @@ describe('GroupReplays', () => {
       expect(screen.getByTestId('loading-indicator')).toBeInTheDocument();
       await waitFor(() => {
         expect(mockReplayCountApi).toHaveBeenCalledTimes(1);
+        expect(mockReplayApi).toHaveBeenCalledTimes(1);
       });
-      expect(mockReplayApi).toHaveBeenCalledTimes(1);
     });
 
     it('should show a list of replays and have the correct values', async () => {
@@ -342,8 +340,8 @@ describe('GroupReplays', () => {
 
       await waitFor(() => {
         expect(mockReplayCountApi).toHaveBeenCalledTimes(1);
+        expect(mockReplayApi).toHaveBeenCalledTimes(1);
       });
-      expect(mockReplayApi).toHaveBeenCalledTimes(1);
 
       // Expect the table to have 2 rows
       expect(screen.getAllByText('testDisplayName')).toHaveLength(2);

+ 3 - 3
static/app/views/issueDetails/groupReplays/groupReplays.tsx

@@ -31,7 +31,7 @@ function GroupReplays({group}: Props) {
   const organization = useOrganization();
   const location = useLocation<ReplayListLocationQuery>();
 
-  const {eventView, fetchError, isFetching, pageLinks} = useReplaysFromIssue({
+  const {eventView, fetchError, pageLinks} = useReplaysFromIssue({
     group,
     location,
     organization,
@@ -52,7 +52,7 @@ function GroupReplays({group}: Props) {
       <StyledLayoutPage withPadding>
         <ReplayTable
           fetchError={fetchError}
-          isFetching={isFetching}
+          isFetching
           replays={[]}
           sort={undefined}
           visibleColumns={VISIBLE_COLUMNS}
@@ -87,7 +87,7 @@ function GroupReplaysTable({
     eventView,
     location,
     organization,
-    queryReferrer: 'groupReplays',
+    queryReferrer: 'issueReplays',
   });
 
   return (

+ 3 - 7
static/app/views/issueDetails/groupReplays/useReplaysFromIssue.spec.tsx

@@ -48,7 +48,6 @@ describe('useReplaysFromIssue', () => {
     expect(result.current).toEqual({
       eventView: null,
       fetchError: undefined,
-      isFetching: true,
       pageLinks: null,
     });
 
@@ -59,7 +58,6 @@ describe('useReplaysFromIssue', () => {
         query: 'id:[replay42,replay256]',
       }),
       fetchError: undefined,
-      isFetching: false,
       pageLinks: null,
     });
   });
@@ -86,7 +84,6 @@ describe('useReplaysFromIssue', () => {
     expect(result.current).toEqual({
       eventView: null,
       fetchError: undefined,
-      isFetching: true,
       pageLinks: null,
     });
 
@@ -97,7 +94,6 @@ describe('useReplaysFromIssue', () => {
         query: 'id:[replay42,replay256]',
       }),
       fetchError: undefined,
-      isFetching: false,
       pageLinks: null,
     });
   });
@@ -122,16 +118,16 @@ describe('useReplaysFromIssue', () => {
     expect(result.current).toEqual({
       eventView: null,
       fetchError: undefined,
-      isFetching: true,
       pageLinks: null,
     });
 
     await waitForNextUpdate();
 
     expect(result.current).toEqual({
-      eventView: null,
+      eventView: expect.objectContaining({
+        query: 'id:[]',
+      }),
       fetchError: undefined,
-      isFetching: false,
       pageLinks: null,
     });
   });

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

@@ -11,7 +11,7 @@ import useApi from 'sentry/utils/useApi';
 import useCleanQueryParamsOnRouteLeave from 'sentry/utils/useCleanQueryParamsOnRouteLeave';
 import {REPLAY_LIST_FIELDS} from 'sentry/views/replays/types';
 
-export default function useReplaysFromIssue({
+function useReplayFromIssue({
   group,
   location,
   organization,
@@ -51,7 +51,7 @@ export default function useReplaysFromIssue({
   }, [api, organization.slug, group.id, dataSource]);
 
   const eventView = useMemo(() => {
-    if (!replayIds || !replayIds.length) {
+    if (!replayIds) {
       return null;
     }
     return EventView.fromSavedQuery({
@@ -59,7 +59,7 @@ export default function useReplaysFromIssue({
       name: '',
       version: 2,
       fields: REPLAY_LIST_FIELDS,
-      query: replayIds.length ? `id:[${String(replayIds)}]` : `id:1`,
+      query: `id:[${String(replayIds)}]`,
       range: '14d',
       projects: [],
       orderby: decodeScalar(location.query.sort, DEFAULT_SORT),
@@ -76,8 +76,9 @@ export default function useReplaysFromIssue({
 
   return {
     eventView,
-    isFetching: replayIds === undefined,
     fetchError,
     pageLinks: null,
   };
 }
+
+export default useReplayFromIssue;

+ 0 - 1
static/app/views/performance/transactionSummary/transactionReplays/transactionReplays.tsx

@@ -144,7 +144,6 @@ function ReplaysContent({
     eventView,
     location,
     organization,
-    queryReferrer: 'transactionReplays',
   });
 
   const replaysWithTx = useReplaysWithTxData({

+ 0 - 1
static/app/views/replays/deadRageClick/exampleReplaysList.tsx

@@ -68,7 +68,6 @@ export default function ExampleReplaysList({
     location: emptyLocation,
     organization,
     perPage: 3,
-    queryReferrer: 'exampleReplaysList',
   });
 
   const routes = useRoutes();

+ 0 - 1
static/app/views/replays/list/replaysList.tsx

@@ -67,7 +67,6 @@ function ReplaysListTable({
     eventView,
     location,
     organization,
-    queryReferrer: 'replaysList',
   });
 
   const {