diff --git a/django/contrib/formtools/tests/__init__.py b/django/contrib/formtools/tests/__init__.py index 3bccb55034..ee93479cbd 100644 --- a/django/contrib/formtools/tests/__init__.py +++ b/django/contrib/formtools/tests/__init__.py @@ -317,7 +317,7 @@ class WizardTests(TestCase): class WizardWithProcessStep(TestWizardClass): def process_step(self, request, form, step): - that.assertTrue(hasattr(form, 'cleaned_data')) + that.assertTrue(form.is_valid()) reached[0] = True wizard = WizardWithProcessStep([WizardPageOneForm, diff --git a/django/forms/forms.py b/django/forms/forms.py index 4bc3ee9d26..4d4cdbe3db 100644 --- a/django/forms/forms.py +++ b/django/forms/forms.py @@ -271,8 +271,6 @@ class BaseForm(StrAndUnicode): self._clean_fields() self._clean_form() self._post_clean() - if self._errors: - del self.cleaned_data def _clean_fields(self): for name, field in self.fields.items(): diff --git a/django/forms/models.py b/django/forms/models.py index b5939b6be3..e6ae357d19 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -506,7 +506,7 @@ class BaseModelFormSet(BaseFormSet): all_unique_checks = set() all_date_checks = set() for form in self.forms: - if not hasattr(form, 'cleaned_data'): + if not form.is_valid(): continue exclude = form._get_validation_exclusions() unique_checks, date_checks = form.instance._get_unique_checks(exclude=exclude) @@ -518,21 +518,21 @@ class BaseModelFormSet(BaseFormSet): for uclass, unique_check in all_unique_checks: seen_data = set() for form in self.forms: - # if the form doesn't have cleaned_data then we ignore it, - # it's already invalid - if not hasattr(form, "cleaned_data"): + if not form.is_valid(): continue # get data for each field of each of unique_check row_data = tuple([form.cleaned_data[field] for field in unique_check if field in form.cleaned_data]) if row_data and not None in row_data: - # if we've aready seen it then we have a uniqueness failure + # if we've already seen it then we have a uniqueness failure if row_data in seen_data: # poke error messages into the right places and mark # the form as invalid errors.append(self.get_unique_error_message(unique_check)) form._errors[NON_FIELD_ERRORS] = self.error_class([self.get_form_error()]) - del form.cleaned_data - break + # remove the data from the cleaned_data dict since it was invalid + for field in unique_check: + if field in form.cleaned_data: + del form.cleaned_data[field] # mark the data as seen seen_data.add(row_data) # iterate over each of the date checks now @@ -540,9 +540,7 @@ class BaseModelFormSet(BaseFormSet): seen_data = set() uclass, lookup, field, unique_for = date_check for form in self.forms: - # if the form doesn't have cleaned_data then we ignore it, - # it's already invalid - if not hasattr(self, 'cleaned_data'): + if not form.is_valid(): continue # see if we have data for both fields if (form.cleaned_data and form.cleaned_data[field] is not None @@ -556,14 +554,15 @@ class BaseModelFormSet(BaseFormSet): else: date_data = (getattr(form.cleaned_data[unique_for], lookup),) data = (form.cleaned_data[field],) + date_data - # if we've aready seen it then we have a uniqueness failure + # if we've already seen it then we have a uniqueness failure if data in seen_data: # poke error messages into the right places and mark # the form as invalid errors.append(self.get_date_error_message(date_check)) form._errors[NON_FIELD_ERRORS] = self.error_class([self.get_form_error()]) - del form.cleaned_data - break + # remove the data from the cleaned_data dict since it was invalid + del form.cleaned_data[field] + # mark the data as seen seen_data.add(data) if errors: raise ValidationError(errors) diff --git a/docs/ref/forms/api.txt b/docs/ref/forms/api.txt index 50488b026a..777d73e015 100644 --- a/docs/ref/forms/api.txt +++ b/docs/ref/forms/api.txt @@ -199,8 +199,8 @@ Note that any text-based field -- such as ``CharField`` or ``EmailField`` -- always cleans the input into a Unicode string. We'll cover the encoding implications later in this document. -If your data does *not* validate, your ``Form`` instance will not have a -``cleaned_data`` attribute:: +If your data does *not* validate, the ``cleaned_data`` dictionary contains +only the valid fields:: >>> data = {'subject': '', ... 'message': 'Hi there', @@ -210,9 +210,12 @@ If your data does *not* validate, your ``Form`` instance will not have a >>> f.is_valid() False >>> f.cleaned_data - Traceback (most recent call last): - ... - AttributeError: 'ContactForm' object has no attribute 'cleaned_data' + {'cc_myself': True, 'message': u'Hi there'} + +.. versionchanged:: 1.5 + +Until Django 1.5, the ``cleaned_data`` attribute wasn't defined at all when +the ``Form`` didn't validate. ``cleaned_data`` will always *only* contain a key for fields defined in the ``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:: >>> f.cleaned_data # Doesn't contain extra_field_1, etc. {'cc_myself': True, 'message': u'Hi there', 'sender': u'foo@example.com', 'subject': u'hello'} -``cleaned_data`` will include a key and value for *all* fields defined in the -``Form``, even if the data didn't include a value for fields that are not -required. In this example, the data dictionary doesn't include a value for the +When the ``Form`` is valid, ``cleaned_data`` will include a key and value for +*all* its fields, even if the data didn't include a value for some optional +fields. In this example, the data dictionary doesn't include a value for the ``nick_name`` field, but ``cleaned_data`` includes it, with an empty value:: >>> class OptionalPersonForm(Form): @@ -583,7 +586,7 @@ lazy developers -- they're not the only way a form object can be displayed. Used to display HTML or access attributes for a single field of a :class:`Form` instance. - + The :meth:`__unicode__` and :meth:`__str__` methods of this object displays the HTML for this field. diff --git a/docs/ref/forms/validation.txt b/docs/ref/forms/validation.txt index 97883d7880..95424d0cd0 100644 --- a/docs/ref/forms/validation.txt +++ b/docs/ref/forms/validation.txt @@ -362,7 +362,9 @@ Secondly, once we have decided that the combined data in the two fields we are considering aren't valid, we must remember to remove them from the ``cleaned_data``. -In fact, Django will currently completely wipe out the ``cleaned_data`` -dictionary if there are any errors in the form. However, this behavior may -change in the future, so it's not a bad idea to clean up after yourself in the -first place. +.. versionchanged:: 1.5 + +Django used to remove the ``cleaned_data`` attribute entirely if there were +any errors in the form. Since version 1.5, ``cleaned_data`` is present even if +the form doesn't validate, but it contains only field values that did +validate. diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index 968d63e1f3..3a9b2d859a 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -239,6 +239,16 @@ database state behind or unit tests that rely on some form of state being preserved after the execution of other tests. Such tests are already very fragile, and must now be changed to be able to run independently. +`cleaned_data` dictionary kept for invalid forms +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The :attr:`~django.forms.Form.cleaned_data` dictionary is now always present +after form validation. When the form doesn't validate, it contains only the +fields that passed validation. You should test the success of the validation +with the :meth:`~django.forms.Form.is_valid()` method and not with the +presence or absence of the :attr:`~django.forms.Form.cleaned_data` attribute +on the form. + Miscellaneous ~~~~~~~~~~~~~ diff --git a/tests/modeltests/model_forms/tests.py b/tests/modeltests/model_forms/tests.py index fc37a25872..1da7f583be 100644 --- a/tests/modeltests/model_forms/tests.py +++ b/tests/modeltests/model_forms/tests.py @@ -638,8 +638,7 @@ class OldFormForXTests(TestCase): f = BaseCategoryForm({'name': '', 'slug': 'not a slug!', 'url': 'foo'}) self.assertEqual(f.errors['name'], ['This field is required.']) self.assertEqual(f.errors['slug'], ["Enter a valid 'slug' consisting of letters, numbers, underscores or hyphens."]) - with self.assertRaises(AttributeError): - f.cleaned_data + self.assertEqual(f.cleaned_data, {'url': 'foo'}) with self.assertRaises(ValueError): f.save() f = BaseCategoryForm({'name': '', 'slug': '', 'url': 'foo'}) diff --git a/tests/regressiontests/forms/tests/forms.py b/tests/regressiontests/forms/tests/forms.py index 7e1c8384a0..a8a28ba806 100644 --- a/tests/regressiontests/forms/tests/forms.py +++ b/tests/regressiontests/forms/tests/forms.py @@ -82,11 +82,7 @@ class FormsTestCase(TestCase): self.assertEqual(p.errors['last_name'], ['This field is required.']) self.assertEqual(p.errors['birthday'], ['This field is required.']) self.assertFalse(p.is_valid()) - try: - p.cleaned_data - self.fail('Attempts to access cleaned_data when validation fails should fail.') - except AttributeError: - pass + self.assertEqual(p.cleaned_data, {}) self.assertHTMLEqual(str(p), """ """) @@ -145,11 +141,7 @@ class FormsTestCase(TestCase): * This field is required. * birthday * This field is required.""") - try: - p.cleaned_data - self.fail('Attempts to access cleaned_data when validation fails should fail.') - except AttributeError: - pass + self.assertEqual(p.cleaned_data, {'last_name': 'Lennon'}) self.assertEqual(p['first_name'].errors, ['This field is required.']) self.assertHTMLEqual(p['first_name'].errors.as_ul(), '') self.assertEqual(p['first_name'].errors.as_text(), '* This field is required.') @@ -1678,11 +1670,7 @@ class FormsTestCase(TestCase): form = SongForm(data, empty_permitted=False) self.assertFalse(form.is_valid()) self.assertEqual(form.errors, {'name': ['This field is required.'], 'artist': ['This field is required.']}) - try: - form.cleaned_data - self.fail('Attempts to access cleaned_data when validation fails should fail.') - except AttributeError: - pass + self.assertEqual(form.cleaned_data, {}) # Now let's show what happens when empty_permitted=True and the form is empty. form = SongForm(data, empty_permitted=True) @@ -1696,11 +1684,7 @@ class FormsTestCase(TestCase): form = SongForm(data, empty_permitted=False) self.assertFalse(form.is_valid()) self.assertEqual(form.errors, {'name': ['This field is required.']}) - try: - form.cleaned_data - self.fail('Attempts to access cleaned_data when validation fails should fail.') - except AttributeError: - pass + self.assertEqual(form.cleaned_data, {'artist': 'The Doors'}) # If a field is not given in the data then None is returned for its data. Lets # make sure that when checking for empty_permitted that None is treated