Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docs note re generated BooleanField being required=False #5665

Merged
merged 4 commits into from Dec 14, 2017

Conversation

carltongibson
Copy link
Collaborator

Closes #5664

  1. Explains/explicitly states that BooleanField is generated with required=False, because models.BooleanField is always blank=True. This has come up a few times so should (help to) clear up confusions.
  2. 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.
  3. Finally adds same link to .serializer_field_mapping docs, since an example (especially needing to use deepcopy) may be handy there too.

@carltongibson carltongibson added this to the v3.7.4 milestone Dec 13, 2017
@@ -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].)
Copy link
Member

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

Copy link
Collaborator Author

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
Copy link
Member

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.

Copy link
Collaborator Author

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.

@carltongibson
Copy link
Collaborator Author

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.

@rpkilby
Copy link
Member

rpkilby commented Dec 13, 2017

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

@carltongibson
Copy link
Collaborator Author

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

@carltongibson carltongibson merged commit 2359d39 into encode:master Dec 14, 2017
@carltongibson carltongibson deleted the 374/boolean-field-docs branch December 14, 2017 10:40
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
…e#5665)

* Note that BooleanField default is required=False

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

Successfully merging this pull request may close these issues.

None yet

3 participants