#22018 closed Bug (fixed)

Checks error on ModelAdmins with multiple fields in one line using lists

Reported by: jwa Owned by: jwa
Component: contrib.admin Version: master
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


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.


class FooAdmin(admin.ModelAdmin):
    fields = ['foo', ['bar', 'baz']]

admin.site.register(Foo, FooAdmin)


  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
  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 Changed 22 months ago by jwa

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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'))

comment:2 Changed 22 months ago by bmispelon

  • Easy pickings set
  • Triage Stage changed from Unreviewed to 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.


comment:3 Changed 22 months ago by jwa

  • Owner changed from nobody to jwa
  • Status changed from new to assigned

comment:4 Changed 22 months ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 23b781cc3d17f12c5158f781b2c8cd9d47550c20:

Fixed #22018 -- Fixed checks for ModelAdmin.fields not handling sub-lists.

Flatten a level of sublists before checking for duplicate fields.

When given sublists such as:


class FooAdmin(admin.ModelAdmin):

fields = ('one', ('one', 'two'))


The previous code did not correctly detect the duplicated 'one' field.

Thanks to jwa for the report.

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