Fixed #22470: Full migration support for order_with_respect_to

django/db/migrations/autodetector.py

@@ -163,6 +163,7 @@ class MigrationAutodetector(object):
         self.generate_altered_fields()
         self.generate_altered_unique_together()
         self.generate_altered_index_together()
+        self.generate_altered_order_with_respect_to()
 
         # Now, reordering to make things possible. The order we have already
         # isn't bad, but we need to pull a few things around so FKs work nicely
@@ -305,9 +306,16 @@ class MigrationAutodetector(object):
                 operation.model_name.lower() == dependency[1].lower() and
                 operation.name.lower() == dependency[2].lower()
             )
+        # order_with_respect_to being unset for a field
+        elif dependency[2] is not None and dependency[3] == "order_wrt_unset":
+            return (
+                isinstance(operation, operations.AlterOrderWithRespectTo) and
+                operation.name.lower() == dependency[1].lower() and
+                (operation.order_with_respect_to or "").lower() != dependency[2].lower()
+            )
         # Unknown dependency. Raise an error.
         else:
-            raise ValueError("Can't handle dependency %r" % dependency)
+            raise ValueError("Can't handle dependency %r" % (dependency, ))
 
     def add_operation(self, app_label, operation, dependencies=None):
         # Dependencies are (app_label, model_name, field_name, create/delete as True/False)
@@ -375,6 +383,7 @@ class MigrationAutodetector(object):
             # Are there unique/index_together to defer?
             unique_together = model_state.options.pop('unique_together', None)
             index_together = model_state.options.pop('index_together', None)
+            order_with_respect_to = model_state.options.pop('order_with_respect_to', None)
             # Generate creation operatoin
             self.add_operation(
                 app_label,
@@ -438,6 +447,17 @@ class MigrationAutodetector(object):
                         for name, field in sorted(related_fields.items())
                     ]
                 )
+            if order_with_respect_to:
+                self.add_operation(
+                    app_label,
+                    operations.AlterOrderWithRespectTo(
+                        name=model_name,
+                        order_with_respect_to=order_with_respect_to,
+                    ),
+                    dependencies=[
+                        (app_label, model_name, order_with_respect_to, True),
+                    ]
+                )
 
     def generate_deleted_models(self):
         """
@@ -595,7 +615,10 @@ class MigrationAutodetector(object):
                 operations.RemoveField(
                     model_name=model_name,
                     name=field_name,
-                )
+                ),
+                # We might need to depend on the removal of an order_with_respect_to;
+                # this is safely ignored if there isn't one
+                dependencies=[(app_label, model_name, field_name, "order_wrt_unset")],
             )
 
     def generate_altered_fields(self):
@@ -659,6 +682,11 @@ class MigrationAutodetector(object):
                 )
 
     def generate_altered_options(self):
+        """
+        Works out if any non-schema-affecting options have changed and
+        makes an operation to represent them in state changes (in case Python
+        code in migrations needs them)
+        """
         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]
@@ -680,6 +708,32 @@ class MigrationAutodetector(object):
                     )
                 )
 
+    def generate_altered_order_with_respect_to(self):
+        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]
+            new_model_state = self.to_state.models[app_label, model_name]
+            if old_model_state.options.get("order_with_respect_to", None) != new_model_state.options.get("order_with_respect_to", None):
+                # Make sure it comes second if we're adding
+                # (removal dependency is part of RemoveField)
+                dependencies = []
+                if new_model_state.options.get("order_with_respect_to", None):
+                    dependencies.append((
+                        app_label,
+                        model_name,
+                        new_model_state.options["order_with_respect_to"],
+                        True,
+                    ))
+                # Actually generate the operation
+                self.add_operation(
+                    app_label,
+                    operations.AlterOrderWithRespectTo(
+                        name=model_name,
+                        order_with_respect_to=new_model_state.options.get('order_with_respect_to', None),
+                    ),
+                    dependencies = dependencies,
+                )
+
     def arrange_for_graph(self, changes, graph):
         """
         Takes in a result from changes() and a MigrationGraph,

django/db/migrations/operations/__init__.py

@@ -1,5 +1,6 @@
 from .models import (CreateModel, DeleteModel, AlterModelTable,
-    AlterUniqueTogether, AlterIndexTogether, RenameModel, AlterModelOptions)
+    AlterUniqueTogether, AlterIndexTogether, RenameModel, AlterModelOptions,
+    AlterOrderWithRespectTo)
 from .fields import AddField, RemoveField, AlterField, RenameField
 from .special import SeparateDatabaseAndState, RunSQL, RunPython
 
@@ -8,4 +9,5 @@ __all__ = [
     'RenameModel', 'AlterIndexTogether', 'AlterModelOptions',
     'AddField', 'RemoveField', 'AlterField', 'RenameField',
     'SeparateDatabaseAndState', 'RunSQL', 'RunPython',
+    'AlterOrderWithRespectTo',
 ]

django/db/migrations/operations/models.py

@@ -285,6 +285,45 @@ class AlterIndexTogether(Operation):
         return "Alter index_together for %s (%s constraints)" % (self.name, len(self.index_together))
 
 
+class AlterOrderWithRespectTo(Operation):
+    """
+    Represents a change with the order_with_respect_to option.
+    """
+
+    def __init__(self, name, order_with_respect_to):
+        self.name = name
+        self.order_with_respect_to = order_with_respect_to
+
+    def state_forwards(self, app_label, state):
+        model_state = state.models[app_label, self.name.lower()]
+        model_state.options['order_with_respect_to'] = self.order_with_respect_to
+
+    def database_forwards(self, app_label, schema_editor, from_state, to_state):
+        from_model = from_state.render().get_model(app_label, self.name)
+        to_model = to_state.render().get_model(app_label, self.name)
+        if router.allow_migrate(schema_editor.connection.alias, to_model):
+            # Remove a field if we need to
+            if from_model._meta.order_with_respect_to and not to_model._meta.order_with_respect_to:
+                schema_editor.remove_field(from_model, from_model._meta.get_field_by_name("_order")[0])
+            # Add a field if we need to (altering the column is untouched as
+            # it's likely a rename)
+            elif to_model._meta.order_with_respect_to and not from_model._meta.order_with_respect_to:
+                field = to_model._meta.get_field_by_name("_order")[0]
+                schema_editor.add_field(
+                    from_model,
+                    field,
+                )
+
+    def database_backwards(self, app_label, schema_editor, from_state, to_state):
+        self.database_forwards(app_label, schema_editor, from_state, to_state)
+
+    def references_model(self, name, app_label=None):
+        return name.lower() == self.name.lower()
+
+    def describe(self):
+        return "Set order_with_respect_to on %s to %s" % (self.name, self.order_with_respect_to)
+
+
 class AlterModelOptions(Operation):
     """
     Sets new model options that don't directly affect the database schema

django/db/migrations/state.py

@@ -5,6 +5,7 @@ from django.apps.registry import Apps, apps as global_apps
 from django.db import models
 from django.db.models.options import DEFAULT_NAMES, normalize_together
 from django.db.models.fields.related import do_pending_lookups
+from django.db.models.fields.proxy import OrderWrt
 from django.conf import settings
 from django.utils import six
 from django.utils.encoding import force_text
@@ -166,6 +167,8 @@ class ModelState(object):
         for field in model._meta.local_fields:
             if getattr(field, "rel", None) and exclude_rels:
                 continue
+            if isinstance(field, OrderWrt):
+                continue
             name, path, args, kwargs = field.deconstruct()
             field_class = import_string(path)
             try:

docs/ref/migration-operations.txt

@@ -99,6 +99,24 @@ Changes the model's set of custom indexes (the
 :attr:`~django.db.models.Options.index_together` option on the ``Meta``
 subclass).
 
+AlterOrderWithRespectTo
+-----------------------
+
+.. class:: AlterIndexTogether(name, order_with_respect_to)
+
+Makes or deletes the ``_order`` column needed for the
+:attr:`~django.db.models.Options.order_with_respect_to` option on the ``Meta``
+subclass.
+
+AlterModelOptions
+-----------------
+
+.. class:: AlterIndexTogether(name, options)
+
+Stores changes to miscellaneous model options (settings on a model's ``Meta``)
+like ``permissions`` and ``verbose_name``. Does not affect the database, but
+persists these changes for :class:`RunPython` instances to use.
+
 AddField
 --------
 

tests/migrations/test_autodetector.py

@@ -31,6 +31,7 @@ class AutodetectorTests(TestCase):
     author_name_deconstructable_3 = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default=models.IntegerField()))])
     author_name_deconstructable_4 = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default=models.IntegerField()))])
     author_with_book = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("book", models.ForeignKey("otherapp.Book"))])
+    author_with_book_order_wrt = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("book", models.ForeignKey("otherapp.Book"))], options={"order_with_respect_to": "book"})
     author_renamed_with_book = ModelState("testapp", "Writer", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("book", models.ForeignKey("otherapp.Book"))])
     author_with_publisher_string = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("publisher_name", models.CharField(max_length=200))])
     author_with_publisher = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200)), ("publisher", models.ForeignKey("testapp.Publisher"))])
@@ -814,3 +815,64 @@ class AutodetectorTests(TestCase):
         self.assertNumberMigrations(changes, "testapp", 1)
         # Right actions in right order?
         self.assertOperationTypes(changes, "testapp", 0, ["AlterModelOptions"])
+
+    def test_set_alter_order_with_respect_to(self):
+        "Tests that setting order_with_respect_to adds a field"
+        # Make state
+        before = self.make_project_state([self.book, self.author_with_book])
+        after = self.make_project_state([self.book, self.author_with_book_order_wrt])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        # Right number of migrations?
+        self.assertNumberMigrations(changes, 'testapp', 1)
+        self.assertOperationTypes(changes, 'testapp', 0, ["AlterOrderWithRespectTo"])
+        self.assertOperationAttributes(changes, 'testapp', 0, 0, name="author", order_with_respect_to="book")
+
+    def test_add_alter_order_with_respect_to(self):
+        """
+        Tests that setting order_with_respect_to when adding the FK too
+        does things in the right order.
+        """
+        # Make state
+        before = self.make_project_state([self.author_name])
+        after = self.make_project_state([self.book, self.author_with_book_order_wrt])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        # Right number of migrations?
+        self.assertNumberMigrations(changes, 'testapp', 1)
+        self.assertOperationTypes(changes, 'testapp', 0, ["AddField", "AlterOrderWithRespectTo"])
+        self.assertOperationAttributes(changes, 'testapp', 0, 0, model_name="author", name="book")
+        self.assertOperationAttributes(changes, 'testapp', 0, 1, name="author", order_with_respect_to="book")
+
+    def test_remove_alter_order_with_respect_to(self):
+        """
+        Tests that removing order_with_respect_to when removing the FK too
+        does things in the right order.
+        """
+        # Make state
+        before = self.make_project_state([self.book, self.author_with_book_order_wrt])
+        after = self.make_project_state([self.author_name])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        # Right number of migrations?
+        self.assertNumberMigrations(changes, 'testapp', 1)
+        self.assertOperationTypes(changes, 'testapp', 0, ["AlterOrderWithRespectTo", "RemoveField"])
+        self.assertOperationAttributes(changes, 'testapp', 0, 0, name="author", order_with_respect_to=None)
+        self.assertOperationAttributes(changes, 'testapp', 0, 1, model_name="author", name="book")
+
+    def test_add_model_order_with_respect_to(self):
+        """
+        Tests that setting order_with_respect_to when adding the whole model
+        does things in the right order.
+        """
+        # Make state
+        before = self.make_project_state([])
+        after = self.make_project_state([self.book, self.author_with_book_order_wrt])
+        autodetector = MigrationAutodetector(before, after)
+        changes = autodetector._detect_changes()
+        # Right number of migrations?
+        self.assertNumberMigrations(changes, 'testapp', 1)
+        self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "AlterOrderWithRespectTo"])
+        self.assertOperationAttributes(changes, 'testapp', 0, 1, name="author", order_with_respect_to="book")
+        # Make sure the _order field is not in the CreateModel fields
+        self.assertNotIn("_order", [name for name, field in changes['testapp'][0].operations[0].fields])

tests/migrations/test_operations.py

@@ -803,6 +803,28 @@ class OperationTests(MigrationTestBase):
         self.assertEqual(len(new_state.models["test_almoop", "pony"].options.get("permissions", [])), 1)
         self.assertEqual(new_state.models["test_almoop", "pony"].options["permissions"][0][0], "can_groom")
 
+    def test_alter_order_with_respect_to(self):
+        """
+        Tests the AlterOrderWithRespectTo operation.
+        """
+        project_state = self.set_up_test_model("test_alorwrtto", related_model=True)
+        # Test the state alteration
+        operation = migrations.AlterOrderWithRespectTo("Rider", "pony")
+        new_state = project_state.clone()
+        operation.state_forwards("test_alorwrtto", new_state)
+        self.assertEqual(project_state.models["test_alorwrtto", "rider"].options.get("order_with_respect_to", None), None)
+        self.assertEqual(new_state.models["test_alorwrtto", "rider"].options.get("order_with_respect_to", None), "pony")
+        # Make sure there's no matching index
+        self.assertColumnNotExists("test_alorwrtto_rider", "_order")
+        # Test the database alteration
+        with connection.schema_editor() as editor:
+            operation.database_forwards("test_alorwrtto", editor, project_state, new_state)
+        self.assertColumnExists("test_alorwrtto_rider", "_order")
+        # And test reversal
+        with connection.schema_editor() as editor:
+            operation.database_backwards("test_alorwrtto", editor, new_state, project_state)
+        self.assertColumnNotExists("test_alorwrtto_rider", "_order")
+
     @unittest.skipIf(sqlparse is None and connection.features.requires_sqlparse_for_splitting, "Missing sqlparse")
     def test_run_sql(self):
         """

tests/migrations/test_state.py

@@ -367,6 +367,37 @@ class StateTests(TestCase):
             1,
         )
 
+    def test_ignore_order_wrt(self):
+        """
+        Makes sure ProjectState doesn't include OrderWrt fields when
+        making from existing models.
+        """
+        new_apps = Apps()
+
+        class Author(models.Model):
+            name = models.TextField()
+
+            class Meta:
+                app_label = "migrations"
+                apps = new_apps
+
+        class Book(models.Model):
+            author = models.ForeignKey(Author)
+
+            class Meta:
+                app_label = "migrations"
+                apps = new_apps
+                order_with_respect_to = "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))
+        self.assertEqual(
+            [name for name, field in project_state.models["migrations", "book"].fields],
+            ["id", "author"],
+        )
+
 
 class ModelStateTests(TestCase):
     def test_custom_model_base(self):