feat(issue-alert): show banner for existing incompatible rules (#41191)

Shows an alert banner for existing rules that have incompatible
conditions.
![Screen Shot 2022-11-09 at 10 03 58
AM](https://user-images.githubusercontent.com/39287272/200922548-018074af-e88f-4794-95b8-0bb61521ed21.png)

Runs the detector that is originally used in the create/edit alert rule
page.

static/app/views/alerts/rules/issue/details/ruleDetails.spec.jsx

@@ -9,7 +9,9 @@ import RuleDetailsContainer from 'sentry/views/alerts/rules/issue/details/index'
 import AlertRuleDetails from 'sentry/views/alerts/rules/issue/details/ruleDetails';
 
 describe('AlertRuleDetails', () => {
-  const context = initializeOrg();
+  const context = initializeOrg({
+    organization: {features: ['issue-alert-incompatible-rules']},
+  });
   const organization = context.organization;
   const project = TestStubs.Project();
   const rule = TestStubs.ProjectAlertRule({
@@ -156,4 +158,43 @@ describe('AlertRuleDetails', () => {
       await screen.findByText('The alert rule you were looking for was not found.')
     ).toBeInTheDocument();
   });
+
+  it('renders incompatible rule filter', async () => {
+    const incompatibleRule = TestStubs.ProjectAlertRule({
+      lastTriggered: moment().subtract(2, 'day').format(),
+      conditions: [
+        {id: 'sentry.rules.conditions.first_seen_event.FirstSeenEventCondition'},
+        {id: 'sentry.rules.conditions.regression_event.RegressionEventCondition'},
+      ],
+    });
+    MockApiClient.mockResponses.splice(
+      MockApiClient.mockResponses.findIndex(
+        response =>
+          response.url ===
+          `/projects/${organization.slug}/${project.slug}/rules/${rule.id}/`
+      ),
+      1
+    );
+    MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/rules/${rule.id}/`,
+      body: incompatibleRule,
+      match: [MockApiClient.matchQuery({expand: 'lastTriggered'})],
+    });
+    createWrapper();
+    expect(
+      await screen.findByText(
+        'The conditions in this alert rule conflict and might not be working properly.'
+      )
+    ).toBeInTheDocument();
+  });
+
+  it('incompatible rule banner hidden for good rule', async () => {
+    createWrapper();
+    expect(await screen.getAllByText('My alert rule')).toHaveLength(2);
+    expect(
+      await screen.queryByText(
+        'The conditions in this alert rule conflict and might not be working properly.'
+      )
+    ).not.toBeInTheDocument();
+  });
 });

static/app/views/alerts/rules/issue/details/ruleDetails.tsx

@@ -3,6 +3,7 @@ import styled from '@emotion/styled';
 import pick from 'lodash/pick';
 import moment from 'moment';
 
+import Alert from 'sentry/components/alert';
 import AsyncComponent from 'sentry/components/asyncComponent';
 import Breadcrumbs from 'sentry/components/breadcrumbs';
 import Button from 'sentry/components/button';
@@ -18,11 +19,12 @@ import {ChangeData} from 'sentry/components/organizations/timeRangeSelector';
 import PageTimeRangeSelector from 'sentry/components/pageTimeRangeSelector';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {IconCopy, IconEdit} from 'sentry/icons';
-import {t} from 'sentry/locale';
+import {t, tct} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {DateString, Member, Organization, Project} from 'sentry/types';
 import {IssueAlertRule} from 'sentry/types/alerts';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
+import {findIncompatibleRules} from 'sentry/views/alerts/rules/issue';
 import {ALERT_DEFAULT_CHART_PERIOD} from 'sentry/views/alerts/rules/metric/details/constants';
 
 import AlertChart from './alertChart';
@@ -184,6 +186,32 @@ class AlertRuleDetails extends AsyncComponent<Props, State> {
     );
   }
 
+  renderIncompatibleAlert() {
+    const {orgId, projectId, ruleId} = this.props.params;
+
+    const incompatibleRule = findIncompatibleRules(this.state.rule);
+    if (
+      incompatibleRule.type !== 'none' &&
+      this.props.organization.features.includes('issue-alert-incompatible-rules')
+    ) {
+      return (
+        <Alert type="error" showIcon>
+          {tct(
+            'The conditions in this alert rule conflict and might not be working properly. [link:Edit alert rule]',
+            {
+              link: (
+                <a
+                  href={`/organizations/${orgId}/alerts/rules/${projectId}/${ruleId}/`}
+                />
+              ),
+            }
+          )}
+        </Alert>
+      );
+    }
+    return null;
+  }
+
   renderBody() {
     const {params, location, organization, project} = this.props;
     const {orgId, ruleId, projectId} = params;
@@ -272,6 +300,7 @@ class AlertRuleDetails extends AsyncComponent<Props, State> {
         </Layout.Header>
         <Layout.Body>
           <Layout.Main>
+            {this.renderIncompatibleAlert()}
             <StyledPageTimeRangeSelector
               organization={organization}
               relative={period ?? ''}

static/app/views/alerts/rules/issue/index.tsx

@@ -123,6 +123,11 @@ type RuleTaskResponse = {
 
 type RouteParams = {orgId: string; projectId?: string; ruleId?: string};
 
+export type IncompatibleRule = {
+  index: number | null;
+  type: 'condition' | 'filter' | 'none';
+};
+
 type Props = {
   location: Location;
   members: Member[] | undefined;
@@ -416,84 +421,12 @@ class IssueRuleEditor extends AsyncView<Props, State> {
 
   // As more incompatible combinations are added, we will need a more generic way to check for incompatibility.
   checkIncompatibleRule = debounce(() => {
-    const {rule} = this.state;
-    if (
-      !rule ||
-      !this.props.organization.features.includes('issue-alert-incompatible-rules')
-    ) {
-      return;
-    }
-
-    const {conditions, filters} = rule;
-    // Check for more than one 'issue state change' condition
-    // or 'FirstSeenEventCondition' + 'EventFrequencyCondition'
-    if (rule.actionMatch === 'all') {
-      let firstSeen = 0;
-      let regression = 0;
-      let reappeared = 0;
-      let eventFrequency = 0;
-      for (let i = 0; i < conditions.length; i++) {
-        const id = conditions[i].id;
-        if (id.endsWith('FirstSeenEventCondition')) {
-          firstSeen = 1;
-        } else if (id.endsWith('RegressionEventCondition')) {
-          regression = 1;
-        } else if (id.endsWith('ReappearedEventCondition')) {
-          reappeared = 1;
-        } else if (id.endsWith('EventFrequencyCondition') && conditions[i].value >= 1) {
-          eventFrequency = 1;
-        }
-        if (firstSeen + regression + reappeared > 1 || firstSeen + eventFrequency > 1) {
-          this.setState({incompatibleCondition: i});
-          return;
-        }
-      }
-    }
-    // Check for 'FirstSeenEventCondition' and ('IssueOccurrencesFilter' or 'AgeComparisonFilter')
-    // Considers the case where filterMatch is 'any' and all filters are incompatible
-    const firstSeen = conditions.some(condition =>
-      condition.id.endsWith('FirstSeenEventCondition')
-    );
-    if (firstSeen && (rule.actionMatch === 'all' || conditions.length === 1)) {
-      let incompatibleFilters = 0;
-      for (let i = 0; i < filters.length; i++) {
-        const filter = filters[i];
-        const id = filter.id;
-        if (id.endsWith('IssueOccurrencesFilter')) {
-          if (
-            (rule.filterMatch === 'all' && filter.value > 1) ||
-            (rule.filterMatch === 'none' && filter.value <= 1)
-          ) {
-            this.setState({incompatibleFilter: i});
-            return;
-          }
-          if (rule.filterMatch === 'any' && filter.value > 1) {
-            incompatibleFilters += 1;
-          }
-        } else if (id.endsWith('AgeComparisonFilter')) {
-          if (rule.filterMatch !== 'none') {
-            if (
-              (filter.comparison_type === 'older' && filter.value >= 0) ||
-              (filter.comparison_type === 'newer' && filter.value <= 0)
-            ) {
-              if (rule.filterMatch === 'all') {
-                this.setState({incompatibleFilter: i});
-                return;
-              }
-              incompatibleFilters += 1;
-            }
-          } else if (
-            (filter.comparison_type === 'older' && filter.value < 0) ||
-            (filter.comparison_type === 'newer' && filter.value > 0)
-          ) {
-            this.setState({incompatibleFilter: i});
-            return;
-          }
-        }
-      }
-      if (incompatibleFilters === filters.length) {
-        this.setState({incompatibleFilter: incompatibleFilters - 1});
-        return;
+    if (this.props.organization.features.includes('issue-alert-incompatible-rules')) {
+      const incompatibleRule = findIncompatibleRules(this.state.rule);
+      if (incompatibleRule.type === 'condition') {
+        this.setState({incompatibleCondition: incompatibleRule.index});
+      } else if (incompatibleRule.type === 'filter') {
+        this.setState({incompatibleFilter: incompatibleRule.index});
       }
     }
   }, 500);
@@ -1488,6 +1421,83 @@ class IssueRuleEditor extends AsyncView<Props, State> {
 
 export default withOrganization(withProjects(IssueRuleEditor));
 
+export const findIncompatibleRules = (
+  rule: IssueAlertRule | UnsavedIssueAlertRule | null | undefined
+): IncompatibleRule => {
+  if (!rule) {
+    return {type: 'none', index: null};
+  }
+
+  const {conditions, filters} = rule;
+  // Check for more than one 'issue state change' condition
+  // or 'FirstSeenEventCondition' + 'EventFrequencyCondition'
+  if (rule.actionMatch === 'all') {
+    let firstSeen = 0;
+    let regression = 0;
+    let reappeared = 0;
+    let eventFrequency = 0;
+    for (let i = 0; i < conditions.length; i++) {
+      const id = conditions[i].id;
+      if (id.endsWith('FirstSeenEventCondition')) {
+        firstSeen = 1;
+      } else if (id.endsWith('RegressionEventCondition')) {
+        regression = 1;
+      } else if (id.endsWith('ReappearedEventCondition')) {
+        reappeared = 1;
+      } else if (id.endsWith('EventFrequencyCondition') && conditions[i].value >= 1) {
+        eventFrequency = 1;
+      }
+      if (firstSeen + regression + reappeared > 1 || firstSeen + eventFrequency > 1) {
+        return {type: 'condition', index: i};
+      }
+    }
+  }
+  // Check for 'FirstSeenEventCondition' and ('IssueOccurrencesFilter' or 'AgeComparisonFilter')
+  // Considers the case where filterMatch is 'any' and all filters are incompatible
+  const firstSeen = conditions.some(condition =>
+    condition.id.endsWith('FirstSeenEventCondition')
+  );
+  if (firstSeen && (rule.actionMatch === 'all' || conditions.length === 1)) {
+    let incompatibleFilters = 0;
+    for (let i = 0; i < filters.length; i++) {
+      const filter = filters[i];
+      const id = filter.id;
+      if (id.endsWith('IssueOccurrencesFilter')) {
+        if (
+          (rule.filterMatch === 'all' && filter.value > 1) ||
+          (rule.filterMatch === 'none' && filter.value <= 1)
+        ) {
+          return {type: 'filter', index: i};
+        }
+        if (rule.filterMatch === 'any' && filter.value > 1) {
+          incompatibleFilters += 1;
+        }
+      } else if (id.endsWith('AgeComparisonFilter')) {
+        if (rule.filterMatch !== 'none') {
+          if (
+            (filter.comparison_type === 'older' && filter.value >= 0) ||
+            (filter.comparison_type === 'newer' && filter.value <= 0)
+          ) {
+            if (rule.filterMatch === 'all') {
+              return {type: 'filter', index: i};
+            }
+            incompatibleFilters += 1;
+          }
+        } else if (
+          (filter.comparison_type === 'older' && filter.value < 0) ||
+          (filter.comparison_type === 'newer' && filter.value > 0)
+        ) {
+          return {type: 'filter', index: i};
+        }
+      }
+    }
+    if (incompatibleFilters === filters.length) {
+      return {type: 'filter', index: incompatibleFilters - 1};
+    }
+  }
+  return {type: 'none', index: null};
+};
+
 // TODO(ts): Understand why styled is not correctly inheriting props here
 const StyledForm = styled(Form)<FormProps>`
   position: relative;