#20982 closed Bug (needsinfo)
forms.BooleanField field issue
Reported by: | george.li | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | 1.7 |
Severity: | Normal | Keywords: | |
Cc: | aryeh.hillman@… | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
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
Change History (8)
comment:1 by , 11 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:2 by , 10 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
Has patch: | set |
Resolution: | worksforme |
Status: | closed → new |
Type: | Uncategorized → Bug |
Version: | 1.5 → 1.7 |
comment:3 by , 10 years ago
In django 1.7, this is an issue. There may have been some regression since this ticket was marked worksforme.
>>> 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!
comment:4 by , 10 years ago
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 docs for BooleanField describe the behavior.
comment:5 by , 10 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:6 by , 10 years ago
@timgraham. This is not the correct implementation. If you set required=False, then supplying an empty value will validate properly, which is definitely undesirable. 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. Your thoughts?
comment:7 by , 10 years ago
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.
comment:8 by , 10 years ago
Resolution: | worksforme → needsinfo |
---|
If you want the boolean field not to be required, you must create it with the
required=False
option.