Opened 12 years ago
Closed 12 years ago
#22018 closed Bug (fixed)
Checks error on ModelAdmins with multiple fields in one line using lists
| Reported by: | Julian Wachholz | Owned by: | Julian Wachholz |
|---|---|---|---|
| Component: | contrib.admin | Version: | dev |
| Severity: | Normal | Keywords: | checks |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
The checks framework will raise an error for ModelAdmin classes that use a fields or fieldset attribute containing multiple fields in one line -
although only when using lists, using tuples works as expected.
Example:
class FooAdmin(admin.ModelAdmin):
fields = ['foo', ['bar', 'baz']]
admin.site.register(Foo, FooAdmin)
Traceback:
File "/home/jwa/projects/django17/apps/foos/admin.py", line 15, in <module>
admin.site.register(Foo, FooAdmin)
File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/sites.py", line 103, in register
admin_class.check(model)
File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/options.py", line 145, in check
return cls.checks_class().check(cls, model, **kwargs)
File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/checks.py", line 489, in check
errors = super(ModelAdminChecks, self).check(cls, model=model, **kwargs)
File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/checks.py", line 27, in check
errors.extend(self._check_fields(cls, model))
File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/checks.py", line 87, in _check_fields
elif len(cls.fields) != len(set(cls.fields)):
TypeError: unhashable type: 'list'
So the _check_fields and _check_fieldsets_item checks need to take care of lists.
Change History (4)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
| Easy pickings: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
As discussed on IRC, while these might be two separate issues, it's likely that both would be fixed by flattening the fields attribute before checking for duplicate fields so I think it makes sense to keep this as one single ticket.
I'll also mark this as easy pickings since it shouldn't be too hard to fix.
Thanks.
comment:3 by , 12 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:4 by , 12 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Additionally, these check methods contain a bug whereas they are trying to prevent duplicate fields on a ModelAdmin form. Using
len(set())on nested tuples will not guarantee that there are, in fact, no duplicates. I think this requires fixing as well.This ModelAdmin for example will pass the checks even though it contains duplicate fields:
class FooAdmin(admin.ModelAdmin): fields = ('one', ('one', 'two'))