From ed668796f6c7e59ca3f35e9d87d940e043a3eff3 Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Wed, 10 Jul 2019 00:27:06 +0200 Subject: [PATCH] Fixed #30543 -- Fixed checks of ModelAdmin.list_display for fields accessible only via instance. Co-Authored-By: Andrew Simons --- django/contrib/admin/checks.py | 42 ++++++++++++++++----------------- tests/modeladmin/test_checks.py | 21 ++++++++++++++++- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index f82c20c2f1..0c32301284 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -720,33 +720,33 @@ class ModelAdminChecks(BaseModelAdminChecks): return [] elif hasattr(obj, item): return [] - elif hasattr(obj.model, item): + try: + field = obj.model._meta.get_field(item) + except FieldDoesNotExist: try: - field = obj.model._meta.get_field(item) - except FieldDoesNotExist: - return [] - else: - if isinstance(field, models.ManyToManyField): - return [ - checks.Error( - "The value of '%s' must not be a ManyToManyField." % label, - obj=obj.__class__, - id='admin.E109', - ) - ] - return [] - else: + field = getattr(obj.model, item) + except AttributeError: + return [ + checks.Error( + "The value of '%s' refers to '%s', which is not a " + "callable, an attribute of '%s', or an attribute or " + "method on '%s.%s'." % ( + label, item, obj.__class__.__name__, + obj.model._meta.app_label, obj.model._meta.object_name, + ), + obj=obj.__class__, + id='admin.E108', + ) + ] + if isinstance(field, models.ManyToManyField): return [ checks.Error( - "The value of '%s' refers to '%s', which is not a callable, " - "an attribute of '%s', or an attribute or method on '%s.%s'." % ( - label, item, obj.__class__.__name__, - obj.model._meta.app_label, obj.model._meta.object_name, - ), + "The value of '%s' must not be a ManyToManyField." % label, obj=obj.__class__, - id='admin.E108', + id='admin.E109', ) ] + return [] def _check_list_display_links(self, obj): """ Check that list_display_links is a unique subset of list_display. diff --git a/tests/modeladmin/test_checks.py b/tests/modeladmin/test_checks.py index a1b7001f68..98cc56d67c 100644 --- a/tests/modeladmin/test_checks.py +++ b/tests/modeladmin/test_checks.py @@ -3,7 +3,7 @@ from django.contrib.admin import BooleanFieldListFilter, SimpleListFilter from django.contrib.admin.options import VERTICAL, ModelAdmin, TabularInline from django.contrib.admin.sites import AdminSite from django.core.checks import Error -from django.db.models import F +from django.db.models import F, Field, Model from django.db.models.functions import Upper from django.forms.models import BaseModelFormSet from django.test import SimpleTestCase @@ -509,6 +509,25 @@ class ListDisplayTests(CheckTestCase): self.assertIsValid(TestModelAdmin, ValidationTestModel) + def test_valid_field_accessible_via_instance(self): + class PositionField(Field): + """Custom field accessible only via instance.""" + def contribute_to_class(self, cls, name): + super().contribute_to_class(cls, name) + setattr(cls, self.name, self) + + def __get__(self, instance, owner): + if instance is None: + raise AttributeError() + + class TestModel(Model): + field = PositionField() + + class TestModelAdmin(ModelAdmin): + list_display = ('field',) + + self.assertIsValid(TestModelAdmin, TestModel) + class ListDisplayLinksCheckTests(CheckTestCase):