From 65b380e74ab79ee011f7556e342dd4d656b018a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Honza=20Kr=C3=A1l?= Date: Sun, 21 Nov 2010 17:27:01 +0000 Subject: [PATCH] Fixed #11418 -- formset.cleaned_data no longer raises AttributeError when is_valid is True. Thanks mlavin! This also introduces a slightly backwards-incompatible change in FormSet's behavior, see the release docs for details. git-svn-id: http://code.djangoproject.com/svn/django/trunk@14667 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/forms/formsets.py | 14 +++--- docs/releases/1.3.txt | 30 ++++++++++++ docs/topics/forms/formsets.txt | 13 +++-- tests/regressiontests/forms/tests/formsets.py | 49 ++++++++++++++++++- 4 files changed, 93 insertions(+), 13 deletions(-) diff --git a/django/forms/formsets.py b/django/forms/formsets.py index 62a25bf737..6d92236686 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -37,8 +37,8 @@ class BaseFormSet(StrAndUnicode): self.is_bound = data is not None or files is not None self.prefix = prefix or self.get_default_prefix() self.auto_id = auto_id - self.data = data - self.files = files + self.data = data or {} + self.files = files or {} self.initial = initial self.error_class = error_class self._errors = None @@ -51,7 +51,7 @@ class BaseFormSet(StrAndUnicode): def _management_form(self): """Returns the ManagementForm instance for this FormSet.""" - if self.data or self.files: + if self.is_bound: form = ManagementForm(self.data, auto_id=self.auto_id, prefix=self.prefix) if not form.is_valid(): raise ValidationError('ManagementForm data is missing or has been tampered with') @@ -66,7 +66,7 @@ class BaseFormSet(StrAndUnicode): def total_form_count(self): """Returns the total number of forms in this FormSet.""" - if self.data or self.files: + if self.is_bound: return self.management_form.cleaned_data[TOTAL_FORM_COUNT] else: initial_forms = self.initial_form_count() @@ -81,7 +81,7 @@ class BaseFormSet(StrAndUnicode): def initial_form_count(self): """Returns the number of forms that are required in this FormSet.""" - if self.data or self.files: + if self.is_bound: return self.management_form.cleaned_data[INITIAL_FORM_COUNT] else: # Use the length of the inital data if it's there, 0 otherwise. @@ -101,7 +101,7 @@ class BaseFormSet(StrAndUnicode): Instantiates and returns the i-th form instance in a formset. """ defaults = {'auto_id': self.auto_id, 'prefix': self.add_prefix(i)} - if self.data or self.files: + if self.is_bound: defaults['data'] = self.data defaults['files'] = self.files if self.initial: @@ -133,7 +133,7 @@ class BaseFormSet(StrAndUnicode): 'prefix': self.add_prefix('__prefix__'), 'empty_permitted': True, } - if self.data or self.files: + if self.is_bound: defaults['data'] = self.data defaults['files'] = self.files defaults.update(kwargs) diff --git a/docs/releases/1.3.txt b/docs/releases/1.3.txt index d1c42496ce..24702111a2 100644 --- a/docs/releases/1.3.txt +++ b/docs/releases/1.3.txt @@ -266,6 +266,36 @@ local flavors: has been removed from the province list in favor of the new official designation "Aceh (ACE)". +FormSet updates +~~~~~~~~~~~~~~~ + +In Django 1.3 ``FormSet`` creation behavior is modified slightly. Historically +the class didn't make a distinction between not being passed data and being +passed empty dictionary. This was inconsistent with behavior in other parts of +the framework. Starting with 1.3 if you pass in empty dictionary the +``FormSet`` will raise a ``ValidationError``. + +For example with a ``FormSet``:: + + >>> class ArticleForm(Form): + ... title = CharField() + ... pub_date = DateField() + >>> ArticleFormSet = formset_factory(ArticleForm) + +the following code will raise a ``ValidationError``:: + + >>> ArticleFormSet({}) + Traceback (most recent call last): + ... + ValidationError: [u'ManagementForm data is missing or has been tampered with'] + +if you need to instantiate an empty ``FormSet``, don't pass in the data or use +``None``:: + + >>> formset = ArticleFormSet() + >>> formset = ArticleFormSet(data=None) + + .. _deprecated-features-1.3: diff --git a/docs/topics/forms/formsets.txt b/docs/topics/forms/formsets.txt index e7b09dc409..fae76c7a2e 100644 --- a/docs/topics/forms/formsets.txt +++ b/docs/topics/forms/formsets.txt @@ -100,7 +100,12 @@ an ``is_valid`` method on the formset to provide a convenient way to validate all forms in the formset:: >>> ArticleFormSet = formset_factory(ArticleForm) - >>> formset = ArticleFormSet({}) + >>> data = { + ... 'form-TOTAL_FORMS': u'1', + ... 'form-INITIAL_FORMS': u'0', + ... 'form-MAX_NUM_FORMS': u'', + ... } + >>> formset = ArticleFormSet(data) >>> formset.is_valid() True @@ -113,7 +118,7 @@ provide an invalid article:: ... 'form-INITIAL_FORMS': u'0', ... 'form-MAX_NUM_FORMS': u'', ... 'form-0-title': u'Test', - ... 'form-0-pub_date': u'16 June 1904', + ... 'form-0-pub_date': u'1904-06-16', ... 'form-1-title': u'Test', ... 'form-1-pub_date': u'', # <-- this date is missing but required ... } @@ -208,9 +213,9 @@ is where you define your own validation that works at the formset level:: ... 'form-INITIAL_FORMS': u'0', ... 'form-MAX_NUM_FORMS': u'', ... 'form-0-title': u'Test', - ... 'form-0-pub_date': u'16 June 1904', + ... 'form-0-pub_date': u'1904-06-16', ... 'form-1-title': u'Test', - ... 'form-1-pub_date': u'23 June 1912', + ... 'form-1-pub_date': u'1912-06-23', ... } >>> formset = ArticleFormSet(data) >>> formset.is_valid() diff --git a/tests/regressiontests/forms/tests/formsets.py b/tests/regressiontests/forms/tests/formsets.py index bf7124fe59..ed2581f6ac 100644 --- a/tests/regressiontests/forms/tests/formsets.py +++ b/tests/regressiontests/forms/tests/formsets.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from django.forms import Form, CharField, IntegerField, ValidationError +from django.forms import Form, CharField, IntegerField, ValidationError, DateField from django.forms.formsets import formset_factory, BaseFormSet from django.utils.unittest import TestCase @@ -741,7 +741,12 @@ class FormsFormsetTestCase(TestCase): formset = FavoriteDrinksFormSet() self.assertEqual(formset.management_form.prefix, 'form') - formset = FavoriteDrinksFormSet(data={}) + data = { + 'form-TOTAL_FORMS': '2', + 'form-INITIAL_FORMS': '0', + 'form-MAX_NUM_FORMS': '0', + } + formset = FavoriteDrinksFormSet(data=data) self.assertEqual(formset.management_form.prefix, 'form') formset = FavoriteDrinksFormSet(initial={}) @@ -795,3 +800,43 @@ class FormsetAsFooTests(TestCase): self.assertEqual(formset.as_ul(),"""
  • Choice:
  • Votes:
    • """) + + +# Regression test for #11418 ################################################# +class ArticleForm(Form): + title = CharField() + pub_date = DateField() + +ArticleFormSet = formset_factory(ArticleForm) + +class TestIsBoundBehavior(TestCase): + def test_no_data_raises_validation_error(self): + self.assertRaises(ValidationError, ArticleFormSet, {}) + + def test_with_management_data_attrs_work_fine(self): + data = { + 'form-TOTAL_FORMS': u'1', + 'form-INITIAL_FORMS': u'0', + } + formset = ArticleFormSet(data) + self.assertEquals(0, formset.initial_form_count()) + self.assertEquals(1, formset.total_form_count()) + self.assertTrue(formset.is_bound) + self.assertTrue(formset.forms[0].is_bound) + self.assertTrue(formset.is_valid()) + self.assertTrue(formset.forms[0].is_valid()) + self.assertEquals([{}], formset.cleaned_data) + + + def test_form_errors_are_cought_by_formset(self): + data = { + 'form-TOTAL_FORMS': u'2', + 'form-INITIAL_FORMS': u'0', + 'form-0-title': u'Test', + 'form-0-pub_date': u'1904-06-16', + 'form-1-title': u'Test', + 'form-1-pub_date': u'', # <-- this date is missing but required + } + formset = ArticleFormSet(data) + self.assertFalse(formset.is_valid()) + self.assertEquals([{}, {'pub_date': [u'This field is required.']}], formset.errors)