Add docs note re generated BooleanField being required=False by carltongibson · Pull Request #5665 · encode/django-rest-framework (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation7 Commits4 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Closes #5664
- Explains/explicitly states that BooleanField is generated with
required=False, becausemodels.BooleanFieldis alwaysblank=True. This has come up a few times so should (help to) clear up confusions. - Additionally adds link to a worked example configuring serialiser generation to use a BooleanField subclass with required=True. This should be helpful, as it's not hard but it's not trivial either.
- Finally adds same link to
.serializer_field_mappingdocs, since an example (especially needing to usedeepcopy) may be handy there too.
| @@ -124,6 +124,8 @@ A boolean representation. |
|---|
| When using HTML encoded form input be aware that omitting a value will always be treated as setting a field to `False`, even if it has a `default=True` option specified. This is because HTML checkbox inputs represent the unchecked state by omitting the value, so REST framework treats omission as if it is an empty checkbox input. |
| Note that default `BooleanField` instances will be generated with a `required=False` option (since Django `models.BooleanField` is always `blank=True`). For JSON-only APIs you may wish explicitly declare BooleanFields in order to control this. ([An example configuring serialiser generation to use a `BooleanField` subclass with `required=True` may be found here][required-by-default-boolean-field].) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "If you want to change this behaviour you should explicitly declare the BooleanField on the serializer class. (For example, JSON-only APIs where you do not want to allow empty values by default against boolean fields.)"
- Let's drop the "an example configuring..."
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will re-word now. You want to drop the link to the example entirely?
| @@ -1181,3 +1181,5 @@ The [drf-writable-nested][drf-writable-nested] package provides writable nested |
|---|
| [drf-serializer-extensions]: https://github.com/evenicoulddoit/django-rest-framework-serializer-extensions |
| [djangorestframework-queryfields]: http://djangorestframework-queryfields.readthedocs.io/ |
| [drf-writable-nested]: http://github.com/beda-software/drf-writable-nested |
| [required-by-default-boolean-field]: http://notes.noumenal.es/2017/12/13/required-by-default.html |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather see us link to this kind of thing in advanced sections of documentation. Not likely to be the right thing to point users at in this particular case.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here "Customizing field mappings" is already advanced no?
"Normally if a ModelSerializer does not generate the fields you need by default then you should either add them to the class explicitly, or simply use a regular Serializer class instead. However..." — Just above
It's the sort of thing people struggle to do.
OK, I reworded the comment.
I removed the reference to the JSON-api stuff, since it didn't add much. (With the "...by default..." bit I am inclined to say that customising the field mapping is the way to go; it's one-off changes vs every single time.)
PR now looks like just Step 1 above. It would be nice to add 2 & 3 if we can, since we see users with issues in these areas.
Just a quick thought... we could use a select widget for boolean fields in the browsable API. Having an empty and false option would bypass the ambiguity that exists with checkboxes.
I don't have the time to whip up a PR right now, but I could revisit it later in a PR for 3.8
@rpkilby I think there's a slightly different issue there: regardless of the UI we present models.BooleanField is always blank=True, so (for us) not required when being generated. (But yes, UI improvements FTW! 🙂)
Let's have this as-is. I can't see a place for the more How-To stuff; we'll just have to trust in google as we've done. It'd be widening the scope of the docs considerably to add too much, and we don't have that capacity.
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request