[Django] #27821: Documentation on form validation is wrong
Django
---------------------------------------------+------------------------
Reporter: Christian Ullrich | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
The documentation on form validation (/ref/forms/validation/) has this to
say regarding the clean_<fieldname> method on a Form subclass:
This method should return the cleaned value obtained from
cleaned_data, regardless of whether it changed anything or not.
This is incorrect; the method should return the value to be used for
further processing, because its result is unconditionally put ''into''
cleaned_data:
(from django.forms.forms.Form._clean_fields():)
{{{
if hasattr(self, 'clean_%s' % name):
value = getattr(self, 'clean_%s' % name)()
self.cleaned_data[name] = value
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27821>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: Christian Ullrich | Owner: nobody
Component: Documentation | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* has_patch: 0 => 1
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
I see how the current wording could be interpreted incorrectly. What do
you think of this proposal:
{{{ #!diff
diff --git a/docs/ref/forms/validation.txt b/docs/ref/forms/validation.txt
index 6d3edf9..622fb68 100644
--- a/docs/ref/forms/validation.txt
+++ b/docs/ref/forms/validation.txt
@@ -69,8 +69,9 @@ overridden:
formfield-specific piece of validation and, possibly,
cleaning/normalizing the data.
- This method should return the cleaned value obtained from
``cleaned_data``,
- regardless of whether it changed anything or not.
+ The return value of this method is inserted into the form's
``cleaned_data``,
+ so it must be the field's value from ``cleaned_data`` (even if you
didn't
+ change it) or a new cleaned value.
* The form subclass's ``clean()`` method can perform validation that
requires
access to multiple form fields. This is where you might put in checks
such as
@@ -315,8 +316,8 @@ write a cleaning method that operates on the
``recipients`` field, like so::
if "fr...@example.com" not in data:
raise forms.ValidationError("You have forgotten about
Fred!")
- # Always return the cleaned data, whether you have changed it
or
- # not.
+ # Always return a value to use as the new cleaned data, even
if
+ # this method didn't change it.
return data
.. _validating-fields-with-clean:
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27821#comment:1>
Django
--------------------------------------+------------------------------------
Component: Documentation | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Christian Ullrich):
"The return value of this method replaces the existing value in
cleaned_data, so [...]", perhaps? Otherwise it looks good to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/27821#comment:2>
Django
--------------------------------------+------------------------------------
Component: Documentation | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"8863c475c53f2b44113f25b749a124a5bf3a02f2" 8863c475]:
{{{
#!CommitTicketReference repository=""
revision="8863c475c53f2b44113f25b749a124a5bf3a02f2"
Fixed #27821 -- Clarified docs of the return value of
Form.clean_<fieldname>().
Thanks Christian Ullrich for the report and review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27821#comment:3>
Django
--------------------------------------+------------------------------------
Component: Documentation | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"638bab2cabceb76b03798fd15f4f2905c9fdf94c" 638bab2c]:
{{{
#!CommitTicketReference repository=""
revision="638bab2cabceb76b03798fd15f4f2905c9fdf94c"
[1.11.x] Fixed #27821 -- Clarified docs of the return value of
Form.clean_<fieldname>().
Thanks Christian Ullrich for the report and review.
Backport of 8863c475c53f2b44113f25b749a124a5bf3a02f2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27821#comment:4>
Django
--------------------------------------+------------------------------------
Component: Documentation | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"f993872e786c921da13de56cd50eefd61ac3f4d0" f993872]:
{{{
#!CommitTicketReference repository=""
revision="f993872e786c921da13de56cd50eefd61ac3f4d0"
[1.10.x] Fixed #27821 -- Clarified docs of the return value of
Form.clean_<fieldname>().
Thanks Christian Ullrich for the report and review.
Backport of 8863c475c53f2b44113f25b749a124a5bf3a02f2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27821#comment:5>