[1.7.x] Fixed #23938 -- Added migration support for m2m to concrete fields and vice versa
Thanks to Michael D. Hoyle for the report and Tim Graham for the review.
Backport of 623ccdd598 from master
This commit is contained in:
Markus Holtermann
Tim Graham
parent
1cbdb49b0a
commit
1702bc52cc
|
|
@ -778,73 +778,71 @@ class MigrationAutodetector(object):
|
|||
Fields that have been added
|
||||
"""
|
||||
for app_label, model_name, field_name in sorted(self.new_field_keys - self.old_field_keys):
|
||||
field = self.new_apps.get_model(app_label, model_name)._meta.get_field_by_name(field_name)[0]
|
||||
# Fields that are foreignkeys/m2ms depend on stuff
|
||||
dependencies = []
|
||||
if field.rel and field.rel.to:
|
||||
# Account for FKs to swappable models
|
||||
swappable_setting = getattr(field, 'swappable_setting', None)
|
||||
if swappable_setting is not None:
|
||||
dep_app_label = "__setting__"
|
||||
dep_object_name = swappable_setting
|
||||
else:
|
||||
dep_app_label = field.rel.to._meta.app_label
|
||||
dep_object_name = field.rel.to._meta.object_name
|
||||
dependencies = [(dep_app_label, dep_object_name, None, True)]
|
||||
if getattr(field.rel, "through", None) and not field.rel.through._meta.auto_created:
|
||||
dependencies.append((
|
||||
field.rel.through._meta.app_label,
|
||||
field.rel.through._meta.object_name,
|
||||
None,
|
||||
True
|
||||
))
|
||||
# You can't just add NOT NULL fields with no default or fields
|
||||
# which don't allow empty strings as default.
|
||||
if (not field.null and not field.has_default() and
|
||||
not isinstance(field, models.ManyToManyField) and
|
||||
not (field.blank and field.empty_strings_allowed)):
|
||||
field = field.clone()
|
||||
field.default = self.questioner.ask_not_null_addition(field_name, model_name)
|
||||
self.add_operation(
|
||||
app_label,
|
||||
operations.AddField(
|
||||
model_name=model_name,
|
||||
name=field_name,
|
||||
field=field,
|
||||
preserve_default=False,
|
||||
),
|
||||
dependencies=dependencies,
|
||||
)
|
||||
self._generate_added_field(app_label, model_name, field_name)
|
||||
|
||||
def _generate_added_field(self, app_label, model_name, field_name):
|
||||
field = self.new_apps.get_model(app_label, model_name)._meta.get_field_by_name(field_name)[0]
|
||||
# Fields that are foreignkeys/m2ms depend on stuff
|
||||
dependencies = []
|
||||
if field.rel and field.rel.to:
|
||||
# Account for FKs to swappable models
|
||||
swappable_setting = getattr(field, 'swappable_setting', None)
|
||||
if swappable_setting is not None:
|
||||
dep_app_label = "__setting__"
|
||||
dep_object_name = swappable_setting
|
||||
else:
|
||||
self.add_operation(
|
||||
app_label,
|
||||
operations.AddField(
|
||||
model_name=model_name,
|
||||
name=field_name,
|
||||
field=field,
|
||||
),
|
||||
dependencies=dependencies,
|
||||
)
|
||||
dep_app_label = field.rel.to._meta.app_label
|
||||
dep_object_name = field.rel.to._meta.object_name
|
||||
dependencies = [(dep_app_label, dep_object_name, None, True)]
|
||||
if getattr(field.rel, "through", None) and not field.rel.through._meta.auto_created:
|
||||
dependencies.append((
|
||||
field.rel.through._meta.app_label,
|
||||
field.rel.through._meta.object_name,
|
||||
None,
|
||||
True,
|
||||
))
|
||||
# You can't just add NOT NULL fields with no default or fields
|
||||
# which don't allow empty strings as default.
|
||||
preserve_default = True
|
||||
if (not field.null and not field.has_default() and
|
||||
not isinstance(field, models.ManyToManyField) and
|
||||
not (field.blank and field.empty_strings_allowed)):
|
||||
field = field.clone()
|
||||
field.default = self.questioner.ask_not_null_addition(field_name, model_name)
|
||||
preserve_default = False
|
||||
self.add_operation(
|
||||
app_label,
|
||||
operations.AddField(
|
||||
model_name=model_name,
|
||||
name=field_name,
|
||||
field=field,
|
||||
preserve_default=preserve_default,
|
||||
),
|
||||
dependencies=dependencies,
|
||||
)
|
||||
|
||||
def generate_removed_fields(self):
|
||||
"""
|
||||
Fields that have been removed.
|
||||
"""
|
||||
for app_label, model_name, field_name in sorted(self.old_field_keys - self.new_field_keys):
|
||||
self.add_operation(
|
||||
app_label,
|
||||
operations.RemoveField(
|
||||
model_name=model_name,
|
||||
name=field_name,
|
||||
),
|
||||
# We might need to depend on the removal of an
|
||||
# order_with_respect_to or index/unique_together operation;
|
||||
# this is safely ignored if there isn't one
|
||||
dependencies=[
|
||||
(app_label, model_name, field_name, "order_wrt_unset"),
|
||||
(app_label, model_name, field_name, "foo_together_change"),
|
||||
],
|
||||
)
|
||||
self._generate_removed_field(app_label, model_name, field_name)
|
||||
|
||||
def _generate_removed_field(self, app_label, model_name, field_name):
|
||||
self.add_operation(
|
||||
app_label,
|
||||
operations.RemoveField(
|
||||
model_name=model_name,
|
||||
name=field_name,
|
||||
),
|
||||
# We might need to depend on the removal of an
|
||||
# order_with_respect_to or index/unique_together operation;
|
||||
# this is safely ignored if there isn't one
|
||||
dependencies=[
|
||||
(app_label, model_name, field_name, "order_wrt_unset"),
|
||||
(app_label, model_name, field_name, "foo_together_change"),
|
||||
],
|
||||
)
|
||||
|
||||
def generate_altered_fields(self):
|
||||
"""
|
||||
|
|
@ -868,25 +866,30 @@ class MigrationAutodetector(object):
|
|||
old_field_dec = self.deep_deconstruct(old_field)
|
||||
new_field_dec = self.deep_deconstruct(new_field)
|
||||
if old_field_dec != new_field_dec:
|
||||
preserve_default = True
|
||||
if (old_field.null and not new_field.null and not new_field.has_default() and
|
||||
if (not isinstance(old_field, models.ManyToManyField) and
|
||||
not isinstance(new_field, models.ManyToManyField)):
|
||||
field = new_field.clone()
|
||||
new_default = self.questioner.ask_not_null_alteration(field_name, model_name)
|
||||
if new_default is not models.NOT_PROVIDED:
|
||||
field.default = new_default
|
||||
preserve_default = False
|
||||
else:
|
||||
field = new_field
|
||||
self.add_operation(
|
||||
app_label,
|
||||
operations.AlterField(
|
||||
model_name=model_name,
|
||||
name=field_name,
|
||||
field=field,
|
||||
preserve_default=preserve_default,
|
||||
preserve_default = True
|
||||
if (old_field.null and not new_field.null and not new_field.has_default() and
|
||||
not isinstance(new_field, models.ManyToManyField)):
|
||||
field = new_field.clone()
|
||||
new_default = self.questioner.ask_not_null_alteration(field_name, model_name)
|
||||
if new_default is not models.NOT_PROVIDED:
|
||||
field.default = new_default
|
||||
preserve_default = False
|
||||
else:
|
||||
field = new_field
|
||||
self.add_operation(
|
||||
app_label,
|
||||
operations.AlterField(
|
||||
model_name=model_name,
|
||||
name=field_name,
|
||||
field=field,
|
||||
preserve_default=preserve_default,
|
||||
)
|
||||
)
|
||||
)
|
||||
else:
|
||||
self._generate_removed_field(app_label, model_name, field_name)
|
||||
self._generate_added_field(app_label, model_name, field_name)
|
||||
|
||||
def _generate_altered_foo_together(self, operation):
|
||||
option_name = operation.option_name
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ Bugfixes
|
|||
* Fixed a migration crash that prevented changing a nullable field with a
|
||||
default to non-nullable with the same default (:ticket:`23738`).
|
||||
|
||||
* Fixed a migrations crash when adding ``GeometryField``\s with ``blank=True``
|
||||
* Fixed a migration crash when adding ``GeometryField``\s with ``blank=True``
|
||||
on PostGIS (:ticket:`23731`).
|
||||
|
||||
* Allowed usage of ``DateTimeField()`` as ``Transform.output_field``
|
||||
|
|
@ -63,7 +63,7 @@ Bugfixes
|
|||
* Fixed a migration crash when a field is renamed that is part of an
|
||||
``index_together`` (:ticket:`23859`).
|
||||
|
||||
* Fixed :djadmin:`squashmigrations` to respect the ``--no-optimize`` parameter
|
||||
* Fixed :djadmin:`squashmigrations` to respect the ``--no-optimize`` parameter
|
||||
(:ticket:`23799`).
|
||||
|
||||
* Made :class:`~django.db.migrations.operations.RenameModel` reversible
|
||||
|
|
@ -144,7 +144,7 @@ Bugfixes
|
|||
* ``makemigrations`` no longer prompts for a default value when adding
|
||||
``TextField()`` or ``CharField()`` without a ``default`` (:ticket:`23405`).
|
||||
|
||||
* Fixed migration crash when adding ``order_with_respect_to`` to a table
|
||||
* Fixed a migration crash when adding ``order_with_respect_to`` to a table
|
||||
with existing rows (:ticket:`23983`).
|
||||
|
||||
* Restored the ``pre_migrate`` signal if all apps have migrations
|
||||
|
|
@ -181,3 +181,6 @@ Bugfixes
|
|||
|
||||
* Supported strings escaped by third-party libraries with the ``__html__``
|
||||
convention in the template engine (:ticket:`23831`).
|
||||
|
||||
* Fixed a migration crash when changing a ``ManyToManyField`` into a concrete
|
||||
field and vice versa (:ticket:`23938`).
|
||||
|
|
|
|||
|
|
@ -116,6 +116,10 @@ class AutodetectorTests(TestCase):
|
|||
("id", models.AutoField(primary_key=True)),
|
||||
("publishers", models.ManyToManyField("testapp.Publisher", through="testapp.Contract")),
|
||||
])
|
||||
author_with_former_m2m = ModelState("testapp", "Author", [
|
||||
("id", models.AutoField(primary_key=True)),
|
||||
("publishers", models.CharField(max_length=100)),
|
||||
])
|
||||
author_with_options = ModelState("testapp", "Author", [
|
||||
("id", models.AutoField(primary_key=True)),
|
||||
], {
|
||||
|
|
@ -1274,6 +1278,39 @@ class AutodetectorTests(TestCase):
|
|||
self.assertOperationAttributes(changes, "testapp", 0, 3, name="publisher", model_name='contract')
|
||||
self.assertOperationAttributes(changes, "testapp", 0, 4, name="Contract")
|
||||
|
||||
def test_concrete_field_changed_to_many_to_many(self):
|
||||
"""
|
||||
#23938 - Tests that changing a concrete field into a ManyToManyField
|
||||
first removes the concrete field and then adds the m2m field.
|
||||
"""
|
||||
before = self.make_project_state([self.author_with_former_m2m])
|
||||
after = self.make_project_state([self.author_with_m2m, self.publisher])
|
||||
autodetector = MigrationAutodetector(before, after)
|
||||
changes = autodetector._detect_changes()
|
||||
# Right number/type of migrations?
|
||||
self.assertNumberMigrations(changes, "testapp", 1)
|
||||
self.assertOperationTypes(changes, "testapp", 0, ["CreateModel", "RemoveField", "AddField"])
|
||||
self.assertOperationAttributes(changes, 'testapp', 0, 0, name='Publisher')
|
||||
self.assertOperationAttributes(changes, 'testapp', 0, 1, name="publishers", model_name='author')
|
||||
self.assertOperationAttributes(changes, 'testapp', 0, 2, name="publishers", model_name='author')
|
||||
|
||||
def test_many_to_many_changed_to_concrete_field(self):
|
||||
"""
|
||||
#23938 - Tests that changing a ManyToManyField into a concrete field
|
||||
first removes the m2m field and then adds the concrete field.
|
||||
"""
|
||||
before = self.make_project_state([self.author_with_m2m, self.publisher])
|
||||
after = self.make_project_state([self.author_with_former_m2m])
|
||||
autodetector = MigrationAutodetector(before, after)
|
||||
changes = autodetector._detect_changes()
|
||||
# Right number/type of migrations?
|
||||
self.assertNumberMigrations(changes, "testapp", 1)
|
||||
self.assertOperationTypes(changes, "testapp", 0, ["RemoveField", "AddField", "DeleteModel"])
|
||||
self.assertOperationAttributes(changes, 'testapp', 0, 0, name="publishers", model_name='author')
|
||||
self.assertOperationAttributes(changes, 'testapp', 0, 1, name="publishers", model_name='author')
|
||||
self.assertOperationAttributes(changes, 'testapp', 0, 2, name='Publisher')
|
||||
self.assertOperationFieldAttributes(changes, 'testapp', 0, 1, max_length=100)
|
||||
|
||||
def test_non_circular_foreignkey_dependency_removal(self):
|
||||
"""
|
||||
If two models with a ForeignKey from one to the other are removed at the
|
||||
|
|
|
|||
Loading…
Reference in New Issue