mirror of
https://github.com/django/django.git
synced 2024-12-22 17:16:24 +00:00
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.
This commit is contained in:
parent
8faaf03b86
commit
efb0100ee6
@ -546,20 +546,24 @@ class BaseModelFormSet(BaseFormSet):
|
|||||||
self._object_dict = dict((o.pk, o) for o in self.get_queryset())
|
self._object_dict = dict((o.pk, o) for o in self.get_queryset())
|
||||||
return self._object_dict.get(pk)
|
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):
|
def _construct_form(self, i, **kwargs):
|
||||||
if self.is_bound and i < self.initial_form_count():
|
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_key = "%s-%s" % (self.add_prefix(i), self.model._meta.pk.name)
|
||||||
pk = self.data[pk_key]
|
pk = self.data[pk_key]
|
||||||
pk_field = self.model._meta.pk
|
pk_field = self.model._meta.pk
|
||||||
pk = pk_field.get_db_prep_lookup('exact', pk,
|
to_python = self._get_to_python(pk_field)
|
||||||
connection=connections[self.get_queryset().db])
|
pk = to_python(pk)
|
||||||
if isinstance(pk, list):
|
|
||||||
pk = pk[0]
|
|
||||||
kwargs['instance'] = self._existing_object(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]
|
kwargs['instance'] = self.get_queryset()[i]
|
||||||
if i >= self.initial_form_count() and self.initial_extra:
|
if i >= self.initial_form_count() and self.initial_extra:
|
||||||
# Set initial values for extra forms
|
# Set initial values for extra forms
|
||||||
@ -711,21 +715,17 @@ class BaseModelFormSet(BaseFormSet):
|
|||||||
saved_instances = []
|
saved_instances = []
|
||||||
forms_to_delete = self.deleted_forms
|
forms_to_delete = self.deleted_forms
|
||||||
for form in self.initial_forms:
|
for form in self.initial_forms:
|
||||||
pk_name = self._pk_field.name
|
obj = form.instance
|
||||||
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)
|
|
||||||
if form in forms_to_delete:
|
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)
|
self.deleted_objects.append(obj)
|
||||||
if commit:
|
if commit:
|
||||||
obj.delete()
|
obj.delete()
|
||||||
continue
|
elif form.has_changed():
|
||||||
if form.has_changed():
|
|
||||||
self.changed_objects.append((obj, form.changed_data))
|
self.changed_objects.append((obj, form.changed_data))
|
||||||
saved_instances.append(self.save_existing(form, obj, commit=commit))
|
saved_instances.append(self.save_existing(form, obj, commit=commit))
|
||||||
if not commit:
|
if not commit:
|
||||||
|
@ -454,3 +454,52 @@ class FormfieldShouldDeleteFormTests(TestCase):
|
|||||||
# verify no "odd" PKs left
|
# verify no "odd" PKs left
|
||||||
odd_ids = [user.pk for user in User.objects.all() if user.pk % 2]
|
odd_ids = [user.pk for user in User.objects.all() if user.pk % 2]
|
||||||
self.assertEqual(len(odd_ids), 0)
|
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)
|
||||||
|
Loading…
Reference in New Issue
Block a user