From efb0100ee67931329f17bc9988ecd5f0619cea14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Wed, 30 Oct 2013 21:14:23 +0200 Subject: [PATCH] Fixed #14877 -- repeated deletion using formsets When a formset contained deletion for an existing instance, and the instance was already deleted, django threw an exception. A common cause for this was resubmit of the formset. Original patch by Trac alias olau. In addition this commit cleaned some code in _construct_form(). This was needed as the primary key value the user submitted wasn't converted correctly to python value in case the primary key field was also a related field. --- django/forms/models.py | 38 ++++++++++----------- tests/model_formsets_regress/tests.py | 49 +++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 19 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index f52fe19bc3..71da43390c 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -546,20 +546,24 @@ class BaseModelFormSet(BaseFormSet): self._object_dict = dict((o.pk, o) for o in self.get_queryset()) return self._object_dict.get(pk) + def _get_to_python(self, field): + """ + If the field is a related field, fetch the concrete field's (that + is, the ultimate pointed-to field's) get_prep_value. + """ + while field.rel is not None: + field = field.rel.get_related_field() + return field.to_python + def _construct_form(self, i, **kwargs): if self.is_bound and i < self.initial_form_count(): - # Import goes here instead of module-level because importing - # django.db has side effects. - from django.db import connections pk_key = "%s-%s" % (self.add_prefix(i), self.model._meta.pk.name) pk = self.data[pk_key] pk_field = self.model._meta.pk - pk = pk_field.get_db_prep_lookup('exact', pk, - connection=connections[self.get_queryset().db]) - if isinstance(pk, list): - pk = pk[0] + to_python = self._get_to_python(pk_field) + pk = to_python(pk) kwargs['instance'] = self._existing_object(pk) - if i < self.initial_form_count() and not kwargs.get('instance'): + if i < self.initial_form_count() and 'instance' not in kwargs: kwargs['instance'] = self.get_queryset()[i] if i >= self.initial_form_count() and self.initial_extra: # Set initial values for extra forms @@ -711,21 +715,17 @@ class BaseModelFormSet(BaseFormSet): saved_instances = [] forms_to_delete = self.deleted_forms for form in self.initial_forms: - pk_name = self._pk_field.name - raw_pk_value = form._raw_value(pk_name) - - # clean() for different types of PK fields can sometimes return - # the model instance, and sometimes the PK. Handle either. - pk_value = form.fields[pk_name].clean(raw_pk_value) - pk_value = getattr(pk_value, 'pk', pk_value) - - obj = self._existing_object(pk_value) + obj = form.instance if form in forms_to_delete: + # If the pk is None, it means that the object can't be + # deleted again. Possible reason for this is that the + # object was already deleted from the DB. Refs #14877. + if obj.pk is None: + continue self.deleted_objects.append(obj) if commit: obj.delete() - continue - if form.has_changed(): + elif form.has_changed(): self.changed_objects.append((obj, form.changed_data)) saved_instances.append(self.save_existing(form, obj, commit=commit)) if not commit: diff --git a/tests/model_formsets_regress/tests.py b/tests/model_formsets_regress/tests.py index a79d3744e0..f8a5b7b3ac 100644 --- a/tests/model_formsets_regress/tests.py +++ b/tests/model_formsets_regress/tests.py @@ -454,3 +454,52 @@ class FormfieldShouldDeleteFormTests(TestCase): # verify no "odd" PKs left odd_ids = [user.pk for user in User.objects.all() if user.pk % 2] self.assertEqual(len(odd_ids), 0) + + +class RedeleteTests(TestCase): + def test_resubmit(self): + u = User.objects.create(username='foo', serial=1) + us = UserSite.objects.create(user=u, data=7) + formset_cls = inlineformset_factory(User, UserSite, fields="__all__") + data = { + 'serial': '1', + 'username': 'foo', + 'usersite_set-TOTAL_FORMS': '1', + 'usersite_set-INITIAL_FORMS': '1', + 'usersite_set-MAX_NUM_FORMS': '1', + 'usersite_set-0-id': six.text_type(us.pk), + 'usersite_set-0-data': '7', + 'usersite_set-0-user': 'foo', + 'usersite_set-0-DELETE': '1' + } + formset = formset_cls(data, instance=u) + self.assertTrue(formset.is_valid()) + formset.save() + self.assertEqual(UserSite.objects.count(), 0) + formset = formset_cls(data, instance=u) + # Even if the "us" object isn't in the DB any more, the form + # validates. + self.assertTrue(formset.is_valid()) + formset.save() + self.assertEqual(UserSite.objects.count(), 0) + + def test_delete_already_deleted(self): + u = User.objects.create(username='foo', serial=1) + us = UserSite.objects.create(user=u, data=7) + formset_cls = inlineformset_factory(User, UserSite, fields="__all__") + data = { + 'serial': '1', + 'username': 'foo', + 'usersite_set-TOTAL_FORMS': '1', + 'usersite_set-INITIAL_FORMS': '1', + 'usersite_set-MAX_NUM_FORMS': '1', + 'usersite_set-0-id': six.text_type(us.pk), + 'usersite_set-0-data': '7', + 'usersite_set-0-user': 'foo', + 'usersite_set-0-DELETE': '1' + } + formset = formset_cls(data, instance=u) + us.delete() + self.assertTrue(formset.is_valid()) + formset.save() + self.assertEqual(UserSite.objects.count(), 0)