Browse Source

feat(issues): Remove child props from group tags (#80647)

Scott Cooper 3 months ago
parent
commit
af19981559

+ 39 - 78
static/app/views/issueDetails/groupTagValues.spec.tsx

@@ -1,4 +1,5 @@
 import {GroupFixture} from 'sentry-fixture/group';
+import {ProjectFixture} from 'sentry-fixture/project';
 import {TagsFixture} from 'sentry-fixture/tags';
 import {TagValuesFixture} from 'sentry-fixture/tagvalues';
 
@@ -8,30 +9,39 @@ import {
   screen,
   userEvent,
   waitFor,
-  waitForElementToBeRemoved,
   within,
 } from 'sentry-test/reactTestingLibrary';
 
+import ProjectsStore from 'sentry/stores/projectsStore';
 import {browserHistory} from 'sentry/utils/browserHistory';
 import {GroupTagValues} from 'sentry/views/issueDetails/groupTagValues';
 
-const group = GroupFixture();
-const tags = TagsFixture();
-
-function init(tagKey: string) {
-  return initializeOrg({
-    router: {
-      location: {
-        query: {},
-        pathname: '/organizations/:orgId/issues/:groupId/tags/:tagKey/',
+describe('GroupTagValues', () => {
+  const group = GroupFixture();
+  const tags = TagsFixture();
+  const project = ProjectFixture();
+
+  function init(tagKey: string, environment?: string[] | string) {
+    return initializeOrg({
+      router: {
+        location: {
+          query: {
+            environment,
+          },
+          pathname: '/organizations/:orgId/issues/:groupId/tags/:tagKey/',
+        },
+        params: {orgId: 'org-slug', groupId: group.id, tagKey},
       },
-      params: {orgId: 'org-slug', groupId: group.id, tagKey},
-    },
-  });
-}
+    });
+  }
 
-describe('GroupTagValues', () => {
   beforeEach(() => {
+    ProjectsStore.init();
+    ProjectsStore.loadInitialData([project]);
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/issues/${group.id}/`,
+      body: group,
+    });
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/issues/1/tags/user/',
       body: tags.find(({key}) => key === 'user'),
@@ -43,27 +53,16 @@ describe('GroupTagValues', () => {
   });
 
   it('renders a list of tag values', async () => {
-    const {router, routerProps, project} = init('user');
+    const {router} = init('user');
 
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/issues/1/tags/user/values/',
       body: TagValuesFixture(),
     });
-    render(
-      <GroupTagValues
-        environments={[]}
-        group={group}
-        project={project}
-        baseUrl=""
-        {...routerProps}
-      />,
-      {router}
-    );
-
-    await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'));
+    render(<GroupTagValues />, {router});
 
     // Special case for user tag - column title changes to Affected Users
-    expect(screen.getByText('Affected Users')).toBeInTheDocument();
+    expect(await screen.findByText('Affected Users')).toBeInTheDocument();
 
     // Affected user column
     expect(screen.getByText('David Cramer')).toBeInTheDocument();
@@ -74,7 +73,7 @@ describe('GroupTagValues', () => {
   });
 
   it('can page through tag values', async () => {
-    const {router, routerProps, project} = init('user');
+    const {router} = init('user');
 
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/issues/1/tags/user/values/',
@@ -85,20 +84,9 @@ describe('GroupTagValues', () => {
           '<https://sentry.io/api/0/organizations/sentry/user-feedback/?statsPeriod=14d&cursor=0:100:0>; rel="next"; results="true"; cursor="0:100:0"',
       },
     });
-    render(
-      <GroupTagValues
-        environments={[]}
-        group={group}
-        project={project}
-        baseUrl=""
-        {...routerProps}
-      />,
-      {router}
-    );
+    render(<GroupTagValues />, {router});
 
-    await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'));
-
-    expect(screen.getByRole('button', {name: 'Previous'})).toBeDisabled();
+    expect(await screen.findByRole('button', {name: 'Previous'})).toBeDisabled();
     expect(screen.getByRole('button', {name: 'Next'})).toBeEnabled();
 
     // Clicking next button loads page with query param ?cursor=0:100:0
@@ -111,24 +99,15 @@ describe('GroupTagValues', () => {
   });
 
   it('navigates to issue details events tab with correct query params', async () => {
-    const {routerProps, router, project} = init('user');
+    const {router} = init('user');
 
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/issues/1/tags/user/values/',
       body: TagValuesFixture(),
     });
-    render(
-      <GroupTagValues
-        environments={[]}
-        group={group}
-        project={project}
-        baseUrl=""
-        {...routerProps}
-      />,
-      {
-        router,
-      }
-    );
+    render(<GroupTagValues />, {
+      router,
+    });
 
     await userEvent.click(await screen.findByRole('button', {name: 'More'}));
     await userEvent.click(
@@ -144,23 +123,14 @@ describe('GroupTagValues', () => {
   });
 
   it('renders an error message if tag values request fails', async () => {
-    const {router, routerProps, project} = init('user');
+    const {router} = init('user', 'staging');
 
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/issues/1/tags/user/values/',
       statusCode: 500,
     });
 
-    render(
-      <GroupTagValues
-        environments={['staging']}
-        group={group}
-        project={project}
-        baseUrl=""
-        {...routerProps}
-      />,
-      {router}
-    );
+    render(<GroupTagValues />, {router});
 
     expect(
       await screen.findByText('There was an error loading tag details')
@@ -168,23 +138,14 @@ describe('GroupTagValues', () => {
   });
 
   it('renders an error message if no tag values are returned because of environment selection', async () => {
-    const {router, routerProps, project} = init('user');
+    const {router} = init('user', 'staging');
 
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/issues/1/tags/user/values/',
       body: [],
     });
 
-    render(
-      <GroupTagValues
-        environments={['staging']}
-        group={group}
-        project={project}
-        baseUrl=""
-        {...routerProps}
-      />,
-      {router}
-    );
+    render(<GroupTagValues />, {router});
 
     expect(
       await screen.findByText(

+ 38 - 16
static/app/views/issueDetails/groupTagValues.tsx

@@ -14,6 +14,7 @@ import * as Layout from 'sentry/components/layouts/thirds';
 import ExternalLink from 'sentry/components/links/externalLink';
 import Link from 'sentry/components/links/link';
 import LoadingError from 'sentry/components/loadingError';
+import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {extractSelectionParameters} from 'sentry/components/organizations/pageFilters/utils';
 import Pagination from 'sentry/components/pagination';
 import {PanelTable} from 'sentry/components/panels/panelTable';
@@ -21,9 +22,7 @@ import TimeSince from 'sentry/components/timeSince';
 import {IconArrow, IconEllipsis, IconMail, IconOpen} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import type {Group} from 'sentry/types/group';
 import type {SavedQueryVersions} from 'sentry/types/organization';
-import type {Project} from 'sentry/types/project';
 import {percent} from 'sentry/utils';
 import EventView from 'sentry/utils/discover/eventView';
 import {SavedQueryDatasets} from 'sentry/utils/discover/types';
@@ -31,11 +30,17 @@ import {isUrl} from 'sentry/utils/string/isUrl';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {useParams} from 'sentry/utils/useParams';
+import useProjectFromSlug from 'sentry/utils/useProjectFromSlug';
 import {hasDatasetSelector} from 'sentry/views/dashboards/utils';
 import GroupEventDetails, {
   type GroupEventDetailsProps,
 } from 'sentry/views/issueDetails/groupEventDetails/groupEventDetails';
-import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils';
+import {useGroup} from 'sentry/views/issueDetails/useGroup';
+import {useGroupDetailsRoute} from 'sentry/views/issueDetails/useGroupDetailsRoute';
+import {
+  useEnvironmentsFromUrl,
+  useHasStreamlinedUI,
+} from 'sentry/views/issueDetails/utils';
 
 type RouteParams = {
   groupId: string;
@@ -43,13 +48,6 @@ type RouteParams = {
   tagKey?: string;
 };
 
-type Props = {
-  baseUrl: string;
-  group: Group;
-  environments?: string[];
-  project?: Project;
-};
-
 const DEFAULT_SORT = 'count';
 
 function useTagQueries({
@@ -59,11 +57,11 @@ function useTagQueries({
   sort,
   cursor,
 }: {
+  environments: string[];
   groupId: string;
   sort: string | string[];
   tagKey: string;
   cursor?: string;
-  environments?: string[];
 }) {
   const organization = useOrganization();
 
@@ -101,24 +99,48 @@ function useTagQueries({
   };
 }
 
-export function GroupTagValues({baseUrl, project, group, environments}: Props) {
+export function GroupTagValues() {
   const organization = useOrganization();
   const location = useLocation();
+  const params = useParams<RouteParams>();
+  const environments = useEnvironmentsFromUrl();
+  const {baseUrl} = useGroupDetailsRoute();
   const {orgId, tagKey = ''} = useParams<RouteParams>();
   const {cursor, page: _page, ...currentQuery} = location.query;
 
+  const {
+    data: group,
+    isPending: isGroupPending,
+    isError: isGroupError,
+    refetch: refetchGroup,
+  } = useGroup({groupId: params.groupId});
+  const project = useProjectFromSlug({organization, projectSlug: group?.project?.slug});
+
   const title = tagKey === 'user' ? t('Affected Users') : tagKey;
   const sort = location.query.sort || DEFAULT_SORT;
   const sortArrow = <IconArrow color="gray300" size="xs" direction="down" />;
 
   const {tagValueList, tag, isLoading, isError, pageLinks} = useTagQueries({
-    groupId: group.id,
+    groupId: params.groupId,
     sort,
     tagKey,
     environments,
     cursor: typeof cursor === 'string' ? cursor : undefined,
   });
 
+  if (isGroupPending) {
+    return <LoadingIndicator />;
+  }
+
+  if (isGroupError) {
+    return (
+      <LoadingError
+        message={t('There was an error loading the issue details')}
+        onRetry={refetchGroup}
+      />
+    );
+  }
+
   const lastSeenColumnHeader = (
     <StyledSortLink
       to={{
@@ -305,7 +327,7 @@ export function GroupTagValues({baseUrl, project, group, environments}: Props) {
           ]}
           emptyMessage={t('Sorry, the tags for this issue could not be found.')}
           emptyAction={
-            environments?.length
+            environments.length
               ? t('No tags were found for the currently selected environments')
               : null
           }
@@ -318,7 +340,7 @@ export function GroupTagValues({baseUrl, project, group, environments}: Props) {
   );
 }
 
-function GroupTagValuesRoute(props: GroupEventDetailsProps & {baseUrl: string}) {
+function GroupTagValuesRoute(props: GroupEventDetailsProps) {
   const hasStreamlinedUI = useHasStreamlinedUI();
 
   // TODO(streamlined-ui): Point the router directly to group event details
@@ -326,7 +348,7 @@ function GroupTagValuesRoute(props: GroupEventDetailsProps & {baseUrl: string})
     return <GroupEventDetails {...props} />;
   }
 
-  return <GroupTagValues {...props} />;
+  return <GroupTagValues />;
 }
 
 const TitleWrapper = styled('div')`

+ 24 - 22
static/app/views/issueDetails/groupTags/groupTagsTab.spec.tsx

@@ -7,26 +7,36 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 import {GroupTagsTab} from './groupTagsTab';
 
 describe('GroupTagsTab', function () {
-  const {routerProps, router, organization} = initializeOrg();
   const group = GroupFixture();
-  let tagsMock;
+  const {router, organization} = initializeOrg({
+    router: {
+      location: {
+        query: {
+          environment: 'dev',
+        },
+      },
+      params: {
+        orgId: 'org-slug',
+        groupId: group.id,
+      },
+    },
+  });
+  let tagsMock: jest.Mock;
+
   beforeEach(function () {
+    MockApiClient.clearMockResponses();
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/issues/${group.id}/`,
+      body: group,
+    });
     tagsMock = MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/issues/1/tags/',
+      url: `/organizations/${organization.slug}/issues/${group.id}/tags/`,
       body: TagsFixture(),
     });
   });
 
   it('navigates to issue details events tab with correct query params', async function () {
-    render(
-      <GroupTagsTab
-        {...routerProps}
-        group={group}
-        environments={['dev']}
-        baseUrl={`/organizations/${organization.slug}/issues/${group.id}/`}
-      />,
-      {router, organization}
-    );
+    render(<GroupTagsTab />, {router, organization});
 
     const headers = await screen.findAllByTestId('tag-title');
 
@@ -49,7 +59,7 @@ describe('GroupTagsTab', function () {
 
     expect(router.push).toHaveBeenCalledWith({
       pathname: '/organizations/org-slug/issues/1/events/',
-      query: {query: 'user.username:david'},
+      query: {query: 'user.username:david', environment: 'dev'},
     });
   });
 
@@ -59,15 +69,7 @@ describe('GroupTagsTab', function () {
       statusCode: 500,
     });
 
-    render(
-      <GroupTagsTab
-        {...routerProps}
-        group={group}
-        environments={['dev']}
-        baseUrl={`/organizations/${organization.slug}/issues/${group.id}/`}
-      />,
-      {router, organization}
-    );
+    render(<GroupTagsTab />, {router, organization});
 
     expect(
       await screen.findByText('There was an error loading issue tags.')

+ 27 - 17
static/app/views/issueDetails/groupTags/groupTagsTab.tsx

@@ -16,20 +16,19 @@ import PanelBody from 'sentry/components/panels/panelBody';
 import Version from 'sentry/components/version';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import type {Group} from 'sentry/types/group';
 import {percent} from 'sentry/utils';
 import {useLocation} from 'sentry/utils/useLocation';
+import {useParams} from 'sentry/utils/useParams';
 import GroupEventDetails, {
   type GroupEventDetailsProps,
 } from 'sentry/views/issueDetails/groupEventDetails/groupEventDetails';
 import {useGroupTags} from 'sentry/views/issueDetails/groupTags/useGroupTags';
-import {useHasStreamlinedUI} from 'sentry/views/issueDetails/utils';
-
-type GroupTagsProps = {
-  baseUrl: string;
-  environments: string[];
-  group: Group;
-};
+import {useGroup} from 'sentry/views/issueDetails/useGroup';
+import {useGroupDetailsRoute} from 'sentry/views/issueDetails/useGroupDetailsRoute';
+import {
+  useEnvironmentsFromUrl,
+  useHasStreamlinedUI,
+} from 'sentry/views/issueDetails/utils';
 
 type SimpleTag = {
   key: string;
@@ -42,8 +41,18 @@ type SimpleTag = {
   totalValues: number;
 };
 
-export function GroupTagsTab({group, baseUrl, environments}: GroupTagsProps) {
+export function GroupTagsTab() {
   const location = useLocation();
+  const environments = useEnvironmentsFromUrl();
+  const {baseUrl} = useGroupDetailsRoute();
+  const params = useParams<{groupId: string}>();
+
+  const {
+    data: group,
+    isPending: isGroupPending,
+    isError: isGroupError,
+    refetch: refetchGroup,
+  } = useGroup({groupId: params.groupId});
 
   const {
     data = [],
@@ -51,21 +60,24 @@ export function GroupTagsTab({group, baseUrl, environments}: GroupTagsProps) {
     isError,
     refetch,
   } = useGroupTags({
-    groupId: group.id,
+    groupId: group?.id,
     environment: environments,
   });
 
   const alphabeticalTags = data.sort((a, b) => a.key.localeCompare(b.key));
 
-  if (isPending) {
+  if (isPending || isGroupPending) {
     return <LoadingIndicator />;
   }
 
-  if (isError) {
+  if (isError || isGroupError) {
     return (
       <LoadingError
         message={t('There was an error loading issue tags.')}
-        onRetry={refetch}
+        onRetry={() => {
+          refetch();
+          refetchGroup();
+        }}
       />
     );
   }
@@ -136,9 +148,7 @@ export function GroupTagsTab({group, baseUrl, environments}: GroupTagsProps) {
   );
 }
 
-function GroupTagsRoute(
-  props: GroupEventDetailsProps & {baseUrl: string; environments: string[]}
-) {
+function GroupTagsRoute(props: GroupEventDetailsProps) {
   const hasStreamlinedUI = useHasStreamlinedUI();
 
   // TODO(streamlined-ui): Point the router to group event details
@@ -146,7 +156,7 @@ function GroupTagsRoute(
     return <GroupEventDetails {...props} />;
   }
 
-  return <GroupTagsTab {...props} />;
+  return <GroupTagsTab />;
 }
 
 export default GroupTagsRoute;