Browse Source

feat(related_issues): Create separate sections for each relation type (#70648)

In #70504 I added support for the `type` parameter which allows getting
the data for same-root issues vs trace-connected issues separately.

This UI change allows each related issues section to fetch and render
the data independently, thus, making the page load faster.

These changes are required to support fetching trace-connected issues
from the Issues Details page without also fetching same-root-connected
data.
Armen Zambrano G 10 months ago
parent
commit
f80216c4e8

+ 34 - 44
static/app/views/issueDetails/groupRelatedIssues/index.spec.tsx

@@ -6,7 +6,8 @@ import {render, screen} from 'sentry-test/reactTestingLibrary';
 import {GroupRelatedIssues} from 'sentry/views/issueDetails/groupRelatedIssues';
 import {GroupRelatedIssues} from 'sentry/views/issueDetails/groupRelatedIssues';
 
 
 describe('Related Issues View', function () {
 describe('Related Issues View', function () {
-  let relatedIssuesMock: jest.Mock;
+  let sameRootIssuesMock: jest.Mock;
+  let traceIssuesMock: jest.Mock;
   let issuesMock: jest.Mock;
   let issuesMock: jest.Mock;
   const router = RouterFixture();
   const router = RouterFixture();
 
 
@@ -21,44 +22,20 @@ describe('Related Issues View', function () {
   const params = {groupId: groupId};
   const params = {groupId: groupId};
   const errorType = 'RuntimeError';
   const errorType = 'RuntimeError';
   const noData = {
   const noData = {
-    data: [
-      {
-        type: 'same_root_cause',
-        data: [],
-      },
-      {
-        type: 'trace_connected',
-        data: [],
-      },
-    ],
+    type: 'irrelevant',
+    data: [],
   };
   };
   const onlySameRootData = {
   const onlySameRootData = {
-    data: [
-      {
-        type: 'same_root_cause',
-        data: [group1, group2],
-      },
-      {
-        type: 'trace_connected',
-        data: [],
-      },
-    ],
+    type: 'same_root_cause',
+    data: [group1, group2],
   };
   };
   const onlyTraceConnectedData = {
   const onlyTraceConnectedData = {
-    data: [
-      {
-        type: 'same_root_cause',
-        data: [],
-      },
-      {
-        type: 'trace_connected',
-        data: [group1, group2],
-        meta: {
-          event_id: 'abcd',
-          trace_id: '1234',
-        },
-      },
-    ],
+    type: 'trace_connected',
+    data: [group1, group2],
+    meta: {
+      event_id: 'abcd',
+      trace_id: '1234',
+    },
   };
   };
   const issuesData = [
   const issuesData = [
     {
     {
@@ -99,8 +76,12 @@ describe('Related Issues View', function () {
   });
   });
 
 
   it('renders with no data', async function () {
   it('renders with no data', async function () {
-    relatedIssuesMock = MockApiClient.addMockResponse({
-      url: `/issues/${groupId}/related-issues/`,
+    sameRootIssuesMock = MockApiClient.addMockResponse({
+      url: `/issues/${groupId}/related-issues/?type=same_root_cause`,
+      body: noData,
+    });
+    traceIssuesMock = MockApiClient.addMockResponse({
+      url: `/issues/${groupId}/related-issues/?type=trace_connected`,
       body: noData,
       body: noData,
     });
     });
     render(
     render(
@@ -121,14 +102,19 @@ describe('Related Issues View', function () {
       await screen.findByText('No trace-connected related issues were found.')
       await screen.findByText('No trace-connected related issues were found.')
     ).toBeInTheDocument();
     ).toBeInTheDocument();
 
 
-    expect(relatedIssuesMock).toHaveBeenCalled();
+    expect(sameRootIssuesMock).toHaveBeenCalled();
+    expect(traceIssuesMock).toHaveBeenCalled();
   });
   });
 
 
   it('renders with same root issues', async function () {
   it('renders with same root issues', async function () {
-    relatedIssuesMock = MockApiClient.addMockResponse({
-      url: `/issues/${groupId}/related-issues/`,
+    sameRootIssuesMock = MockApiClient.addMockResponse({
+      url: `/issues/${groupId}/related-issues/?type=same_root_cause`,
       body: onlySameRootData,
       body: onlySameRootData,
     });
     });
+    MockApiClient.addMockResponse({
+      url: `/issues/${groupId}/related-issues/?type=trace_connected`,
+      body: [],
+    });
     issuesMock = MockApiClient.addMockResponse({
     issuesMock = MockApiClient.addMockResponse({
       url: orgIssuesEndpoint,
       url: orgIssuesEndpoint,
       body: issuesData,
       body: issuesData,
@@ -149,7 +135,7 @@ describe('Related Issues View', function () {
     expect(await screen.findByText(`EARTH-${group1}`)).toBeInTheDocument();
     expect(await screen.findByText(`EARTH-${group1}`)).toBeInTheDocument();
     expect(await screen.findByText(`EARTH-${group2}`)).toBeInTheDocument();
     expect(await screen.findByText(`EARTH-${group2}`)).toBeInTheDocument();
 
 
-    expect(relatedIssuesMock).toHaveBeenCalled();
+    expect(sameRootIssuesMock).toHaveBeenCalled();
     expect(issuesMock).toHaveBeenCalled();
     expect(issuesMock).toHaveBeenCalled();
     expect(
     expect(
       await screen.findByText('No trace-connected related issues were found.')
       await screen.findByText('No trace-connected related issues were found.')
@@ -163,8 +149,12 @@ describe('Related Issues View', function () {
   });
   });
 
 
   it('renders with trace connected issues', async function () {
   it('renders with trace connected issues', async function () {
-    relatedIssuesMock = MockApiClient.addMockResponse({
-      url: `/issues/${groupId}/related-issues/`,
+    MockApiClient.addMockResponse({
+      url: `/issues/${groupId}/related-issues/?type=same_root_cause`,
+      body: [],
+    });
+    traceIssuesMock = MockApiClient.addMockResponse({
+      url: `/issues/${groupId}/related-issues/?type=trace_connected`,
       body: onlyTraceConnectedData,
       body: onlyTraceConnectedData,
     });
     });
     issuesMock = MockApiClient.addMockResponse({
     issuesMock = MockApiClient.addMockResponse({
@@ -186,7 +176,7 @@ describe('Related Issues View', function () {
     expect(await screen.findByText(`EARTH-${group1}`)).toBeInTheDocument();
     expect(await screen.findByText(`EARTH-${group1}`)).toBeInTheDocument();
     expect(await screen.findByText(`EARTH-${group2}`)).toBeInTheDocument();
     expect(await screen.findByText(`EARTH-${group2}`)).toBeInTheDocument();
 
 
-    expect(relatedIssuesMock).toHaveBeenCalled();
+    expect(traceIssuesMock).toHaveBeenCalled();
     expect(issuesMock).toHaveBeenCalled();
     expect(issuesMock).toHaveBeenCalled();
     expect(
     expect(
       await screen.findByText('No same-root-cause related issues were found.')
       await screen.findByText('No same-root-cause related issues were found.')

+ 99 - 100
static/app/views/issueDetails/groupRelatedIssues/index.tsx

@@ -19,59 +19,109 @@ type RouteParams = {
 type Props = RouteComponentProps<RouteParams, {}>;
 type Props = RouteComponentProps<RouteParams, {}>;
 
 
 type RelatedIssuesResponse = {
 type RelatedIssuesResponse = {
-  data: [
-    {
-      data: number[];
-      meta: {
-        event_id: string;
-        trace_id: string;
-      };
-      type: string;
-    },
-  ];
+  data: number[];
+  meta: {
+    event_id: string;
+    trace_id: string;
+  };
+  type: string;
 };
 };
 
 
+interface RelatedIssuesSectionProps {
+  groupId: string;
+  orgSlug: string;
+  relationType: string;
+}
+
 function GroupRelatedIssues({params}: Props) {
 function GroupRelatedIssues({params}: Props) {
   const {groupId} = params;
   const {groupId} = params;
 
 
   const organization = useOrganization();
   const organization = useOrganization();
   const orgSlug = organization.slug;
   const orgSlug = organization.slug;
 
 
+  return (
+    <Fragment>
+      <RelatedIssuesSection
+        groupId={groupId}
+        orgSlug={orgSlug}
+        relationType="same_root_cause"
+      />
+      <RelatedIssuesSection
+        groupId={groupId}
+        orgSlug={orgSlug}
+        relationType="trace_connected"
+      />
+    </Fragment>
+  );
+}
+
+function RelatedIssuesSection({
+  groupId,
+  orgSlug,
+  relationType,
+}: RelatedIssuesSectionProps) {
   // Fetch the list of related issues
   // Fetch the list of related issues
   const {
   const {
     isLoading,
     isLoading,
     isError,
     isError,
     data: relatedIssues,
     data: relatedIssues,
     refetch,
     refetch,
-  } = useApiQuery<RelatedIssuesResponse>([`/issues/${groupId}/related-issues/`], {
-    staleTime: 0,
-  });
-
-  let traceMeta = {
-    trace_id: '',
-    event_id: '',
-  };
-  const {
-    same_root_cause: sameRootCauseIssues = [],
-    trace_connected: traceConnectedIssues = [],
-  } = (relatedIssues?.data ?? []).reduce(
-    (mapping, item) => {
-      if (item.type === 'trace_connected') {
-        traceMeta = {...item.meta};
-      }
-      const issuesList = item.data;
-      mapping[item.type] = issuesList;
-      return mapping;
-    },
-    {same_root_cause: [], trace_connected: []}
+  } = useApiQuery<RelatedIssuesResponse>(
+    [`/issues/${groupId}/related-issues/?type=${relationType}`],
+    {
+      staleTime: 0,
+    }
   );
   );
 
 
+  const traceMeta = relationType === 'trace_connected' ? relatedIssues?.meta : undefined;
+  const issues = relatedIssues?.data ?? [];
+  const query = `issue.id:[${issues}]`;
   // project=-1 allows ensuring that the query will show issues from any projects for the org
   // project=-1 allows ensuring that the query will show issues from any projects for the org
   // This is important for traces since issues can be for any project in the org
   // This is important for traces since issues can be for any project in the org
   const baseUrl = `/organizations/${orgSlug}/issues/?project=-1`;
   const baseUrl = `/organizations/${orgSlug}/issues/?project=-1`;
+  let title;
+  let linkToTrace;
+  let openIssuesButton;
+  if (relationType === 'trace_connected' && traceMeta) {
+    title = t('Issues in the same trace');
+    linkToTrace = (
+      <small>
+        {t('These issues were all found within ')}
+        <Link
+          to={`/organizations/${orgSlug}/performance/trace/${traceMeta.trace_id}/?node=error-${traceMeta.event_id}`}
+        >
+          {t('this trace')}
+        </Link>
+        .
+      </small>
+    );
+    openIssuesButton = (
+      <LinkButton
+        to={`${baseUrl}&query=trace:${traceMeta.trace_id}`}
+        size="xs"
+        analyticsEventName="Clicked Open Issues from trace-connected related issues"
+        analyticsEventKey="similar_issues.trace_connected_issues_clicked_open_issues"
+      >
+        {t('Open in Issues')}
+      </LinkButton>
+    );
+  } else {
+    title = t('Issues caused by the same root cause');
+    openIssuesButton = (
+      <LinkButton
+        to={`${baseUrl}&query=issue.id:[${groupId},${issues}]`}
+        size="xs"
+        analyticsEventName="Clicked Open Issues from same-root related issues"
+        analyticsEventKey="similar_issues.same_root_cause_clicked_open_issues"
+      >
+        {t('Open in Issues')}
+      </LinkButton>
+    );
+  }
 
 
   return (
   return (
-    <Fragment>
+    <HeaderWrapper>
+      <Title>{title}</Title>
       {isLoading ? (
       {isLoading ? (
         <LoadingIndicator />
         <LoadingIndicator />
       ) : isError ? (
       ) : isError ? (
@@ -79,77 +129,26 @@ function GroupRelatedIssues({params}: Props) {
           message={t('Unable to load related issues, please try again later')}
           message={t('Unable to load related issues, please try again later')}
           onRetry={refetch}
           onRetry={refetch}
         />
         />
-      ) : (
+      ) : issues.length > 0 ? (
         <Fragment>
         <Fragment>
-          <div>
-            <HeaderWrapper>
-              <Title>{t('Issues caused by the same root cause')}</Title>
-              {sameRootCauseIssues.length > 0 ? (
-                <div>
-                  <TextButtonWrapper>
-                    <div />
-                    <LinkButton
-                      to={`${baseUrl}&query=issue.id:[${groupId},${sameRootCauseIssues}]`}
-                      size="xs"
-                      analyticsEventName="Clicked Open Issues from same-root related issues"
-                      analyticsEventKey="similar_issues.same_root_cause_clicked_open_issues"
-                    >
-                      {t('Open in Issues')}
-                    </LinkButton>
-                  </TextButtonWrapper>
-                  <GroupList
-                    orgSlug={orgSlug}
-                    queryParams={{query: `issue.id:[${sameRootCauseIssues}]`}}
-                    source="similar-issues-tab"
-                    canSelectGroups={false}
-                    withChart={false}
-                  />
-                </div>
-              ) : (
-                <small>{t('No same-root-cause related issues were found.')}</small>
-              )}
-            </HeaderWrapper>
-          </div>
-          <div>
-            <HeaderWrapper>
-              <Title>{t('Issues in the same trace')}</Title>
-              {traceConnectedIssues.length > 0 ? (
-                <div>
-                  <TextButtonWrapper>
-                    <small>
-                      {t('These issues were all found within ')}
-                      <Link
-                        to={`/organizations/${orgSlug}/performance/trace/${traceMeta.trace_id}/?node=error-${traceMeta.event_id}`}
-                      >
-                        {t('this trace')}
-                      </Link>
-                      .
-                    </small>
-                    <LinkButton
-                      to={`${baseUrl}&query=trace:${traceMeta.trace_id}`}
-                      size="xs"
-                      analyticsEventName="Clicked Open Issues from trace-connected related issues"
-                      analyticsEventKey="similar_issues.trace_connected_issues_clicked_open_issues"
-                    >
-                      {t('Open in Issues')}
-                    </LinkButton>
-                  </TextButtonWrapper>
-                  <GroupList
-                    orgSlug={orgSlug}
-                    queryParams={{query: `issue.id:[${traceConnectedIssues}]`}}
-                    source="similar-issues-tab"
-                    canSelectGroups={false}
-                    withChart={false}
-                  />
-                </div>
-              ) : (
-                <small>{t('No trace-connected related issues were found.')}</small>
-              )}
-            </HeaderWrapper>
-          </div>
+          <TextButtonWrapper>
+            {linkToTrace ?? null}
+            {openIssuesButton ?? null}
+          </TextButtonWrapper>
+          <GroupList
+            orgSlug={orgSlug}
+            queryParams={{query: query}}
+            source="similar-issues-tab"
+            canSelectGroups={false}
+            withChart={false}
+          />
         </Fragment>
         </Fragment>
+      ) : relationType === 'trace_connected' ? (
+        <small>{t('No trace-connected related issues were found.')}</small>
+      ) : (
+        <small>{t('No same-root-cause related issues were found.')}</small>
       )}
       )}
-    </Fragment>
+    </HeaderWrapper>
   );
   );
 }
 }