Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #1829 from akaariai/ticket_14877

Fixed #14877 -- repeated deletion using formsets

Reviewed by Loic Bistuer
  • Loading branch information...
commit ed516a5fc8061615ec94bee37425d4fee8ca872b 2 parents 8faaf03 + efb0100
@akaariai akaariai authored
Showing with 68 additions and 19 deletions.
  1. +19 −19 django/forms/models.py
  2. +49 −0 tests/model_formsets_regress/tests.py
View
38 django/forms/models.py
@@ -546,20 +546,24 @@ def _existing_object(self, pk):
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 @@ def save_existing_objects(self, commit=True):
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:
View
49 tests/model_formsets_regress/tests.py
@@ -454,3 +454,52 @@ def test_custom_delete(self):
# 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)
Please sign in to comment.
Something went wrong with that request. Please try again.