From 384cdf0f7a2c8d1793b120d82a1584776c064f44 Mon Sep 17 00:00:00 2001 From: Eric Blum Date: Thu, 1 May 2025 16:31:06 -0400 Subject: [PATCH] Fixed #36363 -- Added field names to admin duplicated fields error hint. --- django/contrib/admin/checks.py | 28 +++++++++++++++++++++++----- tests/admin_checks/tests.py | 28 ++++++++++++++++++++++++++++ tests/modeladmin/test_checks.py | 3 +++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index a4d7066d10..4bfbe25f22 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -331,11 +331,15 @@ class BaseModelAdminChecks: id="admin.E005", ) ] - fields = flatten(obj.fields) - if len(fields) != len(set(fields)): + field_counts = collections.Counter(flatten(obj.fields)) + if duplicate_fields := [ + field for field, count in field_counts.items() if count > 1 + ]: return [ checks.Error( "The value of 'fields' contains duplicate field(s).", + hint="Remove duplicates of %s." + % ", ".join(map(repr, duplicate_fields)), obj=obj.__class__, id="admin.E006", ) @@ -397,11 +401,20 @@ class BaseModelAdminChecks: id="admin.E008", ) - seen_fields.extend(flatten(fieldset[1]["fields"])) - if len(seen_fields) != len(set(seen_fields)): + fieldset_fields = flatten(fieldset[1]["fields"]) + seen_fields.extend(fieldset_fields) + field_counts = collections.Counter(seen_fields) + fieldset_fields_set = set(fieldset_fields) + if duplicate_fields := [ + field + for field, count in field_counts.items() + if count > 1 and field in fieldset_fields_set + ]: return [ checks.Error( "There are duplicate field(s) in '%s[1]'." % label, + hint="Remove duplicates of %s." + % ", ".join(map(repr, duplicate_fields)), obj=obj.__class__, id="admin.E012", ) @@ -469,10 +482,15 @@ class BaseModelAdminChecks: return must_be( "a list or tuple", option="exclude", obj=obj, id="admin.E014" ) - elif len(obj.exclude) > len(set(obj.exclude)): + field_counts = collections.Counter(obj.exclude) + if duplicate_fields := [ + field for field, count in field_counts.items() if count > 1 + ]: return [ checks.Error( "The value of 'exclude' contains duplicate field(s).", + hint="Remove duplicates of %s." + % ", ".join(map(repr, duplicate_fields)), obj=obj.__class__, id="admin.E015", ) diff --git a/tests/admin_checks/tests.py b/tests/admin_checks/tests.py index 6ca5d6d925..fc87260c9c 100644 --- a/tests/admin_checks/tests.py +++ b/tests/admin_checks/tests.py @@ -479,6 +479,7 @@ class SystemChecksTestCase(SimpleTestCase): expected = [ checks.Error( "The value of 'exclude' contains duplicate field(s).", + hint="Remove duplicates of 'name'.", obj=ExcludedFields2, id="admin.E015", ) @@ -970,6 +971,7 @@ class SystemChecksTestCase(SimpleTestCase): expected = [ checks.Error( "The value of 'fields' contains duplicate field(s).", + hint="Remove duplicates of 'state'.", obj=MyModelAdmin, id="admin.E006", ) @@ -986,12 +988,38 @@ class SystemChecksTestCase(SimpleTestCase): expected = [ checks.Error( "There are duplicate field(s) in 'fieldsets[0][1]'.", + hint="Remove duplicates of 'title', 'album'.", obj=MyModelAdmin, id="admin.E012", ) ] self.assertEqual(errors, expected) + def test_check_multiple_duplicates_across_fieldsets(self): + class MyModelAdmin(admin.ModelAdmin): + fieldsets = [ + ("Header 1", {"fields": ["title", "album"]}), + ("Header 2", {"fields": ["album", "name"]}), + ("Header 3", {"fields": ["name", "other", "title"]}), + ] + + errors = MyModelAdmin(Song, AdminSite()).check() + expected = [ + checks.Error( + "There are duplicate field(s) in 'fieldsets[1][1]'.", + hint="Remove duplicates of 'album'.", + obj=MyModelAdmin, + id="admin.E012", + ), + checks.Error( + "There are duplicate field(s) in 'fieldsets[2][1]'.", + hint="Remove duplicates of 'title', 'name'.", + obj=MyModelAdmin, + id="admin.E012", + ), + ] + self.assertEqual(errors, expected) + def test_list_filter_works_on_through_field_even_when_apps_not_ready(self): """ Ensure list_filter can access reverse fields even when the app registry diff --git a/tests/modeladmin/test_checks.py b/tests/modeladmin/test_checks.py index 94a80ca006..0592be7b4f 100644 --- a/tests/modeladmin/test_checks.py +++ b/tests/modeladmin/test_checks.py @@ -222,6 +222,7 @@ class FieldsetsCheckTests(CheckTestCase): ValidationTestModel, "There are duplicate field(s) in 'fieldsets[0][1]'.", "admin.E012", + "Remove duplicates of 'name'.", ) def test_duplicate_fields_in_fieldsets(self): @@ -236,6 +237,7 @@ class FieldsetsCheckTests(CheckTestCase): ValidationTestModel, "There are duplicate field(s) in 'fieldsets[1][1]'.", "admin.E012", + "Remove duplicates of 'name'.", ) def test_fieldsets_with_custom_form_validation(self): @@ -255,6 +257,7 @@ class FieldsCheckTests(CheckTestCase): ValidationTestModel, "The value of 'fields' contains duplicate field(s).", "admin.E006", + "Remove duplicates of 'name'.", ) def test_inline(self):