NullBooleanField should use forms.NullBooleanField just like django. And BooleanField should support default=True

[pythonwood]

Checklist

• [x] I have verified that that issue exists against the master branch of Django REST framework.
• [x] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• [x] This is not a usage question. (Those should be directed to the discussion group instead.)
• [ ] 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.)
• [x] I have reduced the issue to the simplest possible case.
• [ ] I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

run shell command

django-admin startproject forboolean
cd forboolean
django-admin startapp boolean

boolean/models.py

from django.db import models
from rest_framework import serializers
from rest_framework import viewsets
from rest_framework import permissions

''' not good example but make it small '''

class TestBoolean(models.Model):
    acknowledged = models.BooleanField(null=True, blank=True)
    autoRenewing = models.NullBooleanField()

class TestBooleanSerializer(serializers.ModelSerializer):
    class Meta:
        model = TestBoolean
        fields = '__all__'

class TestBooleanViewSet(viewsets.ModelViewSet):
    queryset = TestBoolean.objects.all()
    serializer_class = TestBooleanSerializer
    permission_classes = [permissions.AllowAny]

add app in forboolean/settings.py

'rest_framework',
'boolean',

modify forboolean/urls.py

from django.urls import path, include

from boolean.models import TestBooleanViewSet
from rest_framework import routers

router = routers.DefaultRouter()
router.register(r'boolean', TestBooleanViewSet)

urlpatterns = [
    path('', include(router.urls)),
]

run shell command

./manage.py makemigrations boolean
./manage migrate
./manage runserver

stdout

......
WARNINGS:
boolean.TestBoolean.autoRenewing: (fields.W903) NullBooleanField is deprecated. Support for it (except in historical migrations) will be removed in Django 4.0.
	HINT: Use BooleanField(null=True) instead.
......

Expected behavior

django doc https://docs.djangoproject.com/en/3.1/ref/models/fields/#booleanfield class BooleanField(**options)¶ A true/false field.

The default form widget for this field is CheckboxInput, or NullBooleanSelect if null=True.

The default value of BooleanField is None when Field.default isn’t defined.

Actual behavior

django rest framework doc

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 Django 2.1 removed the blank kwarg from models.BooleanField. Prior to Django 2.1 models.BooleanField fields were always blank=True. Thus since Django 2.1 default serializers.BooleanField instances will be generated without the required kwarg (i.e. equivalent to required=True) whereas with previous versions of Django, default BooleanField instances will be generated with a required=False option. If you want to control this behaviour manually, explicitly declare the BooleanField on the serializer class, or use the extra_kwargs option to set the required flag.

[tbrknt]

Hello,

Thank you for the steps to reproduce that issue. Due to the title, I'm a bit confused. Which NullBooleanField do you mean?

• The NullBooleanField of DjangoRestFramework is deprecated and it will be removed. Thus, it is not suggested to use. I guess, any further development on that one is not suggested too.
• The NullBooleanField of Django is also deprecated as you shared the warning in your message.

In the current example that you provided, both of the fields are converted to BooleanField of DRF unless they are explicitly stated otherwise.

That is the representation of the serializer:
TestBooleanSerializer():
    id = IntegerField(label='ID', read_only=True)
    acknowledged = BooleanField(allow_null=True, required=False)
    autoRenewing = BooleanField(allow_null=True, label='AutoRenewing', required=False)

Even though both NullBooleanFields are deprecated, do you suggest that if a field is defined in the model as models.BooleanField(null=True) or models.NullBooleanField(), then in the browsable API, the result of the field rendering should be a dropdown like NullBooleanSelect? Is that your suggestion?

Currently, that behavior can be provided with the following change in your sample code:

class TestBoolean(models.Model):
    CHECKLIST_OPTIONS = (
        (None, 'Unknown'),
        (True, 'Yes'),
        (False, 'No'),
    )

    acknowledged = models.BooleanField(null=True, blank=True)
    autoRenewing = models.NullBooleanField()

class TestBooleanSerializer(serializers.ModelSerializer):
    autoRenewing = serializers.NullBooleanField()
    class Meta:
        model = TestBoolean
        fields = '__all__'

So do you think that this should be the default behavior when null=True for a boolean field is provided?

[tomchristie]

So @pythonwood is referring to django.forms.NullBooleanField, not django.db.models.NullBooleanField. (Yes it's easily confused).

And yes, it looks valid to me. I suspect that this broke for use when we moved away from using the deprecated model field, and didn't introduce some behaviour to switch between the correct form field type, depending on if the serializer field is nullable or not.

Blergh.

[tomchristie]

Actually it's less clear to me now if we ever used to render this correctly. We don't use Django's form rendering for the browsable API, instead we have our own templates.

Our rendering for boolean fields is determined here... https://github.com/encode/django-rest-framework/blob/c69e2e4eaafd7270565f0ecab7635f8988bc0f6d/rest_framework/renderers.py#L293-L295

We probably need some explicit override in the render_field method to switch the base_template for BooleanField with null=True ?

[tbrknt]

I understand. So for BooleanField, you suggest having two separate templates for null=True and null=False. What would you think of keeping only one template for BooleanField and change the input element based on the field.allow_null? Would it be not a good idea to combine that logic in one template?

Either way, would it be okay for you if I work on this issue?

[tomchristie]

I don't know. Possibly a bit odd for a template named checkbox.html to sometimes render a checkbox, and sometimes render a select, but... 🤷‍♂️

[tbrknt]

I understand and I agree on that one (: So I will check the following multiple options to change this behavior:

• I saw that model BooleanFields are generated as ChoiceFields if there is a choices parameter on the model field definition. I was thinking to provide a default choices tuple for yes, no, and unknown (as localized) if allow_null=True on serializer BooleanField or null=True on model BooleanField and if there is no choices provided. That way, it can be generated as a ChoiceField and rendered as select. However, there maybe some issues saving the data or loading the existing data. This is one option that I'm checking (: https://github.com/encode/django-rest-framework/blob/f0706190614541fd47aeb7576c2030b58907d68b/rest_framework/serializers.py#L1215-L1218
• The one mentioned by you: To override the template in renderers.py. I think this one could be a clean solution. I just need to check the behavior of choices kwarg on the model field or serializer field if it is provided.
• The one that I mentioned above: To combine two logic in one template but that is not my favorite right now (:

Those are the options that I could come up with. Please let me know if I'm missing anything here. Thank you for your help @tomchristie .

[tomchristie]

Okay, so "I saw that model BooleanFields are generated as ChoiceFields if there is a choices parameter on the model field definition" doesn't actually help here, because that only applies in the case where ModelSerializer is auto-generating a field. We also want things to work correctly when an explicitly listed BooleanField(allow_null=True) is included on the serializer class.

[pythonwood]

I had a little looked into drf source. because form/json data render and parser need to modify both, Maybe it take a lot patch ? by the way, I wonder why drf donot reuse django.forms?

[tbrknt]

I had a little looked into drf source. because form/json data render and parser need to modify both

Which part of the parser needs to be modified? I could not see it.

Here, I was working on the approach of overwriting the rendering template for boolean fields with allow_null=True. I have added some tests and I was planning to add more tests for nested fields. It would be nice to get some early-feedback. That was the desired behavior, right? https://github.com/encode/django-rest-framework/compare/master...tbrknt:fix-null-boolean-rendering-7722

With the following model TestBoolean and model serializer (TestBooleanSerializer), I was able to get the screenshot above.

class TestBoolean(models.Model):
    CHECKLIST_OPTIONS = (
        (None, 'Unknown'),
        (True, 'Yes'),
        (False, 'No'),
    )

    boolean_choices_field = models.BooleanField(null=True, choices=CHECKLIST_OPTIONS)
    nullable_boolean_field_1 = models.BooleanField(null=True)
    nullable_boolean_field_2 = models.BooleanField(null=True)
    nullable_boolean_field_3 = models.BooleanField(null=True)
    null_boolean_field_1 = models.NullBooleanField()
    null_boolean_field_2 = models.NullBooleanField()
    null_boolean_field_3 = models.NullBooleanField()
    regular_boolean_field = models.BooleanField()


class TestBooleanSerializer(serializers.ModelSerializer):
    class Meta:
        model = TestBoolean
        fields = '__all__'

class AnotherSerializer(serializers.Serializer):
    nullable_boolean_field_1 = serializers.BooleanField(allow_null=True)
    nullable_boolean_field_2 = serializers.BooleanField(allow_null=True)
    nullable_boolean_field_3 = serializers.BooleanField(allow_null=True)
    null_boolean_field_1 = serializers.NullBooleanField()
    null_boolean_field_2 = serializers.NullBooleanField()
    null_boolean_field_3 = serializers.NullBooleanField()
    regular_boolean_field = serializers.BooleanField()

With this Serializer, AnotherSerializer, I was able to see the HTML form on the second screenshot. What do you think?

[pythonwood]

有了这个Serializer，AnotherSerializer我能看到的第二个屏幕的HTML表单。你怎么认为？

Yeah, you do it. It is good enought for my case (It take "Unknow" became null is right, thanks).

Hope to good case in nest mode and hope to merge soon.

[HeyHugo]

For anyone having trouble with parsing querystring params in regards to BooleanField(allow_null=True) if parameter is omitted it's set to False and not None (Using version 3.12.4). To get None behaviour you need to also specify default=None: BooleanField(allow_null=True, default=None)

from django.http import QueryDict
from rest_framework import serializers


class FooSerializer(serializers.Serializer):
    foo = serializers.BooleanField(allow_null=True)


s = FooSerializer(data=QueryDict())
s.is_valid()
print(s.validated_data.get("foo"))

>> False

[merwok]

you need to also specify default=True: BooleanField(allow_null=True, default=None)

Sorry, is it True or None?

[HeyHugo]

@merwok Ah yes that's right, I meant None. I made an edit now.

[stale[bot]]

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.

[merwok]

Keep open

[stale[bot]]

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.

[merwok]

Keep 🤖

[stale[bot]]

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.

[merwok]

Should be fixed

[auvipy]

Should be fixed

can you try and review the related PR of this issue?