Forms in model formsets and inline formsets can now be deleted even if they don't validate. Related to #9587.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@10283 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/forms/forms.py

@@ -205,6 +205,15 @@ class BaseForm(StrAndUnicode):
         """
         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

django/forms/formsets.py

@@ -228,9 +228,8 @@ class BaseFormSet(StrAndUnicode):
                 # 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)
+                raw_value = form._raw_value(DELETION_FIELD_NAME)
-                value = field.widget.value_from_datadict(self.data, self.files, prefix)
+                should_delete = field.clean(raw_value)
-                should_delete = field.clean(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.

django/forms/models.py

@@ -425,16 +425,22 @@ class BaseModelFormSet(BaseFormSet):
             existing_objects[obj.pk] = obj
         saved_instances = []
         for form in self.initial_forms:
-            obj = existing_objects[form.cleaned_data[self._pk_field.name].pk]
+            pk_name = self._pk_field.name
-            if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]:
+            raw_pk_value = form._raw_value(pk_name)
-                self.deleted_objects.append(obj)
+            pk_value = form.fields[pk_name].clean(raw_pk_value).pk
-                obj.delete()
+            obj = existing_objects[pk_value]
-            else:
+            if self.can_delete:
-                if form.changed_data:
+                raw_delete_value = form._raw_value(DELETION_FIELD_NAME)
-                    self.changed_objects.append((obj, form.changed_data))
+                should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value)
-                    saved_instances.append(self.save_existing(form, obj, commit=commit))
+                if should_delete:
-                    if not commit:
+                    self.deleted_objects.append(obj)
-                        self.saved_forms.append(form)
+                    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 @@ class BaseModelFormSet(BaseFormSet):
                 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]:
+            if self.can_delete:
-                continue
+                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)

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)

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

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)