Browse Source

ref(tsc): Convert GroupTags to FC and useApiQuery (#58339)

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

There already existed a useApiQuery hook for the endpoint used here
(`useFetchIssueTags`), so I used that here. Had to slightly change the
parameter typings to accommodate the usage.
Malachi Willey 1 year ago
parent
commit
8eb0d9cd1e

+ 2 - 2
static/app/actionCreators/group.tsx

@@ -413,11 +413,11 @@ export type GroupTagsResponse = GroupTagResponseItem[];
 
 type FetchIssueTagsParameters = {
   environment: string[];
-  limit: number;
   orgSlug: string;
-  readable: boolean;
   groupId?: string;
   isStatisticalDetector?: boolean;
+  limit?: number;
+  readable?: boolean;
   statisticalDetectorParameters?: {
     durationBaseline: number;
     end: string;

+ 53 - 3
static/app/views/issueDetails/groupTags.spec.tsx

@@ -3,6 +3,7 @@ import {Tags} from 'sentry-fixture/tags';
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
+import {IssueType} from 'sentry/types';
 import GroupTags from 'sentry/views/issueDetails/groupTags';
 
 describe('GroupTags', function () {
@@ -27,16 +28,22 @@ describe('GroupTags', function () {
       {context: routerContext, organization}
     );
 
+    const headers = await screen.findAllByTestId('tag-title');
+
     expect(tagsMock).toHaveBeenCalledWith(
       '/organizations/org-slug/issues/1/tags/',
       expect.objectContaining({
         query: {environment: ['dev']},
       })
     );
-
-    const headers = screen.getAllByTestId('tag-title').map(header => header.innerHTML);
     // Check headers have been sorted alphabetically
-    expect(headers).toEqual(['browser', 'device', 'environment', 'url', 'user']);
+    expect(headers.map(h => h.innerHTML)).toEqual([
+      'browser',
+      'device',
+      'environment',
+      'url',
+      'user',
+    ]);
 
     await userEvent.click(screen.getByText('david'));
 
@@ -45,4 +52,47 @@ describe('GroupTags', function () {
       query: {query: 'user.username:david'},
     });
   });
+
+  it('navigates correctly when duration regression issue > tags key is clicked', async function () {
+    render(
+      <GroupTags
+        {...routerProps}
+        group={{...group, issueType: IssueType.PERFORMANCE_DURATION_REGRESSION}}
+        environments={['dev']}
+        baseUrl={`/organizations/${organization.slug}/issues/${group.id}/`}
+      />,
+      {context: routerContext, organization}
+    );
+
+    await screen.findAllByTestId('tag-title');
+    await userEvent.click(screen.getByText('browser'));
+
+    expect(router.push).toHaveBeenCalledWith({
+      pathname: '/organizations/org-slug/performance/summary/tags/',
+      query: expect.objectContaining({
+        tagKey: 'browser',
+      }),
+    });
+  });
+
+  it('shows an error message when the request fails', async function () {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/issues/1/tags/',
+      statusCode: 500,
+    });
+
+    render(
+      <GroupTags
+        {...routerProps}
+        group={group}
+        environments={['dev']}
+        baseUrl={`/organizations/${organization.slug}/issues/${group.id}/`}
+      />,
+      {context: routerContext, organization}
+    );
+
+    expect(
+      await screen.findByText('There was an error loading issue tags.')
+    ).toBeInTheDocument();
+  });
 });

+ 161 - 158
static/app/views/issueDetails/groupTags.tsx

@@ -1,195 +1,198 @@
-import {RouteComponentProps} from 'react-router';
+import {useRef} from 'react';
 import styled from '@emotion/styled';
-import isEqual from 'lodash/isEqual';
 
+import {Tag} from 'sentry/actionCreators/events';
+import {GroupTagsResponse, useFetchIssueTags} from 'sentry/actionCreators/group';
 import {Alert} from 'sentry/components/alert';
 import Count from 'sentry/components/count';
-import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
 import {DeviceName} from 'sentry/components/deviceName';
-import {getSampleEventQuery} from 'sentry/components/events/eventStatisticalDetector/eventComparison/eventDisplay';
 import GlobalSelectionLink from 'sentry/components/globalSelectionLink';
 import {sumTagFacetsForTopValues} from 'sentry/components/group/tagFacets';
 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 Panel from 'sentry/components/panels/panel';
 import PanelBody from 'sentry/components/panels/panelBody';
 import Version from 'sentry/components/version';
-import {tct} from 'sentry/locale';
+import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Event, Group, IssueType, Organization, TagWithTopValues} from 'sentry/types';
-import {percent} from 'sentry/utils';
-import withOrganization from 'sentry/utils/withOrganization';
+import {Event, Group, IssueType} from 'sentry/types';
+import {defined, percent} from 'sentry/utils';
+import {useRelativeDateTime} from 'sentry/utils/profiling/hooks/useRelativeDateTime';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
 
-type Props = DeprecatedAsyncComponent['props'] & {
+import {generateTagsRoute} from '../performance/transactionSummary/transactionTags/utils';
+
+type GroupTagsProps = {
   baseUrl: string;
   environments: string[];
   group: Group;
-  organization: Organization;
   event?: Event;
-} & RouteComponentProps<{}, {}>;
-
-type State = DeprecatedAsyncComponent['state'] & {
-  now: number;
-  tagList: null | TagWithTopValues[];
 };
 
-class GroupTags extends DeprecatedAsyncComponent<Props, State> {
-  getDefaultState(): State {
-    return {
-      ...super.getDefaultState(),
-      tagList: null,
-      now: Date.now(),
-    };
-  }
-
-  getEndpoints(): ReturnType<DeprecatedAsyncComponent['getEndpoints']> {
-    const {group, environments, organization, event} = this.props;
+type SimpleTag = {
+  key: string;
+  topValues: Array<{
+    count: number;
+    name: string;
+    value: string;
+    query?: string;
+  }>;
+  totalValues: number;
+};
 
-    if (
-      organization.features.includes('performance-duration-regression-visible') &&
-      group.issueType === IssueType.PERFORMANCE_DURATION_REGRESSION &&
-      this.state
-    ) {
-      if (!event) {
-        // We need the event for its occurrence data to get the timestamps
-        // for querying
-        return [];
-      }
+function isTagFacetsResponse(
+  _: GroupTagsResponse | Tag[] | undefined,
+  shouldUseTagFacetsEndpoint: boolean
+): _ is Tag[] {
+  return shouldUseTagFacetsEndpoint;
+}
 
-      const {transaction, aggregateRange2, breakpoint} =
-        event.occurrence?.evidenceData ?? {};
-      return [
-        [
-          'tagFacets',
-          `/organizations/${organization.slug}/events-facets/`,
-          {
-            query: {
-              environment: environments,
-              transaction,
-              includeAll: true,
-              query: getSampleEventQuery({
-                transaction,
-                durationBaseline: aggregateRange2,
-                addUpperBound: false,
-              }),
-              start: new Date(breakpoint * 1000).toISOString(),
-              end: new Date(this.state.now).toISOString(),
-            },
-          },
-        ],
-      ];
-    }
+function GroupTags({group, baseUrl, environments, event}: GroupTagsProps) {
+  const organization = useOrganization();
+  const location = useLocation();
+  const now = useRef(Date.now()).current;
 
-    return [
-      [
-        'tagList',
-        `/organizations/${organization.slug}/issues/${group.id}/tags/`,
-        {
-          query: {environment: environments},
-        },
-      ],
-    ];
-  }
+  const {transaction, aggregateRange2, breakpoint} =
+    event?.occurrence?.evidenceData ?? {};
 
-  componentDidUpdate(prevProps: Props) {
-    if (
-      !isEqual(prevProps.environments, this.props.environments) ||
-      !isEqual(prevProps.event, this.props.event)
-    ) {
-      this.remountComponent();
-    }
-  }
+  const {start: beforeDateTime, end: afterDateTime} = useRelativeDateTime({
+    anchor: breakpoint,
+    relativeDays: 14,
+  });
 
-  onRequestSuccess({stateKey, data}: {data: any; stateKey: string}): void {
-    if (stateKey === 'tagFacets') {
-      this.setState({
-        tagList: data
-          .filter(({key}) => key !== 'transaction')
-          .map(sumTagFacetsForTopValues),
-        tagFacets: undefined,
-      });
-    }
-  }
+  const shouldUseTagFacetsEndpoint =
+    organization.features.includes('performance-duration-regression-visible') &&
+    defined(event) &&
+    group.issueType === IssueType.PERFORMANCE_DURATION_REGRESSION;
 
-  renderTags() {
-    const {baseUrl, location} = this.props;
-    const {tagList} = this.state;
+  const {
+    data = [],
+    isLoading,
+    isError,
+    refetch,
+  } = useFetchIssueTags({
+    orgSlug: organization.slug,
+    groupId: group.id,
+    environment: environments,
+    isStatisticalDetector: shouldUseTagFacetsEndpoint,
+    statisticalDetectorParameters: shouldUseTagFacetsEndpoint
+      ? {
+          transaction,
+          start: new Date(breakpoint * 1000).toISOString(),
+          end: new Date(now).toISOString(),
+          durationBaseline: aggregateRange2,
+        }
+      : undefined,
+  });
 
-    const alphabeticalTags = (tagList ?? []).sort((a, b) => a.key.localeCompare(b.key));
+  // useFetchIssueTags can return two different types of responses, depending on shouldUseTagFacetsEndpoint
+  // This line will convert the response to a common type for rendering
+  const tagList: SimpleTag[] = isTagFacetsResponse(data, shouldUseTagFacetsEndpoint)
+    ? data.filter(({key}) => key !== 'transaction')?.map(sumTagFacetsForTopValues)
+    : data;
+  const alphabeticalTags = tagList.sort((a, b) => a.key.localeCompare(b.key));
 
-    return (
-      <Container>
-        {alphabeticalTags.map((tag, tagIdx) => (
-          <TagItem key={tagIdx}>
-            <StyledPanel>
-              <PanelBody withPadding>
-                <TagHeading>
-                  <Link
-                    to={{
-                      pathname: `${baseUrl}tags/${tag.key}/`,
-                      query: extractSelectionParameters(location.query),
-                    }}
-                  >
-                    <span data-test-id="tag-title">{tag.key}</span>
-                  </Link>
-                </TagHeading>
-                <UnstyledUnorderedList>
-                  {tag.topValues.map((tagValue, tagValueIdx) => (
-                    <li key={tagValueIdx} data-test-id={tag.key}>
-                      <TagBarGlobalSelectionLink
-                        to={{
-                          pathname: `${baseUrl}events/`,
-                          query: {
-                            query: tagValue.query || `${tag.key}:"${tagValue.value}"`,
-                          },
-                        }}
-                      >
-                        <TagBarBackground
-                          widthPercent={percent(tagValue.count, tag.totalValues) + '%'}
-                        />
-                        <TagBarLabel>
-                          {tag.key === 'release' ? (
-                            <Version version={tagValue.name} anchor={false} />
-                          ) : (
-                            <DeviceName value={tagValue.name} />
-                          )}
-                        </TagBarLabel>
-                        <TagBarCount>
-                          <Count value={tagValue.count} />
-                        </TagBarCount>
-                      </TagBarGlobalSelectionLink>
-                    </li>
-                  ))}
-                </UnstyledUnorderedList>
-              </PanelBody>
-            </StyledPanel>
-          </TagItem>
-        ))}
-      </Container>
-    );
+  if (isLoading) {
+    return <LoadingIndicator />;
   }
 
-  renderBody() {
+  if (isError) {
     return (
-      <Layout.Body>
-        <Layout.Main fullWidth>
-          <Alert type="info">
-            {tct(
-              'Tags are automatically indexed for searching and breakdown charts. Learn how to [link: add custom tags to issues]',
-              {
-                link: (
-                  <ExternalLink href="https://docs.sentry.io/platform-redirect/?next=/enriching-events/tags" />
-                ),
-              }
-            )}
-          </Alert>
-          {this.renderTags()}
-        </Layout.Main>
-      </Layout.Body>
+      <LoadingError
+        message={t('There was an error loading issue tags.')}
+        onRetry={refetch}
+      />
     );
   }
+
+  const getTagKeyTarget = (tag: SimpleTag) => {
+    const pathname =
+      group.issueType === IssueType.PERFORMANCE_DURATION_REGRESSION
+        ? generateTagsRoute({orgSlug: organization.slug})
+        : `${baseUrl}tags/${tag.key}/`;
+
+    const query =
+      group.issueType === IssueType.PERFORMANCE_DURATION_REGRESSION
+        ? {
+            ...extractSelectionParameters(location.query),
+            start: (beforeDateTime as Date).toISOString(),
+            end: (afterDateTime as Date).toISOString(),
+            statsPeriod: undefined,
+            tagKey: tag.key,
+            transaction,
+          }
+        : extractSelectionParameters(location.query);
+
+    return {
+      pathname,
+      query,
+    };
+  };
+
+  return (
+    <Layout.Body>
+      <Layout.Main fullWidth>
+        <Alert type="info">
+          {tct(
+            'Tags are automatically indexed for searching and breakdown charts. Learn how to [link: add custom tags to issues]',
+            {
+              link: (
+                <ExternalLink href="https://docs.sentry.io/platform-redirect/?next=/enriching-events/tags" />
+              ),
+            }
+          )}
+        </Alert>
+        <Container>
+          {alphabeticalTags.map((tag, tagIdx) => (
+            <TagItem key={tagIdx}>
+              <StyledPanel>
+                <PanelBody withPadding>
+                  <TagHeading>
+                    <Link to={getTagKeyTarget(tag)}>
+                      <span data-test-id="tag-title">{tag.key}</span>
+                    </Link>
+                  </TagHeading>
+                  <UnstyledUnorderedList>
+                    {tag.topValues.map((tagValue, tagValueIdx) => (
+                      <li key={tagValueIdx} data-test-id={tag.key}>
+                        <TagBarGlobalSelectionLink
+                          to={{
+                            pathname: `${baseUrl}events/`,
+                            query: {
+                              query: tagValue.query || `${tag.key}:"${tagValue.value}"`,
+                            },
+                          }}
+                        >
+                          <TagBarBackground
+                            widthPercent={percent(tagValue.count, tag.totalValues) + '%'}
+                          />
+                          <TagBarLabel>
+                            {tag.key === 'release' ? (
+                              <Version version={tagValue.name} anchor={false} />
+                            ) : (
+                              <DeviceName value={tagValue.name} />
+                            )}
+                          </TagBarLabel>
+                          <TagBarCount>
+                            <Count value={tagValue.count} />
+                          </TagBarCount>
+                        </TagBarGlobalSelectionLink>
+                      </li>
+                    ))}
+                  </UnstyledUnorderedList>
+                </PanelBody>
+              </StyledPanel>
+            </TagItem>
+          ))}
+        </Container>
+      </Layout.Main>
+    </Layout.Body>
+  );
 }
 
 const Container = styled('div')`
@@ -266,4 +269,4 @@ const TagBarCount = styled('div')`
   font-variant-numeric: tabular-nums;
 `;
 
-export default withOrganization(GroupTags);
+export default GroupTags;