Opened 11 years ago

Closed 11 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 Julian Wachholz, 11 years ago

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

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.

Thanks.

comment:3 by Julian Wachholz, 11 years ago

Owner: changed from nobody to Julian Wachholz
Status: newassigned

comment:4 by Baptiste Mispelon <bmispelon@…>, 11 years ago

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:

`python

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