Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Forms in model formsets and inline formsets can now be deleted even i…

…f they don't validate. Related to #9587.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@10283 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 15becf23a9e4c9b230745738d2d42f6ab8f0f031 1 parent 98f5f68
@jkocherhans jkocherhans authored
View
9 django/forms/forms.py
@@ -205,6 +205,15 @@ def non_field_errors(self):
"""
return self.errors.get(NON_FIELD_ERRORS, self.error_class())
+ def _raw_value(self, fieldname):
+ """
+ Returns the raw_value for a particular field name. This is just a
+ convenient wrapper around widget.value_from_datadict.
+ """
+ field = self.fields[fieldname]
+ prefix = self.add_prefix(fieldname)
+ return field.widget.value_from_datadict(self.data, self.files, prefix)
+
def full_clean(self):
"""
Cleans all of self.data and populates self._errors and
View
5 django/forms/formsets.py
@@ -228,9 +228,8 @@ def is_valid(self):
# more code than we'd like, but the form's cleaned_data will
# not exist if the form is invalid.
field = form.fields[DELETION_FIELD_NAME]
- prefix = form.add_prefix(DELETION_FIELD_NAME)
- value = field.widget.value_from_datadict(self.data, self.files, prefix)
- should_delete = field.clean(value)
+ raw_value = form._raw_value(DELETION_FIELD_NAME)
+ should_delete = field.clean(raw_value)
if should_delete:
# This form is going to be deleted so any of its errors
# should not cause the entire formset to be invalid.
View
33 django/forms/models.py
@@ -425,16 +425,22 @@ def save_existing_objects(self, commit=True):
existing_objects[obj.pk] = obj
saved_instances = []
for form in self.initial_forms:
- obj = existing_objects[form.cleaned_data[self._pk_field.name].pk]
- if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]:
- self.deleted_objects.append(obj)
- obj.delete()
- else:
- if form.changed_data:
- self.changed_objects.append((obj, form.changed_data))
- saved_instances.append(self.save_existing(form, obj, commit=commit))
- if not commit:
- self.saved_forms.append(form)
+ pk_name = self._pk_field.name
+ raw_pk_value = form._raw_value(pk_name)
+ pk_value = form.fields[pk_name].clean(raw_pk_value).pk
+ obj = existing_objects[pk_value]
+ if self.can_delete:
+ raw_delete_value = form._raw_value(DELETION_FIELD_NAME)
+ should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value)
+ if should_delete:
+ self.deleted_objects.append(obj)
+ obj.delete()
+ continue
+ if form.changed_data:
+ self.changed_objects.append((obj, form.changed_data))
+ saved_instances.append(self.save_existing(form, obj, commit=commit))
+ if not commit:
+ self.saved_forms.append(form)
return saved_instances
def save_new_objects(self, commit=True):
@@ -444,8 +450,11 @@ def save_new_objects(self, commit=True):
continue
# If someone has marked an add form for deletion, don't save the
# object.
- if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]:
- continue
+ if self.can_delete:
+ raw_delete_value = form._raw_value(DELETION_FIELD_NAME)
+ should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value)
+ if should_delete:
+ continue
self.new_objects.append(self.save_new(form, commit=commit))
if not commit:
self.saved_forms.append(form)
View
70 tests/modeltests/model_formsets/tests.py
@@ -0,0 +1,70 @@
+from django.test import TestCase
+from django.forms.models import modelformset_factory
+from modeltests.model_formsets.models import Poet, Poem
+
+class DeletionTests(TestCase):
+ def test_deletion(self):
+ PoetFormSet = modelformset_factory(Poet, can_delete=True)
+ poet = Poet.objects.create(name='test')
+ data = {
+ 'form-TOTAL_FORMS': u'1',
+ 'form-INITIAL_FORMS': u'1',
+ 'form-0-id': u'1',
+ 'form-0-name': u'test',
+ 'form-0-DELETE': u'on',
+ }
+ formset = PoetFormSet(data, queryset=Poet.objects.all())
+ formset.save()
+ self.assertTrue(formset.is_valid())
+ self.assertEqual(Poet.objects.count(), 0)
+
+ def test_add_form_deletion_when_invalid(self):
+ """
+ Make sure that an add form that is filled out, but marked for deletion
+ doesn't cause validation errors.
+ """
+ PoetFormSet = modelformset_factory(Poet, can_delete=True)
+ data = {
+ 'form-TOTAL_FORMS': u'1',
+ 'form-INITIAL_FORMS': u'0',
+ 'form-0-id': u'',
+ 'form-0-name': u'x' * 1000,
+ }
+ formset = PoetFormSet(data, queryset=Poet.objects.all())
+ # Make sure this form doesn't pass validation.
+ self.assertEqual(formset.is_valid(), False)
+ self.assertEqual(Poet.objects.count(), 0)
+
+ # Then make sure that it *does* pass validation and delete the object,
+ # even though the data isn't actually valid.
+ data['form-0-DELETE'] = 'on'
+ formset = PoetFormSet(data, queryset=Poet.objects.all())
+ self.assertEqual(formset.is_valid(), True)
+ formset.save()
+ self.assertEqual(Poet.objects.count(), 0)
+
+ def test_change_form_deletion_when_invalid(self):
+ """
+ Make sure that an add form that is filled out, but marked for deletion
+ doesn't cause validation errors.
+ """
+ PoetFormSet = modelformset_factory(Poet, can_delete=True)
+ poet = Poet.objects.create(name='test')
+ data = {
+ 'form-TOTAL_FORMS': u'1',
+ 'form-INITIAL_FORMS': u'1',
+ 'form-0-id': u'1',
+ 'form-0-name': u'x' * 1000,
+ }
+ formset = PoetFormSet(data, queryset=Poet.objects.all())
+ # Make sure this form doesn't pass validation.
+ self.assertEqual(formset.is_valid(), False)
+ self.assertEqual(Poet.objects.count(), 1)
+
+ # Then make sure that it *does* pass validation and delete the object,
+ # even though the data isn't actually valid.
+ data['form-0-DELETE'] = 'on'
+ formset = PoetFormSet(data, queryset=Poet.objects.all())
+ self.assertEqual(formset.is_valid(), True)
+ formset.save()
+ self.assertEqual(Poet.objects.count(), 0)
View
13 tests/regressiontests/inline_formsets/models.py
@@ -13,6 +13,19 @@ class Child(models.Model):
school = models.ForeignKey(School)
name = models.CharField(max_length=100)
+class Poet(models.Model):
+ name = models.CharField(max_length=100)
+
+ def __unicode__(self):
+ return self.name
+
+class Poem(models.Model):
+ poet = models.ForeignKey(Poet)
+ name = models.CharField(max_length=100)
+
+ def __unicode__(self):
+ return self.name
+
__test__ = {'API_TESTS': """
>>> from django.forms.models import inlineformset_factory
View
76 tests/regressiontests/inline_formsets/tests.py
@@ -0,0 +1,76 @@
+from django.test import TestCase
+from django.forms.models import inlineformset_factory
+from regressiontests.inline_formsets.models import Poet, Poem
+
+class DeletionTests(TestCase):
+ def test_deletion(self):
+ PoemFormSet = inlineformset_factory(Poet, Poem, can_delete=True)
+ poet = Poet.objects.create(name='test')
+ poet.poem_set.create(name='test poem')
+ data = {
+ 'poem_set-TOTAL_FORMS': u'1',
+ 'poem_set-INITIAL_FORMS': u'1',
+ 'poem_set-0-id': u'1',
+ 'poem_set-0-poem': u'1',
+ 'poem_set-0-name': u'test',
+ 'poem_set-0-DELETE': u'on',
+ }
+ formset = PoemFormSet(data, instance=poet)
+ formset.save()
+ self.assertTrue(formset.is_valid())
+ self.assertEqual(Poem.objects.count(), 0)
+
+ def test_add_form_deletion_when_invalid(self):
+ """
+ Make sure that an add form that is filled out, but marked for deletion
+ doesn't cause validation errors.
+ """
+ PoemFormSet = inlineformset_factory(Poet, Poem, can_delete=True)
+ poet = Poet.objects.create(name='test')
+ data = {
+ 'poem_set-TOTAL_FORMS': u'1',
+ 'poem_set-INITIAL_FORMS': u'0',
+ 'poem_set-0-id': u'',
+ 'poem_set-0-poem': u'1',
+ 'poem_set-0-name': u'x' * 1000,
+ }
+ formset = PoemFormSet(data, instance=poet)
+ # Make sure this form doesn't pass validation.
+ self.assertEqual(formset.is_valid(), False)
+ self.assertEqual(Poem.objects.count(), 0)
+
+ # Then make sure that it *does* pass validation and delete the object,
+ # even though the data isn't actually valid.
+ data['poem_set-0-DELETE'] = 'on'
+ formset = PoemFormSet(data, instance=poet)
+ self.assertEqual(formset.is_valid(), True)
+ formset.save()
+ self.assertEqual(Poem.objects.count(), 0)
+
+ def test_change_form_deletion_when_invalid(self):
+ """
+ Make sure that a change form that is filled out, but marked for deletion
+ doesn't cause validation errors.
+ """
+ PoemFormSet = inlineformset_factory(Poet, Poem, can_delete=True)
+ poet = Poet.objects.create(name='test')
+ poet.poem_set.create(name='test poem')
+ data = {
+ 'poem_set-TOTAL_FORMS': u'1',
+ 'poem_set-INITIAL_FORMS': u'1',
+ 'poem_set-0-id': u'1',
+ 'poem_set-0-poem': u'1',
+ 'poem_set-0-name': u'x' * 1000,
+ }
+ formset = PoemFormSet(data, instance=poet)
+ # Make sure this form doesn't pass validation.
+ self.assertEqual(formset.is_valid(), False)
+ self.assertEqual(Poem.objects.count(), 1)
+
+ # Then make sure that it *does* pass validation and delete the object,
+ # even though the data isn't actually valid.
+ data['poem_set-0-DELETE'] = 'on'
+ formset = PoemFormSet(data, instance=poet)
+ self.assertEqual(formset.is_valid(), True)
+ formset.save()
+ self.assertEqual(Poem.objects.count(), 0)
Please sign in to comment.
Something went wrong with that request. Please try again.