Fixed #31503 -- Made autodetector remove unique/index_together before altering fields.

2 files changed, 181 insertions, 19 deletions

diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py
index d3ac1c93ab..97977d72ef 100644
--- a/django/db/migrations/autodetector.py
+++ b/django/db/migrations/autodetector.py
@@ -178,8 +178,12 @@ class MigrationAutodetector:
         # Generate index removal operations before field is removed
         self.generate_removed_constraints()
         self.generate_removed_indexes()
-        # Generate field operations
+        # Generate field renaming operations.
         self.generate_renamed_fields()
+        # Generate removal of foo together.
+        self.generate_removed_altered_unique_together()
+        self.generate_removed_altered_index_together()
+        # Generate field operations.
         self.generate_removed_fields()
         self.generate_added_fields()
         self.generate_altered_fields()
@@ -1128,8 +1132,7 @@ class MigrationAutodetector:
             dependencies.append((through_app_label, through_object_name, None, True))
         return dependencies
 
-    def _generate_altered_foo_together(self, operation):
-        option_name = operation.option_name
+    def _get_altered_foo_together_operations(self, option_name):
         for app_label, model_name in sorted(self.kept_model_keys):
             old_model_name = self.renamed_models.get((app_label, model_name), model_name)
             old_model_state = self.from_state.models[app_label, old_model_name]
@@ -1157,13 +1160,49 @@ class MigrationAutodetector:
                             dependencies.extend(self._get_dependencies_for_foreign_key(
                                 app_label, model_name, field, self.to_state,
                             ))
+                yield (
+                    old_value,
+                    new_value,
+                    app_label,
+                    model_name,
+                    dependencies,
+                )
 
+    def _generate_removed_altered_foo_together(self, operation):
+        for (
+            old_value,
+            new_value,
+            app_label,
+            model_name,
+            dependencies,
+        ) in self._get_altered_foo_together_operations(operation.option_name):
+            removal_value = new_value.intersection(old_value)
+            if removal_value or old_value:
                 self.add_operation(
                     app_label,
-                    operation(
-                        name=model_name,
-                        **{option_name: new_value}
-                    ),
+                    operation(name=model_name, **{operation.option_name: removal_value}),
+                    dependencies=dependencies,
+                )
+
+    def generate_removed_altered_unique_together(self):
+        self._generate_removed_altered_foo_together(operations.AlterUniqueTogether)
+
+    def generate_removed_altered_index_together(self):
+        self._generate_removed_altered_foo_together(operations.AlterIndexTogether)
+
+    def _generate_altered_foo_together(self, operation):
+        for (
+            old_value,
+            new_value,
+            app_label,
+            model_name,
+            dependencies,
+        ) in self._get_altered_foo_together_operations(operation.option_name):
+            removal_value = new_value.intersection(old_value)
+            if new_value != removal_value:
+                self.add_operation(
+                    app_label,
+                    operation(name=model_name, **{operation.option_name: new_value}),
                     dependencies=dependencies,
                 )
 
diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py
index 8fb4897f0e..eb98da94e3 100644
--- a/tests/migrations/test_autodetector.py
+++ b/tests/migrations/test_autodetector.py
@@ -1570,9 +1570,26 @@ class AutodetectorTests(TestCase):
         )
         # Right number/type of migrations?
         self.assertNumberMigrations(changes, "otherapp", 1)
-        self.assertOperationTypes(changes, "otherapp", 0, ["AlterUniqueTogether", "AlterIndexTogether"])
-        self.assertOperationAttributes(changes, "otherapp", 0, 0, name="book", unique_together={("title", "author")})
-        self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", index_together={("title", "author")})
+        self.assertOperationTypes(changes, 'otherapp', 0, [
+            'AlterUniqueTogether',
+            'AlterIndexTogether',
+            'AlterUniqueTogether',
+            'AlterIndexTogether',
+        ])
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 0, name='book', unique_together=set(),
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 1, name='book', index_together=set(),
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 2, name='book',
+            unique_together={('title', 'author')},
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 3, name='book',
+            index_together={('title', 'author')},
+        )
 
     def test_add_field_and_foo_together(self):
         """
@@ -1619,10 +1636,100 @@ class AutodetectorTests(TestCase):
         )
         # Right number/type of migrations?
         self.assertNumberMigrations(changes, "otherapp", 1)
-        self.assertOperationTypes(changes, "otherapp", 0, ["AlterUniqueTogether", "AlterIndexTogether", "RemoveField"])
-        self.assertOperationAttributes(changes, "otherapp", 0, 0, name="book", unique_together={("author", "title")})
-        self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", index_together={("author", "title")})
-        self.assertOperationAttributes(changes, "otherapp", 0, 2, model_name="book", name="newfield")
+        self.assertOperationTypes(changes, 'otherapp', 0, [
+            'AlterUniqueTogether',
+            'AlterIndexTogether',
+            'AlterUniqueTogether',
+            'AlterIndexTogether',
+            'RemoveField',
+        ])
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 0, name='book', unique_together=set(),
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 1, name='book', index_together=set(),
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 2, name='book',
+            unique_together={('author', 'title')},
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 3, name='book',
+            index_together={('author', 'title')},
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 4, model_name='book', name='newfield',
+        )
+
+    def test_alter_field_and_foo_together(self):
+        """Fields are altered after deleting some index/unique_together."""
+        initial_author = ModelState('testapp', 'Author', [
+            ('id', models.AutoField(primary_key=True)),
+            ('name', models.CharField(max_length=200)),
+            ('age', models.IntegerField(db_index=True)),
+        ], {
+            'unique_together': {('name',)},
+        })
+        author_reversed_constraints = ModelState('testapp', 'Author', [
+            ('id', models.AutoField(primary_key=True)),
+            ('name', models.CharField(max_length=200, unique=True)),
+            ('age', models.IntegerField()),
+        ], {
+            'index_together': {('age',)},
+        })
+        changes = self.get_changes([initial_author], [author_reversed_constraints])
+
+        self.assertNumberMigrations(changes, 'testapp', 1)
+        self.assertOperationTypes(changes, 'testapp', 0, [
+            'AlterUniqueTogether',
+            'AlterField',
+            'AlterField',
+            'AlterIndexTogether',
+        ])
+        self.assertOperationAttributes(
+            changes, 'testapp', 0, 0, name='author', unique_together=set(),
+        )
+        self.assertOperationAttributes(
+            changes, 'testapp', 0, 1, model_name='author', name='age',
+        )
+        self.assertOperationAttributes(
+            changes, 'testapp', 0, 2, model_name='author', name='name',
+        )
+        self.assertOperationAttributes(
+            changes, 'testapp', 0, 3, name='author', index_together={('age',)},
+        )
+
+    def test_partly_alter_foo_together(self):
+        initial_author = ModelState('testapp', 'Author', [
+            ('id', models.AutoField(primary_key=True)),
+            ('name', models.CharField(max_length=200)),
+            ('age', models.IntegerField()),
+        ], {
+            'unique_together': {('name',), ('age',)},
+            'index_together': {('name',)},
+        })
+        author_reversed_constraints = ModelState('testapp', 'Author', [
+            ('id', models.AutoField(primary_key=True)),
+            ('name', models.CharField(max_length=200)),
+            ('age', models.IntegerField()),
+        ], {
+            'unique_together': {('age',)},
+            'index_together': {('name',), ('age',)},
+        })
+        changes = self.get_changes([initial_author], [author_reversed_constraints])
+
+        self.assertNumberMigrations(changes, 'testapp', 1)
+        self.assertOperationTypes(changes, 'testapp', 0, [
+            'AlterUniqueTogether',
+            'AlterIndexTogether',
+        ])
+        self.assertOperationAttributes(
+            changes, 'testapp', 0, 0, name='author', unique_together={('age',)},
+        )
+        self.assertOperationAttributes(
+            changes, 'testapp', 0, 1, name='author',
+            index_together={('name',), ('age',)},
+        )
 
     def test_rename_field_and_foo_together(self):
         """
@@ -1635,11 +1742,27 @@ class AutodetectorTests(TestCase):
         )
         # Right number/type of migrations?
         self.assertNumberMigrations(changes, "otherapp", 1)
-        self.assertOperationTypes(changes, "otherapp", 0, ["RenameField", "AlterUniqueTogether", "AlterIndexTogether"])
-        self.assertOperationAttributes(changes, "otherapp", 0, 1, name="book", unique_together={
-            ("title", "newfield2")
-        })
-        self.assertOperationAttributes(changes, "otherapp", 0, 2, name="book", index_together={("title", "newfield2")})
+        self.assertOperationTypes(changes, 'otherapp', 0, [
+            'RenameField',
+            'AlterUniqueTogether',
+            'AlterIndexTogether',
+            'AlterUniqueTogether',
+            'AlterIndexTogether',
+        ])
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 1, name='book', unique_together=set(),
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 2, name='book', index_together=set(),
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 3, name='book',
+            unique_together={('title', 'newfield2')},
+        )
+        self.assertOperationAttributes(
+            changes, 'otherapp', 0, 4, name='book',
+            index_together={('title', 'newfield2')},
+        )
 
     def test_proxy(self):
         """The autodetector correctly deals with proxy models."""