Ticket #17252: r17130-admin-order-field-better-test.diff

django/contrib/admin/views/main.py

@@ -169 +169 @@
             ordering = self.lookup_opts.ordering
         return ordering
 
+    def _get_admin_order_field(self, field_name):
+        """
+        Get the name of the admin order field defined for the given
+        element of 'list_display'. The given object can either be the
+        name of a proper field, the name of a method (on the admin or
+        model) with the 'admin_order_field' attribute, or a callable
+        with that attribute. If no admin order is defined for the
+        given object, the method returns None.
+        """
+        try:
+            f = self.lookup_opts.get_field(field_name)
+        except models.FieldDoesNotExist:
+            # See whether field_name is a name of a non-field
+            # that allows sorting.
+            try:
+                if callable(field_name):
+                    attr = field_name
+                elif hasattr(self.model_admin, field_name):
+                    attr = getattr(self.model_admin, field_name)
+                else:
+                    attr = getattr(self.model, field_name)
+                return attr.admin_order_field
+            except AttributeError:
+                return None
+        else:
+            return f.name
+
     def get_ordering(self, request):
         params = self.params
         # For ordering, first check the if exists the "get_ordering" method
@@ -185 +212 @@
                 try:
                     none, pfx, idx = p.rpartition('-')
                     field_name = self.list_display[int(idx)]
-                    try:
-                        f = self.lookup_opts.get_field(field_name)
-                    except models.FieldDoesNotExist:
-                        # See whether field_name is a name of a non-field
-                        # that allows sorting.
-                        try:
-                            if callable(field_name):
-                                attr = field_name
-                            elif hasattr(self.model_admin, field_name):
-                                attr = getattr(self.model_admin, field_name)
-                            else:
-                                attr = getattr(self.model, field_name)
-                            field_name = attr.admin_order_field
-                        except AttributeError:
-                            continue # No 'admin_order_field', skip it
-                    else:
-                        field_name = f.name
+                    order_field = self._get_admin_order_field(field_name)
+                    if not order_field:
+                        continue # No 'admin_order_field', skip it
 
-                    ordering.append(pfx + field_name)
+                    ordering.append(pfx + order_field)
 
                 except (IndexError, ValueError):
                     continue # Invalid ordering specified, skip it.
@@ -235 +248 @@
                     # No match, but there might be a match if we take into
                     # account 'admin_order_field'
                     for _index, attr in enumerate(self.list_display):
-                        if getattr(attr, 'admin_order_field', '') == field:
+                        if self._get_admin_order_field(attr) == field:
                             index = _index
                             break
                 if index is not None:

tests/regressiontests/admin_views/admin.py

@@ -22 +22 @@
     Gadget, Villain, SuperVillain, Plot, PlotDetails, CyclicOne, CyclicTwo,
     WorkHour, Reservation, FoodDelivery, RowLevelChangePermissionModel, Paper,
     CoverLetter, Story, OtherStory, Book, Promo, ChapterXtra1, Pizza, Topping,
-    Album, Question, Answer, ComplexSortedPerson, PrePopulatedPostLargeSlug)
+    Album, Question, Answer, ComplexSortedPerson, PrePopulatedPostLargeSlug,
+    AdminOrderedField, AdminOrderedModelMethod, AdminOrderedAdminMethod,
+    AdminOrderedCallable)
 
 
 def callable_year(dt_value):
@@ -474 +476 @@
         'slug' : ('title',) 
     } 
  
+
+class AdminOrderedFieldAdmin(admin.ModelAdmin):
+    ordering = ('order',)
+    list_display = ('stuff', 'order')
+
+class AdminOrderedModelMethodAdmin(admin.ModelAdmin):
+    ordering = ('order',)
+    list_display = ('stuff', 'some_order')
+
+class AdminOrderedAdminMethodAdmin(admin.ModelAdmin):
+    def some_admin_order(self, obj):
+        return obj.order
+    some_admin_order.admin_order_field = 'order'
+    ordering = ('order',)
+    list_display = ('stuff', 'some_admin_order')
+
+def admin_ordered_callable(obj):
+    return obj.order
+admin_ordered_callable.admin_order_field = 'order'
+class AdminOrderedCallableAdmin(admin.ModelAdmin):
+    ordering = ('order',)
+    list_display = ('stuff', admin_ordered_callable)
+
+
 site = admin.AdminSite(name="admin")
 site.register(Article, ArticleAdmin)
 site.register(CustomArticle, CustomArticleAdmin)
@@ -537 +563 @@
 site.register(Answer)
 site.register(PrePopulatedPost, PrePopulatedPostAdmin)
 site.register(ComplexSortedPerson, ComplexSortedPersonAdmin)
+site.register(PrePopulatedPostLargeSlug, PrePopulatedPostLargeSlugAdmin) 
+site.register(AdminOrderedField, AdminOrderedFieldAdmin)
+site.register(AdminOrderedModelMethod, AdminOrderedModelMethodAdmin)
+site.register(AdminOrderedAdminMethod, AdminOrderedAdminMethodAdmin)
+site.register(AdminOrderedCallable, AdminOrderedCallableAdmin)
 
 # Register core models we need in our tests
 from django.contrib.auth.models import User, Group
 from django.contrib.auth.admin import UserAdmin, GroupAdmin
 site.register(User, UserAdmin)
 site.register(Group, GroupAdmin)
-site.register(PrePopulatedPostLargeSlug, PrePopulatedPostLargeSlugAdmin) 

tests/regressiontests/admin_views/tests.py

@@ -37 +37 @@
     DooHickey, FancyDoodad, Whatsit, Category, Post, Plot, FunkyTag, Chapter,
     Book, Promo, WorkHour, Employee, Question, Answer, Inquisition, Actor,
     FoodDelivery, RowLevelChangePermissionModel, Paper, CoverLetter, Story,
-    OtherStory, ComplexSortedPerson, Parent, Child)
+    OtherStory, ComplexSortedPerson, Parent, Child, AdminOrderedField,
+    AdminOrderedModelMethod, AdminOrderedAdminMethod, AdminOrderedCallable)
 
 
 ERROR_MESSAGE = "Please enter the correct username and password \
@@ -354 +355 @@
             response.content.index(link % p2.id) < response.content.index(link % p1.id)
         )
 
+    def testSortIndicatorsAdminOrder(self):
+        """
+        Ensures that the admin shows default sort indicators for all
+        kinds of 'ordering' fields: field names, method on the model
+        admin and model itself, and other callables. See #17252.
+        """
+        models = [(AdminOrderedField,       'adminorderedfield'      ),
+                  (AdminOrderedModelMethod, 'adminorderedmodelmethod'),
+                  (AdminOrderedAdminMethod, 'adminorderedadminmethod'),
+                  (AdminOrderedCallable,    'adminorderedcallable'   )]
+        for model, url in models:
+            a1 = model.objects.create(stuff='The Last Item',   order=3)
+            a2 = model.objects.create(stuff='The First Item',  order=1)
+            a3 = model.objects.create(stuff='The Middle Item', order=2)
+            response = self.client.get('/test_admin/admin/admin_views/%s/' % url, {})
+            self.assertEqual(response.status_code, 200)
+            # Should have 3 columns including action checkbox col.
+            self.assertContains(response, '<th scope="col"', count=3, msg_prefix=url)
+            # Check if the correct column was selected. 2 is the index of the
+            # 'order' column in the model admin's 'list_display' with 0 being
+            # the implicit 'action_checkbox' and 1 being the column 'stuff'.
+            self.assertEqual(response.context['cl'].get_ordering_field_columns(), {2: 'asc'})
+            # Check order of records.
+            self.assertTrue(response.content.index('The First Item') <
+                response.content.index('The Middle Item') < response.content.index('The Last Item'))
+
     def testLimitedFilter(self):
         """Ensure admin changelist filters do not contain objects excluded via limit_choices_to.
         This also tests relation-spanning filters (e.g. 'color__value').

tests/regressiontests/admin_views/models.py

@@ -538 +538 @@
     age = models.PositiveIntegerField()
     is_employee = models.NullBooleanField()
 
+
 class PrePopulatedPostLargeSlug(models.Model): 
     """ 
     Regression test for #15938: a large max_length for the slugfield must not 
@@ -548 +549 @@
     published = models.BooleanField() 
     slug = models.SlugField(max_length=1000)
     
+
+class AdminOrderedField(models.Model):
+    order = models.IntegerField()
+    stuff = models.CharField(max_length=200)
+
+class AdminOrderedModelMethod(models.Model):
+    order = models.IntegerField()
+    stuff = models.CharField(max_length=200)
+    def some_order(self):
+        return self.order
+    some_order.admin_order_field = 'order'
+
+class AdminOrderedAdminMethod(models.Model):
+    order = models.IntegerField()
+    stuff = models.CharField(max_length=200)
+
+class AdminOrderedCallable(models.Model):
+    order = models.IntegerField()
+    stuff = models.CharField(max_length=200)