[1.7.x] Fixed #23474 -- Prevented migrating backwards from unapplying the wrong migrations.

Backport of abcf28a076 from master

8 files changed, 83 insertions, 5 deletions

diff --git a/django/db/migrations/executor.py b/django/db/migrations/executor.py
index b6275690f9..503ccddc95 100644
--- a/django/db/migrations/executor.py
+++ b/django/db/migrations/executor.py
@@ -36,11 +36,15 @@ class MigrationExecutor(object):
             # If the migration is already applied, do backwards mode,
             # otherwise do forwards mode.
             elif target in applied:
-                backwards_plan = self.loader.graph.backwards_plan(target)[:-1]
-                # We only do this if the migration is not the most recent one
-                # in its app - that is, another migration with the same app
-                # label is in the backwards plan
-                if any(node[0] == target[0] for node in backwards_plan):
+                app_label = target[0]
+                next_migration_prefix = str(int(target[1][:4]) + 1)
+                try:
+                    next_migration = self.loader.get_migration_by_prefix(app_label, next_migration_prefix)
+                except KeyError:
+                    # If `target` is the most recent one in its app, there is nothing to do.
+                    pass
+                else:
+                    backwards_plan = self.loader.graph.backwards_plan(next_migration)
                     for migration in backwards_plan:
                         if migration in applied:
                             plan.append((self.loader.graph.nodes[migration], True))
diff --git a/docs/releases/1.7.1.txt b/docs/releases/1.7.1.txt
index f5f3f515a9..8438b7f6d2 100644
--- a/docs/releases/1.7.1.txt
+++ b/docs/releases/1.7.1.txt
@@ -46,3 +46,6 @@ Bugfixes
 
 * Allowed migrations to work with ``app_label``\s that have the same last
   part (e.g. ``django.contrib.auth`` and ``vendor.auth``) (:ticket:`23483`).
+
+* Fixed bug in migrations that could cause unexpected data loss when executing
+  a backwards or no-op migration (:ticket:`23474`).
diff --git a/tests/migrations/test_executor.py b/tests/migrations/test_executor.py
index d07eecfcef..4631b80577 100644
--- a/tests/migrations/test_executor.py
+++ b/tests/migrations/test_executor.py
@@ -231,3 +231,41 @@ class ExecutorTests(MigrationTestBase):
         executor.migrate([("migrations", None)])
         self.assertTableNotExists("migrations_author")
         self.assertTableNotExists("migrations_tribble")
+
+    @override_settings(
+        MIGRATION_MODULES={
+          "migrations": "migrations.test_migrations_backwards_deps_1",
+          "migrations2": "migrations2.test_migrations_backwards_deps_2",
+        },
+    )
+    def test_backwards_deps(self):
+        """
+        #23474 - Migrating backwards shouldn't cause the wrong migrations to be
+        unapplied.
+
+        Migration dependencies (x -> y === y depends on x):
+        m.0001 -+-> m.0002
+                +-> m2.0001
+
+        1) Migrate m2 to 0001, causing { m.0001, m2.0002 } to be applied.
+        2) Migrate m to 0001. m.0001 has already been applied, so this should
+           be a noop.
+        """
+        executor = MigrationExecutor(connection)
+        executor.migrate([("migrations2", "0001_initial")])
+        try:
+            self.assertTableExists("migrations2_example")
+            # Rebuild the graph to reflect the new DB state
+            executor.loader.build_graph()
+            self.assertEqual(
+                executor.migration_plan([("migrations", "0001_initial")]),
+                [],
+            )
+            executor.migrate([("migrations", "0001_initial")])
+            self.assertTableExists("migrations2_example")
+        finally:
+            # And migrate back to clean up the database
+            executor.loader.build_graph()
+            executor.migrate([("migrations", None)])
+            self.assertTableNotExists("migrations_author")
+            self.assertTableNotExists("migrations_tribble")
diff --git a/tests/migrations/test_migrations_backwards_deps_1/0001_initial.py b/tests/migrations/test_migrations_backwards_deps_1/0001_initial.py
new file mode 100644
index 0000000000..8e1925b472
--- /dev/null
+++ b/tests/migrations/test_migrations_backwards_deps_1/0001_initial.py
@@ -0,0 +1,8 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    operations = []
diff --git a/tests/migrations/test_migrations_backwards_deps_1/0002_second.py b/tests/migrations/test_migrations_backwards_deps_1/0002_second.py
new file mode 100644
index 0000000000..512f32e97c
--- /dev/null
+++ b/tests/migrations/test_migrations_backwards_deps_1/0002_second.py
@@ -0,0 +1,9 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [('migrations', '0001_initial')]
+    operations = []
diff --git a/tests/migrations/test_migrations_backwards_deps_1/__init__.py b/tests/migrations/test_migrations_backwards_deps_1/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/migrations/test_migrations_backwards_deps_1/__init__.py
diff --git a/tests/migrations2/test_migrations_backwards_deps_2/0001_initial.py b/tests/migrations2/test_migrations_backwards_deps_2/0001_initial.py
new file mode 100644
index 0000000000..0b6eafb02f
--- /dev/null
+++ b/tests/migrations2/test_migrations_backwards_deps_2/0001_initial.py
@@ -0,0 +1,16 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [('migrations', '0001_initial')]
+    operations = [
+        migrations.CreateModel(
+            "Example",
+            [
+                ("id", models.AutoField(primary_key=True)),
+            ],
+        ),
+    ]
diff --git a/tests/migrations2/test_migrations_backwards_deps_2/__init__.py b/tests/migrations2/test_migrations_backwards_deps_2/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/migrations2/test_migrations_backwards_deps_2/__init__.py