Repository: getsentry/sentry
Pull request: fix(rulesnooze): Change default mute for integrations
this pr changes how we deal with muting alerts depending on their rule actions. we created 3 categories that an alert rule can fall into. 

1. All the actions are the default (sentry.mail.actions.NotifyEmailAction) 
2. Some of the actions are the default while some are other integrations (Slack, Jira, etc.)
3. All of the actions are other integrations. 

We created these categories because muting an alert means different things for each of them. The main difference is that if an integration action is being used, mute for me is the only way to stop notifications for that action (what does mute for me for a slack notification going to an entire channel mean?). So if an integration action exists, we will make mute for everyone the default mute. If they are all integration actions, then we don't want to provide mute for me as an option while if there are still some of the default notifications, we do want to provide that (muting for me in this situation would only silence the non-integration notifications). 

Scenario 1: 
<img width="1492" alt="Screenshot 2023-05-24 at 1 46 13 PM" src="https://github.com/getsentry/sentry/assets/46740234/f9380501-4d8f-4b5a-8e01-a2da6847b953">

Scenario 2:
<img width="1494" alt="Screenshot 2023-05-24 at 1 40 40 PM" src="https://github.com/getsentry/sentry/assets/46740234/91db26d5-60d8-44c1-b627-45ca5b007d5f">


Scenario 3:
<img width="1487" alt="Screenshot 2023-05-24 at 1 45 43 PM" src="https://github.com/getsentry/sentry/assets/46740234/00d8e98c-0c01-43eb-b6a3-663f12f33aef">

Closes https://github.com/getsentry/sentry/issues/49494



Problem statement:
Change default mute to mute for everyone if alert action is Slack
If you just click "mute" on the alert details page, the alert will be muted just for the user, as opposed to muted for everyone (which is in the dropdown). For alerts that have actions that directly go to a Slack channel or Slack user (e.g. "Send a notification to the {workspace} Slack workspace to {channel or user}") the only mute available should be "mute for everyone".

We can potentially change the button and banner language if the alert action is going to a user, since "mute for everyone" sounds a little odd (but on the backend that's how we need to process it) if that's not too much trouble.

If the alert has multiple actions this might get a bit hairy. If the actions go to multiple Slack channels, keep as "mute for everyone". If the actions go to a bunch of different places, maybe just keep the current behavior and let the user figure it out.

Ensure the link in the emails obey this behavior as well.

Hints from issue discussion:

Commit message: fix test

Patch to review:
diff --git a/static/app/components/alerts/snoozeAlert.tsx b/static/app/components/alerts/snoozeAlert.tsx
index 4251d1fc1abd1c..e7316dc699b845 100644
--- a/static/app/components/alerts/snoozeAlert.tsx
+++ b/static/app/components/alerts/snoozeAlert.tsx
@@ -8,6 +8,7 @@ import ButtonBar from 'sentry/components/buttonBar';
 import {DropdownMenu, MenuItemProps} from 'sentry/components/dropdownMenu';
 import {IconChevron, IconMute, IconSound} from 'sentry/icons';
 import {t} from 'sentry/locale';
+import {RuleActionsCategories} from 'sentry/types/alerts';
 import useApi from 'sentry/utils/useApi';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
@@ -21,10 +22,18 @@ type Props = {
     snoozeForEveryone?: boolean;
   }) => void;
   projectSlug: string;
+  ruleActionCategory: RuleActionsCategories;
   ruleId: string;
 };
 
-function SnoozeAlert({isSnoozed, onSnooze, projectSlug, ruleId, hasAccess}: Props) {
+function SnoozeAlert({
+  isSnoozed,
+  onSnooze,
+  projectSlug,
+  ruleId,
+  ruleActionCategory,
+  hasAccess,
+}: Props) {
   const organization = useOrganization();
   const api = useApi();
   const location = useLocation();
@@ -106,20 +115,24 @@ function SnoozeAlert({isSnoozed, onSnooze, projectSlug, ruleId, hasAccess}: Prop
 
   useEffect(() => {
     if (location.query.mute === '1' && !isSnoozed) {
-      handleMute('me', true);
+      const target =
+        ruleActionCategory !== RuleActionsCategories.AllDefault ? `everyone` : `me`;
+      handleMute(target, true);
     }
-  }, [location.query, isSnoozed, handleMute]);
+  }, [location.query, isSnoozed, handleMute, ruleActionCategory]);
 
   const dropdownItems: MenuItemProps[] = [
     {
       key: 'me',
       label: t('Mute for me'),
       onAction: () => handleMute('me'),
+      hidden: ruleActionCategory !== RuleActionsCategories.SomeDefault,
     },
     {
       key: 'everyone',
       label: t('Mute for everyone'),
       onAction: () => handleMute('everyone'),
+      hidden: ruleActionCategory !== RuleActionsCategories.AllDefault,
     },
   ];
 
@@ -141,22 +154,31 @@ function SnoozeAlert({isSnoozed, onSnooze, projectSlug, ruleId, hasAccess}: Prop
         size="sm"
         icon={<IconSound />}
         disabled={disabled || !hasAccess}
-        onClick={() => handleMute('me')}
+        hasDropdown={ruleActionCategory !== RuleActionsCategories.NoDefault}
+        onClick={() => {
+          const target =
+            ruleActionCategory !== RuleActionsCategories.AllDefault ? 'everyone' : 'me';
+          handleMute(target);
+        }}
       >
-        {t('Mute')}
+        {ruleActionCategory === RuleActionsCategories.AllDefault
+          ? t('Mute for me')
+          : t('Mute for everyone')}
       </MuteButton>
-      <DropdownMenu
-        size="sm"
-        trigger={triggerProps => (
-          <DropdownTrigger
-            {...triggerProps}
-            aria-label={t('Mute alert options')}
-            icon={<IconChevron direction="down" size="xs" />}
-          />
-        )}
-        items={dropdownItems}
-        isDisabled={disabled || !hasAccess}
-      />
+      {ruleActionCategory !== RuleActionsCategories.NoDefault && (
+        <DropdownMenu
+          size="sm"
+          trigger={triggerProps => (
+            <DropdownTrigger
+              {...triggerProps}
+              aria-label={t('Mute alert options')}
+              icon={<IconChevron direction="down" size="xs" />}
+            />
+          )}
+          items={dropdownItems}
+          isDisabled={disabled || !hasAccess}
+        />
+      )}
     </ButtonBar>
   );
 }
@@ -169,7 +191,8 @@ const DropdownTrigger = styled(Button)`
   border-left: none;
 `;
 
-const MuteButton = styled(Button)`
+const MuteButton = styled(Button)<{hasDropdown: boolean}>`
   box-shadow: none;
-  border-radius: ${p => p.theme.borderRadiusLeft};
+  border-radius: ${p =>
+    p.hasDropdown ? p.theme.borderRadiusLeft : p.theme.borderRadius};
 `;
diff --git a/static/app/types/alerts.tsx b/static/app/types/alerts.tsx
index 011f9a92acf037..e2631a709ec987 100644
--- a/static/app/types/alerts.tsx
+++ b/static/app/types/alerts.tsx
@@ -112,3 +112,9 @@ export type NoteType = {
   mentions: string[];
   text: string;
 };
+
+export enum RuleActionsCategories {
+  AllDefault = 'all_default',
+  SomeDefault = 'some_default',
+  NoDefault = 'no_default',
+}
diff --git a/static/app/views/alerts/rules/issue/details/ruleDetails.spec.tsx b/static/app/views/alerts/rules/issue/details/ruleDetails.spec.tsx
index add3c7cfd99cec..ad975cb1bc2b01 100644
--- a/static/app/views/alerts/rules/issue/details/ruleDetails.spec.tsx
+++ b/static/app/views/alerts/rules/issue/details/ruleDetails.spec.tsx
@@ -198,12 +198,12 @@ describe('AlertRuleDetails', () => {
       method: 'DELETE',
     });
     createWrapper();
-    expect(await screen.findByText('Mute')).toBeInTheDocument();
+    expect(await screen.findByText('Mute for everyone')).toBeInTheDocument();
 
-    await userEvent.click(screen.getByRole('button', {name: 'Mute'}));
+    await userEvent.click(screen.getByRole('button', {name: 'Mute for everyone'}));
     expect(postRequest).toHaveBeenCalledWith(
       expect.anything(),
-      expect.objectContaining({data: {target: 'me'}})
+      expect.objectContaining({data: {target: 'everyone'}})
     );
 
     expect(await screen.findByText('Unmute')).toBeInTheDocument();
@@ -230,7 +230,7 @@ describe('AlertRuleDetails', () => {
     expect(request).toHaveBeenCalledWith(
       expect.anything(),
       expect.objectContaining({
-        data: {target: 'me'},
+        data: {target: 'everyone'},
       })
     );
   });
@@ -247,7 +247,7 @@ describe('AlertRuleDetails', () => {
 
     createWrapper({}, contextWithoutAccess, orgWithoutAccess);
 
-    expect(await screen.findByRole('button', {name: 'Mute'})).toBeDisabled();
+    expect(await screen.findByRole('button', {name: 'Mute for everyone'})).toBeDisabled();
   });
 
   it('inserts user email into rule notify action', async () => {
diff --git a/static/app/views/alerts/rules/issue/details/ruleDetails.tsx b/static/app/views/alerts/rules/issue/details/ruleDetails.tsx
index 0c7002ad846b30..f20a5cd8bd184f 100644
--- a/static/app/views/alerts/rules/issue/details/ruleDetails.tsx
+++ b/static/app/views/alerts/rules/issue/details/ruleDetails.tsx
@@ -26,6 +26,7 @@ import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {DateString} from 'sentry/types';
 import type {IssueAlertRule} from 'sentry/types/alerts';
+import {RuleActionsCategories} from 'sentry/types/alerts';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import {
   ApiQueryKey,
@@ -215,6 +216,17 @@ function AlertRuleDetails({params, location, router}: AlertRuleDetailsProps) {
   const hasSnoozeFeature = organization.features.includes('mute-alerts');
   const isSnoozed = rule.snooze;
 
+  const numDefaultActions = rule.actions.filter(
+    action => action.id === 'sentry.mail.actions.NotifyEmailAction'
+  ).length;
+
+  const ruleActionCategory =
+    numDefaultActions === rule.actions.length
+      ? RuleActionsCategories.AllDefault
+      : numDefaultActions === 0
+      ? RuleActionsCategories.NoDefault
+      : RuleActionsCategories.SomeDefault;
+
   const duplicateLink = {
     pathname: `/organizations/${organization.slug}/alerts/new/issue/`,
     query: {
@@ -224,7 +236,6 @@ function AlertRuleDetails({params, location, router}: AlertRuleDetailsProps) {
       referrer: 'issue_rule_details',
     },
   };
-
   function renderIncompatibleAlert() {
     const incompatibleRule = findIncompatibleRules(rule);
     if (
@@ -298,6 +309,7 @@ function AlertRuleDetails({params, location, router}: AlertRuleDetailsProps) {
                     onSnooze={onSnooze}
                     ruleId={rule.id}
                     projectSlug={projectSlug}
+                    ruleActionCategory={ruleActionCategory}
                     hasAccess={hasAccess}
                   />
                 )}
@@ -327,13 +339,18 @@ function AlertRuleDetails({params, location, router}: AlertRuleDetailsProps) {
           {renderIncompatibleAlert()}
           {hasSnoozeFeature && isSnoozed && (
             <Alert showIcon>
-              {tct(
-                "[creator] muted this alert[forEveryone]so you won't get these notifications in the future.",
-                {
-                  creator: rule.snoozeCreatedBy,
-                  forEveryone: rule.snoozeForEveryone ? ' for everyone ' : ' ',
-                }
-              )}
+              {ruleActionCategory === RuleActionsCategories.NoDefault
+                ? tct(
+                    "[creator] muted this alert so these notifications won't be sent in the future.",
+                    {creator: rule.snoozeCreatedBy}
+                  )
+                : tct(
+                    "[creator] muted this alert[forEveryone]so you won't get these notifications in the future.",
+                    {
+                      creator: rule.snoozeCreatedBy,
+                      forEveryone: rule.snoozeForEveryone ? ' for everyone ' : ' ',
+                    }
+                  )}
             </Alert>
           )}
           <StyledPageTimeRangeSelector


Code review comments:
diff_hunk: @@ -141,22 +154,31 @@ function SnoozeAlert({isSnoozed, onSnooze, projectSlug, ruleId, hasAccess}: Prop
         size="sm"
         icon={<IconSound />}
         disabled={disabled || !hasAccess}
-        onClick={() => handleMute('me')}
+        hasDropdown={ruleActionCategory !== RuleActionsCategories.NoDefault}
+        onClick={() => {
+          const target =
+            ruleActionCategory !== RuleActionsCategories.AllDefault ? 'everyone' : 'me';
+          handleMute(target);
+        }}
       >
-        {t('Mute')}
+        {ruleActionCategory === RuleActionsCategories.AllDefault
path: static/app/components/alerts/snoozeAlert.tsx
text: @user1:
Is there a reason why this logic is slightly different from the one in the `onClick`? Would make me feel better if they both shared logic so they can't diverge, maybe earlier do something like 

`const primaryMuteAction = RuleActionsCategories.AllDefault ? 'me' : 'everyone'`

Could even simplify a little more and do

`const primaryMuteAction = dropdownItems.find(item => !item.hidden)`

@author:
no specific reason, i just flipped the check but having shared logic makes sense. i think having it go off the dropdown might make it a little more confusing though? especially since if there are no default integrations then there are no dropdown items since both are hidden. 

@user1:
Ah yeah that's fair, just tossing out ideas that may be worse 😄

diff_hunk: @@ -215,6 +216,17 @@ function AlertRuleDetails({params, location, router}: AlertRuleDetailsProps) {
   const hasSnoozeFeature = organization.features.includes('mute-alerts');
   const isSnoozed = rule.snooze;
 
+  const numDefaultActions = rule.actions.filter(
+    action => action.id === 'sentry.mail.actions.NotifyEmailAction'
+  ).length;
+
+  const ruleActionCategory =
+    numDefaultActions === rule.actions.length
+      ? RuleActionsCategories.AllDefault
path: static/app/views/alerts/rules/issue/details/ruleDetails.tsx
text: @user1:
nit: try to avoid multiple ternaries in a single line for better readability - I'd prefer to extract this (and probably the `numDefaultActions` above as well) into a function