Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed various bugs related to having multiple columns in admin list_d…

…isplay with the same sort field

Thanks to julien for the report

Refs #11868

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16319 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit cb996cce059ec6f384d7ee9b67df6208c92eb719 1 parent 22598d0
@spookylukey spookylukey authored
View
54 django/contrib/admin/templatetags/admin_list.py
@@ -85,7 +85,7 @@ def result_headers(cl):
# We need to know the 'ordering field' that corresponds to each
# item in list_display, and we need other info, so do a pre-pass
# on list_display
- list_display_info = SortedDict()
+ ordering_field_columns = cl.get_ordering_field_columns()
for i, field_name in enumerate(cl.list_display):
admin_order_field = None
text, attr = label_for_field(field_name, cl.model,
@@ -93,36 +93,20 @@ def result_headers(cl):
return_attr = True
)
if attr:
- admin_order_field = getattr(attr, "admin_order_field", None)
- if admin_order_field is None:
- ordering_field_name = field_name
- else:
- ordering_field_name = admin_order_field
- list_display_info[ordering_field_name] = dict(text=text,
- attr=attr,
- index=i,
- admin_order_field=admin_order_field,
- field_name=field_name)
-
- del admin_order_field, text, attr
-
- ordering_fields = cl.get_ordering_fields()
-
- for ordering_field_name, info in list_display_info.items():
- if info['attr']:
# Potentially not sortable
# if the field is the action checkbox: no sorting and special class
- if info['field_name'] == 'action_checkbox':
+ if field_name == 'action_checkbox':
yield {
- "text": info['text'],
+ "text": text,
"class_attrib": mark_safe(' class="action-checkbox-column"')
}
continue
- if not info['admin_order_field']:
+ admin_order_field = getattr(attr, "admin_order_field", None)
+ if not admin_order_field:
# Not sortable
- yield {"text": info['text']}
+ yield {"text": text}
continue
# OK, it is sortable if we got this far
@@ -131,9 +115,9 @@ def result_headers(cl):
new_order_type = 'asc'
sort_pos = 0
# Is it currently being sorted on?
- if ordering_field_name in ordering_fields:
- order_type = ordering_fields.get(ordering_field_name).lower()
- sort_pos = ordering_fields.keys().index(ordering_field_name) + 1
+ if i in ordering_field_columns:
+ order_type = ordering_field_columns.get(i).lower()
+ sort_pos = ordering_field_columns.keys().index(i) + 1
th_classes.append('sorted %sending' % order_type)
new_order_type = {'asc': 'desc', 'desc': 'asc'}[order_type]
@@ -141,27 +125,21 @@ def result_headers(cl):
o_list = []
make_qs_param = lambda t, n: ('-' if t == 'desc' else '') + str(n)
- for f, ot in ordering_fields.items():
- try:
- colnum = list_display_info[f]['index']
- except KeyError:
- continue
-
- if f == ordering_field_name:
+ for j, ot in ordering_field_columns.items():
+ if j == i: # Same column
# We want clicking on this header to bring the ordering to the
# front
- o_list.insert(0, make_qs_param(new_order_type, colnum))
+ o_list.insert(0, make_qs_param(new_order_type, j))
else:
- o_list.append(make_qs_param(ot, colnum))
+ o_list.append(make_qs_param(ot, j))
- if ordering_field_name not in ordering_fields:
- colnum = list_display_info[ordering_field_name]['index']
- o_list.insert(0, make_qs_param(new_order_type, colnum))
+ if i not in ordering_field_columns:
+ o_list.insert(0, make_qs_param(new_order_type, i))
o_list = '.'.join(o_list)
yield {
- "text": info['text'],
+ "text": text,
"sortable": True,
"ascending": order_type == "asc",
"sort_pos": sort_pos,
View
60 django/contrib/admin/views/main.py
@@ -164,16 +164,20 @@ def get_results(self, request):
self.multi_page = multi_page
self.paginator = paginator
+ def _get_default_ordering(self):
+ ordering = []
+ if self.model_admin.ordering:
+ ordering = self.model_admin.ordering
+ elif self.lookup_opts.ordering:
+ ordering = self.lookup_opts.ordering
+ return ordering
+
def get_ordering(self):
- lookup_opts, params = self.lookup_opts, self.params
+ params = self.params
# For ordering, first check the "ordering" parameter in the admin
# options, then check the object's default ordering. Finally, a
# manually-specified ordering from the query string overrides anything.
- ordering = []
- if self.model_admin.ordering:
- ordering = self.model_admin.ordering
- elif lookup_opts.ordering:
- ordering = lookup_opts.ordering
+ ordering = self._get_default_ordering()
if ORDER_VAR in params:
# Clear ordering and used params
@@ -184,7 +188,7 @@ def get_ordering(self):
none, pfx, idx = p.rpartition('-')
field_name = self.list_display[int(idx)]
try:
- f = lookup_opts.get_field(field_name)
+ 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.
@@ -208,12 +212,44 @@ def get_ordering(self):
return ordering
- def get_ordering_fields(self):
- # Returns a SortedDict of ordering fields and asc/desc
+ def get_ordering_field_columns(self):
+ # 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.
+ ordering = self._get_default_ordering()
ordering_fields = SortedDict()
- for o in self.ordering:
- none, t, f = o.rpartition('-')
- ordering_fields[f] = 'desc' if t == '-' else 'asc'
+ if ORDER_VAR not in self.params:
+ # for ordering specified on ModelAdmin or model Meta, we don't know
+ # the right column numbers absolutely, because there might be more
+ # than one column associated with that ordering, so we guess.
+ for f in ordering:
+ if f.startswith('-'):
+ f = f[1:]
+ order_type = 'desc'
+ else:
+ order_type = 'asc'
+ i = None
+ try:
+ # Search for simply field name first
+ i = self.list_display.index(f)
+ except ValueError:
+ # No match, but their might be a match if we take into account
+ # 'admin_order_field'
+ for j, attr in enumerate(self.list_display):
+ if getattr(attr, 'admin_order_field', '') == f:
+ i = j
+ break
+ if i is not None:
+ ordering_fields[i] = order_type
+ else:
+ for p in self.params[ORDER_VAR].split('.'):
+ none, pfx, idx = p.rpartition('-')
+ try:
+ idx = int(idx)
+ except ValueError:
+ continue # skip it
+ ordering_fields[idx] = 'desc' if pfx == '-' else 'asc'
return ordering_fields
def get_lookup_params(self, use_distinct=False):
View
15 tests/regressiontests/admin_views/models.py
@@ -811,6 +811,20 @@ class OtherStoryAdmin(admin.ModelAdmin):
list_editable = ('content', )
ordering = ["-pk"]
+class ComplexSortedPerson(models.Model):
+ name = models.CharField(max_length=100)
+ age = models.PositiveIntegerField()
+ is_employee = models.NullBooleanField()
+
+class ComplexSortedPersonAdmin(admin.ModelAdmin):
+ list_display = ('name', 'age', 'is_employee', 'colored_name')
+ ordering = ('name',)
+
+ def colored_name(self, obj):
+ return '<span style="color: #%s;">%s</span>' % ('ff00ff', obj.name)
+ colored_name.allow_tags = True
+ colored_name.admin_order_field = 'name'
+
admin.site.register(Article, ArticleAdmin)
admin.site.register(CustomArticle, CustomArticleAdmin)
admin.site.register(Section, save_as=True, inlines=[ArticleInline])
@@ -872,3 +886,4 @@ class OtherStoryAdmin(admin.ModelAdmin):
admin.site.register(Question)
admin.site.register(Answer)
admin.site.register(PrePopulatedPost, PrePopulatedPostAdmin)
+admin.site.register(ComplexSortedPerson, ComplexSortedPersonAdmin)
View
39 tests/regressiontests/admin_views/tests.py
@@ -37,7 +37,8 @@
Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit,
Category, Post, Plot, FunkyTag, Chapter, Book, Promo, WorkHour, Employee,
Question, Answer, Inquisition, Actor, FoodDelivery,
- RowLevelChangePermissionModel, Paper, CoverLetter, Story, OtherStory)
+ RowLevelChangePermissionModel, Paper, CoverLetter, Story, OtherStory,
+ ComplexSortedPerson)
class AdminViewBasicTest(TestCase):
@@ -313,6 +314,42 @@ def testChangeListSortingModelAdmin(self):
response.content.index(link % p1.pk) < response.content.index(link % p2.pk)
)
+ def testMultipleSortSameField(self):
+ # Check that we get the columns we expect if we have two columns
+ # that correspond to the same ordering field
+ dt = datetime.datetime.now()
+ p1 = Podcast.objects.create(name="A", release_date=dt)
+ p2 = Podcast.objects.create(name="B", release_date=dt - datetime.timedelta(10))
+
+ link = '<a href="%s/'
+ response = self.client.get('/test_admin/admin/admin_views/podcast/', {})
+ self.assertEqual(response.status_code, 200)
+ self.assertTrue(
+ response.content.index(link % p1.pk) < response.content.index(link % p2.pk)
+ )
+
+ p1 = ComplexSortedPerson.objects.create(name="Bob", age=10)
+ p2 = ComplexSortedPerson.objects.create(name="Amy", age=20)
+ link = '<a href="%s/'
+
+ response = self.client.get('/test_admin/admin/admin_views/complexsortedperson/', {})
+ self.assertEqual(response.status_code, 200)
+ # Should have 5 columns (including action checkbox col)
+ self.assertContains(response, '<th scope="col"', count=5)
+
+ self.assertContains(response, 'Name')
+ self.assertContains(response, 'Colored name')
+
+ # Check order
+ self.assertTrue(
+ response.content.index('Name') < response.content.index('Colored name')
+ )
+
+ # Check sorting - should be by name
+ self.assertTrue(
+ response.content.index(link % p2.id) < response.content.index(link % p1.id)
+ )
+
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').

0 comments on commit cb996cc

Please sign in to comment.
Something went wrong with that request. Please try again.