Browse Source

ref(tsc): Convert GroupTagValue to FC and useApiQuery (#57112)

Ref https://github.com/getsentry/frontend-tsc/issues/2

- Adds error state in the table that didn't exist prior
- No longer waits for tag query to complete - will just display `--` in
the percent column until it successfully fetches (although the value
list usually takes longer)
Malachi Willey 1 year ago
parent
commit
373998e7b3

+ 59 - 1
static/app/actionCreators/group.tsx

@@ -5,7 +5,7 @@ import {Tag} from 'sentry/actionCreators/events';
 import {Client, RequestCallbacks, RequestOptions} from 'sentry/api';
 import {getSampleEventQuery} from 'sentry/components/events/eventStatisticalDetector/eventComparison/eventDisplay';
 import GroupStore from 'sentry/stores/groupStore';
-import {Actor, Group, Member, Note, User} from 'sentry/types';
+import {Actor, Group, Member, Note, Tag as GroupTag, TagValue, User} from 'sentry/types';
 import {buildTeamId, buildUserId, defined} from 'sentry/utils';
 import {uniqueId} from 'sentry/utils/guid';
 import {ApiQueryKey, useApiQuery, UseApiQueryOptions} from 'sentry/utils/queryClient';
@@ -476,3 +476,61 @@ export const useFetchIssueTags = (
     ...options,
   });
 };
+
+type FetchIssueTagValuesParameters = {
+  groupId: string;
+  orgSlug: string;
+  tagKey: string;
+  environment?: string[];
+  sort?: string | string[];
+};
+
+export const makeFetchIssueTagValuesQueryKey = ({
+  orgSlug,
+  groupId,
+  tagKey,
+  environment,
+  sort,
+}: FetchIssueTagValuesParameters): ApiQueryKey => [
+  `/organizations/${orgSlug}/issues/${groupId}/tags/${tagKey}/values/`,
+  {query: {environment, sort}},
+];
+
+export function useFetchIssueTagValues(
+  parameters: FetchIssueTagValuesParameters,
+  options: Partial<UseApiQueryOptions<TagValue[]>> = {}
+) {
+  return useApiQuery<TagValue[]>(makeFetchIssueTagValuesQueryKey(parameters), {
+    staleTime: 0,
+    retry: false,
+    ...options,
+  });
+}
+
+type FetchIssueTagParameters = {
+  groupId: string;
+  orgSlug: string;
+  tagKey: string;
+};
+
+export const makeFetchIssueTagQueryKey = ({
+  orgSlug,
+  groupId,
+  tagKey,
+  environment,
+  sort,
+}: FetchIssueTagValuesParameters): ApiQueryKey => [
+  `/organizations/${orgSlug}/issues/${groupId}/tags/${tagKey}/`,
+  {query: {environment, sort}},
+];
+
+export function useFetchIssueTag(
+  parameters: FetchIssueTagParameters,
+  options: Partial<UseApiQueryOptions<GroupTag>> = {}
+) {
+  return useApiQuery<GroupTag>(makeFetchIssueTagQueryKey(parameters), {
+    staleTime: 0,
+    retry: false,
+    ...options,
+  });
+}

+ 73 - 8
static/app/views/issueDetails/groupTagValues.spec.tsx

@@ -1,12 +1,19 @@
+import {Group} from 'sentry-fixture/group';
 import {Tags} from 'sentry-fixture/tags';
 import {TagValues} from 'sentry-fixture/tagvalues';
 
 import {initializeOrg} from 'sentry-test/initializeOrg';
-import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary';
+import {
+  render,
+  screen,
+  userEvent,
+  waitForElementToBeRemoved,
+  within,
+} from 'sentry-test/reactTestingLibrary';
 
 import GroupTagValues from 'sentry/views/issueDetails/groupTagValues';
 
-const group = TestStubs.Group();
+const group = Group();
 const tags = Tags();
 
 function init(tagKey: string) {
@@ -33,6 +40,37 @@ describe('GroupTagValues', () => {
     MockApiClient.clearMockResponses();
   });
 
+  it('renders a list of tag values', async () => {
+    const {routerProps, routerContext, project} = init('user');
+
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/issues/1/tags/user/values/',
+      body: TagValues(),
+    });
+    render(
+      <GroupTagValues
+        environments={[]}
+        group={group}
+        project={project}
+        baseUrl=""
+        {...routerProps}
+      />,
+      {context: routerContext}
+    );
+
+    await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'));
+
+    // Special case for user tag - column title changes to Affected Users
+    expect(screen.getByText('Affected Users')).toBeInTheDocument();
+
+    // Affected user column
+    expect(screen.getByText('David Cramer')).toBeInTheDocument();
+    // Percent column
+    expect(screen.getByText('16.67%')).toBeInTheDocument();
+    // Count column
+    expect(screen.getByText('3')).toBeInTheDocument();
+  });
+
   it('navigates to issue details events tab with correct query params', async () => {
     const {routerProps, routerContext, router, project} = init('user');
 
@@ -53,7 +91,7 @@ describe('GroupTagValues', () => {
       }
     );
 
-    await userEvent.click(screen.getByRole('button', {name: 'More'}));
+    await userEvent.click(await screen.findByRole('button', {name: 'More'}));
     await userEvent.click(
       within(
         screen.getByRole('menuitemradio', {name: 'Search All Issues with Tag Value'})
@@ -66,14 +104,15 @@ describe('GroupTagValues', () => {
     });
   });
 
-  it('renders an error message if no tag values are returned because of environment selection', () => {
+  it('renders an error message if tag values request fails', async () => {
     const {routerProps, routerContext, project} = init('user');
 
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/issues/1/tags/user/values/',
-      body: [],
+      statusCode: 500,
     });
-    const {container} = render(
+
+    render(
       <GroupTagValues
         environments={['staging']}
         group={group}
@@ -84,8 +123,34 @@ describe('GroupTagValues', () => {
       {context: routerContext}
     );
 
-    expect(container).toHaveTextContent(
-      'No tags were found for the currently selected environments'
+    expect(
+      await screen.findByText('There was an error loading tag details')
+    ).toBeInTheDocument();
+  });
+
+  it('renders an error message if no tag values are returned because of environment selection', async () => {
+    const {routerProps, routerContext, project} = init('user');
+
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/issues/1/tags/user/values/',
+      body: [],
+    });
+
+    render(
+      <GroupTagValues
+        environments={['staging']}
+        group={group}
+        project={project}
+        baseUrl=""
+        {...routerProps}
+      />,
+      {context: routerContext}
     );
+
+    expect(
+      await screen.findByText(
+        'No tags were found for the currently selected environments'
+      )
+    ).toBeInTheDocument();
   });
 });

+ 164 - 151
static/app/views/issueDetails/groupTagValues.tsx

@@ -1,11 +1,11 @@
-import {Fragment} from 'react';
-import {RouteComponentProps, WithRouterProps} from 'react-router';
+import {Fragment, useEffect} from 'react';
 import styled from '@emotion/styled';
 
+import {useFetchIssueTag, useFetchIssueTagValues} from 'sentry/actionCreators/group';
+import {addMessage} from 'sentry/actionCreators/indicator';
 import {Button} from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
 import DataExport, {ExportQueryType} from 'sentry/components/dataExport';
-import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
 import {DeviceName} from 'sentry/components/deviceName';
 import {DropdownMenu} from 'sentry/components/dropdownMenu';
 import GlobalSelectionLink from 'sentry/components/globalSelectionLink';
@@ -13,6 +13,7 @@ import UserBadge from 'sentry/components/idBadge/userBadge';
 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 {extractSelectionParameters} from 'sentry/components/organizations/pageFilters/utils';
 import Pagination from 'sentry/components/pagination';
 import PanelTable from 'sentry/components/panels/panelTable';
@@ -20,19 +21,12 @@ 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 {
-  Group,
-  Organization,
-  Project,
-  SavedQueryVersions,
-  Tag,
-  TagValue,
-} from 'sentry/types';
+import {Group, Project, SavedQueryVersions} from 'sentry/types';
 import {isUrl, percent} from 'sentry/utils';
 import EventView from 'sentry/utils/discover/eventView';
-import withOrganization from 'sentry/utils/withOrganization';
-// eslint-disable-next-line no-restricted-imports
-import withSentryRouter from 'sentry/utils/withSentryRouter';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import {useParams} from 'sentry/utils/useParams';
 
 type RouteParams = {
   groupId: string;
@@ -43,55 +37,110 @@ type RouteParams = {
 type Props = {
   baseUrl: string;
   group: Group;
-  organization: Organization;
   environments?: string[];
   project?: Project;
-} & RouteComponentProps<RouteParams, {}>;
-
-type State = {
-  tag: Tag | null;
-  tagValueList: TagValue[] | null;
-  tagValueListPageLinks: string;
 };
 
 const DEFAULT_SORT = 'count';
 
-class GroupTagValues extends DeprecatedAsyncComponent<
-  Props & DeprecatedAsyncComponent['props'] & WithRouterProps,
-  State & DeprecatedAsyncComponent['state']
-> {
-  getEndpoints(): ReturnType<DeprecatedAsyncComponent['getEndpoints']> {
-    const {environments: environment, organization} = this.props;
-    const {groupId, tagKey} = this.props.params;
-    return [
-      ['tag', `/organizations/${organization.slug}/issues/${groupId}/tags/${tagKey}/`],
-      [
-        'tagValueList',
-        `/organizations/${organization.slug}/issues/${groupId}/tags/${tagKey}/values/`,
-        {query: {environment, sort: this.getSort()}},
-      ],
-    ];
-  }
-
-  getSort(): string {
-    return this.props.location.query.sort || DEFAULT_SORT;
-  }
+function useTagQueries({
+  group,
+  tagKey,
+  environments,
+  sort,
+}: {
+  group: Group;
+  sort: string | string[];
+  tagKey: string;
+  environments?: string[];
+}) {
+  const organization = useOrganization();
+
+  const {
+    data: tagValueList,
+    isLoading: tagValueListIsLoading,
+    isError: tagValueListIsError,
+    getResponseHeader,
+  } = useFetchIssueTagValues({
+    orgSlug: organization.slug,
+    groupId: group.id,
+    tagKey,
+    environment: environments,
+    sort,
+  });
+  const {data: tag, isError: tagIsError} = useFetchIssueTag({
+    orgSlug: organization.slug,
+    groupId: group.id,
+    tagKey,
+  });
+
+  useEffect(() => {
+    if (tagIsError) {
+      addMessage(t('Failed to fetch total tag values'), 'error');
+    }
+  }, [tagIsError]);
+
+  return {
+    tagValueList,
+    tag,
+    isLoading: tagValueListIsLoading,
+    isError: tagValueListIsError,
+    pageLinks: getResponseHeader?.('pageLinks'),
+  };
+}
 
-  renderLoading() {
-    return this.renderBody();
-  }
+function GroupTagValues({baseUrl, project, group, environments}: Props) {
+  const organization = useOrganization();
+  const location = useLocation();
+  const {orgId, tagKey = ''} = useParams<RouteParams>();
+  const {cursor: _cursor, page: _page, ...currentQuery} = location.query;
+
+  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({
+    group,
+    sort,
+    tagKey,
+    environments,
+  });
+
+  const lastSeenColumnHeader = (
+    <StyledSortLink
+      to={{
+        pathname: location.pathname,
+        query: {
+          ...currentQuery,
+          sort: 'date',
+        },
+      }}
+    >
+      {t('Last Seen')} {sort === 'date' && sortArrow}
+    </StyledSortLink>
+  );
+  const countColumnHeader = (
+    <StyledSortLink
+      to={{
+        pathname: location.pathname,
+        query: {
+          ...currentQuery,
+          sort: 'count',
+        },
+      }}
+    >
+      {t('Count')} {sort === 'count' && sortArrow}
+    </StyledSortLink>
+  );
+  const renderResults = () => {
+    if (isError) {
+      return <StyledLoadingError message={t('There was an error loading tag details')} />;
+    }
+
+    if (isLoading) {
+      return null;
+    }
 
-  renderResults() {
-    const {
-      baseUrl,
-      project,
-      environments: environment,
-      group,
-      location: {query},
-      params: {orgId, tagKey},
-      organization,
-    } = this.props;
-    const {tagValueList, tag} = this.state;
     const discoverFields = [
       'title',
       'release',
@@ -100,8 +149,7 @@ class GroupTagValues extends DeprecatedAsyncComponent<
       'timestamp',
     ];
 
-    const globalSelectionParams = extractSelectionParameters(query);
-
+    const globalSelectionParams = extractSelectionParameters(location.query);
     return tagValueList?.map((tagValue, tagValueIdx) => {
       const pct = tag?.totalValues
         ? `${percent(tagValue.count, tag?.totalValues).toFixed(2)}%`
@@ -118,7 +166,7 @@ class GroupTagValues extends DeprecatedAsyncComponent<
         orderby: '-timestamp',
         query: `issue:${group.shortId} ${issuesQuery}`,
         projects: [Number(project?.id)],
-        environment,
+        environment: environments,
         version: 2 as SavedQueryVersions,
         range: '90d',
       });
@@ -199,101 +247,59 @@ class GroupTagValues extends DeprecatedAsyncComponent<
         </Fragment>
       );
     });
-  }
-
-  renderBody() {
-    const {
-      group,
-      params: {orgId, tagKey},
-      location: {query},
-      environments,
-    } = this.props;
-    const {tagValueList, tag, tagValueListPageLinks, loading} = this.state;
-    const {cursor: _cursor, page: _page, ...currentQuery} = query;
-
-    const title = tagKey === 'user' ? t('Affected Users') : tagKey;
-
-    const sort = this.getSort();
-    const sortArrow = <IconArrow color="gray300" size="xs" direction="down" />;
-    const lastSeenColumnHeader = (
-      <StyledSortLink
-        to={{
-          pathname: location.pathname,
-          query: {
-            ...currentQuery,
-            sort: 'date',
-          },
-        }}
-      >
-        {t('Last Seen')} {sort === 'date' && sortArrow}
-      </StyledSortLink>
-    );
-    const countColumnHeader = (
-      <StyledSortLink
-        to={{
-          pathname: location.pathname,
-          query: {
-            ...currentQuery,
-            sort: 'count',
-          },
-        }}
-      >
-        {t('Count')} {sort === 'count' && sortArrow}
-      </StyledSortLink>
-    );
-
-    return (
-      <Layout.Body>
-        <Layout.Main fullWidth>
-          <TitleWrapper>
-            <Title>{t('Tag Details')}</Title>
-            <ButtonBar gap={1}>
-              <Button
-                size="sm"
-                priority="default"
-                href={`/${orgId}/${group.project.slug}/issues/${group.id}/tags/${tagKey}/export/`}
-              >
-                {t('Export Page to CSV')}
-              </Button>
-              <DataExport
-                payload={{
-                  queryType: ExportQueryType.ISSUES_BY_TAG,
-                  queryInfo: {
-                    project: group.project.id,
-                    group: group.id,
-                    key: tagKey,
-                  },
-                }}
-              />
-            </ButtonBar>
-          </TitleWrapper>
-          <StyledPanelTable
-            isLoading={loading}
-            isEmpty={tagValueList?.length === 0}
-            headers={[
-              title,
-              <PercentColumnHeader key="percent">{t('Percent')}</PercentColumnHeader>,
-              countColumnHeader,
-              lastSeenColumnHeader,
-              '',
-            ]}
-            emptyMessage={t('Sorry, the tags for this issue could not be found.')}
-            emptyAction={
-              environments?.length
-                ? t('No tags were found for the currently selected environments')
-                : null
-            }
-          >
-            {tagValueList && tag && this.renderResults()}
-          </StyledPanelTable>
-          <StyledPagination pageLinks={tagValueListPageLinks} />
-        </Layout.Main>
-      </Layout.Body>
-    );
-  }
+  };
+
+  return (
+    <Layout.Body>
+      <Layout.Main fullWidth>
+        <TitleWrapper>
+          <Title>{t('Tag Details')}</Title>
+          <ButtonBar gap={1}>
+            <Button
+              size="sm"
+              priority="default"
+              href={`/${orgId}/${group.project.slug}/issues/${group.id}/tags/${tagKey}/export/`}
+            >
+              {t('Export Page to CSV')}
+            </Button>
+            <DataExport
+              payload={{
+                queryType: ExportQueryType.ISSUES_BY_TAG,
+                queryInfo: {
+                  project: group.project.id,
+                  group: group.id,
+                  key: tagKey,
+                },
+              }}
+            />
+          </ButtonBar>
+        </TitleWrapper>
+        <StyledPanelTable
+          isLoading={isLoading}
+          isEmpty={!isError && tagValueList?.length === 0}
+          headers={[
+            title,
+            <PercentColumnHeader key="percent">{t('Percent')}</PercentColumnHeader>,
+            countColumnHeader,
+            lastSeenColumnHeader,
+            '',
+          ]}
+          emptyMessage={t('Sorry, the tags for this issue could not be found.')}
+          emptyAction={
+            environments?.length
+              ? t('No tags were found for the currently selected environments')
+              : null
+          }
+        >
+          {renderResults()}
+        </StyledPanelTable>
+        <StyledPagination pageLinks={pageLinks} />
+      </Layout.Main>
+    </Layout.Body>
+  );
 }
 
-export default withSentryRouter(withOrganization(GroupTagValues));
+export default GroupTagValues;
 
 const TitleWrapper = styled('div')`
   display: flex;
@@ -322,6 +328,13 @@ const StyledPanelTable = styled(PanelTable)`
   }
 `;
 
+const StyledLoadingError = styled(LoadingError)`
+  grid-column: 1 / -1;
+  margin-bottom: ${space(4)};
+  border-radius: 0;
+  border-width: 1px 0;
+`;
+
 const PercentColumnHeader = styled('div')`
   text-align: right;
 `;