Opened 16 years ago
Closed 16 years ago
#5104 closed (fixed)
CheckboxInput should return False if not found
Reported by: | Chris Beaven | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This is the crux of the problem people are having with BooleanField
. A CheckboxInput
widget will only ever return something if it's checked. So if it isn't in cleaned_data
then it should be assumed False
, not None
.
Attachments (3)
Change History (14)
Changed 16 years ago by
Attachment: | checkboxinput.patch added |
---|
comment:1 Changed 16 years ago by
Has patch: | set |
---|
comment:2 Changed 16 years ago by
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Changed 16 years ago by
Attachment: | checkboxinput.2.patch added |
---|
comment:3 Changed 16 years ago by
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
Summary: | CheckboxInput should only return True or False → CheckboxInput should return False if not found |
There, that's better.
I also changed the documentation to get rid of the "No validation" messages because they are confusing. They really mean "No further validation" because the Field
validation still applies.
comment:4 Changed 16 years ago by
This should be mentioned on http://code.djangoproject.com/wiki/BackwardsIncompatibleChanges because some people could be (probably are) relying on the current behaviour of BooleanField
(with default widget) raising a ValidationError
if the key doesn't exist.
Note: This is still possible to check for, you just have to do it in a clean_*
method.
comment:5 Changed 16 years ago by
Triage Stage: | Unreviewed → Ready for checkin |
---|
comment:6 Changed 16 years ago by
#4480 is a duplicate of this one, closed because we have a good patch here.
comment:8 Changed 16 years ago by
Closing #3130 in favor of getting the BooleanField
/Checkbox
/etc. issues worked out all in one ticket -- this one -- instead of spread out.
comment:9 Changed 16 years ago by
Okay, I like the approach Chris is taking here and I'm going to commit a small variation on this patch (predominantly, including some more documentation clarifications). I'm not going to remove the "validates nothing" lines in the third patch, since explicitly including them makes it clear we haven't just forgotten to list the validation conditions.
I agree that this will make the natural behaviour clearer for people and I think this should put the issue to rest.
I'm not going to change the current default for BooleanField subclasses to make it required=False because it's not possible to make that sort of change and break existing code, so people writing BooleanField()
at the moment will have their validation conditions weakened and might not notice it (we try to make all backwards incompatible changes fatal if you don't notice them). It simply isn't that hard to type required=False
to make that risk necessary. A little annoying for some people; hardly fatal, though.
comment:10 Changed 16 years ago by
comment:11 Changed 16 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
[6563] contained the main code changes, but didn't seem to update this ticket automatically.
Oops, that's not the right patch yet...