migrations(crons): Slugify invalid monitor slugs

getsentry · Jul 7, 2023 · 0fd61b0 · 0fd61b0
1 parent 23e3fb5
commit 0fd61b0
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 7 deletions.
diff --git a/src/sentry/migrations/0507_slugify_invalid_monitors.py b/src/sentry/migrations/0507_slugify_invalid_monitors.py
@@ -1,5 +1,5 @@
 # Generated by Django 2.2.28 on 2023-07-07 15:51
-from django.db import migrations
+from django.db import IntegrityError, migrations
 from django.utils.text import slugify
 
 from sentry.new_migrations.migrations import CheckedMigration
@@ -10,6 +10,7 @@ def migrate_monitor_slugs(apps, schema_editor):
     Monitor = apps.get_model("sentry", "Monitor")
 
     MAX_SLUG_LENGTH = 50
+    monitors_to_clean_up = []
 
     for monitor in RangeQuerySetWrapperWithProgressBar(Monitor.objects.all()):
         monitor_slug = monitor.slug
@@ -20,7 +21,13 @@ def migrate_monitor_slugs(apps, schema_editor):
             continue
 
         monitor.slug = slugified
-        monitor.save()
+        try:
+            monitor.save()
+        except IntegrityError:
+            # If there is a collision, delete the old monitor as the new one is receiving all check-ins
+            monitors_to_clean_up.append(monitor.id)
+
+    Monitor.objects.filter(id__in=monitors_to_clean_up).delete()
 
 
 class Migration(CheckedMigration):

diff --git a/tests/sentry/migrations/test_0507_slugify_invalid_monitors.py b/tests/sentry/migrations/test_0507_slugify_invalid_monitors.py
@@ -1,3 +1,4 @@
+import pytest
 from django.utils.text import slugify
 
 from sentry.monitors.models import Monitor, MonitorType, ScheduleType
@@ -27,9 +28,27 @@ def setup_before_migration(self, apps):
             config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
         )
 
-        self.valid_monitor = Monitor.objects.create(
-            name="valid",
-            slug="valid",
+        self.invalid_monitor_3 = Monitor.objects.create(
+            name="invalid_2",
+            slug="/api/cron/3",
+            organization_id=self.organization.id,
+            project_id=self.project.id,
+            type=MonitorType.CRON_JOB,
+            config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
+        )
+
+        self.valid_monitor_1 = Monitor.objects.create(
+            name="valid_1",
+            slug="valid_1",
+            organization_id=self.organization.id,
+            project_id=self.project.id,
+            type=MonitorType.CRON_JOB,
+            config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
+        )
+
+        self.valid_monitor_2 = Monitor.objects.create(
+            name="valid_2",
+            slug="apicron3",
             organization_id=self.organization.id,
             project_id=self.project.id,
             type=MonitorType.CRON_JOB,
@@ -41,12 +60,20 @@ def test(self):
 
         invalid_monitor_1 = Monitor.objects.get(id=self.invalid_monitor_1.id)
         invalid_monitor_2 = Monitor.objects.get(id=self.invalid_monitor_2.id)
-        valid_monitor = Monitor.objects.get(id=self.valid_monitor.id)
+
+        valid_monitor_1 = Monitor.objects.get(id=self.valid_monitor_1.id)
+        valid_monitor_2 = Monitor.objects.get(id=self.valid_monitor_2.id)
 
         assert invalid_monitor_1.slug == slugify(self.invalid_monitor_1.slug)[
             :MAX_SLUG_LENGTH
         ].strip("-")
         assert invalid_monitor_2.slug == slugify(self.invalid_monitor_2.slug)[
             :MAX_SLUG_LENGTH
         ].strip("-")
-        assert valid_monitor.slug == self.valid_monitor.slug
+
+        # assert colliding monitor does not exist
+        with pytest.raises(Monitor.DoesNotExist):
+            Monitor.objects.get(id=self.invalid_monitor_3.id)
+
+        assert valid_monitor_1.slug == self.valid_monitor_1.slug
+        assert valid_monitor_2.slug == self.valid_monitor_2.slug