Revert "Fixed #22397 -- Issues removing M2M field with explicit through model."

This reverts commit 00e3b9a2a992ee0b7288eeeb03e7cbd52ebc6dce.

It's causing a regression when tested with the proxy_model_inheritance tests.

django/db/backends/sqlite3/schema.py

@@ -121,15 +121,11 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
         Removes a field from a model. Usually involves deleting a column,
         but for M2Ms may involve deleting a table.
         """
-        # M2M fields are a special case
-        if isinstance(field, ManyToManyField):
-            # For implicit M2M tables, delete the auto-created table
-            if field.rel.through._meta.auto_created:
-                self.delete_model(field.rel.through)
-            # For explicit "through" M2M fields, do nothing
+        # Special-case implicit M2M tables
+        if isinstance(field, ManyToManyField) and field.rel.through._meta.auto_created:
+            return self.delete_model(field.rel.through)
         # For everything else, remake.
-        else:
-            self._remake_table(model, delete_fields=[field])
+        self._remake_table(model, delete_fields=[field])
 
     def alter_field(self, model, old_field, new_field, strict=False):
         """

django/db/migrations/autodetector.py

@@ -223,6 +223,16 @@ class MigrationAutodetector(object):
                     unique_together=unique_together
                 )
             )
+        # Removing models
+        removed_models = set(old_model_keys) - set(new_model_keys)
+        for app_label, model_name in removed_models:
+            model_state = self.from_state.models[app_label, model_name]
+            self.add_to_migration(
+                app_label,
+                operations.DeleteModel(
+                    model_state.name,
+                )
+            )
         # Changes within models
         kept_models = set(old_model_keys).intersection(new_model_keys)
         old_fields = set()
@@ -338,16 +348,6 @@ class MigrationAutodetector(object):
                 )
         for app_label, operation in unique_together_operations:
             self.add_to_migration(app_label, operation)
-        # Removing models
-        removed_models = set(old_model_keys) - set(new_model_keys)
-        for app_label, model_name in removed_models:
-            model_state = self.from_state.models[app_label, model_name]
-            self.add_to_migration(
-                app_label,
-                operations.DeleteModel(
-                    model_state.name,
-                )
-            )
         # Alright, now add internal dependencies
         for app_label, migrations in self.migrations.items():
             for m1, m2 in zip(migrations, migrations[1:]):

django/db/migrations/migration.py

@@ -127,7 +127,6 @@ class Migration(object):
         to_run.reverse()
         for operation, to_state, from_state in to_run:
             operation.database_backwards(self.app_label, schema_editor, from_state, to_state)
-        return project_state
 
 
 def swappable_dependency(value):

django/db/migrations/state.py

@@ -51,16 +51,6 @@ class ProjectState(object):
                 if len(new_unrendered_models) == len(unrendered_models):
                     raise InvalidBasesError("Cannot resolve bases for %r" % new_unrendered_models)
                 unrendered_models = new_unrendered_models
-            # make sure apps has no dangling references
-            if self.apps._pending_lookups:
-                # Raise an error with a best-effort helpful message
-                # (only for the first issue). Error message should look like:
-                # "ValueError: Lookup failed for model referenced by
-                # field migrations.Book.author: migrations.Author"
-                dangling_lookup = list(self.apps._pending_lookups.items())[0]
-                raise ValueError("Lookup failed for model referenced by field {field}: {model[0]}.{model[1]}".format(
-                    field=dangling_lookup[1][0][1],
-                    model=dangling_lookup[0]))
         return self.apps
 
     @classmethod

tests/migrations/test_autodetector.py

@@ -33,15 +33,11 @@ class AutodetectorTests(TestCase):
     other_stable = ModelState("otherapp", "Stable", [("id", models.AutoField(primary_key=True))])
     third_thing = ModelState("thirdapp", "Thing", [("id", models.AutoField(primary_key=True))])
     book = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))])
-    book_with_no_author = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("title", models.CharField(max_length=200))])
     book_with_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))])
     book_with_field_and_author_renamed = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("writer", models.ForeignKey("testapp.Writer")), ("title", models.CharField(max_length=200))])
-    book_with_multiple_authors = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("authors", models.ManyToManyField("testapp.Author")), ("title", models.CharField(max_length=200))])
-    book_with_multiple_authors_through_attribution = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("authors", models.ManyToManyField("testapp.Author", through="otherapp.Attribution")), ("title", models.CharField(max_length=200))])
     book_unique = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("author", "title")]})
     book_unique_2 = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("title", "author")]})
     book_unique_3 = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("newfield", models.IntegerField()), ("author", models.ForeignKey("testapp.Author")), ("title", models.CharField(max_length=200))], {"unique_together": [("title", "newfield")]})
-    attribution = ModelState("otherapp", "Attribution", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("testapp.Author")), ("book", models.ForeignKey("otherapp.Book"))])
     edition = ModelState("thirdapp", "Edition", [("id", models.AutoField(primary_key=True)), ("book", models.ForeignKey("otherapp.Book"))])
     custom_user = ModelState("thirdapp", "CustomUser", [("id", models.AutoField(primary_key=True)), ("username", models.CharField(max_length=255))])
     knight = ModelState("eggs", "Knight", [("id", models.AutoField(primary_key=True))])
@@ -556,65 +552,18 @@ class AutodetectorTests(TestCase):
         """
         # Make state
         before = self.make_project_state([self.author_with_publisher_string])
-        after = self.make_project_state([self.author_with_publisher, self.publisher])
+        after = self.make_project_state([self.author_with_publisher])
         autodetector = MigrationAutodetector(before, after)
         changes = autodetector._detect_changes()
         # Right number of migrations?
         self.assertEqual(len(changes['testapp']), 1)
         # Right number of actions?
         migration = changes['testapp'][0]
-        self.assertEqual(len(migration.operations), 3)
+        self.assertEqual(len(migration.operations), 2)
         # Right actions?
         action = migration.operations[0]
-        self.assertEqual(action.__class__.__name__, "CreateModel")
-        self.assertEqual(action.name, "Publisher")
-        action = migration.operations[1]
         self.assertEqual(action.__class__.__name__, "AddField")
         self.assertEqual(action.name, "publisher")
-        action = migration.operations[2]
-        self.assertEqual(action.__class__.__name__, "RemoveField")
-        self.assertEqual(action.name, "publisher_name")
-
-    def test_foreign_key_removed_before_target_model(self):
-        """
-        Removing an FK and the model it targets in the same change must remove
-        the FK field before the model to maintain consistency.
-        """
-        before = self.make_project_state([self.author_with_publisher, self.publisher])
-        after = self.make_project_state([self.author_name])  # removes both the model and FK
-        autodetector = MigrationAutodetector(before, after)
-        changes = autodetector._detect_changes()
-        # Right number of migrations?
-        self.assertEqual(len(changes['testapp']), 1)
-        # Right number of actions?
-        migration = changes['testapp'][0]
-        self.assertEqual(len(migration.operations), 2)
-        # Right actions in right order?
-        action = migration.operations[0]
-        self.assertEqual(action.__class__.__name__, "RemoveField")
-        self.assertEqual(action.name, "publisher")
         action = migration.operations[1]
-        self.assertEqual(action.__class__.__name__, "DeleteModel")
-        self.assertEqual(action.name, "Publisher")
-
-    def test_many_to_many_removed_before_through_model(self):
-        """
-        Removing a ManyToManyField and the "through" model in the same change must remove
-        the field before the model to maintain consistency.
-        """
-        before = self.make_project_state([self.book_with_multiple_authors_through_attribution, self.author_name, self.attribution])
-        after = self.make_project_state([self.book_with_no_author, self.author_name])  # removes both the through model and ManyToMany
-        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), 2)
-        # Right actions in right order?
-        action = migration.operations[0]
         self.assertEqual(action.__class__.__name__, "RemoveField")
-        self.assertEqual(action.name, "authors")
-        action = migration.operations[1]
-        self.assertEqual(action.__class__.__name__, "DeleteModel")
-        self.assertEqual(action.name, "Attribution")
+        self.assertEqual(action.name, "publisher_name")

tests/migrations/test_operations.py

@@ -1,12 +1,9 @@
 import unittest
-
-from django.db import connection, migrations, models, router
-from django.db.migrations.migration import Migration
-from django.db.migrations.state import ProjectState
+from django.db import connection, models, migrations, router
 from django.db.models.fields import NOT_PROVIDED
 from django.db.transaction import atomic
 from django.db.utils import IntegrityError
-
+from django.db.migrations.state import ProjectState
 from .test_base import MigrationTestBase
 
 
@@ -18,16 +15,15 @@ class OperationTests(MigrationTestBase):
     """
 
     def apply_operations(self, app_label, project_state, operations):
-        migration = Migration('name', app_label)
-        migration.operations = operations
-        with connection.schema_editor() as editor:
-            return migration.apply(project_state, editor)
+        new_state = project_state.clone()
+        for operation in operations:
+            operation.state_forwards(app_label, new_state)
 
-    def unapply_operations(self, app_label, project_state, operations):
-        migration = Migration('name', app_label)
-        migration.operations = operations
+        # Set up the database
         with connection.schema_editor() as editor:
-            return migration.unapply(project_state, editor)
+            for operation in operations:
+                operation.database_forwards(app_label, editor, project_state, new_state)
+        return new_state
 
     def set_up_test_model(self, app_label, second_model=False, related_model=False, mti_model=False):
         """
@@ -400,38 +396,6 @@ class OperationTests(MigrationTestBase):
         Pony = new_apps.get_model("test_alflmm", "Pony")
         self.assertTrue(Pony._meta.get_field('stables').blank)
 
-    def test_remove_field_m2m(self):
-        project_state = self.set_up_test_model("test_rmflmm", second_model=True)
-
-        project_state = self.apply_operations("test_rmflmm", project_state, operations=[
-            migrations.AddField("Pony", "stables", models.ManyToManyField("Stable", related_name="ponies"))
-        ])
-        self.assertTableExists("test_rmflmm_pony_stables")
-
-        operations = [migrations.RemoveField("Pony", "stables")]
-        self.apply_operations("test_rmflmm", project_state, operations=operations)
-        self.assertTableNotExists("test_rmflmm_pony_stables")
-
-        # And test reversal
-        self.unapply_operations("test_rmflmm", project_state, operations=operations)
-        self.assertTableExists("test_rmflmm_pony_stables")
-
-    def test_remove_field_m2m_with_through(self):
-        project_state = self.set_up_test_model("test_rmflmmwt", second_model=True)
-
-        self.assertTableNotExists("test_rmflmmwt_ponystables")
-        project_state = self.apply_operations("test_rmflmmwt", project_state, operations=[
-            migrations.CreateModel("PonyStables", fields=[
-                ("pony", models.ForeignKey('test_rmflmmwt.Pony')),
-                ("stable", models.ForeignKey('test_rmflmmwt.Stable')),
-            ]),
-            migrations.AddField("Pony", "stables", models.ManyToManyField("Stable", related_name="ponies", through='test_rmflmmwt.PonyStables'))
-        ])
-        self.assertTableExists("test_rmflmmwt_ponystables")
-
-        operations = [migrations.RemoveField("Pony", "stables")]
-        self.apply_operations("test_rmflmmwt", project_state, operations=operations)
-
     def test_remove_field(self):
         """
         Tests the RemoveField operation.

tests/migrations/test_state.py

@@ -291,33 +291,3 @@ class StateTests(TestCase):
         ))
         self.assertNotEqual(project_state, other_state)
         self.assertEqual(project_state == other_state, False)
-
-    def test_dangling_references_throw_error(self):
-        class Author(models.Model):
-            name = models.TextField()
-
-        class Book(models.Model):
-            author = models.ForeignKey(Author)
-
-        class Magazine(models.Model):
-            authors = models.ManyToManyField(Author)
-
-        # Make a valid ProjectState and render it
-        project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(Author))
-        project_state.add_model_state(ModelState.from_model(Book))
-        project_state.add_model_state(ModelState.from_model(Magazine))
-        rendered_state = project_state.render()
-        self.assertEqual(len(rendered_state.get_models()), 3)
-
-        # now make an invalid one with a ForeignKey
-        project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(Book))
-        with self.assertRaises(ValueError):
-            rendered_state = project_state.render()
-
-        # and another with ManyToManyField
-        project_state = ProjectState()
-        project_state.add_model_state(ModelState.from_model(Magazine))
-        with self.assertRaises(ValueError):
-            rendered_state = project_state.render()