Просмотр исходного кода

feat(issue-alerts): detect incompatible rules (#40733)

Adds a banner below incompatible rule.
![Screen Shot 2022-10-28 at 10 52 28
AM](https://user-images.githubusercontent.com/39287272/198700926-f2082e3c-ef4a-4099-aa83-8169a255facc.png)

The logic behind detecting these incompatible rules is still very simple
and can be expanded on later if the function gets too complicated. An
idea for this is in the
[doc](https://www.notion.so/sentry/Issue-Alert-Previews-Testing-ba0d8a43ea564ab9ba6c72b1814274f4#ec0bd3e95e4c4ab29285f6cebb7c5d8a)
(still might be overkill)
Andrew Xue 2 лет назад
Родитель
Сommit
20323914a9

+ 30 - 0
static/app/views/alerts/create.spec.jsx

@@ -528,4 +528,34 @@ describe('ProjectAlertsCreate', function () {
       ).toBeInTheDocument();
     });
   });
+
+  it('shows error for incompatible conditions', async () => {
+    const organization = TestStubs.Organization({
+      features: ['issue-alert-incompatible-rules'],
+    });
+    createWrapper({organization});
+    await selectEvent.select(screen.getByText('Add optional trigger...'), [
+      'A new issue is created',
+    ]);
+    await selectEvent.select(screen.getByText('Add optional trigger...'), [
+      'The issue changes state from resolved to unresolved',
+    ]);
+    expect(
+      screen.getByText(
+        'This condition conflicts with other condition(s) above. Please select a different condition'
+      )
+    ).toBeInTheDocument();
+
+    expect(screen.getByRole('button', {name: 'Save Rule'})).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
+
+    userEvent.click(screen.getAllByLabelText('Delete Node')[0]);
+    expect(
+      screen.queryByText(
+        'This condition conflicts with other condition(s) above. Please select a different condition'
+      )
+    ).not.toBeInTheDocument();
+  });
 });

+ 81 - 4
static/app/views/alerts/rules/issue/index.tsx

@@ -144,6 +144,8 @@ type State = AsyncView['state'] & {
     [key: string]: string[];
   };
   environments: Environment[] | null;
+  incompatibleCondition: number | null;
+  incompatibleFilter: number | null;
   issueCount: number;
   loadingPreview: boolean;
   previewCursor: string | null | undefined;
@@ -184,8 +186,13 @@ class IssueRuleEditor extends AsyncView<Props, State> {
     if (prevState.previewCursor !== this.state.previewCursor) {
       this.fetchPreview();
     } else if (this.isRuleStateChange(prevState)) {
-      this.setState({loadingPreview: true});
+      this.setState({
+        loadingPreview: true,
+        incompatibleCondition: null,
+        incompatibleFilter: null,
+      });
       this.fetchPreviewDebounced();
+      this.checkIncompatibleRule();
     }
     if (prevState.project.id === this.state.project.id) {
       return;
@@ -237,6 +244,8 @@ class IssueRuleEditor extends AsyncView<Props, State> {
       previewPage: 0,
       loadingPreview: false,
       sendingNotification: false,
+      incompatibleCondition: null,
+      incompatibleFilter: null,
     };
 
     const projectTeamIds = new Set(project.teams.map(({id}) => id));
@@ -405,6 +414,60 @@ class IssueRuleEditor extends AsyncView<Props, State> {
     this.fetchPreview(true);
   }, 1000);
 
+  // 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'
+    const firstSeen = conditions.some(condition =>
+      condition.id.endsWith('FirstSeenEventCondition')
+    );
+    if (
+      firstSeen &&
+      (rule.actionMatch === 'all' || conditions.length === 1) &&
+      (rule.filterMatch === 'all' || (rule.filterMatch === 'any' && filters.length === 1))
+    ) {
+      for (let i = 0; i < filters.length; i++) {
+        const id = filters[i].id;
+        if (id.endsWith('IssueOccurrencesFilter') && filters[i].value > 1) {
+          this.setState({incompatibleFilter: i});
+          return;
+        }
+      }
+    }
+  }, 500);
+
   onPreviewCursor: CursorHandler = (cursor, _1, _2, direction) => {
     this.setState({
       previewCursor: cursor,
@@ -1064,8 +1127,16 @@ class IssueRuleEditor extends AsyncView<Props, State> {
 
   renderBody() {
     const {organization} = this.props;
-    const {project, rule, detailedError, loading, ownership, sendingNotification} =
-      this.state;
+    const {
+      project,
+      rule,
+      detailedError,
+      loading,
+      ownership,
+      sendingNotification,
+      incompatibleCondition,
+      incompatibleFilter,
+    } = this.state;
     const {actions, filters, conditions, frequency} = rule || {};
 
     const environment =
@@ -1092,7 +1163,11 @@ class IssueRuleEditor extends AsyncView<Props, State> {
                   frequency: `${frequency}`,
                   projectId: project.id,
                 }}
-                submitDisabled={disabled}
+                submitDisabled={
+                  disabled ||
+                  incompatibleCondition !== null ||
+                  incompatibleFilter !== null
+                }
                 submitLabel={t('Save Rule')}
                 extraButton={
                   isSavedAlertRule(rule) ? (
@@ -1196,6 +1271,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
                                   </StyledAlert>
                                 )
                               }
+                              incompatibleRule={incompatibleCondition}
                             />
                           </StepContent>
                         </StepContainer>
@@ -1266,6 +1342,7 @@ class IssueRuleEditor extends AsyncView<Props, State> {
                                   </StyledAlert>
                                 )
                               }
+                              incompatibleRule={incompatibleFilter}
                             />
                           </StepContent>
                         </StepContainer>

+ 16 - 0
static/app/views/alerts/rules/issue/ruleNode.tsx

@@ -229,6 +229,7 @@ interface Props {
   onReset: (rowIndex: number, name: string, value: string) => void;
   organization: Organization;
   project: Project;
+  incompatibleRule?: boolean;
   node?: IssueAlertRuleActionTemplate | IssueAlertRuleConditionTemplate | null;
   ownership?: null | IssueOwnership;
 }
@@ -244,6 +245,7 @@ function RuleNode({
   onPropertyChange,
   onReset,
   ownership,
+  incompatibleRule,
 }: Props) {
   const handleDelete = useCallback(() => {
     onDelete(index);
@@ -447,6 +449,19 @@ function RuleNode({
     return null;
   }
 
+  function renderIncompatibleRuleBanner() {
+    if (!incompatibleRule) {
+      return null;
+    }
+    return (
+      <MarginlessAlert type="error" showIcon>
+        {t(
+          'This condition conflicts with other condition(s) above. Please select a different condition'
+        )}
+      </MarginlessAlert>
+    );
+  }
+
   /**
    * Update all the AlertRuleAction's fields from the TicketRuleModal together
    * only after the user clicks "Apply Changes".
@@ -562,6 +577,7 @@ function RuleNode({
           icon={<IconDelete />}
         />
       </RuleRow>
+      {renderIncompatibleRuleBanner()}
       {conditionallyRenderHelpfulBanner()}
     </RuleRowContainer>
   );

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

@@ -45,6 +45,7 @@ type Props = {
    */
   placeholder: string;
   project: Project;
+  incompatibleRule?: number | null;
   ownership?: null | IssueOwnership;
   selectType?: 'grouped';
 };
@@ -152,6 +153,7 @@ class RuleNodeList extends Component<Props> {
       disabled,
       error,
       selectType,
+      incompatibleRule,
     } = this.props;
 
     const enabledNodes = nodes ? nodes.filter(({enabled}) => enabled) : [];
@@ -244,6 +246,7 @@ class RuleNodeList extends Component<Props> {
               project={project}
               disabled={disabled}
               ownership={ownership}
+              incompatibleRule={incompatibleRule === idx}
             />
           ))}
         </RuleNodes>