Browse Source

fix update logic to only remove AlertRuleProjects for rules being mod… (#69323)

…ified & fix tests

Previously:
- whenever ANY rule was modified, we would delete ALL AlertRuleProjects
that were not being modified

Fix:
- ONLY delete AlertRuleProjects _for alert rules being modified_
Nathan Hsieh 10 months ago
parent
commit
423c356082
2 changed files with 16 additions and 3 deletions
  1. 4 1
      src/sentry/incidents/logic.py
  2. 12 2
      tests/sentry/incidents/test_logic.py

+ 4 - 1
src/sentry/incidents/logic.py

@@ -832,7 +832,10 @@ def update_alert_rule(
             ]
             updated_project_slugs = {project.slug for project in projects}
 
-            AlertRuleProjects.objects.exclude(project__slug__in=updated_project_slugs).delete()
+            # Delete any projects for the alert rule that were removed as part of this update
+            AlertRuleProjects.objects.filter(
+                alert_rule_id=alert_rule.id, project__slug__in=existing_project_slugs
+            ).exclude(project__slug__in=updated_project_slugs).delete()
             for project in projects:
                 alert_rule.projects.add(project)
             # Find any subscriptions that were removed as part of this update

+ 12 - 2
tests/sentry/incidents/test_logic.py

@@ -801,11 +801,21 @@ class UpdateAlertRuleTest(TestCase, BaseIncidentsTest):
         assert alert_rule.snuba_query.query == ""
 
     def test_delete_projects(self):
+        # Testing delete projects from update
         alert_rule = self.create_alert_rule(
             projects=[self.project, self.create_project(fire_project_created=True)]
         )
-        update_alert_rule(alert_rule, projects=[self.project])
-        assert self.alert_rule.snuba_query.subscriptions.get().project == self.project
+        unaffected_alert_rule = self.create_alert_rule(
+            projects=[self.project, self.create_project(fire_project_created=True)]
+        )
+        with self.tasks():
+            update_alert_rule(alert_rule, projects=[self.project])
+        # NOTE: subscribing alert rule to projects creates a new subscription per project
+        subscriptions = alert_rule.snuba_query.subscriptions.all()
+        assert subscriptions.count() == 1
+        assert alert_rule.snuba_query.subscriptions.get().project == self.project
+        assert alert_rule.projects.all().count() == 1
+        assert unaffected_alert_rule.projects.all().count() == 2
 
     def test_new_updated_deleted_projects(self):
         alert_rule = self.create_alert_rule(