Skip to content

Commit

Permalink
Fixed #23916 -- Allowed makemigrations to handle related model name c…
Browse files Browse the repository at this point in the history 
…ase changes.

Made autodetector ignore related model name case changes so unnecessary
migrations are not created.
  • Loading branch information
@adamchainz @felixxm
adamchainz authored and felixxm committed Mar 25, 2020
1 parent f3da09d commit 9e1b6b8
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 28 deletions.
7 changes: 5 additions & 2 deletions django/db/migrations/autodetector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,10 +496,13 @@ def generate_renamed_models(self):
dependencies=dependencies,
)
self.renamed_models[app_label, model_name] = rem_model_name
renamed_models_rel_key = '%s.%s' % (rem_model_state.app_label, rem_model_state.name)
renamed_models_rel_key = '%s.%s' % (
rem_model_state.app_label,
rem_model_state.name_lower,
)
self.renamed_models_rel[renamed_models_rel_key] = '%s.%s' % (
model_state.app_label,
model_state.name,
model_state.name_lower,
)
self.old_model_keys.remove((rem_app_label, rem_model_name))
self.old_model_keys.add((app_label, model_name))
Expand Down
8 changes: 2 additions & 6 deletions django/db/models/fields/related.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -581,14 +581,10 @@ def deconstruct(self):

if self.remote_field.parent_link:
kwargs['parent_link'] = self.remote_field.parent_link
# Work out string form of "to"
if isinstance(self.remote_field.model, str):
kwargs['to'] = self.remote_field.model
kwargs['to'] = self.remote_field.model.lower()
else:
kwargs['to'] = "%s.%s" % (
self.remote_field.model._meta.app_label,
self.remote_field.model._meta.object_name,
)
kwargs['to'] = self.remote_field.model._meta.label_lower
# If swappable is True, then see if we're actually pointing to the target
# of a swap.
swappable_setting = self.swappable_setting
Expand Down
38 changes: 19 additions & 19 deletions tests/field_deconstruction/tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,47 +202,47 @@ def test_foreign_key(self):
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "on_delete": models.CASCADE})
self.assertFalse(hasattr(kwargs['to'], "setting_name"))
# Test swap detection for swappable model
field = models.ForeignKey("auth.User", models.CASCADE)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.User", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.user", "on_delete": models.CASCADE})
self.assertEqual(kwargs['to'].setting_name, "AUTH_USER_MODEL")
# Test nonexistent (for now) model
field = models.ForeignKey("something.Else", models.CASCADE)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "something.Else", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "something.else", "on_delete": models.CASCADE})
# Test on_delete
field = models.ForeignKey("auth.User", models.SET_NULL)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.User", "on_delete": models.SET_NULL})
self.assertEqual(kwargs, {"to": "auth.user", "on_delete": models.SET_NULL})
# Test to_field preservation
field = models.ForeignKey("auth.Permission", models.CASCADE, to_field="foobar")
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "to_field": "foobar", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "to_field": "foobar", "on_delete": models.CASCADE})
# Test related_name preservation
field = models.ForeignKey("auth.Permission", models.CASCADE, related_name="foobar")
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "related_name": "foobar", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "related_name": "foobar", "on_delete": models.CASCADE})
# Test related_query_name
field = models.ForeignKey("auth.Permission", models.CASCADE, related_query_name="foobar")
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(
kwargs,
{"to": "auth.Permission", "related_query_name": "foobar", "on_delete": models.CASCADE}
{"to": "auth.permission", "related_query_name": "foobar", "on_delete": models.CASCADE}
)
# Test limit_choices_to
field = models.ForeignKey("auth.Permission", models.CASCADE, limit_choices_to={'foo': 'bar'})
Expand All @@ -251,14 +251,14 @@ def test_foreign_key(self):
self.assertEqual(args, [])
self.assertEqual(
kwargs,
{"to": "auth.Permission", "limit_choices_to": {'foo': 'bar'}, "on_delete": models.CASCADE}
{"to": "auth.permission", "limit_choices_to": {'foo': 'bar'}, "on_delete": models.CASCADE}
)
# Test unique
field = models.ForeignKey("auth.Permission", models.CASCADE, unique=True)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "unique": True, "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "unique": True, "on_delete": models.CASCADE})

@override_settings(AUTH_USER_MODEL="auth.Permission")
def test_foreign_key_swapped(self):
Expand All @@ -270,7 +270,7 @@ def test_foreign_key_swapped(self):

self.assertEqual(path, "django.db.models.ForeignKey")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "on_delete": models.CASCADE})
self.assertEqual(kwargs['to'].setting_name, "AUTH_USER_MODEL")

def test_one_to_one(self):
Expand All @@ -282,47 +282,47 @@ def test_one_to_one(self):
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "on_delete": models.CASCADE})
self.assertFalse(hasattr(kwargs['to'], "setting_name"))
# Test swap detection for swappable model
field = models.OneToOneField("auth.User", models.CASCADE)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.User", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.user", "on_delete": models.CASCADE})
self.assertEqual(kwargs['to'].setting_name, "AUTH_USER_MODEL")
# Test nonexistent (for now) model
field = models.OneToOneField("something.Else", models.CASCADE)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "something.Else", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "something.else", "on_delete": models.CASCADE})
# Test on_delete
field = models.OneToOneField("auth.User", models.SET_NULL)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.User", "on_delete": models.SET_NULL})
self.assertEqual(kwargs, {"to": "auth.user", "on_delete": models.SET_NULL})
# Test to_field
field = models.OneToOneField("auth.Permission", models.CASCADE, to_field="foobar")
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "to_field": "foobar", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "to_field": "foobar", "on_delete": models.CASCADE})
# Test related_name
field = models.OneToOneField("auth.Permission", models.CASCADE, related_name="foobar")
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "related_name": "foobar", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "related_name": "foobar", "on_delete": models.CASCADE})
# Test related_query_name
field = models.OneToOneField("auth.Permission", models.CASCADE, related_query_name="foobar")
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(
kwargs,
{"to": "auth.Permission", "related_query_name": "foobar", "on_delete": models.CASCADE}
{"to": "auth.permission", "related_query_name": "foobar", "on_delete": models.CASCADE}
)
# Test limit_choices_to
field = models.OneToOneField("auth.Permission", models.CASCADE, limit_choices_to={'foo': 'bar'})
Expand All @@ -331,14 +331,14 @@ def test_one_to_one(self):
self.assertEqual(args, [])
self.assertEqual(
kwargs,
{"to": "auth.Permission", "limit_choices_to": {'foo': 'bar'}, "on_delete": models.CASCADE}
{"to": "auth.permission", "limit_choices_to": {'foo': 'bar'}, "on_delete": models.CASCADE}
)
# Test unique
field = models.OneToOneField("auth.Permission", models.CASCADE, unique=True)
name, path, args, kwargs = field.deconstruct()
self.assertEqual(path, "django.db.models.OneToOneField")
self.assertEqual(args, [])
self.assertEqual(kwargs, {"to": "auth.Permission", "on_delete": models.CASCADE})
self.assertEqual(kwargs, {"to": "auth.permission", "on_delete": models.CASCADE})

def test_image_field(self):
field = models.ImageField(upload_to="foo/barness", width_field="width", height_field="height")
Expand Down
18 changes: 17 additions & 1 deletion tests/migrations/test_autodetector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ def test_rename_related_field_preserved_db_column(self):
'renamed_foo',
'django.db.models.ForeignKey',
[],
{'to': 'app.Foo', 'on_delete': models.CASCADE, 'db_column': 'foo_id'},
{'to': 'app.foo', 'on_delete': models.CASCADE, 'db_column': 'foo_id'},
))

def test_rename_model(self):
Expand All @@ -1032,6 +1032,22 @@ def test_rename_model(self):
# no AlterField for the related field.
self.assertNumberMigrations(changes, 'otherapp', 0)

def test_rename_model_case(self):
"""
Model name is case-insensitive. Changing case doesn't lead to any
autodetected operations.
"""
author_renamed = ModelState('testapp', 'author', [
('id', models.AutoField(primary_key=True)),
])
changes = self.get_changes(
[self.author_empty, self.book],
[author_renamed, self.book],
questioner=MigrationQuestioner({'ask_rename_model': True}),
)
self.assertNumberMigrations(changes, 'testapp', 0)
self.assertNumberMigrations(changes, 'otherapp', 0)

def test_rename_m2m_through_model(self):
"""
Tests autodetection of renamed models that are used in M2M relations as
Expand Down

0 comments on commit 9e1b6b8

Please sign in to comment.