Opened 11 years ago

Closed 11 years ago

#20699 closed Bug (fixed)

Extra blank choice '-----' when using models.fields.NullBooleanField with custom choices.

Reported by: tomas.krajca@… Owned by: nobody
Component: Database layer (models, ORM) Version: 1.5
Severity: Normal Keywords: NullBooleanField extra blank choice
Cc: tomas.krajca@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When I define my model class Test like below:

NULL_CHOICES = (
    (None, 'Not applicable'),
    (False, 'Incomplete'),
    (True, 'Complete'),
)

class Test(models.Model):
    t = models.NullBooleanField(choices=NULL_CHOICES, default=False)

The default admin/model form renders t with four options, as if the choices were like:
((, '-----'), (None, 'Not applicable'), (False, 'Incomplete'), (True, 'Complete'))

I think this happens because django.db.models.fields.Field.formfield adds the 'blank' choice to the choices and then changes the form field class to forms.TypedChoiceField.

Change History (4)

comment:1 by mindsocket, 11 years ago

Triage Stage: UnreviewedAccepted

I've reviewed this and can confirm it. Some options to consider for fixing this:

  • A change to allow blank=True in the NullBooleanField declaration (currently the constructor overrides it). Then, when blank=True is specified along with choices, the additional choice is not rendered. This feels a bit hacky.
  • Detect this situation (choices being set) in NullBooleanField.formfield and pass better hints to the super call, or deal with it itself
  • Declare this behaviour unsupported/wontfix, possibly document as such
  • Provide another way to relabel the options

comment:2 by tomas.krajca@…, 11 years ago

Cc: tomas.krajca@… added

I like the option number 2 - detect the situation. The NullBooleanField.formfield could be easily fixed like this:

    def formfield(self, **kwargs):
        defaults = {
            'form_class': forms.NullBooleanField,
            'required': not self.blank,
            'label': capfirst(self.verbose_name),
            'help_text': self.help_text}
        if self.choices:
            defaults['choices'] = self.get_choices(include_blank=False)

        defaults.update(kwargs)
        return super(NullBooleanField, self).formfield(**defaults)

This is inspired by BooleanField.formfield, I assume that if the user gives custom choices, he/she takes care of the blank field (it's a NullBooleanField so the Null option should be implied). This looks reasonably simple to me and I am happy to create a pull request if it is accepted. Would this also need tests?

I think customizing the option labels is a reasonably common use case so I would be against not supporting it.

comment:3 by anonymous, 11 years ago

The code snippet is for django 1.5 as a quick patch, I could make a proper patch for the master.

comment:4 by Tim Graham, 11 years ago

Resolution: fixed
Status: newclosed

I believe this is fixed by #20649 (the extra blank choice will no longer appear).

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