From a89abbec9e045208d60df6d9b937bca16c91cf6e Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 22 Nov 2024 14:14:45 -0800 Subject: [PATCH] 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. --- .../__init__.py | 0 .../migrations/0001_initial.py | 55 +++++++++++++++++++ .../migrations/0002_delete_without_pending.py | 17 ++++++ .../migrations/__init__.py | 0 .../models.py | 18 ++++++ src/sentry/new_migrations/monkey/fields.py | 8 ++- .../integration/test_migrations.py | 9 +++ 7 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/__init__.py create mode 100644 fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0001_initial.py create mode 100644 fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0002_delete_without_pending.py create mode 100644 fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/__init__.py create mode 100644 fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/models.py diff --git a/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/__init__.py b/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0001_initial.py b/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0001_initial.py new file mode 100644 index 0000000000000..0c0b198c43b88 --- /dev/null +++ b/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", + ), + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0002_delete_without_pending.py b/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/0002_delete_without_pending.py new file mode 100644 index 0000000000000..7cd3b1a05e1e0 --- /dev/null +++ b/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, + ), + ] diff --git a/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/__init__.py b/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/migrations/__init__.py new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/models.py b/fixtures/safe_migrations_apps/good_flow_delete_field_pending_with_not_null_m2m_app/models.py new file mode 100644 index 0000000000000..90c1bc2b9e0f5 --- /dev/null +++ b/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) diff --git a/src/sentry/new_migrations/monkey/fields.py b/src/sentry/new_migrations/monkey/fields.py index 119e64fb8dafc..b11e363dfb68c 100644 --- a/src/sentry/new_migrations/monkey/fields.py +++ b/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 @@ def state_forwards(self, app_label: str, state: SentryProjectState) -> None: # 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" diff --git a/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py b/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py index 80dec14906ed3..23e5077acf28c 100644 --- a/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py +++ b/tests/sentry/db/postgres/schema/safe_migrations/integration/test_migrations.py @@ -381,6 +381,15 @@ def test(self): 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 = ""