From ec2c9c353113bb1db6e32ed3f0b6c28bc06ca2eb Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Thu, 14 Jun 2018 14:47:20 -0400 Subject: [PATCH] Refs #29428 -- Fixed admin check crash when using a query expression in ModelAdmin.ordering. --- django/contrib/admin/checks.py | 9 ++++++++- docs/releases/2.0.7.txt | 3 +++ tests/modeladmin/test_checks.py | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py index e0f3a21550..cf5b312369 100644 --- a/django/contrib/admin/checks.py +++ b/django/contrib/admin/checks.py @@ -10,6 +10,7 @@ from django.core import checks from django.core.exceptions import FieldDoesNotExist from django.db import models from django.db.models.constants import LOOKUP_SEP +from django.db.models.expressions import Combinable, F, OrderBy from django.forms.models import ( BaseModelForm, BaseModelFormSet, _get_foreign_key, ) @@ -489,7 +490,13 @@ class BaseModelAdminChecks: def _check_ordering_item(self, obj, model, field_name, label): """ Check that `ordering` refers to existing fields. """ - + if isinstance(field_name, (Combinable, OrderBy)): + if not isinstance(field_name, OrderBy): + field_name = field_name.asc() + if isinstance(field_name.expression, F): + field_name = field_name.expression.name + else: + return [] if field_name == '?' and len(obj.ordering) != 1: return [ checks.Error( diff --git a/docs/releases/2.0.7.txt b/docs/releases/2.0.7.txt index 46d6e86073..4890ee2dba 100644 --- a/docs/releases/2.0.7.txt +++ b/docs/releases/2.0.7.txt @@ -11,3 +11,6 @@ Bugfixes * Fixed admin changelist crash when using a query expression without ``asc()`` or ``desc()`` in the page's ordering (:ticket:`29428`). + +* Fixed admin check crash when using a query expression in + ``ModelAdmin.ordering`` (:ticket:`29428`). diff --git a/tests/modeladmin/test_checks.py b/tests/modeladmin/test_checks.py index f76b78078a..5a0433deb4 100644 --- a/tests/modeladmin/test_checks.py +++ b/tests/modeladmin/test_checks.py @@ -3,6 +3,8 @@ 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.functions import Upper from django.forms.models import BaseModelFormSet from django.test import SimpleTestCase @@ -829,6 +831,23 @@ class OrderingCheckTests(CheckTestCase): self.assertIsValid(TestModelAdmin, ValidationTestModel) + def test_invalid_expression(self): + class TestModelAdmin(ModelAdmin): + ordering = (F('nonexistent'), ) + + self.assertIsInvalid( + TestModelAdmin, ValidationTestModel, + "The value of 'ordering[0]' refers to 'nonexistent', which is not " + "an attribute of 'modeladmin.ValidationTestModel'.", + 'admin.E033' + ) + + def test_valid_expression(self): + class TestModelAdmin(ModelAdmin): + ordering = (Upper('name'), Upper('band__name').desc()) + + self.assertIsValid(TestModelAdmin, ValidationTestModel) + class ListSelectRelatedCheckTests(CheckTestCase):