Opened 3 years ago

Closed 3 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: 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']], FooAdmin)


  File "/home/jwa/projects/django17/apps/foos/", line 15, in <module>, FooAdmin)
  File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/", line 103, in register
  File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/", 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/", 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/", line 27, in check
    errors.extend(self._check_fields(cls, model))
  File "/home/jwa/.venvs/django17/lib/python3.3/site-packages/django/contrib/admin/", 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 3 years ago by Julian Wachholz

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 3 years ago by Baptiste Mispelon

Easy pickings: set
Triage Stage: UnreviewedAccepted

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 3 years ago by Julian Wachholz

Owner: changed from nobody to Julian Wachholz
Status: newassigned

comment:4 Changed 3 years ago by Baptiste Mispelon <bmispelon@…>

Resolution: fixed
Status: assignedclosed

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