Skip to content

Correct allow_null behaviour when required=False #5888

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Mar 20, 2018

Conversation

carltongibson
Copy link
Collaborator

@carltongibson carltongibson commented Mar 20, 2018

This PR reverts #5639. There required=False would override the implicit default coming from allow_null, omitting the field from inclusion. This was incorrect, as reported in #5708.

Closes #5708.

There's a slight BC issue here — although we're restoring an older behaviour. Overriding data to remove the offending field if None would serve as a fallback.

TODO:

  • Release notes for behaviour change here.

@carltongibson carltongibson added this to the 3.8 Release milestone Mar 20, 2018
@carltongibson carltongibson requested a review from rpkilby March 20, 2018 13:00
Ref encode#5708: allow_null should imply default=None, even for non-required fields.

flake8
default is prior to allow_null. allow_null implies an outgoing default=None.
carltongibson added a commit that referenced this pull request Mar 20, 2018
@carltongibson carltongibson merged commit 6c0c69e into encode:master Mar 20, 2018
@carltongibson carltongibson deleted the 38/allow_null branch March 20, 2018 20:24
carltongibson added a commit that referenced this pull request Mar 26, 2018
carltongibson added a commit that referenced this pull request Apr 3, 2018
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
* Revert "Non-required fields with 'allow_null=True' should not imply a default value (encode#5639)"
    This reverts commit 905a557.
    Closes encode#5708

* Add test for allow_null + required=False
    Ref encode#5708: allow_null should imply default=None, even for non-required fields.

* Re-order allow_null and default in field docs
    default is prior to allow_null. allow_null implies an outgoing default=None.

* Adjust allow_null note.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant