[Django] #23547: BooleanField that is required and have value False always will raise ValidationError
Django
ValidationError
----------------------------+-------------------------------
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.6
Severity: Normal | Keywords: Form BooleanField
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+-------------------------------
class BooleanField(Field):
def to_python(self, value):
if isinstance(value, six.string_types) and value.lower() in
('false', '0'):
value = False
else:
value = bool(value)
return super(BooleanField, self).to_python(value)
def validate(self, value):
if not value and self.required:
raise ValidationError(self.error_messages['required'],
code='required')
In method "validate" value is "True" or "False" as python object, because
called after to_python.
So if value is valid ('0' or 'false') and field is required, will be
raised ValidationError.
Anyway value in "validate" will be False if value is not '0' or 'false',
so method validate should be empty because always is valid.
--
Ticket URL: <https://code.djangoproject.com/ticket/23547>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
ValidationError
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.6
Severity: Normal | Resolution: duplicate
Keywords: Form BooleanField | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
* resolution: => duplicate
Old description:
> class BooleanField(Field):
> def to_python(self, value):
> if isinstance(value, six.string_types) and value.lower() in
> ('false', '0'):
> value = False
> else:
> value = bool(value)
> return super(BooleanField, self).to_python(value)
>
> def validate(self, value):
> if not value and self.required:
> raise ValidationError(self.error_messages['required'],
> code='required')
> In method "validate" value is "True" or "False" as python object, because
> called after to_python.
> So if value is valid ('0' or 'false') and field is required, will be
> raised ValidationError.
> Anyway value in "validate" will be False if value is not '0' or 'false',
> so method validate should be empty because always is valid.
New description:
{{{
class BooleanField(Field):
def to_python(self, value):
if isinstance(value, six.string_types) and value.lower() in
('false', '0'):
value = False
else:
value = bool(value)
return super(BooleanField, self).to_python(value)
def validate(self, value):
if not value and self.required:
raise ValidationError(self.error_messages['required'],
code='required')
}}}
In method "validate" value is "True" or "False" as python object, because
called after to_python.
So if value is valid ('0' or 'false') and field is required, will be
raised ValidationError.
Anyway value in "validate" will be False if value is not '0' or 'false',
so method validate should be empty because always is valid.
--
Comment:
I think your complaint is the same as #23130. If not, could you please
reopen with a proposed patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/23547#comment:1>
Django
ValidationError
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.6
Keywords: Form BooleanField | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Lagovas):
* cc: Lagovas (added)
* status: closed => new
* has_patch: 0 => 1
* resolution: duplicate =>
--
Ticket URL: <https://code.djangoproject.com/ticket/23547#comment:2>
Django
ValidationError
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.6
Keywords: Form BooleanField | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
The tests do not pass with your proposed change:
{{{
======================================================================
FAIL: test_booleanfield
(forms_tests.tests.test_error_messages.FormsErrorMessagesTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File
"/home/tim/code/django/tests/forms_tests/tests/test_error_messages.py",
line 169, in test_booleanfield
self.assertFormErrors(['REQUIRED'], f.clean, '')
File
"/home/tim/code/django/tests/forms_tests/tests/test_error_messages.py",
line 23, in assertFormErrors
self.fail("Testing the 'clean' method on %s failed to raise a
ValidationError.")
AssertionError: Testing the 'clean' method on %s failed to raise a
ValidationError.
======================================================================
FAIL: test_booleanfield_1 (forms_tests.tests.test_fields.FieldsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tim/code/django/tests/forms_tests/tests/test_fields.py",
line 891, in test_booleanfield_1
self.assertRaisesMessage(ValidationError, "'This field is required.'",
f.clean, '')
File "/home/tim/code/django/django/test/testcases.py", line 579, in
assertRaisesMessage
re.escape(expected_message), callable_obj, *args, **kwargs)
File "/home/tim/code/django/django/utils/six.py", line 690, in
assertRaisesRegex
return getattr(self, _assertRaisesRegex)(*args, **kwargs)
AssertionError: ValidationError not raised
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23547#comment:3>
Django
ValidationError
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.6
Keywords: Form BooleanField | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by Lagovas):
Added new patch. For correct way, in "to_python" will saving raw_value and
then in "validate" check that it value not in empty_values if field is
required. And removed 2 tests, that expect ValidationError with correct
false value.
--
Ticket URL: <https://code.djangoproject.com/ticket/23547#comment:4>
Django
ValidationError
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.6
Keywords: Form BooleanField | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Your proposed change is backwards incompatible. You can't just call the
existing behavior a bug and remove tests that don't conform to how you
expect things to work.
[https://docs.djangoproject.com/en/dev/ref/forms/fields/#booleanfield As
documented]: "Since all `Field` subclasses have `required=True` by
default, the validation condition here is important. If you want to
include a boolean in your form that can be either `True` or `False` (e.g.
a checked or unchecked checkbox), you must remember to pass in
`required=False` when creating the `BooleanField`."
--
Ticket URL: <https://code.djangoproject.com/ticket/23547#comment:5>
Django
ValidationError
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.6
Keywords: Form BooleanField | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by Lagovas):
Sorry. Revised documentation and saw note about this (after error first of
all went to code to see from what and why raises and code behavior was
nonobvious. But in my opinion, it's not good that field can't be required
and return all valid values. Ticket can be closed i guess.
--
Ticket URL: <https://code.djangoproject.com/ticket/23547#comment:6>
Django
ValidationError
Reporter: Lagovas | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 1.6
Severity: Normal | Resolution: worksforme
Keywords: Form BooleanField | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by Lagovas):
* status: new => closed
* resolution: => worksforme
--
Ticket URL: <https://code.djangoproject.com/ticket/23547#comment:7>