diff options
| -rw-r--r-- | django/db/migrations/autodetector.py | 157 | ||||
| -rw-r--r-- | docs/releases/1.7.2.txt | 9 | ||||
| -rw-r--r-- | 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 080e53182e..16e7a7d40c 100644 --- a/django/db/migrations/autodetector.py +++ b/django/db/migrations/autodetector.py @@ -773,73 +773,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): """ @@ -863,25 +861,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 328ccf05db..c022a2e0d8 100644 --- a/tests/migrations/test_autodetector.py +++ b/tests/migrations/test_autodetector.py @@ -126,6 +126,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)), ], { @@ -1326,6 +1330,39 @@ class AutodetectorTests(TestCase): self.assertOperationAttributes(changes, "testapp", 0, 3, name="Author") 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 |