Browse Source

fix(alerts): Display event frequency conditions as text (#37061)

Scott Cooper 2 years ago
parent
commit
107c9a4f36

+ 111 - 137
static/app/views/alerts/rules/issue/details/sidebar.tsx

@@ -1,4 +1,4 @@
-import {Fragment, PureComponent} from 'react';
+import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import ActorAvatar from 'sentry/components/avatar/actorAvatar';
@@ -12,58 +12,38 @@ import space from 'sentry/styles/space';
 import {Actor, Member, Team} from 'sentry/types';
 import {IssueAlertRule} from 'sentry/types/alerts';
 
+import {TextAction, TextCondition} from './textRule';
+
 type Props = {
   memberList: Member[];
   rule: IssueAlertRule;
   teams: Team[];
 };
 
-class Sidebar extends PureComponent<Props> {
-  renderConditions() {
-    const {rule, memberList, teams} = this.props;
-
-    const conditions = rule.conditions.length
-      ? rule.conditions.map(condition => (
-          <ConditionsBadge key={condition.id}>{condition.name}</ConditionsBadge>
-        ))
-      : null;
-    const filters = rule.filters.length
-      ? rule.filters.map(filter => (
-          <ConditionsBadge key={filter.id}>
-            {filter.time ? filter.name + '(s)' : filter.name}
-          </ConditionsBadge>
-        ))
-      : null;
-    const actions = rule.actions.length ? (
-      rule.actions.map(action => {
-        let name = action.name;
-        if (action.targetType === 'Member') {
-          const user = memberList.find(
-            member => member.user.id === `${action.targetIdentifier}`
-          );
-          name = t('Send a notification to %s', user?.email);
-        }
-        if (action.targetType === 'Team') {
-          const team = teams.find(tm => tm.id === `${action.targetIdentifier}`);
-          name = t('Send a notification to #%s', team?.name);
-        }
-        if (
-          action.id === 'sentry.integrations.slack.notify_action.SlackNotifyServiceAction'
-        ) {
-          // Remove (optionally, an ID: XXX) from slack action
-          name = name.replace(/\(optionally.*\)/, '');
-          // Remove tags if they aren't used
-          name = name.replace(' and show tags [] in notification', '');
-        }
-
-        return <ConditionsBadge key={action.id}>{name}</ConditionsBadge>;
-      })
-    ) : (
-      <ConditionsBadge>{t('Do nothing')}</ConditionsBadge>
-    );
-
-    return (
-      <PanelBody>
+function Conditions({rule, teams, memberList}: Props) {
+  return (
+    <PanelBody>
+      <Step>
+        <StepContainer>
+          <ChevronContainer>
+            <IconChevron color="gray200" isCircled direction="right" size="sm" />
+          </ChevronContainer>
+          <StepContent>
+            <StepLead>
+              {tct('[when:When] an event is captured [selector]', {
+                when: <Badge />,
+                selector: rule.conditions.length ? t('and %s...', rule.actionMatch) : '',
+              })}
+            </StepLead>
+            {rule.conditions.map((condition, idx) => (
+              <ConditionsBadge key={idx}>
+                <TextCondition condition={condition} />
+              </ConditionsBadge>
+            ))}
+          </StepContent>
+        </StepContainer>
+      </Step>
+      {rule.filters.length ? (
         <Step>
           <StepContainer>
             <ChevronContainer>
@@ -71,108 +51,102 @@ class Sidebar extends PureComponent<Props> {
             </ChevronContainer>
             <StepContent>
               <StepLead>
-                {tct('[when:When] an event is captured [selector]', {
-                  when: <Badge />,
-                  selector: conditions?.length ? t('and %s...', rule.actionMatch) : '',
+                {tct('[if:If] [selector] of these filters match', {
+                  if: <Badge />,
+                  selector: rule.filterMatch,
                 })}
               </StepLead>
-              {conditions}
+              {rule.filters.map((filter, idx) => (
+                <ConditionsBadge key={idx}>
+                  {filter.time ? filter.name + '(s)' : filter.name}
+                </ConditionsBadge>
+              ))}
             </StepContent>
           </StepContainer>
         </Step>
-        {filters && (
-          <Step>
-            <StepContainer>
-              <ChevronContainer>
-                <IconChevron color="gray200" isCircled direction="right" size="sm" />
-              </ChevronContainer>
-              <StepContent>
-                <StepLead>
-                  {tct('[if:If] [selector] of these filters match', {
-                    if: <Badge />,
-                    selector: rule.filterMatch,
-                  })}
-                </StepLead>
-                {filters}
-              </StepContent>
-            </StepContainer>
-          </Step>
-        )}
-        <Step>
-          <StepContainer>
-            <ChevronContainer>
-              <IconChevron isCircled color="gray200" direction="right" size="sm" />
-            </ChevronContainer>
-            <div>
-              <StepLead>
-                {tct('[then:Then] perform these actions', {
-                  then: <Badge />,
-                })}
-              </StepLead>
-              {actions}
-            </div>
-          </StepContainer>
-        </Step>
-      </PanelBody>
-    );
-  }
-
-  render() {
-    const {rule} = this.props;
-
-    const ownerId = rule.owner?.split(':')[1];
-    const teamActor = ownerId && {type: 'team' as Actor['type'], id: ownerId, name: ''};
+      ) : null}
+      <Step>
+        <StepContainer>
+          <ChevronContainer>
+            <IconChevron isCircled color="gray200" direction="right" size="sm" />
+          </ChevronContainer>
+          <div>
+            <StepLead>
+              {tct('[then:Then] perform these actions', {
+                then: <Badge />,
+              })}
+            </StepLead>
+            {rule.actions.length ? (
+              rule.actions.map((action, idx) => {
+                return (
+                  <ConditionsBadge key={idx}>
+                    <TextAction action={action} memberList={memberList} teams={teams} />
+                  </ConditionsBadge>
+                );
+              })
+            ) : (
+              <ConditionsBadge>{t('Do nothing')}</ConditionsBadge>
+            )}
+          </div>
+        </StepContainer>
+      </Step>
+    </PanelBody>
+  );
+}
 
-    return (
-      <Fragment>
-        <StatusContainer>
-          <HeaderItem>
-            <Heading noMargin>{t('Last Triggered')}</Heading>
-            <Status>
-              {rule.lastTriggered ? (
-                <TimeSince date={rule.lastTriggered} />
-              ) : (
-                t('No alerts triggered')
-              )}
-            </Status>
-          </HeaderItem>
-        </StatusContainer>
-        <SidebarGroup>
-          <Heading noMargin>{t('Alert Conditions')}</Heading>
-          {this.renderConditions()}
-        </SidebarGroup>
-        <SidebarGroup>
-          <Heading>{t('Alert Rule Details')}</Heading>
-          <KeyValueTable>
+function Sidebar({rule, teams, memberList}: Props) {
+  const ownerId = rule.owner?.split(':')[1];
+  const teamActor = ownerId && {type: 'team' as Actor['type'], id: ownerId, name: ''};
+
+  return (
+    <Fragment>
+      <StatusContainer>
+        <HeaderItem>
+          <Heading noMargin>{t('Last Triggered')}</Heading>
+          <Status>
+            {rule.lastTriggered ? (
+              <TimeSince date={rule.lastTriggered} />
+            ) : (
+              t('No alerts triggered')
+            )}
+          </Status>
+        </HeaderItem>
+      </StatusContainer>
+      <SidebarGroup>
+        <Heading noMargin>{t('Alert Conditions')}</Heading>
+        <Conditions rule={rule} teams={teams} memberList={memberList} />
+      </SidebarGroup>
+      <SidebarGroup>
+        <Heading>{t('Alert Rule Details')}</Heading>
+        <KeyValueTable>
+          <KeyValueTableRow
+            keyName={t('Environment')}
+            value={<OverflowTableValue>{rule.environment ?? '-'}</OverflowTableValue>}
+          />
+          {rule.dateCreated && (
             <KeyValueTableRow
-              keyName={t('Environment')}
-              value={<OverflowTableValue>{rule.environment ?? '-'}</OverflowTableValue>}
+              keyName={t('Date Created')}
+              value={<TimeSince date={rule.dateCreated} suffix={t('ago')} />}
             />
-            {rule.dateCreated && (
-              <KeyValueTableRow
-                keyName={t('Date Created')}
-                value={<TimeSince date={rule.dateCreated} suffix={t('ago')} />}
-              />
-            )}
-            {rule.createdBy && (
-              <KeyValueTableRow
-                keyName={t('Created By')}
-                value={
-                  <OverflowTableValue>{rule.createdBy.name ?? '-'}</OverflowTableValue>
-                }
-              />
-            )}
+          )}
+          {rule.createdBy && (
             <KeyValueTableRow
-              keyName={t('Team')}
+              keyName={t('Created By')}
               value={
-                teamActor ? <ActorAvatar actor={teamActor} size={24} /> : 'Unassigned'
+                <OverflowTableValue>{rule.createdBy.name ?? '-'}</OverflowTableValue>
               }
             />
-          </KeyValueTable>
-        </SidebarGroup>
-      </Fragment>
-    );
-  }
+          )}
+          <KeyValueTableRow
+            keyName={t('Team')}
+            value={
+              teamActor ? <ActorAvatar actor={teamActor} size={24} /> : t('Unassigned')
+            }
+          />
+        </KeyValueTable>
+      </SidebarGroup>
+    </Fragment>
+  );
 }
 
 export default Sidebar;

+ 75 - 0
static/app/views/alerts/rules/issue/details/textRule.tsx

@@ -0,0 +1,75 @@
+import {Fragment} from 'react';
+
+import {t} from 'sentry/locale';
+import type {Member, Team} from 'sentry/types';
+import type {IssueAlertRule} from 'sentry/types/alerts';
+import {AlertRuleComparisonType} from 'sentry/views/alerts/rules/metric/types';
+
+/**
+ * Translate Issue Alert Conditions to text
+ */
+export function TextCondition({
+  condition,
+}: {
+  condition: IssueAlertRule['conditions'][number];
+}) {
+  if (
+    condition.id === 'sentry.rules.conditions.event_frequency.EventFrequencyCondition'
+  ) {
+    if (condition.comparisonType === AlertRuleComparisonType.PERCENT) {
+      return (
+        <Fragment>
+          {t(
+            // Double %% escapes
+            'Number of events in an issue is %s%% higher in %s compared to %s ago',
+            condition.value,
+            condition.comparisonInterval,
+            condition.interval
+          )}
+        </Fragment>
+      );
+    }
+
+    return (
+      <Fragment>
+        {t(
+          'Number of events in an issue is more than %s in %s',
+          condition.value,
+          condition.interval
+        )}
+      </Fragment>
+    );
+  }
+
+  return <Fragment>{condition.name}</Fragment>;
+}
+
+// TODO(scttcper): Remove the teams/memberList prop drilling
+export function TextAction({
+  action,
+  memberList,
+  teams,
+}: {
+  action: IssueAlertRule['actions'][number];
+  memberList: Member[];
+  teams: Team[];
+}) {
+  if (action.targetType === 'Member') {
+    const user = memberList.find(
+      member => member.user.id === `${action.targetIdentifier}`
+    );
+    return <Fragment>{t('Send a notification to %s', user?.email)}</Fragment>;
+  }
+
+  if (action.targetType === 'Team') {
+    const team = teams.find(tm => tm.id === `${action.targetIdentifier}`);
+    return <Fragment>{t('Send a notification to #%s', team?.name)}</Fragment>;
+  }
+
+  if (action.id === 'sentry.integrations.slack.notify_action.SlackNotifyServiceAction') {
+    // Remove (optionally, an ID: XXX) from slack action
+    return <Fragment>{action.name.replace(/\(optionally.*\)/, '')}</Fragment>;
+  }
+
+  return <Fragment>{action.name}</Fragment>;
+}

+ 3 - 1
static/app/views/alerts/rules/issue/ruleNodeList.tsx

@@ -20,6 +20,8 @@ import {
 } from 'sentry/views/alerts/utils/constants';
 import {EVENT_FREQUENCY_PERCENT_CONDITION} from 'sentry/views/projectInstall/issueAlertOptions';
 
+import {AlertRuleComparisonType} from '../metric/types';
+
 import RuleNode from './ruleNode';
 
 type Props = {
@@ -106,7 +108,7 @@ class RuleNodeList extends Component<Props> {
         ),
       };
 
-      if (item.comparisonType === 'percent') {
+      if (item.comparisonType === AlertRuleComparisonType.PERCENT) {
         if (!item.comparisonInterval) {
           // comparisonInterval value in IssueRuleEditor state
           // is undefined even if initial value is defined

+ 1 - 0
static/app/views/alerts/rules/metric/types.tsx

@@ -18,6 +18,7 @@ export enum AlertRuleTriggerType {
 export enum AlertRuleComparisonType {
   COUNT = 'count',
   CHANGE = 'change',
+  PERCENT = 'percent',
 }
 
 export enum Dataset {

+ 47 - 0
tests/js/spec/views/alerts/details/textRule.spec.tsx

@@ -0,0 +1,47 @@
+import {render} from 'sentry-test/reactTestingLibrary';
+
+import {TextCondition} from 'sentry/views/alerts/rules/issue/details/textRule';
+
+describe('AlertRuleDetails', () => {
+  it('displays EventFrequencyCondition percentage', () => {
+    const wrapper = render(
+      <TextCondition
+        condition={{
+          comparisonType: 'count',
+          id: 'sentry.rules.conditions.event_frequency.EventFrequencyCondition',
+          interval: '1h',
+          name: 'The issue is seen more than 1000 times in 1h',
+          value: 1000,
+
+          // TODO(scttcper): label and prompt only exist in the type definition
+          label: '',
+          prompt: '',
+        }}
+      />
+    );
+    expect(wrapper.container).toHaveTextContent(
+      'Number of events in an issue is more than 1000 in 1h'
+    );
+  });
+  it('displays EventFrequencyCondition count', () => {
+    const wrapper = render(
+      <TextCondition
+        condition={{
+          comparisonInterval: '1h',
+          comparisonType: 'percent',
+          id: 'sentry.rules.conditions.event_frequency.EventFrequencyCondition',
+          interval: '1h',
+          name: 'The issue is seen more than 150 times in 1h',
+          value: 150,
+
+          // TODO(scttcper): label and prompt only exist in the type definition
+          label: '',
+          prompt: '',
+        }}
+      />
+    );
+    expect(wrapper.container).toHaveTextContent(
+      'Number of events in an issue is 150% higher in 1h compared to 1h ago'
+    );
+  });
+});