Fixed #28386 -- Made operations within non-atomic migrations honor the operation's atomic flag when migrating backwards.

AUTHORS

@@ -254,6 +254,7 @@ answer newbie questions, and generally made Django that much better:
     Esdras Beleza <linux@esdrasbeleza.com>
     Espen Grindhaug <http://grindhaug.org/>
     Eugene Lazutkin <http://lazutkin.com/blog/>
+    Evan Grim <https://github.com/egrim>
     Fabrice Aneche <akh@nobugware.com>
     favo@exoweb.net
     fdr <drfarina@gmail.com>

django/db/migrations/migration.py

@@ -162,8 +162,10 @@ class Migration:
                 schema_editor.collected_sql.append("--")
                 if not operation.reduces_to_sql:
                     continue
-            if not schema_editor.connection.features.can_rollback_ddl and operation.atomic:
-                # We're forcing a transaction on a non-transactional-DDL backend
+            atomic_operation = operation.atomic or (self.atomic and operation.atomic is not False)
+            if not schema_editor.atomic_migration and atomic_operation:
+                # Force a transaction on a non-transactional-DDL backend or an
+                # atomic operation inside a non-atomic migration.
                 with atomic(schema_editor.connection.alias):
                     operation.database_backwards(self.app_label, schema_editor, from_state, to_state)
             else:

tests/migrations/test_executor.py

@@ -126,6 +126,14 @@ class ExecutorTests(MigrationTestBase):
         migrations_apps = executor.loader.project_state(("migrations", "0001_initial")).apps
         Editor = migrations_apps.get_model("migrations", "Editor")
         self.assertFalse(Editor.objects.exists())
+        # Record previous migration as successful.
+        executor.migrate([("migrations", "0001_initial")], fake=True)
+        # Rebuild the graph to reflect the new DB state.
+        executor.loader.build_graph()
+        # Migrating backwards is also atomic.
+        with self.assertRaisesMessage(RuntimeError, "Abort migration"):
+            executor.migrate([("migrations", None)])
+        self.assertFalse(Editor.objects.exists())
 
     @override_settings(MIGRATION_MODULES={
         "migrations": "migrations.test_migrations",

tests/migrations/test_migrations_atomic_operation/0001_initial.py

@@ -18,5 +18,5 @@ class Migration(migrations.Migration):
                 ("name", models.CharField(primary_key=True, max_length=255)),
             ],
         ),
-        migrations.RunPython(raise_error, atomic=True),
+        migrations.RunPython(raise_error, reverse_code=raise_error, atomic=True),
     ]

tests/migrations/test_operations.py

@@ -2055,9 +2055,11 @@ class OperationTests(OperationTestBase):
             with self.assertRaises(ValueError):
                 with connection.schema_editor() as editor:
                     atomic_migration.unapply(project_state, editor)
+            self.assertEqual(project_state.apps.get_model("test_runpythonatomic", "Pony").objects.count(), 0)
             with self.assertRaises(ValueError):
                 with connection.schema_editor() as editor:
                     non_atomic_migration.unapply(project_state, editor)
+            self.assertEqual(project_state.apps.get_model("test_runpythonatomic", "Pony").objects.count(), 1)
         # Verify deconstruction.
         definition = non_atomic_migration.operations[0].deconstruct()
         self.assertEqual(definition[0], "RunPython")