Skip to content

Conversation

mx-moth
Copy link

@mx-moth mx-moth commented Jan 21, 2020

Not-null Boolean columns fail this requirement, for example.
<input type="checkbox" required> will not allow the page to be submitted
until it is checked, despite an unchecked/falsey checkbox being valid
data at the model level.

@mx-moth mx-moth mentioned this pull request Jan 21, 2020
@mx-moth mx-moth force-pushed the not-nullable-required branch from ddf74b7 to baa0442 Compare January 22, 2020 00:10
@mlenzen
Copy link
Owner

mlenzen commented Jan 25, 2020

I agree that this isn't clear or obvious. My concern with this is that it breaks backwards compatibility. I know I've used this in the past for checkboxes that have to be checked, e.g. "I accept the terms and conditions". Is there another way to express this?

@mx-moth
Copy link
Author

mx-moth commented Jan 25, 2020

It does break backwards compatibility, but I would argue that it breaks it towards being more correct. Required in wtforms is taken to mean True in the context of a checkbox, while not-nullable still means False is a valid value, so the current behaviour does not match what is expected or desired most of the time when using a boolean model field.

The current behaviour is difficult to work with, as the addition of the required/optional validator is done in a section of code that is very difficult to override and modify the behaviour. If you were interested in keeping backwards compatibility, the _nullable_required() call could also be added to the conv_Boolean() method. Developers after behaviour that makes sense could then override either conv_Boolean() to not add the validator, or override _nullable_required() to check the field type before adding the validator.

Not-null Boolean columns fail this requirement, for example.
<input type="checkbox" required> will not allow the page to be submitted
until it is checked, despite an unchecked/falsey checkbox being valid
data at the model level.
@mx-moth mx-moth force-pushed the not-nullable-required branch from baa0442 to 3315884 Compare January 31, 2020 00:58
@mlenzen
Copy link
Owner

mlenzen commented Feb 6, 2020

Yeah, I just had a brain fart. I used the Required validator when I wanted the checkbox to have to be checked. A non-nullable Boolean column should be allowed to be false.

@mlenzen mlenzen merged commit 2f879b2 into mlenzen:master Feb 6, 2020
mlenzen added a commit that referenced this pull request Feb 6, 2020
@mx-moth
Copy link
Author

mx-moth commented Feb 6, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants