Opened 8 years ago

Closed 6 years ago

Last modified 4 years ago

#5563 closed (fixed)

BooleanField should raise an error if null=True

Reported by: shaunc <shaun@…> Owned by: SamBull
Component: Core (Serialization) Version: master
Severity: Keywords:
Cc: shaun@…, david Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

django.db.models.fields.BooleanField.to_python raises an error on "None" input, but this is valid if null=True for the field.

Attachments (2)

booleanfield-to-python-can-be-none.patch (533 bytes) - added by shaunc <shaun@…> 8 years ago.
(patch to fix django.db.models.fields.init.py
boolean_no_null_validation.patch (3.2 KB) - added by SamBull 6 years ago.
patch to raise a validation error for BooleanField(null=True)

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by shaunc <shaun@…>

(patch to fix django.db.models.fields.init.py

comment:1 Changed 8 years ago by shaunc <shaun@…>

  • Cc shaun@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 8 years ago by anonymous

Isn't this what NullBooleanField is for?

comment:3 Changed 8 years ago by shaunc <shaun@…>

  • Has patch unset
  • Summary changed from BooleanField can be None if null=True to BooleanField should raise an error if null=True

um... well, how about that: I never noticed NullBooleanField before. I'd close this, but I'll leave it up to others to decide if the following should be done so others don't go astray:

1) doc for BooleanField should mention that one should use NullBooleanField rather than null=True.
2) passing null=True to BooleanField init should cause an exception to be raised (whose error text says: "use NullBooleanField")

comment:4 Changed 8 years ago by ubernostrum

  • Triage Stage changed from Unreviewed to Design decision needed

There's been a lot of discussion lately about BooleanField and NullBooleanField, and it's debatable whether NullBooleanField is really appropriate for this use case.

comment:5 Changed 8 years ago by brosner

Marked #6229 as a duplicate.

comment:6 Changed 7 years ago by david

  • Cc david added
  • Has patch set

I can confirm, it breaks json deserialization with None values.

== True from the patch is probably useless?

comment:7 Changed 6 years ago by jacob

  • milestone set to 1.1
  • Triage Stage changed from Design decision needed to Accepted

We either need to fix this or raise a validation warning about BooleanField(null=True).

comment:8 Changed 6 years ago by SamBull

  • Owner changed from nobody to SamBull
  • Status changed from new to assigned

Changed 6 years ago by SamBull

patch to raise a validation error for BooleanField(null=True)

comment:9 Changed 6 years ago by SamBull

I've added a new patch. This adds a model validation check that raises an error if null=True is set within a BooleanField.

I also added a test for the new behaviour and updated a serialization test that was failing after the change.

comment:10 Changed 6 years ago by jacob

  • Triage Stage changed from Accepted to Design decision needed

comment:11 Changed 6 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [10456]) Fixed #5563: BooleanField(null=True) now raises a validation warning telling users to use NullBooleanField instead. Thanks, SamBull.

comment:12 Changed 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Note: See TracTickets for help on using tickets.
Back to Top