Author: russellm
Date: 2010-03-12 09:51:00 -0600 (Fri, 12 Mar 2010)
New Revision: 12771

Modified:
   django/trunk/django/forms/formsets.py
   django/trunk/tests/regressiontests/forms/formsets.py
Log:
Fixed #11801 -- Corrected form validation to ensure you can still get 
deleted_forms and ordered_forms when a form that is being deleted doesn't 
validate. Thanks to dantallis for the report and patch.

Modified: django/trunk/django/forms/formsets.py
===================================================================
--- django/trunk/django/forms/formsets.py       2010-03-12 15:32:06 UTC (rev 
12770)
+++ django/trunk/django/forms/formsets.py       2010-03-12 15:51:00 UTC (rev 
12771)
@@ -163,7 +163,7 @@
                 # if this is an extra form and hasn't changed, don't consider 
it
                 if i >= self.initial_form_count() and not form.has_changed():
                     continue
-                if form.cleaned_data[DELETION_FIELD_NAME]:
+                if self._should_delete_form(form):
                     self._deleted_form_indexes.append(i)
         return [self.forms[i] for i in self._deleted_form_indexes]
     deleted_forms = property(_get_deleted_forms)
@@ -187,7 +187,7 @@
                 if i >= self.initial_form_count() and not form.has_changed():
                     continue
                 # don't add data marked for deletion to self.ordered_data
-                if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]:
+                if self.can_delete and self._should_delete_form(form):
                     continue
                 self._ordering.append((i, 
form.cleaned_data[ORDERING_FIELD_NAME]))
             # After we're done populating self._ordering, sort it.
@@ -231,6 +231,15 @@
         return self._errors
     errors = property(_get_errors)
 
+    def _should_delete_form(self, form):
+        # The way we lookup the value of the deletion field here takes
+        # 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]
+        raw_value = form._raw_value(DELETION_FIELD_NAME)
+        should_delete = field.clean(raw_value)
+        return should_delete
+
     def is_valid(self):
         """
         Returns True if form.errors is empty for every form in self.forms.
@@ -243,13 +252,7 @@
         for i in range(0, self.total_form_count()):
             form = self.forms[i]
             if self.can_delete:
-                # The way we lookup the value of the deletion field here takes
-                # 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]
-                raw_value = form._raw_value(DELETION_FIELD_NAME)
-                should_delete = field.clean(raw_value)
-                if should_delete:
+                if self._should_delete_form(form):
                     # This form is going to be deleted so any of its errors
                     # should not cause the entire formset to be invalid.
                     continue

Modified: django/trunk/tests/regressiontests/forms/formsets.py
===================================================================
--- django/trunk/tests/regressiontests/forms/formsets.py        2010-03-12 
15:32:06 UTC (rev 12770)
+++ django/trunk/tests/regressiontests/forms/formsets.py        2010-03-12 
15:51:00 UTC (rev 12771)
@@ -332,6 +332,26 @@
 >>> formset.is_valid()
 False
 
+Should be able to get deleted_forms from a valid formset even if a
+deleted form would have been invalid.
+
+>>> class Person(Form):
+...     name = CharField()
+
+>>> PeopleForm = formset_factory(
+...     form=Person,
+...     can_delete=True)
+
+>>> p = PeopleForm(
+...     {'form-0-name': u'', 'form-0-DELETE': u'on', # no name!
+...      'form-TOTAL_FORMS': 1, 'form-INITIAL_FORMS': 1,
+...      'form-MAX_NUM_FORMS': 1})
+
+>>> p.is_valid()
+True
+>>> len(p.deleted_forms)
+1
+
 # FormSets with ordering ######################################################
 
 We can also add ordering ability to a FormSet with an agrument to
@@ -492,7 +512,27 @@
 >>> [form.cleaned_data for form in formset.deleted_forms]
 [{'votes': 900, 'DELETE': True, 'ORDER': 2, 'choice': u'Fergie'}]
 
+Should be able to get ordered forms from a valid formset even if a
+deleted form would have been invalid.
 
+>>> class Person(Form):
+...     name = CharField()
+
+>>> PeopleForm = formset_factory(
+...     form=Person,
+...     can_delete=True,
+...     can_order=True)
+
+>>> p = PeopleForm(
+...     {'form-0-name': u'', 'form-0-DELETE': u'on', # no name!
+...      'form-TOTAL_FORMS': 1, 'form-INITIAL_FORMS': 1,
+...      'form-MAX_NUM_FORMS': 1})
+
+>>> p.is_valid()
+True
+>>> p.ordered_forms
+[]
+
 # FormSet clean hook ##########################################################
 
 FormSets have a hook for doing extra validation that shouldn't be tied to any

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Reply via email to