From 73a6ab6382809d5452907dcff5767403d8d66985 Mon Sep 17 00:00:00 2001 From: Ana Vojnovic Date: Sat, 7 Nov 2015 15:26:25 +0100 Subject: [PATCH] Fixed #25551 -- Fixed migration operations ordering when adding fields and a unique_together constraint. --- django/db/migrations/autodetector.py | 63 +++++++++++++-------------- tests/migrations/test_autodetector.py | 28 ++++++++++++ 2 files changed, 58 insertions(+), 33 deletions(-) diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 513bb543d1..f28c89cc09 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -558,22 +558,7 @@ class MigrationAutodetector(object): # Generate operations for each related field for name, field in sorted(related_fields.items()): - # Account for FKs to swappable models - swappable_setting = getattr(field, 'swappable_setting', None) - if swappable_setting is not None: - dep_app_label = "__setting__" - dep_object_name = swappable_setting - else: - dep_app_label = field.remote_field.model._meta.app_label - dep_object_name = field.remote_field.model._meta.object_name - dependencies = [(dep_app_label, dep_object_name, None, True)] - if getattr(field.remote_field, "through", None) and not field.remote_field.through._meta.auto_created: - dependencies.append(( - field.remote_field.through._meta.app_label, - field.remote_field.through._meta.object_name, - None, - True - )) + dependencies = self._get_dependecies_for_foreign_key(field) # Depend on our own model being created dependencies.append((app_label, model_name, None, True)) # Make operation @@ -810,22 +795,7 @@ class MigrationAutodetector(object): # Fields that are foreignkeys/m2ms depend on stuff dependencies = [] if field.remote_field and field.remote_field.model: - # Account for FKs to swappable models - swappable_setting = getattr(field, 'swappable_setting', None) - if swappable_setting is not None: - dep_app_label = "__setting__" - dep_object_name = swappable_setting - else: - dep_app_label = field.remote_field.model._meta.app_label - dep_object_name = field.remote_field.model._meta.object_name - dependencies = [(dep_app_label, dep_object_name, None, True)] - if getattr(field.remote_field, "through", None) and not field.remote_field.through._meta.auto_created: - dependencies.append(( - field.remote_field.through._meta.app_label, - field.remote_field.through._meta.object_name, - None, - True, - )) + dependencies.extend(self._get_dependecies_for_foreign_key(field)) # You can't just add NOT NULL fields with no default or fields # which don't allow empty strings as default. preserve_default = True @@ -925,6 +895,25 @@ class MigrationAutodetector(object): self._generate_removed_field(app_label, model_name, field_name) self._generate_added_field(app_label, model_name, field_name) + def _get_dependecies_for_foreign_key(self, field): + # Account for FKs to swappable models + swappable_setting = getattr(field, 'swappable_setting', None) + if swappable_setting is not None: + dep_app_label = "__setting__" + dep_object_name = swappable_setting + else: + dep_app_label = field.remote_field.model._meta.app_label + dep_object_name = field.remote_field.model._meta.object_name + dependencies = [(dep_app_label, dep_object_name, None, True)] + if getattr(field.remote_field, "through", None) and not field.remote_field.through._meta.auto_created: + dependencies.append(( + field.remote_field.through._meta.app_label, + field.remote_field.through._meta.object_name, + None, + True, + )) + return dependencies + def _generate_altered_foo_together(self, operation): option_name = operation.option_name for app_label, model_name in sorted(self.kept_model_keys): @@ -948,12 +937,20 @@ class MigrationAutodetector(object): new_value = set(new_value) if old_value != new_value: + dependencies = [] + for foo_togethers in new_value: + for field_name in foo_togethers: + field = self.new_apps.get_model(app_label, model_name)._meta.get_field(field_name) + if field.remote_field and field.remote_field.model: + dependencies.extend(self._get_dependecies_for_foreign_key(field)) + self.add_operation( app_label, operation( name=model_name, **{option_name: new_value} - ) + ), + dependencies=dependencies, ) def generate_altered_unique_together(self): diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index 40ad1d450c..1ed74226f8 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -1174,6 +1174,34 @@ class AutodetectorTests(TestCase): self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", unique_together={("title", "newfield")}) self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", index_together={("title", "newfield")}) + def test_create_model_and_unique_together(self): + author = ModelState("otherapp", "Author", [ + ("id", models.AutoField(primary_key=True)), + ("name", models.CharField(max_length=200)), + ]) + book_with_author = ModelState("otherapp", "Book", [ + ("id", models.AutoField(primary_key=True)), + ("author", models.ForeignKey("otherapp.Author", models.CASCADE)), + ("title", models.CharField(max_length=200)), + ], { + "index_together": {("title", "author")}, + "unique_together": {("title", "author")}, + }) + before = self.make_project_state([self.book_with_no_author]) + after = self.make_project_state([author, book_with_author]) + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes() + # Right number of migrations? + self.assertEqual(len(changes['otherapp']), 1) + # Right number of actions? + migration = changes['otherapp'][0] + self.assertEqual(len(migration.operations), 4) + # Right actions order? + self.assertOperationTypes( + changes, 'otherapp', 0, + ['CreateModel', 'AddField', 'AlterUniqueTogether', 'AlterIndexTogether'] + ) + def test_remove_field_and_foo_together(self): """ Tests that removed fields will be removed after updating