diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index a9daa33124..e937e10b80 100644 --- a/django/db/migrations/autodetector.py +++ b/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, diff --git a/django/db/migrations/operations/__init__.py b/django/db/migrations/operations/__init__.py index b8a0ec7569..a6bee2d24a 100644 --- a/django/db/migrations/operations/__init__.py +++ b/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', ] diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index 04dd957019..d8f98b7926 100644 --- a/django/db/migrations/operations/models.py +++ b/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 diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index b8c614d1a1..34bd83deff 100644 --- a/django/db/migrations/state.py +++ b/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: diff --git a/docs/ref/migration-operations.txt b/docs/ref/migration-operations.txt index 980b26cdbd..cdf1f6b458 100644 --- a/docs/ref/migration-operations.txt +++ b/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 -------- diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index bd305a874f..9a8be73f27 100644 --- a/tests/migrations/test_autodetector.py +++ b/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]) diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index d1a23a375f..64703d9140 100644 --- a/tests/migrations/test_operations.py +++ b/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): """ diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py index 9be04f3dfc..45b3d363de 100644 --- a/tests/migrations/test_state.py +++ b/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):