Fixed #25551 -- Fixed migration operations ordering when adding fields and a unique_together constraint.

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