Fixed #17252 -- Fixed a minor regression introduced by the work in #11868, where the default sorted columns wouldn't correctly be visually represented in the changelist table headers if those columns referred to non model fields. Thanks to sebastian for the report and patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@17143 bcc190cf-cafb-0310-a4f2-bffc1f526a37

3 files changed, 93 insertions, 18 deletions

diff --git a/tests/regressiontests/admin_views/admin.py b/tests/regressiontests/admin_views/admin.py
index 66e76dd369..514538fb6d 100644
--- a/tests/regressiontests/admin_views/admin.py
+++ b/tests/regressiontests/admin_views/admin.py
@@ -22,7 +22,9 @@ from .models import (Article, Chapter, Account, Media, Child, Parent, Picture,
     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):
@@ -469,11 +471,35 @@ class WorkHourAdmin(admin.ModelAdmin):
     list_filter = ('employee',)
 
 
-class PrePopulatedPostLargeSlugAdmin(admin.ModelAdmin): 
-    prepopulated_fields = { 
-        'slug' : ('title',) 
-    } 
- 
+class PrePopulatedPostLargeSlugAdmin(admin.ModelAdmin):
+    prepopulated_fields = {
+        '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,10 +563,14 @@ site.register(Question)
 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) 
diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py
index d1a5d19dd0..eb56f9b6e3 100644
--- a/tests/regressiontests/admin_views/models.py
+++ b/tests/regressiontests/admin_views/models.py
@@ -538,13 +538,31 @@ class ComplexSortedPerson(models.Model):
     age = models.PositiveIntegerField()
     is_employee = models.NullBooleanField()
 
-class PrePopulatedPostLargeSlug(models.Model): 
-    """ 
-    Regression test for #15938: a large max_length for the slugfield must not 
-    be localized in prepopulated_fields_js.html or it might end up breaking 
-    the javascript (ie, using THOUSAND_SEPARATOR ends up with maxLength=1,000) 
-    """ 
-    title = models.CharField(max_length=100) 
-    published = models.BooleanField() 
+class PrePopulatedPostLargeSlug(models.Model):
+    """
+    Regression test for #15938: a large max_length for the slugfield must not
+    be localized in prepopulated_fields_js.html or it might end up breaking
+    the javascript (ie, using THOUSAND_SEPARATOR ends up with maxLength=1,000)
+    """
+    title = models.CharField(max_length=100)
+    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)
diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py
index f4ec63f8ae..f176708366 100644
--- a/tests/regressiontests/admin_views/tests.py
+++ b/tests/regressiontests/admin_views/tests.py
@@ -37,7 +37,8 @@ from .models import (Article, BarAccount, CustomArticle, EmptyModel, FooAccount,
     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,6 +355,32 @@ class AdminViewBasicTest(TestCase):
             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').
@@ -2757,7 +2784,7 @@ class PrePopulatedTest(TestCase):
         self.assertNotContains(response, "id: '#id_slug'")
         self.assertNotContains(response, "field['dependency_ids'].push('#id_title');")
         self.assertNotContains(response, "id: '#id_prepopulatedsubpost_set-0-subslug',")
-    
+
     @override_settings(USE_THOUSAND_SEPARATOR = True, USE_L10N = True)
     def test_prepopulated_maxlength_localized(self):
         """