Browse Source

fix(migrations): Allow m2m columns to be deleted without first making them not null (#81209)

These columns don't actually exist on the table, they just reference a
bridging table.
Dan Fuller 3 months ago
parent
commit
a89abbec9e

+ 0 - 0
fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/__init__.py


+ 55 - 0
fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0001_initial.py

@@ -0,0 +1,55 @@
+from django.db import migrations, models
+
+from sentry.db.models import FlexibleForeignKey
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+
+    initial = True
+    checked = False
+
+    dependencies = []
+
+    operations = [
+        migrations.CreateModel(
+            name="OtherTable",
+            fields=[
+                ("id", models.AutoField(auto_created=True, primary_key=True, serialize=False)),
+            ],
+        ),
+        migrations.CreateModel(
+            name="M2MTable",
+            fields=[
+                ("id", models.AutoField(auto_created=True, primary_key=True, serialize=False)),
+                (
+                    "alert_rule",
+                    FlexibleForeignKey(
+                        on_delete=models.deletion.CASCADE,
+                        to="good_flow_delete_field_pending_with_not_null_m2m_app.othertable",
+                    ),
+                ),
+            ],
+        ),
+        migrations.CreateModel(
+            name="TestTable",
+            fields=[
+                ("id", models.AutoField(auto_created=True, primary_key=True, serialize=False)),
+                (
+                    "excluded_projects",
+                    models.ManyToManyField(
+                        through="good_flow_delete_field_pending_with_not_null_m2m_app.M2MTable",
+                        to="good_flow_delete_field_pending_with_not_null_m2m_app.othertable",
+                    ),
+                ),
+            ],
+        ),
+        migrations.AddField(
+            model_name="m2mtable",
+            name="test_table",
+            field=FlexibleForeignKey(
+                on_delete=models.deletion.CASCADE,
+                to="good_flow_delete_field_pending_with_not_null_m2m_app.testtable",
+            ),
+        ),
+    ]

+ 17 - 0
fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0002_delete_without_pending.py

@@ -0,0 +1,17 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.fields import SafeRemoveField
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+    dependencies = [
+        ("good_flow_delete_field_pending_with_not_null_m2m_app", "0001_initial"),
+    ]
+
+    operations = [
+        SafeRemoveField(
+            model_name="testtable",
+            name="excluded_projects",
+            deletion_action=DeletionAction.MOVE_TO_PENDING,
+        ),
+    ]

+ 0 - 0
fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/__init__.py


+ 18 - 0
fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/models.py

@@ -0,0 +1,18 @@
+from django.db import models
+
+from sentry.db.models import FlexibleForeignKey
+
+
+class OtherTable(models.Model):
+    pass
+
+
+class M2MTable(models.Model):
+    alert_rule = FlexibleForeignKey(OtherTable)
+    test_table = FlexibleForeignKey(
+        "good_flow_delete_field_pending_with_not_null_m2m_app.TestTable"
+    )
+
+
+class TestTable(models.Model):
+    excluded_projects = models.ManyToManyField(OtherTable, through=M2MTable)

+ 6 - 2
src/sentry/new_migrations/monkey/fields.py

@@ -1,5 +1,5 @@
 from django.db.migrations import RemoveField
-from django.db.models import Field
+from django.db.models import Field, ManyToManyField
 from django.db.models.fields import NOT_PROVIDED
 from django_zero_downtime_migrations.backends.postgres.schema import UnsafeOperationException
 
@@ -37,7 +37,11 @@ class SafeRemoveField(RemoveField):
                     f"Foreign key db constraint must be removed before dropping {app_label}.{self.model_name_lower}.{self.name}. "
                     "More info: https://develop.sentry.dev/api-server/application-domains/database-migrations/#deleting-columns"
                 )
-            if not field.null and field.db_default is NOT_PROVIDED:
+            if (
+                not isinstance(field, ManyToManyField)
+                and not field.null
+                and field.db_default is NOT_PROVIDED
+            ):
                 raise UnsafeOperationException(
                     f"Field {app_label}.{self.model_name_lower}.{self.name} must either be nullable or have a db_default before dropping. "
                     "More info: https://develop.sentry.dev/api-server/application-domains/database-migrations/#deleting-columns"

+ 9 - 0
tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py

@@ -381,6 +381,15 @@ class DeletionFieldBadDeletePendingWithNotNull(BaseSafeMigrationTest):
             self.run_migration()
 
 
+class DeletionFieldGoodDeletePendingWithNotNullM2M(BaseSafeMigrationTest):
+    app = "good_flow_delete_field_pending_with_not_null_m2m_app"
+    migrate_from = "0001"
+    migrate_to = "0002"
+
+    def test(self):
+        self.run_migration()
+
+
 class ColExistsMixin:
     app = ""