Permalink
Browse files

Fixed #17252 -- Fixed a minor regression introduced by the work in #1…

…1868, 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
  • Loading branch information...
jphalip committed Nov 22, 2011
1 parent 658abb0 commit 8ddecc39dbbb4da17a4eccff29bcf95efed991b8
@@ -169,14 +169,35 @@ def _get_default_ordering(self):
ordering = self.lookup_opts.ordering ordering = self.lookup_opts.ordering
return 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): def get_ordering(self, request):
params = self.params params = self.params
# For ordering, first check the if exists the "get_ordering" method # For ordering, first check the if exists the "get_ordering" method
# in model admin, then check "ordering" parameter in the admin # in model admin, then check "ordering" parameter in the admin
# options, then check the object's default ordering. Finally, a # options, then check the object's default ordering. Finally, a
# manually-specified ordering from the query string overrides anything. # manually-specified ordering from the query string overrides anything.
ordering = self.model_admin.get_ordering(request) or self._get_default_ordering() ordering = self.model_admin.get_ordering(request) or self._get_default_ordering()
if ORDER_VAR in params: if ORDER_VAR in params:
# Clear ordering and used params # Clear ordering and used params
ordering = [] ordering = []
@@ -185,33 +206,18 @@ def get_ordering(self, request):
try: try:
none, pfx, idx = p.rpartition('-') none, pfx, idx = p.rpartition('-')
field_name = self.list_display[int(idx)] field_name = self.list_display[int(idx)]
try: order_field = self.get_ordering_field(field_name)
f = self.lookup_opts.get_field(field_name) if not order_field:
except models.FieldDoesNotExist: continue # No 'admin_order_field', skip it
# See whether field_name is a name of a non-field ordering.append(pfx + order_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)
except (IndexError, ValueError): except (IndexError, ValueError):
continue # Invalid ordering specified, skip it. continue # Invalid ordering specified, skip it.
return ordering return ordering
def get_ordering_field_columns(self): 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 # We must cope with more than one column having the same underlying sort
# field, so we base things on column numbers. # field, so we base things on column numbers.
@@ -227,19 +233,10 @@ def get_ordering_field_columns(self):
order_type = 'desc' order_type = 'desc'
else: else:
order_type = 'asc' order_type = 'asc'
index = None for index, attr in enumerate(self.list_display):
try: if self.get_ordering_field(attr) == field:
# Search for simply field name first ordering_fields[index] = order_type
index = list(self.list_display).index(field) break
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
else: else:
for p in self.params[ORDER_VAR].split('.'): for p in self.params[ORDER_VAR].split('.'):
none, pfx, idx = p.rpartition('-') none, pfx, idx = p.rpartition('-')
@@ -22,7 +22,9 @@
Gadget, Villain, SuperVillain, Plot, PlotDetails, CyclicOne, CyclicTwo, Gadget, Villain, SuperVillain, Plot, PlotDetails, CyclicOne, CyclicTwo,
WorkHour, Reservation, FoodDelivery, RowLevelChangePermissionModel, Paper, WorkHour, Reservation, FoodDelivery, RowLevelChangePermissionModel, Paper,
CoverLetter, Story, OtherStory, Book, Promo, ChapterXtra1, Pizza, Topping, 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): def callable_year(dt_value):
@@ -469,11 +471,35 @@ class WorkHourAdmin(admin.ModelAdmin):
list_filter = ('employee',) list_filter = ('employee',)
class PrePopulatedPostLargeSlugAdmin(admin.ModelAdmin): class PrePopulatedPostLargeSlugAdmin(admin.ModelAdmin):
prepopulated_fields = { prepopulated_fields = {
'slug' : ('title',) '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 = admin.AdminSite(name="admin")
site.register(Article, ArticleAdmin) site.register(Article, ArticleAdmin)
site.register(CustomArticle, CustomArticleAdmin) site.register(CustomArticle, CustomArticleAdmin)
@@ -537,10 +563,14 @@ class PrePopulatedPostLargeSlugAdmin(admin.ModelAdmin):
site.register(Answer) site.register(Answer)
site.register(PrePopulatedPost, PrePopulatedPostAdmin) site.register(PrePopulatedPost, PrePopulatedPostAdmin)
site.register(ComplexSortedPerson, ComplexSortedPersonAdmin) 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 # Register core models we need in our tests
from django.contrib.auth.models import User, Group from django.contrib.auth.models import User, Group
from django.contrib.auth.admin import UserAdmin, GroupAdmin from django.contrib.auth.admin import UserAdmin, GroupAdmin
site.register(User, UserAdmin) site.register(User, UserAdmin)
site.register(Group, GroupAdmin) site.register(Group, GroupAdmin)
site.register(PrePopulatedPostLargeSlug, PrePopulatedPostLargeSlugAdmin)
@@ -538,13 +538,31 @@ class ComplexSortedPerson(models.Model):
age = models.PositiveIntegerField() age = models.PositiveIntegerField()
is_employee = models.NullBooleanField() is_employee = models.NullBooleanField()
class PrePopulatedPostLargeSlug(models.Model): class PrePopulatedPostLargeSlug(models.Model):
""" """
Regression test for #15938: a large max_length for the slugfield must not 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 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) the javascript (ie, using THOUSAND_SEPARATOR ends up with maxLength=1,000)
""" """
title = models.CharField(max_length=100) title = models.CharField(max_length=100)
published = models.BooleanField() published = models.BooleanField()
slug = models.SlugField(max_length=1000) 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)
@@ -37,7 +37,8 @@
DooHickey, FancyDoodad, Whatsit, Category, Post, Plot, FunkyTag, Chapter, DooHickey, FancyDoodad, Whatsit, Category, Post, Plot, FunkyTag, Chapter,
Book, Promo, WorkHour, Employee, Question, Answer, Inquisition, Actor, Book, Promo, WorkHour, Employee, Question, Answer, Inquisition, Actor,
FoodDelivery, RowLevelChangePermissionModel, Paper, CoverLetter, Story, 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 \ ERROR_MESSAGE = "Please enter the correct username and password \
@@ -354,6 +355,32 @@ def testMultipleSortSameField(self):
response.content.index(link % p2.id) < response.content.index(link % p1.id) 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): def testLimitedFilter(self):
"""Ensure admin changelist filters do not contain objects excluded via limit_choices_to. """Ensure admin changelist filters do not contain objects excluded via limit_choices_to.
This also tests relation-spanning filters (e.g. 'color__value'). This also tests relation-spanning filters (e.g. 'color__value').
@@ -2757,7 +2784,7 @@ def test_prepopulated_off(self):
self.assertNotContains(response, "id: '#id_slug'") self.assertNotContains(response, "id: '#id_slug'")
self.assertNotContains(response, "field['dependency_ids'].push('#id_title');") self.assertNotContains(response, "field['dependency_ids'].push('#id_title');")
self.assertNotContains(response, "id: '#id_prepopulatedsubpost_set-0-subslug',") self.assertNotContains(response, "id: '#id_prepopulatedsubpost_set-0-subslug',")
@override_settings(USE_THOUSAND_SEPARATOR = True, USE_L10N = True) @override_settings(USE_THOUSAND_SEPARATOR = True, USE_L10N = True)
def test_prepopulated_maxlength_localized(self): def test_prepopulated_maxlength_localized(self):
""" """

0 comments on commit 8ddecc3

Please sign in to comment.