ref(issue-stream): Refactor issue stream action logic with better typ…

…ing and flexibility (getsentry#65443) Before adding priority to the list of issue actions that remove issues for the stream (getsentry#65212), I wanted to do some cleanup since there is a lot of existing logic here that can be simplified and typed better. Some of the changes: - The "For Review" tab and other issue actions were using separate code paths, I've combined them. This means that `onMarkReviewed` was removed, and the `removeItems` state which was only used by that code path was also removed. - Removed the `actionTakenGroupData` state. Instead of storing that in state, we send it in through the `onActionTaken` callback.
fubop · Feb 20, 2024 · fc44acc · fc44acc
1 parent 0219a51
commit fc44acc
Show file tree

Hide file tree

Showing 8 changed files with 435 additions and 135 deletions.
diff --git a/static/app/utils/analytics/issueAnalyticsEvents.tsx b/static/app/utils/analytics/issueAnalyticsEvents.tsx
@@ -191,7 +191,7 @@ export type IssueEventParameters = {
   };
   'issues_stream.archived': {
     action_status_details?: string;
-    action_substatus?: string;
+    action_substatus?: string | null;
   };
   'issues_stream.issue_assigned': IssueStream & {
     assigned_type: string;

diff --git a/static/app/views/issueList/actions/actionSet.tsx b/static/app/views/issueList/actions/actionSet.tsx
@@ -18,6 +18,7 @@ import type {IssueTypeConfig} from 'sentry/utils/issueTypeConfig/types';
 import Projects from 'sentry/utils/projects';
 import useMedia from 'sentry/utils/useMedia';
 import useOrganization from 'sentry/utils/useOrganization';
+import type {IssueUpdateData} from 'sentry/views/issueList/types';
 
 import ResolveActions from './resolveActions';
 import ReviewAction from './reviewAction';
@@ -31,7 +32,7 @@ type Props = {
   onDelete: () => void;
   onMerge: () => void;
   onShouldConfirm: (action: ConfirmAction) => boolean;
-  onUpdate: (data?: any) => void;
+  onUpdate: (data: IssueUpdateData) => void;
   query: string;
   queryCount: number;
   selectedProjectSlug?: string;
@@ -165,7 +166,7 @@ function ActionSet({
       onAction: () => {
         openConfirmModal({
           bypass: !onShouldConfirm(ConfirmAction.UNRESOLVE),
-          onConfirm: () => onUpdate({status: GroupStatus.UNRESOLVED}),
+          onConfirm: () => onUpdate({status: GroupStatus.UNRESOLVED, statusDetails: {}}),
           message: confirm({action: ConfirmAction.UNRESOLVE, canBeUndone: true}),
           confirmText: label('unresolve'),
         });
@@ -225,7 +226,8 @@ function ActionSet({
           onClick={() => {
             openConfirmModal({
               bypass: !onShouldConfirm(ConfirmAction.UNRESOLVE),
-              onConfirm: () => onUpdate({status: GroupStatus.UNRESOLVED}),
+              onConfirm: () =>
+                onUpdate({status: GroupStatus.UNRESOLVED, statusDetails: {}}),
               message: confirm({action: ConfirmAction.UNRESOLVE, canBeUndone: true}),
               confirmText: label('unarchive'),
             });

diff --git a/static/app/views/issueList/actions/index.spec.tsx b/static/app/views/issueList/actions/index.spec.tsx
@@ -300,7 +300,7 @@ describe('IssueListActions', function () {
       expect.anything(),
       expect.objectContaining({
         query: expect.objectContaining({id: ['1'], project: [1]}),
-        data: {status: 'unresolved'},
+        data: {status: 'unresolved', statusDetails: {}},
       })
     );
   });
@@ -327,7 +327,7 @@ describe('IssueListActions', function () {
 
   describe('mark reviewed', function () {
     it('acknowledges group', async function () {
-      const mockOnMarkReviewed = jest.fn();
+      const mockOnActionTaken = jest.fn();
 
       MockApiClient.addMockResponse({
         url: '/organizations/org-slug/issues/',
@@ -347,13 +347,13 @@ describe('IssueListActions', function () {
           },
         });
       });
-      render(<WrappedComponent onMarkReviewed={mockOnMarkReviewed} />);
+      render(<WrappedComponent onActionTaken={mockOnActionTaken} />);
 
       const reviewButton = screen.getByRole('button', {name: 'Mark Reviewed'});
       expect(reviewButton).toBeEnabled();
       await userEvent.click(reviewButton);
 
-      expect(mockOnMarkReviewed).toHaveBeenCalledWith(['1', '2', '3']);
+      expect(mockOnActionTaken).toHaveBeenCalledWith(['1', '2', '3'], {inbox: false});
     });
 
     it('mark reviewed disabled for group that is already reviewed', function () {

diff --git a/static/app/views/issueList/actions/index.tsx b/static/app/views/issueList/actions/index.tsx
@@ -19,6 +19,7 @@ import useApi from 'sentry/utils/useApi';
 import useMedia from 'sentry/utils/useMedia';
 import useOrganization from 'sentry/utils/useOrganization';
 import {useSyncedLocalStorageState} from 'sentry/utils/useSyncedLocalStorageState';
+import type {IssueUpdateData} from 'sentry/views/issueList/types';
 import {SAVED_SEARCHES_SIDEBAR_OPEN_LOCALSTORAGE_KEY} from 'sentry/views/issueList/utils';
 
 import ActionSet from './actionSet';
@@ -38,8 +39,7 @@ type IssueListActionsProps = {
   selection: PageFilters;
   sort: string;
   statsPeriod: string;
-  onActionTaken?: (itemIds: string[]) => void;
-  onMarkReviewed?: (itemIds: string[]) => void;
+  onActionTaken?: (itemIds: string[], data: IssueUpdateData) => void;
 };
 
 function IssueListActions({
@@ -48,7 +48,6 @@ function IssueListActions({
   groupIds,
   onActionTaken,
   onDelete,
-  onMarkReviewed,
   onSelectStatsPeriod,
   onSortChange,
   queryCount,
@@ -142,15 +141,16 @@ function IssueListActions({
     });
   }
 
-  function handleUpdate(data?: any) {
-    if (data.status === 'ignored') {
-      const statusDetails = data.statusDetails.ignoreCount
-        ? 'ignoreCount'
-        : data.statusDetails.ignoreDuration
-          ? 'ignoreDuration'
-          : data.statusDetails.ignoreUserCount
-            ? 'ignoreUserCount'
-            : undefined;
+  function handleUpdate(data: IssueUpdateData) {
+    if ('status' in data && data.status === 'ignored') {
+      const statusDetails =
+        'ignoreCount' in data.statusDetails
+          ? 'ignoreCount'
+          : 'ignoreDuration' in data.statusDetails
+            ? 'ignoreDuration'
+            : 'ignoreUserCount' in data.statusDetails
+              ? 'ignoreUserCount'
+              : undefined;
       trackAnalytics('issues_stream.archived', {
         action_status_details: statusDetails,
         action_substatus: data.substatus,
@@ -159,12 +159,6 @@ function IssueListActions({
     }
 
     actionSelectedGroups(itemIds => {
-      if (data?.inbox === false) {
-        onMarkReviewed?.(itemIds ?? []);
-      }
-
-      onActionTaken?.(itemIds ?? []);
-
       // If `itemIds` is undefined then it means we expect to bulk update all items
       // that match the query.
       //
@@ -184,7 +178,11 @@ function IssueListActions({
           ...projectConstraints,
           ...selection.datetime,
         },
-        {}
+        {
+          complete: () => {
+            onActionTaken?.(itemIds ?? [], data);
+          },
+        }
       );
     });
   }

diff --git a/static/app/views/issueList/actions/reviewAction.tsx b/static/app/views/issueList/actions/reviewAction.tsx
@@ -2,9 +2,10 @@ import ActionLink from 'sentry/components/actions/actionLink';
 import type {TooltipProps} from 'sentry/components/tooltip';
 import {IconIssues} from 'sentry/icons';
 import {t} from 'sentry/locale';
+import type {IssueUpdateData} from 'sentry/views/issueList/types';
 
 type Props = {
-  onUpdate: (data: {inbox: boolean}) => void;
+  onUpdate: (data: IssueUpdateData) => void;
   disabled?: boolean;
   tooltip?: string;
   tooltipProps?: Omit<TooltipProps, 'children' | 'title' | 'skipWrapper'>;