Skip to content

Fix BooleanField's allow_null behavior #8614

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

Merged
merged 3 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,12 @@ class BooleanField(Field):
}
NULL_VALUES = {'null', 'Null', 'NULL', '', None}

def __init__(self, **kwargs):
if kwargs.get('allow_null', False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allow null false or true? can you elaborate please

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTML checkboxes do not send any value and should be treated
as None by BooleanField if allow_null is True. from a comment of the tests in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If allow_null is true (false if not set) then set those default values, that was the problem,. If not set, It was consideres as false on booleanfield, It needs another widget to have the there values (none, true, false). As nullboolean field addesed that but It has been deprecated, the current booleanfield treats unexistant as false, the fix correcta that as show in the tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for compatibility reason we can consider this PR. just have to carefully handle any breaking or compatibility breaking changes. may be we can just mention that on release notes later.

self.default_empty_html = None
self.initial = None
super().__init__(**kwargs)

def to_internal_value(self, data):
with contextlib.suppress(TypeError):
if data in self.TRUE_VALUES:
Expand Down
33 changes: 31 additions & 2 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ class TestBooleanHTMLInput:
def test_empty_html_checkbox(self):
"""
HTML checkboxes do not send any value, but should be treated
as `False` by BooleanField.
as `False` by BooleanField if allow_null=False.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField()
Expand All @@ -381,7 +381,8 @@ class TestSerializer(serializers.Serializer):
def test_empty_html_checkbox_not_required(self):
"""
HTML checkboxes do not send any value, but should be treated
as `False` by BooleanField, even if the field is required=False.
as `False` by BooleanField when the field is required=False
and allow_null=False.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField(required=False)
Expand All @@ -390,6 +391,34 @@ class TestSerializer(serializers.Serializer):
assert serializer.is_valid()
assert serializer.validated_data == {'archived': False}

def test_empty_html_checkbox_allow_null(self):
"""
HTML checkboxes do not send any value and should be treated
as `None` by BooleanField if allow_null is True.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField(allow_null=True)

serializer = TestSerializer(data=QueryDict(''))
assert serializer.is_valid()
assert serializer.validated_data == {'archived': None}

def test_empty_html_checkbox_allow_null_with_default(self):
"""
BooleanField should respect default if set and still allow
setting null values.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField(allow_null=True, default=True)

serializer = TestSerializer(data=QueryDict(''))
assert serializer.is_valid()
assert serializer.validated_data == {'archived': True}

serializer = TestSerializer(data=QueryDict('archived='))
assert serializer.is_valid()
assert serializer.validated_data == {'archived': None}


class TestHTMLInput:
def test_empty_html_charfield_with_default(self):
Expand Down