Fixed #11418 -- formset.cleaned_data no longer raises AttributeError when…

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

@@ -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)

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:
 

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()

tests/regressiontests/forms/tests/formsets.py

 # -*- 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(),"""<input type="hidden" name="choices-TOTAL_FORMS" value="1" /><input type="hidden" name="choices-INITIAL_FORMS" value="0" /><input type="hidden" name="choices-MAX_NUM_FORMS" value="0" />
 <li>Choice: <input type="text" name="choices-0-choice" value="Calexico" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" value="100" /></li>""")
+
+
+# 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)