diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py index 1020e3ccd94..c3a3424cd10 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -47,7 +47,7 @@ class MigrationAutodetector(object): Takes a graph to base names on and an optional set of apps to try and restrict to (restriction is not guaranteed) """ - changes = self._detect_changes(convert_apps) + changes = self._detect_changes(convert_apps, graph) changes = self.arrange_for_graph(changes, graph) if trim_to_apps: changes = self._trim_to_apps(changes, trim_to_apps) @@ -90,7 +90,7 @@ class MigrationAutodetector(object): fields_def.append(deconstruction) return fields_def - def _detect_changes(self, convert_apps=None): + def _detect_changes(self, convert_apps=None, graph=None): """ Returns a dict of migration plans which will achieve the change from from_state to to_state. The dict has app labels @@ -98,6 +98,12 @@ class MigrationAutodetector(object): The resulting migrations aren't specially named, but the names do matter for dependencies inside the set. + + convert_apps is the list of apps to convert to use migrations + (i.e. to make initial migrations for, in the usual case) + + graph is an optional argument that, if provided, can help improve + dependency generation and avoid potential circular dependencies. """ # The first phase is generating all the operations for each app @@ -245,10 +251,16 @@ class MigrationAutodetector(object): if self.migrations.get(dep[0], None): operation_dependencies.add((dep[0], self.migrations[dep[0]][-1].name)) else: - # If we can't find the other app, we add a __first__ dependency, + # If we can't find the other app, we add a first/last dependency, # but only if we've already been through once and checked everything if chop_mode: - operation_dependencies.add((dep[0], "__first__")) + # If the app already exists, we add __last__, as we don't know which + # migration contains the target field. + # If it's not yet migrated or has no migrations, we use __first__ + if graph and not graph.root_nodes(dep[0]): + operation_dependencies.add((dep[0], "__first__")) + else: + operation_dependencies.add((dep[0], "__last__")) else: deps_satisfied = False if deps_satisfied: diff --git a/tests/migrations/models.py b/tests/migrations/models.py index 87165ab6e3d..fc063f5cb05 100644 --- a/tests/migrations/models.py +++ b/tests/migrations/models.py @@ -42,3 +42,11 @@ class UnserializableModel(models.Model): class Meta: # Disable auto loading of this model as we load it on our own apps = Apps() + + +class UnmigratedModel(models.Model): + """ + A model that is in a migration-less app (which this app is + if its migrations directory has not been repointed) + """ + pass diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py index c093e5c6e66..bc4478b22d7 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -4,7 +4,8 @@ from django.db.migrations.autodetector import MigrationAutodetector from django.db.migrations.questioner import MigrationQuestioner from django.db.migrations.state import ProjectState, ModelState from django.db.migrations.graph import MigrationGraph -from django.db import models +from django.db.migrations.loader import MigrationLoader +from django.db import models, connection from django.contrib.auth.models import AbstractBaseUser @@ -58,6 +59,7 @@ class AutodetectorTests(TestCase): 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_proxy_fk = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("thirdapp.AuthorProxy")), ("title", models.CharField(max_length=200))]) + book_migrations_fk = ModelState("otherapp", "Book", [("id", models.AutoField(primary_key=True)), ("author", models.ForeignKey("migrations.UnmigratedModel")), ("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))]) @@ -1017,3 +1019,42 @@ class AutodetectorTests(TestCase): self.assertOperationTypes(changes, 'testapp', 0, ["CreateModel", "CreateModel"]) self.assertOperationAttributes(changes, 'testapp', 0, 0, name="Author") self.assertOperationAttributes(changes, 'testapp', 0, 1, name="Aardvark") + + def test_first_dependency(self): + """ + Tests that a dependency to an app with no migrations uses __first__. + """ + # Load graph + loader = MigrationLoader(connection) + # Make state + before = self.make_project_state([]) + after = self.make_project_state([self.book_migrations_fk]) + after.real_apps = ["migrations"] + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes(graph=loader.graph) + # Right number of migrations? + self.assertNumberMigrations(changes, 'otherapp', 1) + self.assertOperationTypes(changes, 'otherapp', 0, ["CreateModel"]) + self.assertOperationAttributes(changes, 'otherapp', 0, 0, name="Book") + # Right dependencies? + self.assertEqual(changes['otherapp'][0].dependencies, [("migrations", "__first__")]) + + @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"}) + def test_last_dependency(self): + """ + Tests that a dependency to an app with existing migrations uses __first__. + """ + # Load graph + loader = MigrationLoader(connection) + # Make state + before = self.make_project_state([]) + after = self.make_project_state([self.book_migrations_fk]) + after.real_apps = ["migrations"] + autodetector = MigrationAutodetector(before, after) + changes = autodetector._detect_changes(graph=loader.graph) + # Right number of migrations? + self.assertNumberMigrations(changes, 'otherapp', 1) + self.assertOperationTypes(changes, 'otherapp', 0, ["CreateModel"]) + self.assertOperationAttributes(changes, 'otherapp', 0, 0, name="Book") + # Right dependencies? + self.assertEqual(changes['otherapp'][0].dependencies, [("migrations", "__last__")]) diff --git a/tests/migrations/test_migrations_no_changes/0003_third.py b/tests/migrations/test_migrations_no_changes/0003_third.py index f8f3db9386c..2418bd5e9f9 100644 --- a/tests/migrations/test_migrations_no_changes/0003_third.py +++ b/tests/migrations/test_migrations_no_changes/0003_third.py @@ -16,8 +16,15 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), ], - options={ - }, + options={}, + bases=(models.Model,), + ), + migrations.CreateModel( + name='UnmigratedModel', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ], + options={}, bases=(models.Model,), ), migrations.DeleteModel(