Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #5524 -- Do not remove cleaned_data when a form fails validation

cleaned_data is no longer deleted when form validation fails but only
contains the data that did validate.
Thanks to the various contributors to this patch (see ticket).
  • Loading branch information...
commit 121fd109de09ece4e263e508f9034df9b583da46 1 parent 10f979f
Claude Paroz authored August 04, 2012
2  django/contrib/formtools/tests/__init__.py
@@ -317,7 +317,7 @@ def test_14498(self):
317 317
 
318 318
         class WizardWithProcessStep(TestWizardClass):
319 319
             def process_step(self, request, form, step):
320  
-                that.assertTrue(hasattr(form, 'cleaned_data'))
  320
+                that.assertTrue(form.is_valid())
321 321
                 reached[0] = True
322 322
 
323 323
         wizard = WizardWithProcessStep([WizardPageOneForm,
2  django/forms/forms.py
@@ -271,8 +271,6 @@ def full_clean(self):
271 271
         self._clean_fields()
272 272
         self._clean_form()
273 273
         self._post_clean()
274  
-        if self._errors:
275  
-            del self.cleaned_data
276 274
 
277 275
     def _clean_fields(self):
278 276
         for name, field in self.fields.items():
25  django/forms/models.py
@@ -506,7 +506,7 @@ def validate_unique(self):
506 506
         all_unique_checks = set()
507 507
         all_date_checks = set()
508 508
         for form in self.forms:
509  
-            if not hasattr(form, 'cleaned_data'):
  509
+            if not form.is_valid():
510 510
                 continue
511 511
             exclude = form._get_validation_exclusions()
512 512
             unique_checks, date_checks = form.instance._get_unique_checks(exclude=exclude)
@@ -518,21 +518,21 @@ def validate_unique(self):
518 518
         for uclass, unique_check in all_unique_checks:
519 519
             seen_data = set()
520 520
             for form in self.forms:
521  
-                # if the form doesn't have cleaned_data then we ignore it,
522  
-                # it's already invalid
523  
-                if not hasattr(form, "cleaned_data"):
  521
+                if not form.is_valid():
524 522
                     continue
525 523
                 # get data for each field of each of unique_check
526 524
                 row_data = tuple([form.cleaned_data[field] for field in unique_check if field in form.cleaned_data])
527 525
                 if row_data and not None in row_data:
528  
-                    # if we've aready seen it then we have a uniqueness failure
  526
+                    # if we've already seen it then we have a uniqueness failure
529 527
                     if row_data in seen_data:
530 528
                         # poke error messages into the right places and mark
531 529
                         # the form as invalid
532 530
                         errors.append(self.get_unique_error_message(unique_check))
533 531
                         form._errors[NON_FIELD_ERRORS] = self.error_class([self.get_form_error()])
534  
-                        del form.cleaned_data
535  
-                        break
  532
+                        # remove the data from the cleaned_data dict since it was invalid
  533
+                        for field in unique_check:
  534
+                            if field in form.cleaned_data:
  535
+                                del form.cleaned_data[field]
536 536
                     # mark the data as seen
537 537
                     seen_data.add(row_data)
538 538
         # iterate over each of the date checks now
@@ -540,9 +540,7 @@ def validate_unique(self):
540 540
             seen_data = set()
541 541
             uclass, lookup, field, unique_for = date_check
542 542
             for form in self.forms:
543  
-                # if the form doesn't have cleaned_data then we ignore it,
544  
-                # it's already invalid
545  
-                if not hasattr(self, 'cleaned_data'):
  543
+                if not form.is_valid():
546 544
                     continue
547 545
                 # see if we have data for both fields
548 546
                 if (form.cleaned_data and form.cleaned_data[field] is not None
@@ -556,14 +554,15 @@ def validate_unique(self):
556 554
                     else:
557 555
                         date_data = (getattr(form.cleaned_data[unique_for], lookup),)
558 556
                     data = (form.cleaned_data[field],) + date_data
559  
-                    # if we've aready seen it then we have a uniqueness failure
  557
+                    # if we've already seen it then we have a uniqueness failure
560 558
                     if data in seen_data:
561 559
                         # poke error messages into the right places and mark
562 560
                         # the form as invalid
563 561
                         errors.append(self.get_date_error_message(date_check))
564 562
                         form._errors[NON_FIELD_ERRORS] = self.error_class([self.get_form_error()])
565  
-                        del form.cleaned_data
566  
-                        break
  563
+                        # remove the data from the cleaned_data dict since it was invalid
  564
+                        del form.cleaned_data[field]
  565
+                    # mark the data as seen
567 566
                     seen_data.add(data)
568 567
         if errors:
569 568
             raise ValidationError(errors)
21  docs/ref/forms/api.txt
@@ -199,8 +199,8 @@ Note that any text-based field -- such as ``CharField`` or ``EmailField`` --
199 199
 always cleans the input into a Unicode string. We'll cover the encoding
200 200
 implications later in this document.
201 201
 
202  
-If your data does *not* validate, your ``Form`` instance will not have a
203  
-``cleaned_data`` attribute::
  202
+If your data does *not* validate, the ``cleaned_data`` dictionary contains
  203
+only the valid fields::
204 204
 
205 205
     >>> data = {'subject': '',
206 206
     ...         'message': 'Hi there',
@@ -210,9 +210,12 @@ If your data does *not* validate, your ``Form`` instance will not have a
210 210
     >>> f.is_valid()
211 211
     False
212 212
     >>> f.cleaned_data
213  
-    Traceback (most recent call last):
214  
-    ...
215  
-    AttributeError: 'ContactForm' object has no attribute 'cleaned_data'
  213
+    {'cc_myself': True, 'message': u'Hi there'}
  214
+
  215
+.. versionchanged:: 1.5
  216
+
  217
+Until Django 1.5, the ``cleaned_data`` attribute wasn't defined at all when
  218
+the ``Form`` didn't validate.
216 219
 
217 220
 ``cleaned_data`` will always *only* contain a key for fields defined in the
218 221
 ``Form``, even if you pass extra data when you define the ``Form``. In this
@@ -232,9 +235,9 @@ but ``cleaned_data`` contains only the form's fields::
232 235
     >>> f.cleaned_data # Doesn't contain extra_field_1, etc.
233 236
     {'cc_myself': True, 'message': u'Hi there', 'sender': u'foo@example.com', 'subject': u'hello'}
234 237
 
235  
-``cleaned_data`` will include a key and value for *all* fields defined in the
236  
-``Form``, even if the data didn't include a value for fields that are not
237  
-required. In this example, the data dictionary doesn't include a value for the
  238
+When the ``Form`` is valid, ``cleaned_data`` will include a key and value for
  239
+*all* its fields, even if the data didn't include a value for some optional
  240
+fields. In this example, the data dictionary doesn't include a value for the
238 241
 ``nick_name`` field, but ``cleaned_data`` includes it, with an empty value::
239 242
 
240 243
     >>> class OptionalPersonForm(Form):
@@ -583,7 +586,7 @@ lazy developers -- they're not the only way a form object can be displayed.
583 586
 
584 587
    Used to display HTML or access attributes for a single field of a
585 588
    :class:`Form` instance.
586  
-   
  589
+
587 590
    The :meth:`__unicode__` and :meth:`__str__` methods of this object displays
588 591
    the HTML for this field.
589 592
 
10  docs/ref/forms/validation.txt
@@ -362,7 +362,9 @@ Secondly, once we have decided that the combined data in the two fields we are
362 362
 considering aren't valid, we must remember to remove them from the
363 363
 ``cleaned_data``.
364 364
 
365  
-In fact, Django will currently completely wipe out the ``cleaned_data``
366  
-dictionary if there are any errors in the form. However, this behavior may
367  
-change in the future, so it's not a bad idea to clean up after yourself in the
368  
-first place.
  365
+.. versionchanged:: 1.5
  366
+
  367
+Django used to remove the ``cleaned_data`` attribute entirely if there were
  368
+any errors in the form. Since version 1.5, ``cleaned_data`` is present even if
  369
+the form doesn't validate, but it contains only field values that did
  370
+validate.
10  docs/releases/1.5.txt
@@ -239,6 +239,16 @@ database state behind or unit tests that rely on some form of state being
239 239
 preserved after the execution of other tests. Such tests are already very
240 240
 fragile, and must now be changed to be able to run independently.
241 241
 
  242
+`cleaned_data` dictionary kept for invalid forms
  243
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  244
+
  245
+The :attr:`~django.forms.Form.cleaned_data` dictionary is now always present
  246
+after form validation. When the form doesn't validate, it contains only the
  247
+fields that passed validation. You should test the success of the validation
  248
+with the :meth:`~django.forms.Form.is_valid()` method and not with the
  249
+presence or absence of the :attr:`~django.forms.Form.cleaned_data` attribute
  250
+on the form.
  251
+
242 252
 Miscellaneous
243 253
 ~~~~~~~~~~~~~
244 254
 
3  tests/modeltests/model_forms/tests.py
@@ -638,8 +638,7 @@ def test_with_data(self):
638 638
         f = BaseCategoryForm({'name': '', 'slug': 'not a slug!', 'url': 'foo'})
639 639
         self.assertEqual(f.errors['name'], ['This field is required.'])
640 640
         self.assertEqual(f.errors['slug'], ["Enter a valid 'slug' consisting of letters, numbers, underscores or hyphens."])
641  
-        with self.assertRaises(AttributeError):
642  
-            f.cleaned_data
  641
+        self.assertEqual(f.cleaned_data, {'url': 'foo'})
643 642
         with self.assertRaises(ValueError):
644 643
             f.save()
645 644
         f = BaseCategoryForm({'name': '', 'slug': '', 'url': 'foo'})
24  tests/regressiontests/forms/tests/forms.py
@@ -82,11 +82,7 @@ def test_empty_dict(self):
82 82
         self.assertEqual(p.errors['last_name'], ['This field is required.'])
83 83
         self.assertEqual(p.errors['birthday'], ['This field is required.'])
84 84
         self.assertFalse(p.is_valid())
85  
-        try:
86  
-            p.cleaned_data
87  
-            self.fail('Attempts to access cleaned_data when validation fails should fail.')
88  
-        except AttributeError:
89  
-            pass
  85
+        self.assertEqual(p.cleaned_data, {})
90 86
         self.assertHTMLEqual(str(p), """<tr><th><label for="id_first_name">First name:</label></th><td><ul class="errorlist"><li>This field is required.</li></ul><input type="text" name="first_name" id="id_first_name" /></td></tr>
91 87
 <tr><th><label for="id_last_name">Last name:</label></th><td><ul class="errorlist"><li>This field is required.</li></ul><input type="text" name="last_name" id="id_last_name" /></td></tr>
92 88
 <tr><th><label for="id_birthday">Birthday:</label></th><td><ul class="errorlist"><li>This field is required.</li></ul><input type="text" name="birthday" id="id_birthday" /></td></tr>""")
@@ -145,11 +141,7 @@ def test_unicode_values(self):
145 141
   * This field is required.
146 142
 * birthday
147 143
   * This field is required.""")
148  
-        try:
149  
-            p.cleaned_data
150  
-            self.fail('Attempts to access cleaned_data when validation fails should fail.')
151  
-        except AttributeError:
152  
-            pass
  144
+        self.assertEqual(p.cleaned_data, {'last_name': 'Lennon'})
153 145
         self.assertEqual(p['first_name'].errors, ['This field is required.'])
154 146
         self.assertHTMLEqual(p['first_name'].errors.as_ul(), '<ul class="errorlist"><li>This field is required.</li></ul>')
155 147
         self.assertEqual(p['first_name'].errors.as_text(), '* This field is required.')
@@ -1678,11 +1670,7 @@ class SongForm(Form):
1678 1670
         form = SongForm(data, empty_permitted=False)
1679 1671
         self.assertFalse(form.is_valid())
1680 1672
         self.assertEqual(form.errors, {'name': ['This field is required.'], 'artist': ['This field is required.']})
1681  
-        try:
1682  
-            form.cleaned_data
1683  
-            self.fail('Attempts to access cleaned_data when validation fails should fail.')
1684  
-        except AttributeError:
1685  
-            pass
  1673
+        self.assertEqual(form.cleaned_data, {})
1686 1674
 
1687 1675
         # Now let's show what happens when empty_permitted=True and the form is empty.
1688 1676
         form = SongForm(data, empty_permitted=True)
@@ -1696,11 +1684,7 @@ class SongForm(Form):
1696 1684
         form = SongForm(data, empty_permitted=False)
1697 1685
         self.assertFalse(form.is_valid())
1698 1686
         self.assertEqual(form.errors, {'name': ['This field is required.']})
1699  
-        try:
1700  
-            form.cleaned_data
1701  
-            self.fail('Attempts to access cleaned_data when validation fails should fail.')
1702  
-        except AttributeError:
1703  
-            pass
  1687
+        self.assertEqual(form.cleaned_data, {'artist': 'The Doors'})
1704 1688
 
1705 1689
         # If a field is not given in the data then None is returned for its data. Lets
1706 1690
         # make sure that when checking for empty_permitted that None is treated

0 notes on commit 121fd10

Please sign in to comment.
Something went wrong with that request. Please try again.