diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 4eed0160e2..0294be187e 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -169,6 +169,28 @@ class ChangeList(object): ordering = self.lookup_opts.ordering return ordering + def get_ordering_field(self, field_name): + """ + Returns the proper model field name corresponding to the given + field_name to use for ordering. field_name may either be the name of a + proper model field or the name of a method (on the admin or model) or a + callable with the 'admin_order_field' attribute. Returns None if no + proper model field name can be matched. + """ + try: + field = self.lookup_opts.get_field(field_name) + return field.name + except models.FieldDoesNotExist: + # See whether field_name is a name of a non-field + # that allows sorting. + 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 getattr(attr, 'admin_order_field', None) + def get_ordering(self, request): params = self.params # For ordering, first check the if exists the "get_ordering" method @@ -176,7 +198,6 @@ class ChangeList(object): # options, then check the object's default ordering. Finally, a # manually-specified ordering from the query string overrides anything. ordering = self.model_admin.get_ordering(request) or self._get_default_ordering() - if ORDER_VAR in params: # Clear ordering and used params ordering = [] @@ -185,33 +206,18 @@ class ChangeList(object): 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 - - ordering.append(pfx + field_name) - + order_field = self.get_ordering_field(field_name) + if not order_field: + continue # No 'admin_order_field', skip it + ordering.append(pfx + order_field) except (IndexError, ValueError): continue # Invalid ordering specified, skip it. - return ordering def get_ordering_field_columns(self): - # Returns a SortedDict of ordering field column numbers and asc/desc + """ + Returns a SortedDict of ordering field column numbers and asc/desc + """ # We must cope with more than one column having the same underlying sort # field, so we base things on column numbers. @@ -227,19 +233,10 @@ class ChangeList(object): order_type = 'desc' else: order_type = 'asc' - index = None - try: - # Search for simply field name first - index = list(self.list_display).index(field) - except ValueError: - # 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: - index = _index - break - if index is not None: - ordering_fields[index] = order_type + for index, attr in enumerate(self.list_display): + if self.get_ordering_field(attr) == field: + ordering_fields[index] = order_type + break else: for p in self.params[ORDER_VAR].split('.'): none, pfx, idx = p.rpartition('-') 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, '