From 1702bc52cc20ed0729893177fc8f4391b4b3183c Mon Sep 17 00:00:00 2001
From: Markus Holtermann <info@markusholtermann.eu>
Date: Tue, 2 Dec 2014 19:25:46 +0100
Subject: [PATCH] [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 623ccdd598625591d1a12fc1564cf3ef9a87581f from master
---
django/db/migrations/autodetector.py | 157 +++++++++++++-------------
docs/releases/1.7.2.txt | 9 +-
tests/migrations/test_autodetector.py | 37 ++++++
3 files changed, 123 insertions(+), 80 deletions(-)
diff --git a/django/db/migrations/autodetector.py b/django/db/migrations/autodetector.py
index b323182df9..b0720dd51f 100644
--- a/django/db/migrations/autodetector.py
+++ b/django/db/migrations/autodetector.py
@@ -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
diff --git a/docs/releases/1.7.2.txt b/docs/releases/1.7.2.txt
index da6721a1ba..34f3eb4675 100644
--- a/docs/releases/1.7.2.txt
+++ b/docs/releases/1.7.2.txt
@@ -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`).
diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py
index 8beda8e552..d097d4f2da 100644
--- a/tests/migrations/test_autodetector.py
+++ b/tests/migrations/test_autodetector.py
@@ -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