[Django] #20982: forms.BooleanField field issue
Django
-------------------------------+--------------------
Reporter: george.li | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
in django 1.4 1.5.1
{{{
from django import forms
forms.BooleanField().clean(True)
}}}
this case is success
But
{{{
forms.BooleanField().clean(False)
}}}
{{{
forms.BooleanField().clean('false')
}}}
in this two case are
*** ValidationError: [u'This field is required.']
source code
{{{
621 class BooleanField(Field):
622 widget = CheckboxInput
623
624 def to_python(self, value):
625 ¦ """Returns a Python boolean object."""
626 ¦ # Explicitly check for the string 'False', which is what a
hidden field
627 ¦ # will submit for False. Also check for '0', since this is
what
628 ¦ # RadioSelect will provide. Because bool("True") == bool('1')
== True,
629 ¦ # we don't need to handle that explicitly.
630 ¦ if isinstance(value, six.string_types) and value.lower() in
('false', '0'):
631 ¦ ¦ value = False
632 ¦ else:
633 ¦ ¦ value = bool(value)
634 ¦ value = super(BooleanField, self).to_python(value)
635 ¦ if not value and self.required:
636 ¦ ¦ raise ValidationError(self.error_messages['required'])
637 ¦ return value
}}}
in 630, 'false' and '0' Transform to boolean False
but 635 then decide False is no input value
--
Ticket URL: <https://code.djangoproject.com/ticket/20982>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------+--------------------------------------
Reporter: george.li | Owner: nobody
Type: Uncategorized | Status: closed
Component: Forms | Version: 1.5
Severity: Normal | Resolution: worksforme
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* status: new => closed
* needs_docs: => 0
* resolution: => worksforme
* needs_tests: => 0
* needs_better_patch: => 0
Comment:
If you want the boolean field not to be required, you must create it with
the `required=False` option.
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:1>
Django
---------------------------+--------------------------------------
Reporter: george.li | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by abhillman):
* status: closed => new
* cc: aryeh.hillman@… (added)
* type: Uncategorized => Bug
* version: 1.5 => 1.7
* has_patch: 0 => 1
* resolution: worksforme =>
Old description:
> in django 1.4 1.5.1
>
New description:
in django 1.4 1.5.1
But
{{{
forms.BooleanField().clean(False)
}}}
{{{
forms.BooleanField().clean('false')
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:2>
Django
---------------------------+--------------------------------------
Type: Bug | Status: new
Component: Forms | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by abhillman):
In django 1.7, this is an issue. There may have been some regression. For
example:
{{{
>>> from django import forms
>>> forms.BooleanField().clean(True)
True
>>> forms.BooleanField().clean(False)
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/Library/Python/2.7/site-packages/django/forms/fields.py", line
151, in clean
self.validate(value)
File "/Library/Python/2.7/site-packages/django/forms/fields.py", line
735, in validate
raise ValidationError(self.error_messages['required'],
code='required')
ValidationError: [u'This field is required.']
}}}
The bug is in the validate method on forms.BooleanField.
It's code is:
{{{
def validate(self, value):
if not value and self.required:
raise ValidationError(self.error_messages['required'],
code='required')
}}}
On forms.Field, however, there is a class variable, empty_values that
should be used. That is, the correct implementation would be:
{{{
def validate(self, value):
if value in self.empty_values and self.required:
raise ValidationError(self.error_messages['required'],
code='required')
}}}
Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:3>
Django
---------------------------+--------------------------------------
Type: Bug | Status: new
Component: Forms | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Looking at your example, the field is working as expected. As noted in
comment 1, if you want to accept `False`, you need to use
`required=False`. The
[https://docs.djangoproject.com/en/dev/ref/forms/fields/#booleanfield docs
for BooleanField] describe the behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:4>
Django
---------------------------+--------------------------------------
Type: Bug | Status: closed
Component: Forms | Version: 1.7
Severity: Normal | Resolution: worksforme
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => worksforme
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:5>
Django
---------------------------+--------------------------------------
Type: Bug | Status: closed
Component: Forms | Version: 1.7
Severity: Normal | Resolution: worksforme
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by abhillman):
timgraham. This is not the correct implementation. If you set
required=False, then supplying an empty value will validate as will False.
I understand that there could be a backwards compatibility issue here,
which is fine. But it seems to me that this is almost definitely the wrong
implementation.
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:6>
Django
---------------------------+--------------------------------------
Type: Bug | Status: closed
Component: Forms | Version: 1.7
Severity: Normal | Resolution: worksforme
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by abhillman):
It seems to me that the original thinking on this was the following:
making a boolean field required is good for cases where a user must, say,
check a box to accept terms of service. This seems to me, however, that it
should be handled at the form-level clean or with a custom validator on
the field — not by setting required=False. Another argument is that
BooleanField is one of the only fields that overrides forms.Field's
validate method. Whereas the other fields rely on self.empty_values,
forms.BooleanField is the only field that relies on the provided value's
implicit boolean value. Definitely inconsistent.
But again, I understand completely if this is a backwards compatibility
case. It just is terribly confusing in many cases, especially when wanting
to, say, use forms for validating some JSON data. Say you always want a
boolean field to show up in the data, then, really, the best way to deal
with this situation is to sub-class BooleanField and override its validate
method. Or write a custom validator — but its a bit hacky to circularly
reference the calling class' empty_values from an outside method.
Again, I understand your reasoning to some degree. It IS in fact
documented, if otherwise confusing.
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:7>
Django
---------------------------+--------------------------------------
Type: Bug | Status: closed
Component: Forms | Version: 1.7
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by abhillman):
* resolution: worksforme => needsinfo
--
Ticket URL: <https://code.djangoproject.com/ticket/20982#comment:8>