Browse Source

ref(feedback): use expand integrationIssues for list signals (#63785)

Uses the following changes to reduce the number of API calls that our
feedback list is calling to display our issue tracking signals:

- https://github.com/getsentry/sentry/pull/63530
- https://github.com/getsentry/sentry/pull/63403


More context:
This reduces the number of API calls needed for user feedback, which
makes the display of the user feedback external issues indicators more
efficient (the little GitHub/Jira/etc icons):


![image](https://github.com/getsentry/sentry/assets/56095982/982d79f7-334d-4f90-90ff-3fa616c11c79)
Michelle Zhang 1 year ago
parent
commit
fde6a1d323

+ 14 - 28
static/app/components/feedback/list/issueTrackingSignals.tsx

@@ -1,38 +1,23 @@
+import useExternalIssueDataFeedback from 'sentry/components/feedback/list/useHasLinkedIssues';
 import {
-  ExternalIssueComponent,
   IntegrationComponent,
   PluginActionComponent,
   PluginIssueComponent,
   SentryAppIssueComponent,
 } from 'sentry/components/group/externalIssuesList/types';
-import useExternalIssueData from 'sentry/components/group/externalIssuesList/useExternalIssueData';
 import {Tooltip} from 'sentry/components/tooltip';
 import {IconLink} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {Event, Group} from 'sentry/types';
-import {getIntegrationIcon} from 'sentry/utils/integrationUtil';
+import {
+  getIntegrationDisplayName,
+  getIntegrationIcon,
+} from 'sentry/utils/integrationUtil';
 
 interface Props {
   group: Group;
 }
 
-function filterLinkedPlugins(actions: ExternalIssueComponent[]) {
-  // Plugins: need to do some extra logic to detect if the issue is linked,
-  // by checking if there exists an issue object
-  const plugins = actions.filter(
-    a =>
-      (a.type === 'plugin-issue' || a.type === 'plugin-action') &&
-      'issue' in a.props.plugin
-  );
-
-  // Non plugins: can read directly from the `hasLinkedIssue` property
-  const nonPlugins = actions.filter(
-    a => a.hasLinkedIssue && !(a.type === 'plugin-issue' || a.type === 'plugin-action')
-  );
-
-  return plugins.concat(nonPlugins);
-}
-
 function getPluginNames(pluginIssue: PluginIssueComponent | PluginActionComponent) {
   return {
     name: pluginIssue.props.plugin.name ?? '',
@@ -41,13 +26,16 @@ function getPluginNames(pluginIssue: PluginIssueComponent | PluginActionComponen
 }
 
 function getIntegrationNames(integrationIssue: IntegrationComponent) {
-  if (!integrationIssue.props.configurations.length) {
-    return {name: '', icon: ''};
-  }
+  const icon = integrationIssue.props.externalIssue
+    ? integrationIssue.props.externalIssue.integrationKey
+    : '';
+  const name = integrationIssue.props.externalIssue
+    ? getIntegrationDisplayName(integrationIssue.props.externalIssue.integrationKey)
+    : '';
 
   return {
-    name: integrationIssue.props.configurations[0].provider.name ?? '',
-    icon: integrationIssue.key ?? '',
+    name,
+    icon,
   };
 }
 
@@ -59,14 +47,12 @@ function getAppIntegrationNames(integrationIssue: SentryAppIssueComponent) {
 }
 
 export default function IssueTrackingSignals({group}: Props) {
-  const {actions} = useExternalIssueData({
+  const {linkedIssues} = useExternalIssueDataFeedback({
     group,
     event: {} as Event,
     project: group.project,
   });
 
-  const linkedIssues = filterLinkedPlugins(actions);
-
   if (!linkedIssues.length) {
     return null;
   }

+ 130 - 0
static/app/components/feedback/list/useHasLinkedIssues.tsx

@@ -0,0 +1,130 @@
+import {ExternalIssueComponent} from 'sentry/components/group/externalIssuesList/types';
+import useFetchSentryAppData from 'sentry/components/group/externalIssuesList/useFetchSentryAppData';
+import useIssueTrackingFilter from 'sentry/components/group/externalIssuesList/useIssueTrackingFilter';
+import ExternalIssueStore from 'sentry/stores/externalIssueStore';
+import SentryAppInstallationStore from 'sentry/stores/sentryAppInstallationsStore';
+import {useLegacyStore} from 'sentry/stores/useLegacyStore';
+import type {Group, Project} from 'sentry/types';
+import type {Event} from 'sentry/types/event';
+import useOrganization from 'sentry/utils/useOrganization';
+import useSentryAppComponentsStore from 'sentry/utils/useSentryAppComponentsStore';
+
+type Props = {
+  event: Event;
+  group: Group;
+  project: Project;
+};
+
+export default function useExternalIssueData({group, event, project}: Props) {
+  const organization = useOrganization();
+  useFetchSentryAppData({group, organization}); // TODO: add this info onto the group directly
+  const issueTrackingFilter = useIssueTrackingFilter();
+  const components = useSentryAppComponentsStore({componentType: 'issue-link'});
+  const externalIssues = useLegacyStore(ExternalIssueStore);
+  const sentryAppInstallations = useLegacyStore(SentryAppInstallationStore);
+
+  const renderSentryAppIssues = (): ExternalIssueComponent[] => {
+    return components
+      .map<ExternalIssueComponent | null>(component => {
+        const {sentryApp, error: disabled} = component;
+        const installation = sentryAppInstallations.find(
+          i => i.app.uuid === sentryApp.uuid
+        );
+        // should always find a match but TS complains if we don't handle this case
+        if (!installation) {
+          return null;
+        }
+
+        const issue = (externalIssues || []).find(i => i.serviceType === sentryApp.slug);
+
+        return {
+          type: 'sentry-app-issue',
+          key: sentryApp.slug,
+          disabled,
+          hasLinkedIssue: !!issue,
+          props: {
+            sentryApp,
+            group,
+            organization,
+            event,
+            sentryAppComponent: component,
+            sentryAppInstallation: installation,
+            externalIssue: issue,
+            disabled,
+          },
+        };
+      })
+      .filter((x): x is ExternalIssueComponent => x !== null);
+  };
+
+  const renderIntegrationIssues = (): ExternalIssueComponent[] => {
+    return (
+      group.integrationIssues?.map(issue => ({
+        type: 'integration-issue',
+        key: issue.key,
+        disabled: false,
+        hasLinkedIssue: true,
+        props: {
+          configurations: [],
+          externalIssue: issue,
+          group,
+          onChange: () => {},
+        },
+      })) ?? []
+    );
+  };
+
+  const renderPluginIssues = (): ExternalIssueComponent[] => {
+    return group.pluginIssues?.map((plugin, i) => ({
+      type: 'plugin-issue',
+      key: `plugin-issue-${i}`,
+      disabled: false,
+      hasLinkedIssue: true,
+      props: {
+        group,
+        project,
+        plugin,
+      },
+    }));
+  };
+
+  const renderPluginActions = (): ExternalIssueComponent[] => {
+    return (
+      group.pluginActions?.map((plugin, i) => ({
+        type: 'plugin-action',
+        key: `plugin-action-${i}`,
+        disabled: false,
+        hasLinkedIssue: false,
+        props: {plugin},
+      })) ?? []
+    );
+  };
+
+  const linkedIssues = [
+    ...renderSentryAppIssues(),
+    ...renderIntegrationIssues(),
+    ...renderPluginIssues(),
+    ...renderPluginActions(),
+  ].filter(issue => !issueTrackingFilter || issue.key === issueTrackingFilter);
+
+  // Plugins: need to do some extra logic to detect if the issue is linked,
+  // by checking if there exists an issue object
+  const plugins = linkedIssues.filter(
+    a =>
+      (a.type === 'plugin-issue' || a.type === 'plugin-action') &&
+      'issue' in a.props.plugin
+  );
+
+  // Sentry app issues: read from `hasLinkedIssue` property
+  const sentryAppIssues = linkedIssues.filter(
+    a =>
+      a.hasLinkedIssue &&
+      a.type === 'sentry-app-issue' &&
+      a.props.externalIssue?.issueId === group.id
+  );
+
+  // Integration issues
+  const integrationIssues = linkedIssues.filter(a => a.type === 'integration-issue');
+
+  return {linkedIssues: plugins.concat(integrationIssues).concat(sentryAppIssues)};
+}

+ 1 - 0
static/app/components/feedback/useFeedbackListQueryKey.tsx

@@ -92,6 +92,7 @@ export default function useFeedbackListQueryKey({
                 'stats', // Gives us `firstSeen`
                 'pluginActions', // Gives us plugin actions available
                 'pluginIssues', // Gives us plugin issues available
+                'integrationIssues', // Gives us integration issues available
               ],
           shortIdLookup: 0,
           query: `issue.category:feedback status:${mailbox} ${fixedQueryView.query}`,

+ 2 - 0
static/app/components/group/externalIssuesList/types.tsx

@@ -1,4 +1,5 @@
 import type {
+  ExternalIssue,
   Group,
   GroupIntegration,
   Organization,
@@ -40,6 +41,7 @@ export interface IntegrationComponent extends BaseIssueComponent {
     configurations: GroupIntegration[];
     group: Group;
     onChange: () => void;
+    externalIssue?: ExternalIssue;
   };
   type: 'integration-issue';
 }

+ 7 - 3
static/app/types/group.tsx

@@ -715,9 +715,12 @@ export interface BaseGroup {
   participants: Array<UserParticipant | TeamParticipant>;
   permalink: string;
   platform: PlatformKey;
-  pluginActions: any[]; // TODO(ts)
-  pluginContexts: any[]; // TODO(ts)
-  pluginIssues: any[]; // TODO(ts)
+  pluginActions: any[];
+  // TODO(ts)
+  pluginContexts: any[];
+  // TODO(ts)
+  pluginIssues: any[];
+  // TODO(ts)
   project: Project;
   seenBy: User[];
   shareId: string;
@@ -729,6 +732,7 @@ export interface BaseGroup {
   type: EventOrGroupType;
   userReportCount: number;
   inbox?: InboxDetails | null | false;
+  integrationIssues?: any[];
   latestEvent?: Event;
   owners?: SuggestedOwner[] | null;
   substatus?: GroupSubstatus | null;

+ 10 - 0
static/app/types/integrations.tsx

@@ -420,6 +420,16 @@ export type PlatformExternalIssue = {
   webUrl: string;
 };
 
+export type ExternalIssue = {
+  description: string;
+  displayName: string;
+  id: string;
+  integrationKey: string;
+  integrationName: string;
+  key: string;
+  title: string;
+};
+
 /**
  * The issue config form fields we get are basically the form fields we use in
  * the UI but with some extra information. Some fields marked optional in the

+ 23 - 0
static/app/utils/integrationUtil.tsx

@@ -211,6 +211,29 @@ export const getIntegrationIcon = (
   }
 };
 
+export const getIntegrationDisplayName = (integrationType?: string) => {
+  switch (integrationType) {
+    case 'asana':
+      return 'Asana';
+    case 'bitbucket':
+      return 'Bitbucket';
+    case 'gitlab':
+      return 'GitLab';
+    case 'github':
+    case 'github_enterprise':
+      return 'GitHub';
+    case 'jira':
+    case 'jira_server':
+      return 'Jira';
+    case 'vsts':
+      return 'VSTS';
+    case 'codecov':
+      return 'Codeov';
+    default:
+      return '';
+  }
+};
+
 export const getIntegrationSourceUrl = (
   integrationType: string,
   sourceUrl: string,