feat(issues): Only update group hasSeen when user is member (#92597)

Author: scttcper

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

@@ -18,17 +18,12 @@ 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';
 import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils';
 
 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(),
-}));
-
 jest.mock('sentry/views/issueDetails/utils', () => ({
   ...jest.requireActual('sentry/views/issueDetails/utils'),
   useHasStreamlinedUI: jest.fn(),
@@ -39,31 +34,16 @@ describe('groupDetails', () => {
   const group = GroupFixture({issueCategory: IssueCategory.ERROR});
   const event = EventFixture();
   const project = ProjectFixture({teams: [TeamFixture()]});
+  let hasSeenMock!: jest.Mock;
 
-  const routes = [
-    {path: '/', childRoutes: []},
-    {childRoutes: []},
-    {
-      path: '/organizations/:orgId/issues/:groupId/',
-      childRoutes: [],
-    },
-    {},
-  ];
-
-  const initRouter = {
+  const initialRouterConfig = {
     location: {
       pathname: `/organizations/org-slug/issues/${group.id}/`,
-      query: {project: group.project.id},
-    },
-    params: {
-      groupId: group.id,
     },
-    routes,
+    route: '/organizations/:orgId/issues/:groupId/',
   };
 
-  const defaultInit = initializeOrg<{groupId: string}>({
-    router: initRouter,
-  });
+  const defaultInit = initializeOrg<{groupId: string}>({});
 
   const recommendedUser = UserFixture({
     options: {
@@ -88,17 +68,19 @@ describe('groupDetails', () => {
     return <div>Group Details Mock</div>;
   }
 
-  const createWrapper = (init = defaultInit) => {
+  const createWrapper = (
+    routerConfig = initialRouterConfig,
+    organization = defaultInit.organization
+  ) => {
     // Add project id to the url to skip over the _allp redirect
     window.location.search = qs.stringify({project: group.project.id});
     return render(
-      <GroupDetails {...init.routerProps}>
+      <GroupDetails>
         <MockComponent />
       </GroupDetails>,
       {
-        organization: init.organization,
-        router: init.router,
-        deprecatedRouterMocks: true,
+        organization,
+        initialRouterConfig: routerConfig,
       }
     );
   };
@@ -109,7 +91,6 @@ describe('groupDetails', () => {
     MockApiClient.clearMockResponses();
     OrganizationStore.onUpdate(defaultInit.organization);
     act(() => ProjectsStore.loadInitialData(defaultInit.projects));
-    jest.mocked(useNavigate).mockReturnValue(mockNavigate);
 
     MockApiClient.addMockResponse({
       url: `/assistant/`,
@@ -126,7 +107,7 @@ describe('groupDetails', () => {
         ...event,
       },
     });
-    MockApiClient.addMockResponse({
+    hasSeenMock = MockApiClient.addMockResponse({
       url: `/projects/org-slug/${project.slug}/issues/`,
       method: 'PUT',
       body: {
@@ -190,6 +171,7 @@ describe('groupDetails', () => {
 
     // Sample event alert should not show up
     expect(screen.queryByText(SAMPLE_EVENT_ALERT_TEXT)).not.toBeInTheDocument();
+    expect(hasSeenMock).toHaveBeenCalled();
   });
 
   it('renders error when issue is not found', async function () {
@@ -237,16 +219,14 @@ describe('groupDetails', () => {
       body: group,
     });
 
-    const init = initializeOrg({
-      router: {
-        ...initRouter,
-        location: LocationFixture({
-          ...initRouter.location,
-          query: {environment: 'staging', project: group.project.id},
-        }),
-      },
-    });
-    createWrapper(init);
+    const routerConfig = {
+      ...initialRouterConfig,
+      location: LocationFixture({
+        ...initialRouterConfig.location,
+        query: {environment: 'staging', project: group.project.id},
+      }),
+    };
+    createWrapper(routerConfig);
 
     await waitFor(() =>
       expect(mock).toHaveBeenCalledWith(
@@ -272,10 +252,7 @@ describe('groupDetails', () => {
         substatus: 'ongoing',
       },
     });
-    createWrapper({
-      ...defaultInit,
-      organization: {...defaultInit.organization},
-    });
+    createWrapper();
     expect(await screen.findByText('Ongoing')).toBeInTheDocument();
   });
 
@@ -336,30 +313,24 @@ describe('groupDetails', () => {
       },
     });
 
-    createWrapper({
-      ...defaultInit,
-      router: {
-        ...defaultInit.router,
-        location: LocationFixture({
-          query: {
-            query: 'foo:bar',
-            statsPeriod: '14d',
-          },
-        }),
-      },
-    });
+    const routerConfig = {
+      ...initialRouterConfig,
+      location: LocationFixture({
+        ...initialRouterConfig.location,
+        query: {query: 'foo:bar', statsPeriod: '14d'},
+      }),
+    };
+    const {router} = createWrapper(routerConfig);
 
     await waitFor(() => expect(recommendedWithSearchMock).toHaveBeenCalledTimes(1));
 
     await waitFor(() =>
-      expect(mockNavigate).toHaveBeenCalledWith(
-        expect.objectContaining(
-          // query and period should be removed
-          {query: {}}
-        ),
-        {
-          replace: true,
-        }
+      expect(router.location).toEqual(
+        expect.objectContaining({
+          pathname: routerConfig.location.pathname,
+          // Query has been removed
+          query: {},
+        })
       )
     );
   });
@@ -460,4 +431,31 @@ describe('groupDetails', () => {
 
     await waitFor(() => expect(recommendedMock).toHaveBeenCalledTimes(1));
   });
+
+  it('does not send hasSeen request when user is not a project member', async function () {
+    const nonMemberProject = ProjectFixture({
+      teams: [TeamFixture()],
+      isMember: false,
+    });
+
+    // Mock the group to belong to the non-member project
+    MockApiClient.addMockResponse({
+      url: `/organizations/${defaultInit.organization.slug}/issues/${group.id}/`,
+      body: {
+        ...group,
+        project: nonMemberProject,
+        hasSeen: false,
+      },
+    });
+
+    act(() => ProjectsStore.loadInitialData([nonMemberProject]));
+
+    createWrapper();
+
+    // Wait for the component to render
+    expect(await screen.findByText(group.shortId)).toBeInTheDocument();
+
+    // Verify that the hasSeen request was NOT made
+    expect(hasSeenMock).not.toHaveBeenCalled();
+  });
 });

static/app/views/issueDetails/groupDetails.tsx

@@ -235,6 +235,7 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
   const navigate = useNavigate();
   const defaultIssueEvent = useDefaultIssueEvent();
   const hasStreamlinedUI = useHasStreamlinedUI();
+  const {projects} = useProjects();
 
   const [allProjectChanged, setAllProjectChanged] = useState<boolean>(false);
 
@@ -399,17 +400,11 @@ function useFetchGroupDetails(): FetchGroupDetailsState {
       return;
     }
 
-    if (!group.hasSeen) {
+    const project = projects.find(p => p.id === group?.project.id);
+    if (!group.hasSeen && project?.isMember) {
       markEventSeen(api, organization.slug, matchingProjectSlug, params.groupId);
     }
-  }, [
-    api,
-    group?.hasSeen,
-    group?.project?.id,
-    group?.project?.slug,
-    organization.slug,
-    params.groupId,
-  ]);
+  }, [api, group?.hasSeen, group?.project, organization.slug, params.groupId, projects]);
 
   useEffect(() => {
     const locationQuery = qs.parse(window.location.search) || {};