ref(rules): Migrate 'bad' rules to be disabled (#55941)

Migrate alert rules that have no action or are exact duplicates of
another rule in the same project to be disabled. We now prevent these
from being created, this will just clean up old ones.

Closes #55502

migrations_lockfile.txt

@@ -8,5 +8,5 @@ will then be regenerated, and you should be able to merge without conflicts.
 feedback: 0001_feedback
 nodestore: 0002_nodestore_no_dictfield
 replays: 0003_add_size_to_recording_segment
-sentry: 0549_re_add_groupsubscription_columns
+sentry: 0550_migrate_no_action_dupe_issue_alerts
 social_auth: 0002_default_auto_field

src/sentry/migrations/0550_migrate_no_action_dupe_issue_alerts.py

@@ -0,0 +1,92 @@
+# Generated by Django 3.2.20 on 2023-09-12 18:15
+
+from django.db import migrations
+
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.utils.query import RangeQuerySetWrapperWithProgressBar
+
+
+class ObjectStatus:
+    ACTIVE = 0
+    HIDDEN = 1
+    PENDING_DELETION = 2
+    DELETION_IN_PROGRESS = 3
+
+    DISABLED = 1
+
+
+def has_duplicate_rule(apps, rule_data, project, rule_id):
+    Rule = apps.get_model("sentry", "Rule")
+
+    matchers = {key for key in list(rule_data.keys()) if key not in ("name", "user_id")}
+    extra_fields = ["actions", "environment"]
+    matchers.update(extra_fields)
+    existing_rules = Rule.objects.exclude(id=rule_id).filter(
+        project=project, status=ObjectStatus.ACTIVE
+    )
+    for existing_rule in existing_rules:
+        keys = 0
+        matches = 0
+        for matcher in matchers:
+            if existing_rule.data.get(matcher) and rule_data.get(matcher):
+                keys += 1
+
+                if existing_rule.data[matcher] == rule_data[matcher]:
+                    matches += 1
+
+            elif matcher in extra_fields:
+                if not existing_rule.data.get(matcher) and not rule_data.get(matcher):
+                    # neither rule has the matcher
+                    continue
+
+                elif matcher == "environment":
+                    if existing_rule.environment_id and rule_data.get(matcher):
+                        keys += 1
+                        if existing_rule.environment_id == rule_data.get(matcher):
+                            matches += 1
+                    else:
+                        keys += 1
+                else:
+                    # one rule has the matcher and the other one doesn't
+                    keys += 1
+
+        if keys == matches:
+            return True
+    return False
+
+
+def migrate_bad_rules(apps, schema_editor):
+    Rule = apps.get_model("sentry", "Rule")
+
+    for rule in RangeQuerySetWrapperWithProgressBar(Rule.objects.all()):
+        if not rule.data.get("actions", []) or has_duplicate_rule(
+            apps, rule.data, rule.project, rule.id
+        ):
+            rule.status = ObjectStatus.DISABLED
+            rule.save(update_fields=["status"])
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = True
+
+    dependencies = [
+        ("sentry", "0549_re_add_groupsubscription_columns"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            migrate_bad_rules,
+            migrations.RunPython.noop,
+            hints={"tables": ["sentry_rule"]},
+        ),
+    ]

src/sentry/testutils/factories.py

@@ -426,23 +426,27 @@ def create_project_bookmark(project, user):
     def create_project_rule(
         project,
         action_data=None,
+        allow_no_action_data=False,
         condition_data=None,
         name="",
         action_match="all",
         filter_match="all",
         **kwargs,
     ):
-        action_data = action_data or [
-            {
-                "id": "sentry.rules.actions.notify_event.NotifyEventAction",
-                "name": "Send a notification (for all legacy integrations)",
-            },
-            {
-                "id": "sentry.rules.actions.notify_event_service.NotifyEventServiceAction",
-                "service": "mail",
-                "name": "Send a notification via mail",
-            },
-        ]
+        actions = None
+        if not allow_no_action_data:
+            action_data = action_data or [
+                {
+                    "id": "sentry.rules.actions.notify_event.NotifyEventAction",
+                    "name": "Send a notification (for all legacy integrations)",
+                },
+                {
+                    "id": "sentry.rules.actions.notify_event_service.NotifyEventServiceAction",
+                    "service": "mail",
+                    "name": "Send a notification via mail",
+                },
+            ]
+            actions = action_data
         condition_data = condition_data or [
             {
                 "id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition",
@@ -453,15 +457,18 @@ def create_project_rule(
                 "name": "The event occurs",
             },
         ]
+        data = {
+            "conditions": condition_data,
+            "action_match": action_match,
+            "filter_match": filter_match,
+        }
+        if actions:
+            data["actions"] = actions
+
         return Rule.objects.create(
             label=name,
             project=project,
-            data={
-                "conditions": condition_data,
-                "actions": action_data,
-                "action_match": action_match,
-                "filter_match": filter_match,
-            },
+            data=data,
             **kwargs,
         )
 

tests/sentry/migrations/test_0550_migrate_no_action_dupe_issue_alerts.py

@@ -0,0 +1,25 @@
+from sentry.constants import ObjectStatus
+from sentry.testutils.cases import TestMigrations
+
+
+class MigrateNoActionDupleIssueAlerts(TestMigrations):
+    migrate_from = "0549_re_add_groupsubscription_columns"
+    migrate_to = "0550_migrate_no_action_dupe_issue_alerts"
+
+    def setup_initial_state(self):
+        self.no_action_rule = self.create_project_rule(
+            project=self.project, allow_no_action_data=True
+        )
+        self.rule1 = self.create_project_rule(project=self.project)
+        self.rule2 = self.create_project_rule(project=self.project)
+        assert self.no_action_rule.status == ObjectStatus.ACTIVE
+        assert self.rule2.status == ObjectStatus.ACTIVE
+        assert self.rule1.status == ObjectStatus.ACTIVE
+
+    def test(self):
+        self.no_action_rule.refresh_from_db()
+        self.rule1.refresh_from_db()
+        self.rule2.refresh_from_db()
+        assert self.no_action_rule.status == ObjectStatus.DISABLED
+        assert self.rule1.status == ObjectStatus.DISABLED
+        assert self.rule2.status == ObjectStatus.ACTIVE