Opened 4 years ago

Closed 4 years ago

#23547 closed Bug (worksforme)

BooleanField that is required and have value False always will raise ValidationError

Reported by: Lagovas Owned by: nobody
Component: Forms Version: 1.6
Severity: Normal Keywords: Form BooleanField
Cc: Lagovas 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 Tim Graham)

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.

Attachments (2)

0001-fix-BooleanField-in-case-when-field-is-required-and-.patch (918 bytes) - added by Lagovas 4 years ago.
Patch for ticket
0001-fix-BooleanField-ticket-23547-in-case-when-field-is-.patch (2.5 KB) - added by Lagovas 4 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by Tim Graham

Description: modified (diff)
Resolution: duplicate
Status: newclosed

I think your complaint is the same as #23130. If not, could you please reopen with a proposed patch?

Changed 4 years ago by Lagovas

Patch for ticket

comment:2 Changed 4 years ago by Lagovas

Cc: Lagovas added
Has patch: set
Resolution: duplicate
Status: closednew

comment:3 Changed 4 years ago by Tim Graham

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

comment:4 Changed 4 years ago 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.

comment:5 Changed 4 years ago by Tim Graham

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.

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."

comment:6 Changed 4 years ago 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.

comment:7 Changed 4 years ago by Lagovas

Resolution: worksforme
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top