From 8ddecc39dbbb4da17a4eccff29bcf95efed991b8 Mon Sep 17 00:00:00 2001 From: Julien Phalip Date: Tue, 22 Nov 2011 09:14:09 +0000 Subject: [PATCH] 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 --- django/contrib/admin/views/main.py | 69 ++++++++++----------- tests/regressiontests/admin_views/admin.py | 44 ++++++++++--- tests/regressiontests/admin_views/models.py | 36 ++++++++--- tests/regressiontests/admin_views/tests.py | 31 ++++++++- 4 files changed, 126 insertions(+), 54 deletions(-) diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 4eed0160e29..0294be187e6 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 66e76dd369e..514538fb6df 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 d1a5d19dd0c..eb56f9b6e3b 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 f4ec63f8ae9..f176708366d 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, '