Browse Source

feat(alerts): Migrate project ownerships settings to issue alert fallback (#42796)

Migrate the `ProjectOwnership.fallthrough` setting to the new Issue alert fallback, defaulting to ActiveMembers if no fallthrough is set.

WOR-2388

[WOR-2388]:
https://getsentry.atlassian.net/browse/WOR-2388?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Snigdha Sharma 2 years ago
parent
commit
47226d3b4e

+ 1 - 1
migrations_lockfile.txt

@@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
 will then be regenerated, and you should be able to merge without conflicts.
 
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0348_add_outbox_and_tombstone_tables
+sentry: 0349_issue_alert_fallback
 social_auth: 0001_initial

+ 107 - 0
src/sentry/migrations/0349_issue_alert_fallback.py

@@ -0,0 +1,107 @@
+# Generated by Django 2.2.28 on 2023-01-04 19:14
+
+import logging
+from enum import Enum
+
+from django.db import migrations, transaction
+
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.utils.query import RangeQuerySetWrapper, RangeQuerySetWrapperWithProgressBar
+
+
+# Redefining the enums here so that we can reliably use them in the migration.
+class RuleStatus(Enum):
+    ACTIVE = 0
+
+
+class ProjectStatus(Enum):
+    ACTIVE = 0
+    DISABLED = 1
+
+
+class OrganizationStatus(Enum):
+    ACTIVE = 0
+
+
+def set_issue_alert_fallback(rule, fallthrough_choice):
+    actions = rule.data.get("actions", [])
+    rule_changed = False
+    for action in actions:
+        id = action.get("id")
+        target_type = action.get("targetType")
+        if id == "sentry.mail.actions.NotifyEmailAction" and target_type == "IssueOwners":
+            if "fallthroughType" not in action:
+                action.update({"fallthroughType": fallthrough_choice})
+            rule_changed = True
+
+    if rule_changed:
+        rule.data["actions"] = actions
+        rule.save()
+
+
+def migrate_project_ownership_to_issue_alert_fallback(project, ProjectOwnership, Rule):
+    with transaction.atomic():
+        # Determine whether this project has a fallback setting.
+        fallthrough_choice = None
+        try:
+            ownership = ProjectOwnership.objects.get(project=project)
+            fallthrough_choice = "AllMembers" if ownership and ownership.fallthrough else "NoOne"
+        except ProjectOwnership.DoesNotExist:
+            # Projects without fallbacks will be assigned the new "ActiveMembers" default.
+            fallthrough_choice = "ActiveMembers"
+
+        # We only migrate rules that are not pending deletion.
+        for rule in Rule.objects.filter(project=project, status=RuleStatus.ACTIVE.value):
+            set_issue_alert_fallback(rule, fallthrough_choice)
+
+
+def migrate_to_issue_alert_fallback(apps, schema_editor):
+    Project = apps.get_model("sentry", "Project")
+    ProjectOwnership = apps.get_model("sentry", "ProjectOwnership")
+    Organization = apps.get_model("sentry", "Organization")
+    Rule = apps.get_model("sentry", "Rule")
+
+    # We migrate a project at a time, but we prefer to group by org so that for the
+    # most part an org will see the changes all at once.
+    for org in RangeQuerySetWrapperWithProgressBar(
+        Organization.objects.filter(status=OrganizationStatus.ACTIVE.value)
+    ):
+        # We only migrate projects that are not pending deletion.
+        for project in RangeQuerySetWrapper(
+            Project.objects.filter(
+                organization=org,
+                status__in=[ProjectStatus.ACTIVE.value, ProjectStatus.DISABLED.value],
+            )
+        ):
+            try:
+                migrate_project_ownership_to_issue_alert_fallback(project, ProjectOwnership, Rule)
+            except Exception:
+                # If a project fails we'll just log and continue. We shouldn't see any
+                # failures, but if we do we can analyze them and run a new migration.
+                logging.exception(f"Error migrating project {project.id}")
+
+
+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", "0348_add_outbox_and_tombstone_tables"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            migrate_to_issue_alert_fallback,
+            reverse_code=migrations.RunPython.noop,
+            hints={"tables": ["sentry_rule"]},
+        )
+    ]

+ 68 - 0
tests/sentry/migrations/test_0349_issue_alert_fallback.py

@@ -0,0 +1,68 @@
+from sentry.models.project import Project
+from sentry.models.projectownership import ProjectOwnership
+from sentry.models.rule import Rule
+from sentry.testutils.cases import TestMigrations
+
+
+class MigrateAlertFallbackTest(TestMigrations):
+    migrate_from = "0348_add_outbox_and_tombstone_tables"
+    migrate_to = "0349_issue_alert_fallback"
+
+    def create_issue_alert(self, name, project, set_fallthrough=False):
+        rule = Rule()
+        rule.project = project
+        rule.label = name
+        action = {
+            "id": "sentry.mail.actions.NotifyEmailAction",
+            "targetType": "IssueOwners",
+            "targetIdentifier": "None",
+        }
+        if set_fallthrough:
+            action["fallthroughType"] = "AllMembers"
+
+        rule.data["actions"] = [action]
+        rule.save()
+        return rule
+
+    def setup_before_migration(self, apps):
+        self.project_no_alerts = Project.objects.create(
+            organization_id=self.organization.id, name="p0"
+        )
+        project_no_fallback = Project.objects.create(
+            organization_id=self.organization.id, name="p1"
+        )
+        project_with_fallback_on = Project.objects.create(
+            organization_id=self.organization.id, name="p2"
+        )
+        project_with_fallback_off = Project.objects.create(
+            organization_id=self.organization.id, name="p3"
+        )
+        project_with_fallback_set = Project.objects.create(
+            organization_id=self.organization.id, name="p4"
+        )
+        ProjectOwnership.objects.create(project=project_with_fallback_on, fallthrough="True")
+        ProjectOwnership.objects.create(project=project_with_fallback_off, fallthrough="False")
+        ProjectOwnership.objects.create(project=project_with_fallback_set, fallthrough="False")
+
+        self.alerts = [
+            self.create_issue_alert("alert1", project)
+            for project in [
+                project_no_fallback,
+                project_with_fallback_on,
+                project_with_fallback_off,
+            ]
+        ]
+
+        self.alerts.append(
+            self.create_issue_alert("alert1", project_with_fallback_set, set_fallthrough=True)
+        )
+
+    def test(self):
+        assert not Rule.objects.filter(project_id=self.project_no_alerts.id).exists()
+        for alert, expected_type in zip(
+            self.alerts,
+            ["ActiveMembers", "AllMembers", "NoOne", "AllMembers"],
+        ):
+            alert = Rule.objects.get(id=alert.id)
+            action = alert.data["actions"][0]
+            assert action["fallthroughType"] == expected_type