From 23b781cc3d17f12c5158f781b2c8cd9d47550c20 Mon Sep 17 00:00:00 2001
From: Aaron France <aaron.l.france@gmail.com>
Date: Sat, 15 Feb 2014 11:28:09 +0100
Subject: [PATCH] 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.
---
django/contrib/admin/checks.py | 24 +++++++++++++-----------
django/contrib/admin/utils.py | 20 +++++++++++++++-----
tests/admin_checks/tests.py | 34 ++++++++++++++++++++++++++++++++++
tests/admin_util/tests.py | 15 +++++++++++++--
4 files changed, 75 insertions(+), 18 deletions(-)
diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py
index 855e916eaa..4238297b22 100644
--- a/django/contrib/admin/checks.py
+++ b/django/contrib/admin/checks.py
@@ -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,11 +94,11 @@ class BaseModelAdminChecks(object):
id='admin.E006',
)
]
- else:
- return list(chain(*[
+
+ return list(chain(*[
self._check_field_spec(cls, model, field_name, 'fields')
for field_name in cls.fields
- ]))
+ ]))
def _check_fieldsets(self, cls, model):
""" Check that fieldsets is properly formatted and doesn't contain
@@ -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,11 +144,10 @@ 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']
- ]))
+ return list(chain(*[
+ self._check_field_spec(cls, model, fields, '%s[1][\'fields\']' % label)
+ for fields in fieldset[1]['fields']
+ ]))
def _check_field_spec(self, cls, model, fields, label):
""" `fields` should be an item of `fields` or an item of
diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py
index a2f2e9fa7b..b52300756b 100644
--- a/django/contrib/admin/utils.py
+++ b/django/contrib/admin/utils.py
@@ -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
diff --git a/tests/admin_checks/tests.py b/tests/admin_checks/tests.py
index ba9faea03f..109161ebba 100644
--- a/tests/admin_checks/tests.py
+++ b/tests/admin_checks/tests.py
@@ -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)
diff --git a/tests/admin_util/tests.py b/tests/admin_util/tests.py
index 4cb2d843fb..20980efffa 100644
--- a/tests/admin_util/tests.py
+++ b/tests/admin_util/tests.py
@@ -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">&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