makemigrations generates wrong dependencies when a new model has an FK to a renamed model in a different app

[Gavin Wahl]

When adding a new model with a ForeignKey to a model that is also being renamed in a different app, makemigrations generates dependencies that put the new model with a FK to renamed model's new name before renaming the referenced model and migrate fails.

1. App shop has class T0(models.Model): pass. Run makemigrations.

   Migrations for 'shop':
     shop/migrations/0001_initial.py
       + Create model T0
   

2. Rename T0 to T0r, and add app zeta with:

   class T1(models.Model):
       t0 = models.ForeignKey("shop.T0r", models.CASCADE)
   

3. Run makemigrations and answer yes to the rename.

   Migrations for 'zeta':
     zeta/migrations/0001_initial.py
       + Create model T1
   Migrations for 'shop':
     shop/migrations/0002_rename_t0_t0r.py
       ~ Rename model T0 to T0r
   

4. Run migrate.

The dependencies come out backwards. shop/0002_rename_t0_t0r depends on ('zeta', '0001_initial'), and zeta/0001_initial depends only on ('shop', '0001_initial') even though its foreign key points at shop.t0r:

Applying zeta.0001_initial...
ValueError: Related model 'shop.t0r' cannot be resolved

This is a regression in Django 2.0 introduced by #28493's change to generate_renamed_models(). The same steps work on 1.11 and fail on 2.0 and later. This is essentially the reverse structure of #28493: its foreign key used the old name and had to run before the rename. Here it uses the new name and has to run after.

Changing the dependency in zeta.0001_initial to shop.0002_rename_t0_t0r and removing the zeta.0001_initial dependency from shop.0002_rename_t0_t0r makes it work. That's the order generated by Django 1.11, and the only order that works migrating forwards and back.

I haven't had time to investigate further, but I played around a little bit with this patch and it appears to work:

django/db/migrations/autodetector.py

@@ -596 +596 @@
                             rem_model_state, model_state
                         ):
                             dependencies = []
+                            # A relation to the old name must be created
+                            # before the rename. A relation added in this run
+                            # refers to the new name and is created after it.
+                            old_relations = self.from_state.relations.get(
+                                (rem_app_label, rem_model_name), {}
+                            )
                             fields = list(model_state.fields.values()) + [
                                 field.remote_field
-                                for relations in self.to_state.relations[
+                                for related_key, relations in self.to_state.relations[
                                     app_label, model_name
-                                ].values()
-                                for field in relations.values()
+                                ].items()
+                                for field_name, field in relations.items()
+                                if field_name in old_relations.get(related_key, {})
                             ]
                             for field in fields:
                                 if field.is_relation:

I found this with the same fuzzer as #37493, which tests a migration extension that manages database views with Django migrations. Its random changes include renaming a model and adding a model with a foreign key to an existing one, across multiple apps. One change it produced failed to migrate. I then produced a minimal repro using Django alone.

This looks semi-related to #37477, but the proposed patch only applies to changes in a single app at a time.