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: | 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 , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 11 years ago
Cc: | 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 , 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 , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I believe this is fixed by #20649 (the extra blank choice will no longer appear).
I've reviewed this and can confirm it. Some options to consider for fixing this: