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

Incorrect Serializer behaviour for BooleanField(required=False) when passed a QueryDict #8300

Open
2 of 3 tasks
Florian-Projects opened this issue Dec 16, 2021 · 16 comments
Open
2 of 3 tasks
Labels

Comments

@Florian-Projects
Copy link

Florian-Projects commented Dec 16, 2021

Checklist

  • Raised initially as discussion #...
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.

The Documentation states the following about the required keyword argument:
"Normally an error will be raised if a field is not supplied during deserialization. Set to false if this field is not required to be present during deserialization.

Setting this to False also allows the object attribute or dictionary key to be omitted from output when serializing the instance. If the key is not present it will simply not be included in the output representation. "

However this doesn't work correctly with django.http.QueryDict. When passing a QueryDict to the Serializer, the not required fields are not omitted in the output but instead are set to False.

Affected Version:

django_rest_framework <= 3.12
found when upgrading from 3.11 to latest version

A Code Example

from django.http import QueryDict
from rest_framework import serializers

data_dict = {'required': True}
data_querydict = QueryDict('required=True')


class TestSerializer(serializers.Serializer):
    required = serializers.BooleanField(required=True)
    param = serializers.BooleanField(required=False)


serializer1 = TestSerializer(data=data_dict)
serializer1.is_valid()

serializer2 = TestSerializer(data=data_querydict)
serializer2.is_valid()

print(f"Serialized Dict: {serializer1.validated_data}")
print(f"Serialized QueryDict: {serializer2.validated_data}")

if serializer1.validated_data != serializer2.validated_data:
    print("There is a Bug")

When executing this Example you will see that:
serializer1.validated_data == OrderedDict([('required', True)]) while serializer2.validated_data == OrderedDict([('required', True), ('param', False)]).

This diverging behaviour is not present in other types of Field (Tested myself: CharField, IntegerField)
This is potentially an API breaking change

Possible Workaround

As Described here

from rest_framework import fields, serializers


class DefaultEmptyBooleanField(fields.BooleanField):
    default_empty_html = fields.empty


class MySerializer(serializers.Serializer):
    boolean_field = DefaultEmptyBooleanField(required=False)

Expected Behavior

not required fields are omitted in the output for both Dict and QueryDict

@sspross
Copy link

sspross commented Feb 1, 2022

Oh we have a same problem with NullBooleanFields:

class TestSerializer(serializers.Serializer):
    test = serializers.NullBooleanField(required=False)

bla = TestSerializer(data=QueryDict())
bla.is_valid()
bla.validated_data

OrderedDict([('test', False)])

Quick fix for us atm TestSerializer(data=self.request.query_params.dict())

@sudosubin
Copy link
Contributor

You can also use partial option for temporary purposes.

serializer_partial = TestSerializer(data=data_querydict, partial=True)
serializer_partial.is_valid()
serializer_partial.validated_data

And the BooleanField's test cases show the reason: for HTML checkboxes.

class TestBooleanHTMLInput:
def test_empty_html_checkbox(self):
"""
HTML checkboxes do not send any value, but should be treated
as `False` by BooleanField.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField()
serializer = TestSerializer(data=QueryDict(''))
assert serializer.is_valid()
assert serializer.validated_data == {'archived': False}
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.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField(required=False)
serializer = TestSerializer(data=QueryDict(''))
assert serializer.is_valid()
assert serializer.validated_data == {'archived': False}

Doc: https://www.django-rest-framework.org/api-guide/serializers/#partial-updates

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@grazzolini
Copy link

I have this exact same issue, but with a URLField. In my scenario, the field is not present on the data, but given it has allow_null=True, allow_blank=True and also required=False, the field, even if not present on the initial JSON data, is set to None on the serialized data, after is_valid() is called.

@stale stale bot removed the stale label Apr 21, 2022
@grazzolini
Copy link

I think a few more checks should be done here: https://github.com/encode/django-rest-framework/blob/master/rest_framework/fields.py#L440-L447. I'll try to put a PR up to fix this.

@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 13, 2022
@grazzolini
Copy link

This is not stale, this bug is still present on DRF.

@TonyMistark
Copy link

TonyMistark commented Oct 27, 2022

from rest_framework.serializers import Serializer
from rest_framework.serializers import BooleanField
from django.http import QueryDict
class TestSerializer(Serializer):
    bool_field_1 = BooleanField(required=False)
    bool_field_2 = BooleanField(required=False, default=False)
s = TestSerializer(data=QueryDict(), partial=True)
s.is_valid(raise_exception=True)
s.validated_data
# output
# OrderedDict()

s = TestSerializer(data={}, partial=True)
s.is_valid(raise_exception=True)
s.validated_data
# output
# OrderedDict()

bool_field_2 = BooleanField(required=False, default=False)
bool_field_2 had set default value
so

Expected output should be:

OrderedDict([('bool_field_2', False)])

send partial=True, It will not solve the problem completely, but will create new problems

@TonyMistark
Copy link

TonyMistark commented Oct 27, 2022

You can also use partial option for temporary purposes.

serializer_partial = TestSerializer(data=data_querydict, partial=True)
serializer_partial.is_valid()
serializer_partial.validated_data

And the BooleanField's test cases show the reason: for HTML checkboxes.

class TestBooleanHTMLInput:
def test_empty_html_checkbox(self):
"""
HTML checkboxes do not send any value, but should be treated
as `False` by BooleanField.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField()
serializer = TestSerializer(data=QueryDict(''))
assert serializer.is_valid()
assert serializer.validated_data == {'archived': False}
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.
"""
class TestSerializer(serializers.Serializer):
archived = serializers.BooleanField(required=False)
serializer = TestSerializer(data=QueryDict(''))
assert serializer.is_valid()
assert serializer.validated_data == {'archived': False}

Doc: https://www.django-rest-framework.org/api-guide/serializers/#partial-updates

from rest_framework.serializers import Serializer
from rest_framework.serializers import BooleanField
from django.http import QueryDict
class TestSerializer(Serializer):
    bool_field_1 = BooleanField(required=False)
    bool_field_2 = BooleanField(required=False, default=False)
s = TestSerializer(data=QueryDict(), partial=True)
s.is_valid(raise_exception=True)
s.validated_data
# output
# OrderedDict()

s = TestSerializer(data={}, partial=True)
s.is_valid(raise_exception=True)
s.validated_data
# output
# OrderedDict()

bool_field_2 = BooleanField(required=False, default=False)
bool_field_2 had set default value
so

Expected output should be:

OrderedDict([('bool_field_2', False)])

send partial=True, It will not solve the problem completely, but will create new problems

@TonyMistark
Copy link

TonyMistark commented Oct 28, 2022

Oh we have a same problem with NullBooleanFields:

class TestSerializer(serializers.Serializer):
    test = serializers.NullBooleanField(required=False)

bla = TestSerializer(data=QueryDict())
bla.is_valid()
bla.validated_data

OrderedDict([('test', False)])

Quick fix for us atm TestSerializer(data=self.request.query_params.dict())

NullBooleanField resolved my problom

Both types of empty dictionary input give the same result

from rest_framework.serializers import Serializer
from rest_framework.serializers import NullBooleanField
from django.http import QueryDict

class TestSerializer(Serializer):
    null_bool = NullBooleanField(required=False)
s = TestSerializer(data={})
s.is_valid(raise_exception=True)
print("input {}: ", s.validated_data)
# output
# OrderedDict()

s = TestSerializer(data=QueryDict())
s.is_valid(raise_exception=True)
print("input QueryDict: ", s.validated_data)
# output
# OrderedDict()
input {}:  OrderedDict()
input QueryDict:  OrderedDict()

SET the default value None situation

from rest_framework.serializers import Serializer
from rest_framework.serializers import NullBooleanField
from django.http import QueryDict

class TestSerializer(Serializer):
    # SET the default value None
    null_bool = NullBooleanField(required=False, default=None)
s = TestSerializer(data={})
s.is_valid(raise_exception=True)
print("input {}: ", s.validated_data)
# output
# OrderedDict()

s = TestSerializer(data=QueryDict())
s.is_valid(raise_exception=True)
print("input QueryDict: ", s.validated_data)
# output
# OrderedDict()
input {}:  OrderedDict([('null_bool', None)])
input QueryDict:  OrderedDict([('null_bool', None)])

@auvipy
Copy link
Member

auvipy commented Dec 4, 2022

I would love your inputs and trying this PR #8614. also please contribute some suggested test cases to the test suite of this project.

@auvipy auvipy added the Bug label Dec 4, 2022
@tanushree05
Copy link

I want to contribute to this issue please give me permission

@merwok
Copy link
Contributor

merwok commented Jan 17, 2023

I don’t think you need permission, you can report if the changes in #8614 fix the problems and if not submit a pull request!

@stale
Copy link

stale bot commented Apr 2, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 2, 2023
@trevnocap
Copy link

This issue is not stale, and still needs resolved

from rest_framework.serializers import Serializer
from rest_framework.views import APIView

class TestAPI(APIView):

class InputSerializer(serializers.Serializer):
    id = serializers.UUIDField()
    required_field1 = serializers.BooleanField(required=True)
    required_field2 = serializers.BooleanField(required=True)

def put(self, request):
    serializer = self.InputSerializer(data= request.data)
    print(serializer.initial_data) 
    serializer.is_valid(raise_exception= True)
    print(serializer.validated_data)

request.data is <QueryDict: {'id: ['uuid_value'], 'required_field1': ['true']}>

    serializer.validated_data prints: OrderedDict([('division_setting_id', UUID('uuid_value')), ('require_field1', True), **('required_fields2', False)])**

@stale stale bot removed the stale label Dec 8, 2023
@ArtemIsmagilov
Copy link

ArtemIsmagilov commented Jan 28, 2024

In the web api, I want to add the first_id and second_id parameters, but these parameters are not required in the web form, although in the model from which the serializer was created, these ForeigenKey parameters are required.

I think it's wrong to close the problem just because of the activity.

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

No branches or pull requests

10 participants