Ticket #17522: r17364-admin-ordering-validation.diff

docs/ref/contrib/admin/index.txt

@@ -732 +732 @@
 
     Set ``ordering`` to specify how lists of objects should be ordered in the
     Django admin views. This should be a list or tuple in the same format as a
-    model's :attr:`~django.db.models.Options.ordering` parameter.
+    model's :attr:`~django.db.models.Options.ordering` parameter. In addition
+    to field names, ``ordering`` may contain the names of methods of the model
+    or admin class which define the ``admin_order_field`` attribute (cf.
+    :attr:`~ModelAdmin.list_display`).
 
     If this isn't provided, the Django admin will use the model's default
     ordering.

tests/regressiontests/admin_validation/tests.py

@@ -4 +4 @@
 from django.contrib import admin
 from django.contrib.admin.validation import validate, validate_inline
 from django.core.exceptions import ImproperlyConfigured
+from django.db.models import Count
 from django.test import TestCase
 
 from .models import Song, Book, Album, TwoAlbumFKAndAnE, State, City
@@ -281 +282 @@
             fields = ['extra_data', 'title']
 
         validate(FieldsOnFormOnlyAdmin, Song)
+
+    def test_ordering_regular_field(self):
+        """
+        Ordering with proper field must succeed.
+        """
+        class SongAdmin(admin.ModelAdmin):
+            ordering = ('title',)
+        validate(SongAdmin, Song)
+
+    def test_ordering_nonexisting_field(self):
+        """
+        Ordering for an unknown field must fail.
+        """
+        class SongAdmin(admin.ModelAdmin):
+            ordering = ('spam',)
+        self.assertRaisesMessage(ImproperlyConfigured,
+            "'SongAdmin.ordering[0]' refers to field 'spam' that is missing from model 'admin_validation.Song'.",
+            validate,
+            SongAdmin, Song)
+
+    def test_ordering_readonly_method(self):
+        """
+        Ordering for methods that do not specify admin_order_field
+        must fail.
+        """
+        class ModelSongAdmin(admin.ModelAdmin):
+            ordering = ('readonly_method_on_model',)
+        self.assertRaisesMessage(ImproperlyConfigured,
+            "'ModelSongAdmin.ordering[0]' refers to method 'readonly_method_on_model' with admin_order_field unset.",
+            validate,
+            ModelSongAdmin, Song)
+
+        class AdminSongAdmin(admin.ModelAdmin):
+            ordering = ('readonly_method_on_admin',)
+            def readonly_method_on_admin(self, song):
+                pass
+        self.assertRaisesMessage(ImproperlyConfigured,
+            "'AdminSongAdmin.ordering[0]' refers to method 'readonly_method_on_admin' with admin_order_field unset.",
+            validate,
+            AdminSongAdmin, Song)
+
+    def test_ordering_sortable_method_field(self):
+        """
+        Ordering for methods with admin_order_field set to a proper
+        field must succeed.
+        """
+        class ModelSongAdmin(admin.ModelAdmin):
+            ordering = ('sortable_method_on_model',)
+        validate(ModelSongAdmin, Song)
+
+        class AdminSongAdmin(admin.ModelAdmin):
+            ordering = ('sortable_method_on_admin',)
+            def sortable_method_on_admin(self, song):
+                pass
+            sortable_method_on_admin.admin_order_field = 'title'
+        validate(AdminSongAdmin, Song)
+
+    def test_ordering_sortable_method_queryset(self):
+        """
+        Ordering for methods with admin_order_field set to name of
+        an additional field (e.g. annotation) returned by a custom
+        queryset must succeed.
+        """
+        class AlbumAdmin(admin.ModelAdmin):
+            ordering = ('advanced_sortable_method_on_admin',)
+            def queryset(self, request):
+                return Album.objects.annotate(songs_count=Count('song_set'))
+            def advanced_sortable_method_on_admin(self, album):
+                pass
+            advanced_sortable_method_on_admin.admin_order_field = 'songs_count'
+        validate(AlbumAdmin, Album)

tests/regressiontests/admin_validation/models.py

@@ -11 +11 @@
 
 class Song(models.Model):
     title = models.CharField(max_length=150)
-    album = models.ForeignKey(Album)
+    album = models.ForeignKey(Album, related_name="song_set")
     original_release = models.DateField(editable=False)
 
     class Meta:
@@ -24 +24 @@
         # does nothing
         pass
 
+    def sortable_method_on_model(self):
+        pass
+    sortable_method_on_model.admin_order_field = 'title'
 
+
 class TwoAlbumFKAndAnE(models.Model):
     album1 = models.ForeignKey(Album, related_name="album1_set")
     album2 = models.ForeignKey(Album, related_name="album2_set")

django/contrib/admin/validation.py

@@ -154 +154 @@
                 continue
             if field.startswith('-'):
                 field = field[1:]
+            # Check whether `field` is a name of a non-field that
+            # allows for sorting, i.e. name of a method on either
+            # the admin cls or model with admin_order_field set.
+            attr = (getattr(cls, field, None) or
+                    getattr(model, field, None))
+            if callable(attr):
+                if not hasattr(attr, 'admin_order_field'):
+                    # Found method but no admin_order_field.
+                    raise ImproperlyConfigured("'%s.%s' refers "
+                            "to method '%s' with admin_order_field unset."
+                            % (cls.__name__, 'ordering[%d]' % idx, field))
+                # No further checks for methods with
+                # admin_order_field. In particular, we do not enforce
+                # admin_order_field to refer to an actual model field:
+                # it might also be an annotation returned by the model
+                # admin's custom queryset() method etc.
+                continue
             # Skip ordering in the format field1__field2 (FIXME: checking
             # this format would be nice, but it's a little fiddly).
             if '__' in field: