From fb1dd6b13a0e6b1ef64eac88467321d097942cd2 Mon Sep 17 00:00:00 2001 From: Marc Tamlyn Date: Thu, 8 Aug 2013 14:05:55 +0100 Subject: [PATCH] Form.clean() does not need to return cleaned_data. If it does, that will be used as the cleaned_data. The default implementation has been changed to match this change. --- django/forms/forms.py | 7 +++++-- docs/ref/forms/validation.txt | 12 ++---------- docs/releases/1.7.txt | 5 +++++ tests/forms_tests/tests/test_extra.py | 18 ++++++++++++++++++ 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/django/forms/forms.py b/django/forms/forms.py index 9905eee7c72..97ee72e98ed 100644 --- a/django/forms/forms.py +++ b/django/forms/forms.py @@ -297,9 +297,12 @@ class BaseForm(object): def _clean_form(self): try: - self.cleaned_data = self.clean() + cleaned_data = self.clean() except ValidationError as e: self._errors[NON_FIELD_ERRORS] = self.error_class(e.messages) + else: + if cleaned_data is not None: + self.cleaned_data = cleaned_data def _post_clean(self): """ @@ -315,7 +318,7 @@ class BaseForm(object): not be associated with a particular field; it will have a special-case association with the field named '__all__'. """ - return self.cleaned_data + pass def has_changed(self): """ diff --git a/docs/ref/forms/validation.txt b/docs/ref/forms/validation.txt index 87f9741c1bd..c6d2722bf73 100644 --- a/docs/ref/forms/validation.txt +++ b/docs/ref/forms/validation.txt @@ -75,10 +75,8 @@ overridden: any validation that requires access to multiple fields from the form at once. This is where you might put in things to check that if field ``A`` is supplied, field ``B`` must contain a valid email address and the - like. The data that this method returns is the final ``cleaned_data`` - attribute for the form, so don't forget to return the full list of - cleaned data if you override this method (by default, ``Form.clean()`` - just returns ``self.cleaned_data``). + like. This method can return a completely different dictionary if it wishes, + which will be used as the ``cleaned_data``. Note that any errors raised by your ``Form.clean()`` override will not be associated with any field in particular. They go into a special @@ -403,9 +401,6 @@ example:: raise forms.ValidationError("Did not send for 'help' in " "the subject despite CC'ing yourself.") - # Always return the full collection of cleaned data. - return cleaned_data - In this code, if the validation error is raised, the form will display an error message at the top of the form (normally) describing the problem. @@ -443,9 +438,6 @@ sample) looks like this:: del cleaned_data["cc_myself"] del cleaned_data["subject"] - # Always return the full collection of cleaned data. - return cleaned_data - As you can see, this approach requires a bit more effort, not withstanding the extra design effort to create a sensible form display. The details are worth noting, however. Firstly, earlier we mentioned that you might need to check if diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index fbbf93f627b..6bf13834d2c 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -127,6 +127,11 @@ Minor features for each individual field will be respected, and a new ``incomplete`` validation error will be raised when any required fields are empty. +* The :meth:`~django.forms.Form.clean` method on a form no longer needs to + return ``self.cleaned_data``. If it does return a changed dictionary then + that will still be used. The default implementation no longer returns + ``self.cleaned_data``. + Backwards incompatible changes in 1.7 ===================================== diff --git a/tests/forms_tests/tests/test_extra.py b/tests/forms_tests/tests/test_extra.py index afdfaf5281a..ba835495c65 100644 --- a/tests/forms_tests/tests/test_extra.py +++ b/tests/forms_tests/tests/test_extra.py @@ -620,6 +620,24 @@ class FormsExtraTestCase(TestCase, AssertFormErrorsMixin): self.assertTrue(f.is_valid()) self.assertEqual(f.cleaned_data['username'], 'sirrobin') + def test_changing_cleaned_data_in_clean(self): + class UserForm(Form): + username = CharField(max_length=10) + password = CharField(widget=PasswordInput) + + def clean(self): + data = self.cleaned_data + + # Return a different dict. We have not changed self.cleaned_data. + return { + 'username': data['username'].lower(), + 'password': 'this_is_not_a_secret', + } + + f = UserForm({'username': 'SirRobin', 'password': 'blue'}) + self.assertTrue(f.is_valid()) + self.assertEqual(f.cleaned_data['username'], 'sirrobin') + def test_overriding_errorlist(self): @python_2_unicode_compatible class DivErrorList(ErrorList):