From 4120a181e9109383ab35df52dbb3621302a75a72 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 12 Mar 2010 15:51:00 +0000 Subject: [PATCH] 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. git-svn-id: http://code.djangoproject.com/svn/django/trunk@12771 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/forms/formsets.py | 21 +++++++------ tests/regressiontests/forms/formsets.py | 40 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/django/forms/formsets.py b/django/forms/formsets.py index 7acf238795..ec14f813e9 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -163,7 +163,7 @@ class BaseFormSet(StrAndUnicode): # 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 @@ class BaseFormSet(StrAndUnicode): 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 @@ class BaseFormSet(StrAndUnicode): 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 @@ class BaseFormSet(StrAndUnicode): 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 diff --git a/tests/regressiontests/forms/formsets.py b/tests/regressiontests/forms/formsets.py index d9d4082f71..3eecb7f7be 100644 --- a/tests/regressiontests/forms/formsets.py +++ b/tests/regressiontests/forms/formsets.py @@ -332,6 +332,26 @@ If we remove the deletion flag now we will have our validation back. >>> 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,6 +512,26 @@ True >>> [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 ##########################################################