Opened 8 years ago

Closed 4 years ago

Last modified 4 years ago

#27445 closed Cleanup/optimization (fixed)

RadioSelect widget does not work for NullBooleanField

Reported by: Anne Fleischer Owned by: David Smith
Component: Documentation Version: dev
Severity: Normal Keywords: NullBooleanField, RadioSelect
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The default widget for NullBooleanFields result in a selectbox with 3 options being shown.
In my opinion it should also be possible to represent those 3 options by radio inputs.
However, simply changing the form widget for a NullBooleanField to be a RadioSelect, will result in no form inputs whatsoever being shown.

Here is the code to reproduce it:

from django.db import models

class MyTestModel(models.Model):
    foo = models.NullBooleanField(blank=True, null=True)
from django import forms

class MyTestModelForm(forms.ModelForm):
    class Meta:
        model = MyTestModel
        fields = ('foo',)
        widgets = {
            'foo': forms.RadioSelect,
        }

I'm not sure if this is a bug or if the bahaviour is intentional.
As a workaround I tried to just initialize the choices manually in the widget, like this:

widgets = {
            'foo': forms.RadioSelect(choices=(('', 'Unknown'), ('1', 'Yes'), ('0', 'No'))),
        }

However, this works in terms of storing the correct value, but it does not render the selected radio input as checked.

I'd like to work in this if someone can confirm that this behaviour is not intended.

Change History (16)

comment:1 by Takis Issaris, 8 years ago

Hi,

I can confirm that specifying the widget removes all input/select tags:

from app27445.forms import MyTestModelForm
str(MyTestModelForm())
<tr><th><label for="id_foo">Foo:</label></th><td><select id="id_foo" name="foo">\n<option value="1" selected>Unknown</option>\n<option value="2">Yes</option>\n<option value="3">No</option>\n</select></td></tr>
from app27445.forms import MyTestModelForm
str(MyTestModelForm())
<tr><th><label for="id_foo_0">Foo:</label></th><td><ul id="id_foo"></ul></td></tr>

But I do not think this is specific to NullBooleanField as I get the same output when using the BooleanField.

comment:2 by Kenneth Veldman, 8 years ago

Owner: changed from nobody to Kenneth Veldman
Status: newassigned

comment:3 by Kenneth Veldman, 8 years ago

Triage Stage: UnreviewedAccepted

Was able to reproduce using the same model and forms. I added a couple of more checks by using multiple fields like so:

class MyTestModel(models.Model):
    foo = models.NullBooleanField(blank=True, null=True)
    bar = models.BooleanField(blank=True)
    baz = models.BooleanField()
    bor = models.NullBooleanField()

All results in an empty form when using the forms.RadioSelect widget.

comment:4 by Jacob Rief, 8 years ago

Owner: changed from Kenneth Veldman to Jacob Rief

comment:5 by Tim Graham, 8 years ago

Not sure how much effort the fix is here, but for what it's worth, there's a proposal that might involve deprecating NullBooleanField (#23130). Completing that ticket might be more fruitful in the long run.

comment:6 by Takis Issaris, 8 years ago

After digging through the code for quite some time, I decided to cheat a little and have another go at search StackOverflow.

This is what I found regarding the saved value not being reflected in the rendered form (in an UpdateView for example).

It was suggested http://stackoverflow.com/a/28262878 that one should use the following choices to make it work:

choices=((None, 'N/A'), (True, 'Yes'), (False, 'No'))

That works for me for True and False, but not for the None value of the NullBooleanField.

On the same thread a user named 'x0nix' suggests this snippet http://stackoverflow.com/a/28906939:

class NullBooleanRadioSelect(RadioSelect):
    def __init__(self, *args, **kwargs):
        choices = (
            (None, ugettext_lazy('Unknown')),
            (True, ugettext_lazy('Yes')),
            (False, ugettext_lazy('No'))
        )
        super(NullBooleanRadioSelect, self).__init__(choices=choices, *args, **kwargs)

    _empty_value = None
...
        widgets = {'foo': NullBooleanRadioSelect()}

With this snippet all three values seem to be rendered correctly.

comment:7 by Takis Issaris, 8 years ago

After looking at the empty_value in the forms.widgets.RadioSelect, I tried using the following for choices:

        choices = (('', 'Unknown'), (True, 'Yes'), (False, 'No'))

With this the issue of the HTML rendering not selecting the saved value is gone, so please ignore my previous post...

comment:8 by Takis Issaris, 8 years ago

Should the need for specifying the choices field as mentioned above be documented?
Or should the behaviour be modified so that the default choices field is set as in the previous post automatically?

comment:9 by Jacob Rief, 8 years ago

While #23130 "BooleanField should not override 'blank' if choices are specified", might make sense on the long term, this report describes a real bug.
I spoke with various people on the sprints, which had their concerns about fixing #23130, so even if at any time NullBooleanField is removed, it does not harm to fix this ticket.

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:10 by Tim Graham, 8 years ago

Has patch: set
Patch needs improvement: set

Repeating my comment from the PR, we're targeting template-based widget rendering (#15667) for 1.11 which conflicts with this patch because things like RendererMixin are removed. I think the widget would just need to be tweaked based on the changes in that branch, but I want to look at ticket #23130 to see if that idea might obsolete this fix so that we don't have to add a widget only to deprecated it later.

I wonder if a documentation suggestion of how to make things work without this widget could suffice for now, which would also help users of older Django versions.

Also, it would be nice if you could summarize the concerns about fixing #23130 that you discussed at the sprint.

comment:11 by David Smith, 4 years ago

Owner: changed from Jacob Rief to David Smith
Patch needs improvement: unset

comment:12 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:13 by David Smith, 4 years ago

Patch needs improvement: unset

comment:14 by Carlton Gibson, 4 years ago

Component: FormsDocumentation
Triage Stage: AcceptedReady for checkin
Type: New featureCleanup/optimization

We're going to resolve this documenting providing widget choices explicitly. PR

comment:15 by GitHub <noreply@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In d976c254:

Fixed #23681, Fixed #27445 -- Doc'd setting choices for NullBooleanField widgets.

Thanks to David Smith for investigation and review.

Co-authored-by: Carlton Gibson <carlton.gibson@…>

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 82bdc51:

[3.1.x] Fixed #23681, Fixed #27445 -- Doc'd setting choices for NullBooleanField widgets.

Thanks to David Smith for investigation and review.

Co-authored-by: Carlton Gibson <carlton.gibson@…>
Backport of d976c254fc76e5f04d81dfd9d142c58e933c9c92 from master

Note: See TracTickets for help on using tickets.
Back to Top