Opened 19 years ago
Closed 17 years ago
#2037 closed enhancement (fixed)
Doesn't make sense to have a blank option in dropdowns for non-nullable, non-blankable fields with defaults
| Reported by: | Joeboy | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.admin | Version: | dev |
| Severity: | minor | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description
It makes sense if null or blank is a valid option, or if there's no default (so the user can't mistakenly skip the field). Otherwise it's a (admittedly minor) blemish on your spangly admin section.
Attachments (2)
Change History (13)
comment:1 by , 19 years ago
comment:2 by , 19 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
Here's one:
from django.db import models
# Create your models here.
class TestModel(models.Model):
""" Simple Test Model. """
CHOICES = (
(0, 'State A'),
(1, 'State B'),
)
emptynotallowed = models.CharField(maxlength=1, choices=CHOICES)
emptyallowed = models.CharField(maxlength=1, choices=CHOICES, null=True, blank=True)
defaultemptynotallowed = models.CharField(maxlength=1, choices=CHOICES, default=2)
defaultemptyallowed = models.CharField(maxlength=1, choices=CHOICES, null=True, blank=True, default=2)
class Admin:
pass
So - only emptynotallowed should have the blank drop-down option
comment:3 by , 18 years ago
From my POV, Field.get_choices() actually adds the BLANK line if the parameter include_blank is True (which is the default) it does not consider self.blank which is the parameter given when defining the field.
The expected behaviour is that if self.blank = False then Field.get_choices must not add the blank line, I will upload a patch following this reasoning in a while.
by , 18 years ago
Patching the behaviour of Field.get_choices() + tests
comment:4 by , 18 years ago
| Has patch: | set |
|---|---|
| Patch needs improvement: | set |
The attached patch does:
- Change the behaviour of Field.get_choices() so it never adds the Blank option if self.blank = False
- Adds Field.get_choices() tests on the choices tests
- Changes tests/modeltests/manipulators/models.py
The issue reported in this ticket also affects ForeignKeys, on [source:django/trunk/tests/modeltests/manipulators/models.py] there's Album which has a ForeignKey to Musician and on the validation below it expects a blank choice for the relation while it's defined null=False so I stripped the blank choice as the patch above will strip it.
Maybe a second test should be added with a ForeignKey set null=True to make sure that there's always a blank choice ;)
(mtredinnick: this is the patch I owe for #4330 (comment 6) hehe).
comment:5 by , 18 years ago
| Patch needs improvement: | unset |
|---|
Ok, get_choices() has to check self.blank and self.null, if any of both is True the blank choice is given.
I also added tests on ForeignKey for the future. That should be "Ready for Check-in" but I'll leave that in hands of a triager ;))
comment:6 by , 18 years ago
| Patch needs improvement: | set |
|---|
The patch looks fine (well, I'm not great fan of inline "and and and or" constructs), but no longer applies cleanly. Got time to clean it up?
comment:7 by , 18 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:8 by , 18 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
Releasing.... should be patched against newforms
comment:9 by , 18 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
Will port it to newforms (as asked at #django-sprint)
comment:10 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
comment:11 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
r6733 accomplished what was being asked for here.
Could you paste a sample model? That'd really save some time in verifying and fixing this.