[Django] #22018: Checks error on ModelAdmins with multiple fields in one line using lists

[Django]

#22018: Checks error on ModelAdmins with multiple fields in one line using lists
-------------------------------+--------------------
Reporter: jwa | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords: checks
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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.

--
Ticket URL: <https://code.djangoproject.com/ticket/22018>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

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

-------------------------------+--------------------------------------

Reporter: jwa | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master

Severity: Normal | Resolution:

Keywords: checks | Triage Stage: Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------+--------------------------------------
Changes (by jwa):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0

Comment:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/22018#comment:1>

[Django]

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

-------------------------------+------------------------------------

Reporter: jwa | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: checks | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by bmispelon):

* easy: 0 => 1
* stage: Unreviewed => Accepted

Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/22018#comment:2>

[Django]

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

-------------------------------+------------------------------------
Reporter: jwa | Owner: jwa
Type: Bug | Status: assigned
Component: contrib.admin | Version: master

Severity: Normal | Resolution:
Keywords: checks | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------

Changes (by jwa):

* owner: nobody => jwa
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/22018#comment:3>

[Django]

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

-------------------------------+------------------------------------
Reporter: jwa | Owner: jwa

Type: Bug | Status: closed
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed

Keywords: checks | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------

Changes (by Baptiste Mispelon <bmispelon@…>):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"23b781cc3d17f12c5158f781b2c8cd9d47550c20"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22018#comment:4>