Commit d6e73a87 authored by Andrew Godwin's avatar Andrew Godwin
Browse files

Fixed #23121: AlterModelOptions operation not changing state right

parent cb60d22b
Loading
Loading
Loading
Loading
+3 −13
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ from django.db.migrations import operations
from django.db.migrations.migration import Migration
from django.db.migrations.questioner import MigrationQuestioner
from django.db.migrations.optimizer import MigrationOptimizer
from django.db.migrations.operations.models import AlterModelOptions


class MigrationAutodetector(object):
@@ -25,17 +26,6 @@ class MigrationAutodetector(object):
    if it wishes, with the caveat that it may not always be possible.
    """

    # Model options we want to compare and preserve in an AlterModelOptions op
    ALTER_OPTION_KEYS = [
        "get_latest_by",
        "ordering",
        "permissions",
        "default_permissions",
        "select_on_save",
        "verbose_name",
        "verbose_name_plural",
    ]

    def __init__(self, from_state, to_state, questioner=None):
        self.from_state = from_state
        self.to_state = to_state
@@ -864,11 +854,11 @@ class MigrationAutodetector(object):
            new_model_state = self.to_state.models[app_label, model_name]
            old_options = dict(
                option for option in old_model_state.options.items()
                if option[0] in self.ALTER_OPTION_KEYS
                if option[0] in AlterModelOptions.ALTER_OPTION_KEYS
            )
            new_options = dict(
                option for option in new_model_state.options.items()
                if option[0] in self.ALTER_OPTION_KEYS
                if option[0] in AlterModelOptions.ALTER_OPTION_KEYS
            )
            if old_options != new_options:
                self.add_operation(
+14 −0
Original line number Diff line number Diff line
@@ -337,6 +337,17 @@ class AlterModelOptions(Operation):
    may still need them.
    """

    # Model options we want to compare and preserve in an AlterModelOptions op
    ALTER_OPTION_KEYS = [
        "get_latest_by",
        "ordering",
        "permissions",
        "default_permissions",
        "select_on_save",
        "verbose_name",
        "verbose_name_plural",
    ]

    def __init__(self, name, options):
        self.name = name
        self.options = options
@@ -345,6 +356,9 @@ class AlterModelOptions(Operation):
        model_state = state.models[app_label, self.name.lower()]
        model_state.options = dict(model_state.options)
        model_state.options.update(self.options)
        for key in self.ALTER_OPTION_KEYS:
            if key not in self.options and key in model_state.options:
                del model_state.options[key]

    def database_forwards(self, app_label, schema_editor, from_state, to_state):
        pass
+7 −0
Original line number Diff line number Diff line
@@ -897,6 +897,13 @@ class AutodetectorTests(TestCase):
        self.assertNumberMigrations(changes, "testapp", 1)
        # Right actions in right order?
        self.assertOperationTypes(changes, "testapp", 0, ["AlterModelOptions"])
        # Changing them back to empty should also make a change
        before = self.make_project_state([self.author_with_options])
        after = self.make_project_state([self.author_empty])
        autodetector = MigrationAutodetector(before, after)
        changes = autodetector._detect_changes()
        self.assertNumberMigrations(changes, "testapp", 1)
        self.assertOperationTypes(changes, "testapp", 0, ["AlterModelOptions"])

    def test_alter_model_options_proxy(self):
        """
+21 −5
Original line number Diff line number Diff line
@@ -46,7 +46,7 @@ class OperationTestBase(MigrationTestBase):
        operation.state_forwards(app_label, new_state)
        return project_state, new_state

    def set_up_test_model(self, app_label, second_model=False, third_model=False, related_model=False, mti_model=False, proxy_model=False, unique_together=False):
    def set_up_test_model(self, app_label, second_model=False, third_model=False, related_model=False, mti_model=False, proxy_model=False, unique_together=False, options=False):
        """
        Creates a test model state and database table.
        """
@@ -76,6 +76,12 @@ class OperationTestBase(MigrationTestBase):
            except DatabaseError:
                pass
        # Make the "current" state
        model_options = {
            "swappable": "TEST_SWAP_MODEL",
            "unique_together": [["pink", "weight"]] if unique_together else [],
        }
        if options:
            model_options["permissions"] = [("can_groom", "Can groom")]
        operations = [migrations.CreateModel(
            "Pony",
            [
@@ -83,10 +89,7 @@ class OperationTestBase(MigrationTestBase):
                ("pink", models.IntegerField(default=3)),
                ("weight", models.FloatField()),
            ],
            options={
                "swappable": "TEST_SWAP_MODEL",
                "unique_together": [["pink", "weight"]] if unique_together else [],
            },
            options=model_options,
        )]
        if second_model:
            operations.append(migrations.CreateModel(
@@ -975,6 +978,19 @@ class OperationTests(OperationTestBase):
        self.assertEqual(len(new_state.models["test_almoop", "pony"].options.get("permissions", [])), 1)
        self.assertEqual(new_state.models["test_almoop", "pony"].options["permissions"][0][0], "can_groom")

    def test_alter_model_options_emptying(self):
        """
        Tests that the AlterModelOptions operation removes keys from the dict (#23121)
        """
        project_state = self.set_up_test_model("test_almoop", options=True)
        # Test the state alteration (no DB alteration to test)
        operation = migrations.AlterModelOptions("Pony", {})
        self.assertEqual(operation.describe(), "Change Meta options on Pony")
        new_state = project_state.clone()
        operation.state_forwards("test_almoop", new_state)
        self.assertEqual(len(project_state.models["test_almoop", "pony"].options.get("permissions", [])), 1)
        self.assertEqual(len(new_state.models["test_almoop", "pony"].options.get("permissions", [])), 0)

    def test_alter_order_with_respect_to(self):
        """
        Tests the AlterOrderWithRespectTo operation.