Commit 23b781cc authored by Aaron France's avatar Aaron France Committed by Baptiste Mispelon
Browse files

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.
parent 2ebccebf

django/contrib/admin/checks.py

+13 −11
Original line number Diff line number Diff line
@@ -3,7 +3,7 @@ from __future__ import unicode_literals 


from itertools import chain



from django.contrib.admin.utils import get_fields_from_path, NotRelationField

from django.contrib.admin.utils import get_fields_from_path, NotRelationField, flatten

from django.core import checks

from django.db import models

from django.db.models.fields import FieldDoesNotExist
@@ -84,7 +84,8 @@ class BaseModelAdminChecks(object): 
                    id='admin.E005',

                )

            ]

        elif len(cls.fields) != len(set(cls.fields)):

        fields = flatten(cls.fields)

        if len(fields) != len(set(fields)):

            return [

                checks.Error(

                    'There are duplicate field(s) in "fields".',
@@ -93,7 +94,7 @@ class BaseModelAdminChecks(object): 
                    id='admin.E006',

                )

            ]

        else:



        return list(chain(*[

                self._check_field_spec(cls, model, field_name, 'fields')

                for field_name in cls.fields
@@ -132,7 +133,9 @@ class BaseModelAdminChecks(object): 
                    id='admin.E011',

                )

            ]

        elif len(fieldset[1]['fields']) != len(set(fieldset[1]['fields'])):



        fields = flatten(fieldset[1]['fields'])

        if len(fields) != len(set(fields)):

            return [

                checks.Error(

                    'There are duplicate field(s) in "%s[1]".' % label,
@@ -141,7 +144,6 @@ class BaseModelAdminChecks(object): 
                    id='admin.E012',

                )

            ]

        else:

        return list(chain(*[

            self._check_field_spec(cls, model, fields, '%s[1][\'fields\']' % label)

            for fields in fieldset[1]['fields']

django/contrib/admin/utils.py

+15 −5
Original line number Diff line number Diff line
@@ -83,15 +83,25 @@ def unquote(s): 
    return "".join(res)





def flatten(fields):

    """Returns a list which is a single level of flattening of the

    original list."""

    flat = []

    for field in fields:

        if isinstance(field, (list, tuple)):

            flat.extend(field)

        else:

            flat.append(field)

    return flat





def flatten_fieldsets(fieldsets):

    """Returns a list of field names from an admin fieldsets structure."""

    field_names = []

    for name, opts in fieldsets:

        for field in opts['fields']:

            if isinstance(field, (list, tuple)):

                field_names.extend(field)

            else:

                field_names.append(field)

        field_names.extend(

            flatten(opts['fields'])

        )

    return field_names

tests/admin_checks/tests.py

+34 −0
Original line number Diff line number Diff line
@@ -463,3 +463,37 @@ class SystemChecksTestCase(TestCase): 
                )

            ]

            self.assertEqual(errors, expected)



    def test_check_sublists_for_duplicates(self):

        class MyModelAdmin(admin.ModelAdmin):

            fields = ['state', ['state']]



        errors = MyModelAdmin.check(model=Song)

        expected = [

            checks.Error(

                'There are duplicate field(s) in "fields".',

                hint=None,

                obj=MyModelAdmin,

                id='admin.E006'

            )

        ]

        self.assertEqual(errors, expected)



    def test_check_fieldset_sublists_for_duplicates(self):

        class MyModelAdmin(admin.ModelAdmin):

            fieldsets = [

                (None, {

                    'fields': ['title', 'album', ('title', 'album')]

                }),

            ]



        errors = MyModelAdmin.check(model=Song)

        expected = [

            checks.Error(

                'There are duplicate field(s) in "fieldsets[0][1]".',

                hint=None,

                obj=MyModelAdmin,

                id='admin.E012'

            )

        ]

        self.assertEqual(errors, expected)

tests/admin_util/tests.py

+13 −2
Original line number Diff line number Diff line
@@ -5,8 +5,8 @@ from datetime import datetime 
from django.conf import settings

from django.contrib import admin

from django.contrib.admin import helpers

from django.contrib.admin.utils import (display_for_field, flatten_fieldsets,

    label_for_field, lookup_field, NestedObjects)

from django.contrib.admin.utils import (display_for_field, flatten,

    flatten_fieldsets, label_for_field, lookup_field, NestedObjects)

from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE

from django.contrib.sites.models import Site

from django.db import models, DEFAULT_DB_ALIAS
@@ -323,6 +323,17 @@ class UtilTests(SimpleTestCase): 
        self.assertHTMLEqual(helpers.AdminField(form, 'cb', is_first=False).label_tag(),

                             '<label for="id_cb" class="vCheckboxLabel required inline">&amp;cb</label>')



    def test_flatten(self):

        flat_all = ['url', 'title', 'content', 'sites']

        inputs = (

            ((), []),

            (('url', 'title', ('content', 'sites')), flat_all),

            (('url', 'title', 'content', 'sites'), flat_all),

            ((('url', 'title'), ('content', 'sites')), flat_all)

        )

        for orig, expected in inputs:

            self.assertEqual(flatten(orig), expected)



    def test_flatten_fieldsets(self):

        """

        Regression test for #18051