Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #11868 - Multiple sort in admin changelist.

Many thanks to bendavis78 for the initial patch, and for input from others.

Also fixed #7309. If people were relying on the undocumented default ordering
applied by the admin before, they will need to add 'ordering = ["-pk"]' to
their ModelAdmin.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16316 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 5434ce231d75004bdbe5cf2b7b24ce67a2a6e737 1 parent 78b3797
@spookylukey spookylukey authored
View
28 django/contrib/admin/media/css/base.css
@@ -326,6 +326,34 @@ table thead th.descending a {
background: url(../img/admin/arrow-down.gif) right .4em no-repeat;
}
+table thead th.sorted a span.text {
+ display: block;
+ float: left;
+}
+
+table thead th.sorted a span.sortpos {
+ display: block;
+ float: right;
+ font-size: .6em;
+}
+
+table thead th.sorted a img {
+ vertical-align: bottom;
+}
+
+table thead th.sorted a span.clear {
+ display: block;
+ clear: both;
+}
+
+#sorting-popup-div {
+ position: absolute;
+ background-color: white;
+ border: 1px solid #ddd;
+ z-index: 2000; /* more than filters on right */
+ padding-right: 10px;
+}
+
/* ORDERABLE TABLES */
table.orderable tbody tr td:hover {
View
8 django/contrib/admin/media/css/rtl.css
@@ -91,6 +91,14 @@ table thead th.descending a {
background-position: left;
}
+table thead th.sorted a span.text {
+ float: right;
+}
+
+table thead th.sorted a span.sortpos {
+ float: left;
+}
+
/* dashboard styles */
.dashboard .module table td a {
View
BIN  django/contrib/admin/media/img/admin/icon_cog.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
View
67 django/contrib/admin/templates/admin/change_list_results.html
@@ -1,3 +1,5 @@
+{% load adminmedia %}
+{% load i18n %}
{% if result_hidden_fields %}
<div class="hiddenfields">{# DIV for HTML validation #}
{% for item in result_hidden_fields %}{{ item }}{% endfor %}
@@ -8,10 +10,18 @@
<table id="result_list">
<thead>
<tr>
-{% for header in result_headers %}<th scope="col"{{ header.class_attrib }}>
-{% if header.sortable %}<a href="{{ header.url }}">{% endif %}
-{{ header.text|capfirst }}
-{% if header.sortable %}</a>{% endif %}</th>{% endfor %}
+{% for header in result_headers %}
+<th scope="col" {{ header.class_attrib }}>
+ {% if header.sortable %}<a href="{{ header.url }}">{% endif %}
+ <span class="text">{{ header.text|capfirst }}</span>
+ {% if header.sortable %}
+ {% if header.sort_pos > 0 %}<span class="sortpos">
+ {% if header.sort_pos == 1 %}<img id="primary-sort-icon" src="{% admin_media_prefix %}img/admin/icon_cog.gif" alt="" />&nbsp;{% endif %}
+ {{ header.sort_pos }}</span>
+ {% endif %}
+ <span class="clear"></span></a>
+ {% endif %}
+</th>{% endfor %}
</tr>
</thead>
<tbody>
@@ -24,4 +34,53 @@
</tbody>
</table>
</div>
+
+{# Sorting popup: #}
+<div style="display: none;" id="sorting-popup-div">
+<p>{% trans "Sorting by:" %}</p>
+<ol>
+{% for header in result_headers|dictsort:"sort_pos" %}
+ {% if header.sort_pos > 0 %}
+ {% if header.ascending %}
+ <li>{% blocktrans with fieldname=header.text %}{{ fieldname }} (ascending){% endblocktrans %}</li>
+ {% else %}
+ <li>{% blocktrans with fieldname=header.text %}{{ fieldname }} (descending){% endblocktrans %}</li>
+ {% endif %}
+ {% endif %}
+{% endfor %}
+</ol>
+<p><a href="{{ reset_sorting_url }}">{% trans "Reset sorting" %}</a></p>
+</div>
+<script type="text/javascript">
+<!--
+(function($) {
+ $(document).ready(function() {
+ var popup = $('#sorting-popup-div');
+ /* These next lines seems necessary to prime the popup: */
+ popup.offset({left:-1000, top:0});
+ popup.show();
+ var popupWidth = popup.width();
+ popup.hide();
+
+ $('#primary-sort-icon').toggle(function(ev) {
+ ev.preventDefault();
+ var img = $(this);
+ var pos = img.offset();
+ pos.top += img.height();
+ if (pos.left + popupWidth >
+ $(window).width()) {
+ pos.left -= popupWidth;
+ }
+ popup.show();
+ popup.offset(pos);
+ },
+ function(ev) {
+ ev.preventDefault();
+ popup.hide();
+ });
+ });
+})(django.jQuery);
+//-->
+</script>
+
{% endif %}
View
93 django/contrib/admin/templatetags/admin_list.py
@@ -7,6 +7,7 @@
from django.core.exceptions import ObjectDoesNotExist
from django.db import models
from django.utils import formats
+from django.utils.datastructures import SortedDict
from django.utils.html import escape, conditional_escape
from django.utils.safestring import mark_safe
from django.utils.text import capfirst
@@ -81,43 +82,90 @@ def result_headers(cl):
"""
Generates the list column headers.
"""
- lookup_opts = cl.lookup_opts
-
+ # 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()
for i, field_name in enumerate(cl.list_display):
- header, attr = label_for_field(field_name, cl.model,
+ admin_order_field = None
+ text, attr = label_for_field(field_name, cl.model,
model_admin = cl.model_admin,
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 field_name == 'action_checkbox':
+ if info['field_name'] == 'action_checkbox':
yield {
- "text": header,
+ "text": info['text'],
"class_attrib": mark_safe(' class="action-checkbox-column"')
}
continue
- # It is a non-field, but perhaps one that is sortable
- admin_order_field = getattr(attr, "admin_order_field", None)
- if not admin_order_field:
- yield {"text": header}
+ if not info['admin_order_field']:
+ # Not sortable
+ yield {"text": info['text']}
continue
- # So this _is_ a sortable non-field. Go to the yield
- # after the else clause.
- else:
- admin_order_field = None
-
+ # OK, it is sortable if we got this far
th_classes = []
+ order_type = ''
new_order_type = 'asc'
- if field_name == cl.order_field or admin_order_field == cl.order_field:
- th_classes.append('sorted %sending' % cl.order_type.lower())
- new_order_type = {'asc': 'desc', 'desc': 'asc'}[cl.order_type.lower()]
+ 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
+ th_classes.append('sorted %sending' % order_type)
+ new_order_type = {'asc': 'desc', 'desc': 'asc'}[order_type]
+
+ # build new ordering param
+ 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:
+ # We want clicking on this header to bring the ordering to the
+ # front
+ o_list.insert(0, make_qs_param(new_order_type, colnum))
+ else:
+ o_list.append(make_qs_param(ot, colnum))
+
+ 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))
+
+ o_list = '.'.join(o_list)
yield {
- "text": header,
+ "text": info['text'],
"sortable": True,
- "url": cl.get_query_string({ORDER_VAR: i, ORDER_TYPE_VAR: new_order_type}),
+ "ascending": order_type == "asc",
+ "sort_pos": sort_pos,
+ "url": cl.get_query_string({ORDER_VAR: o_list}),
"class_attrib": mark_safe(th_classes and ' class="%s"' % ' '.join(th_classes) or '')
}
@@ -228,9 +276,14 @@ def result_list(cl):
"""
Displays the headers and data list together
"""
+ headers = list(result_headers(cl))
+ for h in headers:
+ # Sorting in templates depends on sort_pos attribute
+ h.setdefault('sort_pos', 0)
return {'cl': cl,
'result_hidden_fields': list(result_hidden_fields(cl)),
- 'result_headers': list(result_headers(cl)),
+ 'result_headers': headers,
+ 'reset_sorting_url': cl.get_query_string(remove=[ORDER_VAR]),
'results': list(results(cl))}
@register.inclusion_tag('admin/date_hierarchy.html')
View
81 django/contrib/admin/views/main.py
@@ -3,6 +3,7 @@
from django.core.exceptions import SuspiciousOperation
from django.core.paginator import InvalidPage
from django.db import models
+from django.utils.datastructures import SortedDict
from django.utils.encoding import force_unicode, smart_str
from django.utils.translation import ugettext, ugettext_lazy
from django.utils.http import urlencode
@@ -75,7 +76,7 @@ def __init__(self, request, model, list_display, list_display_links,
self.list_editable = ()
else:
self.list_editable = list_editable
- self.order_field, self.order_type = self.get_ordering()
+ self.ordering = self.get_ordering()
self.query = request.GET.get(SEARCH_VAR, '')
self.query_set = self.get_query_set(request)
self.get_results(request)
@@ -166,40 +167,54 @@ def get_results(self, request):
def get_ordering(self):
lookup_opts, params = self.lookup_opts, self.params
# For ordering, first check the "ordering" parameter in the admin
- # options, then check the object's default ordering. If neither of
- # those exist, order descending by ID by default. Finally, look for
- # manually-specified ordering from the query string.
- ordering = self.model_admin.ordering or lookup_opts.ordering or ['-' + lookup_opts.pk.name]
+ # 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
- if ordering[0].startswith('-'):
- order_field, order_type = ordering[0][1:], 'desc'
- else:
- order_field, order_type = ordering[0], 'asc'
if ORDER_VAR in params:
- try:
- field_name = self.list_display[int(params[ORDER_VAR])]
+ # Clear ordering and used params
+ ordering = []
+ order_params = params[ORDER_VAR].split('.')
+ for p in order_params:
try:
- f = lookup_opts.get_field(field_name)
- except models.FieldDoesNotExist:
- # See whether field_name is a name of a non-field
- # that allows sorting.
+ none, pfx, idx = p.rpartition('-')
+ field_name = self.list_display[int(idx)]
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)
- order_field = attr.admin_order_field
- except AttributeError:
- pass
- else:
- order_field = f.name
- except (IndexError, ValueError):
- pass # Invalid ordering specified. Just use the default.
- if ORDER_TYPE_VAR in params and params[ORDER_TYPE_VAR] in ('asc', 'desc'):
- order_type = params[ORDER_TYPE_VAR]
- return order_field, order_type
+ f = 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)
+
+ except (IndexError, ValueError):
+ continue # Invalid ordering specified, skip it.
+
+ return ordering
+
+ def get_ordering_fields(self):
+ # Returns a SortedDict of ordering fields and asc/desc
+ ordering_fields = SortedDict()
+ for o in self.ordering:
+ none, t, f = o.rpartition('-')
+ ordering_fields[f] = 'desc' if t == '-' else 'asc'
+ return ordering_fields
def get_lookup_params(self, use_distinct=False):
lookup_params = self.params.copy() # a dictionary of the query string
@@ -290,8 +305,8 @@ def get_query_set(self, request):
break
# Set ordering.
- if self.order_field:
- qs = qs.order_by('%s%s' % ((self.order_type == 'desc' and '-' or ''), self.order_field))
+ if self.ordering:
+ qs = qs.order_by(*self.ordering)
# Apply keyword searches.
def construct_search(field_name):
View
6 docs/ref/contrib/admin/index.txt
@@ -696,10 +696,10 @@ subclass::
If this isn't provided, the Django admin will use the model's default
ordering.
- .. admonition:: Note
+ .. versionchanged:: 1.4
- Django will only honor the first element in the list/tuple; any others
- will be ignored.
+ Django honors all elements in the list/tuple; before 1.4, only the first
+ was respected.
.. attribute:: ModelAdmin.paginator
View
8 docs/releases/1.4.txt
@@ -46,6 +46,14 @@ not custom filters. This has been rectified with a simple API previously
known as "FilterSpec" which was used internally. For more details, see the
documentation for :attr:`~django.contrib.admin.ModelAdmin.list_filter`.
+Multiple sort in admin interface
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The admin change list now supports sorting on multiple columns. It respects all
+elements of the :attr:`~django.contrib.admin.ModelAdmin.ordering` attribute, and
+sorting on multiple columns by clicking on headers is designed to work similarly
+to how desktop GUIs do it.
+
Tools for cryptographic signing
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
View
4 tests/regressiontests/admin_filters/tests.py
@@ -67,11 +67,11 @@ class CustomUserAdmin(UserAdmin):
class BookAdmin(ModelAdmin):
list_filter = ('year', 'author', 'contributors', 'is_best_seller', 'date_registered', 'no')
- order_by = '-id'
+ ordering = ('-id',)
class DecadeFilterBookAdmin(ModelAdmin):
list_filter = ('author', DecadeListFilterWithTitleAndParameter)
- order_by = '-id'
+ ordering = ('-id',)
class DecadeFilterBookAdminWithoutTitle(ModelAdmin):
list_filter = (DecadeListFilterWithoutTitle,)
View
14 tests/regressiontests/admin_views/models.py
@@ -243,9 +243,6 @@ class Person(models.Model):
def __unicode__(self):
return self.name
- class Meta:
- ordering = ["id"]
-
class BasePersonModelFormSet(BaseModelFormSet):
def clean(self):
for person_dict in self.cleaned_data:
@@ -259,13 +256,17 @@ class PersonAdmin(admin.ModelAdmin):
list_editable = ('gender', 'alive')
list_filter = ('gender',)
search_fields = ('^name',)
- ordering = ["id"]
save_as = True
def get_changelist_formset(self, request, **kwargs):
return super(PersonAdmin, self).get_changelist_formset(request,
formset=BasePersonModelFormSet, **kwargs)
+ def queryset(self, request):
+ # Order by a field that isn't in list display, to be able to test
+ # whether ordering is preserved.
+ return super(PersonAdmin, self).queryset(request).order_by('age')
+
class Persona(models.Model):
"""
@@ -357,6 +358,9 @@ class Media(models.Model):
class Podcast(Media):
release_date = models.DateField()
+ class Meta:
+ ordering = ('release_date',) # overridden in PodcastAdmin
+
class PodcastAdmin(admin.ModelAdmin):
list_display = ('name', 'release_date')
list_editable = ('release_date',)
@@ -795,6 +799,7 @@ class StoryAdmin(admin.ModelAdmin):
list_display_links = ('title',) # 'id' not in list_display_links
list_editable = ('content', )
form = StoryForm
+ ordering = ["-pk"]
class OtherStory(models.Model):
title = models.CharField(max_length=100)
@@ -804,6 +809,7 @@ class OtherStoryAdmin(admin.ModelAdmin):
list_display = ('id', 'title', 'content')
list_display_links = ('title', 'id') # 'id' in list_display_links
list_editable = ('content', )
+ ordering = ["-pk"]
admin.site.register(Article, ArticleAdmin)
admin.site.register(CustomArticle, CustomArticleAdmin)
View
85 tests/regressiontests/admin_views/tests.py
@@ -32,7 +32,7 @@
# local test models
from models import (Article, BarAccount, CustomArticle, EmptyModel,
- FooAccount, Gallery, GalleryAdmin, ModelWithStringPrimaryKey,
+ FooAccount, Gallery, PersonAdmin, ModelWithStringPrimaryKey,
Person, Persona, Picture, Podcast, Section, Subscriber, Vodcast,
Language, Collector, Widget, Grommet, DooHickey, FancyDoodad, Whatsit,
Category, Post, Plot, FunkyTag, Chapter, Book, Promo, WorkHour, Employee,
@@ -204,7 +204,7 @@ def testChangeListSortingCallable(self):
Ensure we can sort on a list_display field that is a callable
(column 2 is callable_year in ArticleAdmin)
"""
- response = self.client.get('/test_admin/%s/admin_views/article/' % self.urlbit, {'ot': 'asc', 'o': 2})
+ response = self.client.get('/test_admin/%s/admin_views/article/' % self.urlbit, {'o': 2})
self.assertEqual(response.status_code, 200)
self.assertTrue(
response.content.index('Oldest content') < response.content.index('Middle content') and
@@ -217,7 +217,7 @@ def testChangeListSortingModel(self):
Ensure we can sort on a list_display field that is a Model method
(colunn 3 is 'model_year' in ArticleAdmin)
"""
- response = self.client.get('/test_admin/%s/admin_views/article/' % self.urlbit, {'ot': 'dsc', 'o': 3})
+ response = self.client.get('/test_admin/%s/admin_views/article/' % self.urlbit, {'o': '-3'})
self.assertEqual(response.status_code, 200)
self.assertTrue(
response.content.index('Newest content') < response.content.index('Middle content') and
@@ -230,7 +230,7 @@ def testChangeListSortingModelAdmin(self):
Ensure we can sort on a list_display field that is a ModelAdmin method
(colunn 4 is 'modeladmin_year' in ArticleAdmin)
"""
- response = self.client.get('/test_admin/%s/admin_views/article/' % self.urlbit, {'ot': 'asc', 'o': 4})
+ response = self.client.get('/test_admin/%s/admin_views/article/' % self.urlbit, {'o': '4'})
self.assertEqual(response.status_code, 200)
self.assertTrue(
response.content.index('Oldest content') < response.content.index('Middle content') and
@@ -238,6 +238,81 @@ def testChangeListSortingModelAdmin(self):
"Results of sorting on ModelAdmin method are out of order."
)
+ def testChangeListSortingMultiple(self):
+ p1 = Person.objects.create(name="Chris", gender=1, alive=True)
+ p2 = Person.objects.create(name="Chris", gender=2, alive=True)
+ p3 = Person.objects.create(name="Bob", gender=1, alive=True)
+ link = '<a href="%s/'
+
+ # Sort by name, gender
+ # This hard-codes the URL because it'll fail if it runs against the
+ # 'admin2' custom admin (which doesn't have the Person model).
+ response = self.client.get('/test_admin/admin/admin_views/person/', {'o': '1.2'})
+ self.assertEqual(response.status_code, 200)
+ self.assertTrue(
+ response.content.index(link % p3.id) < response.content.index(link % p1.id) and
+ response.content.index(link % p1.id) < response.content.index(link % p2.id)
+ )
+
+ # Sort by gender descending, name
+ response = self.client.get('/test_admin/admin/admin_views/person/', {'o': '-2.1'})
+ self.assertEqual(response.status_code, 200)
+ self.assertTrue(
+ response.content.index(link % p2.id) < response.content.index(link % p3.id) and
+ response.content.index(link % p3.id) < response.content.index(link % p1.id)
+ )
+
+ def testChangeListSortingPreserveQuerySetOrdering(self):
+ # If no ordering on ModelAdmin, or query string, the underlying order of
+ # the queryset should not be changed.
+
+ p1 = Person.objects.create(name="Amy", gender=1, alive=True, age=80)
+ p2 = Person.objects.create(name="Bob", gender=1, alive=True, age=70)
+ p3 = Person.objects.create(name="Chris", gender=2, alive=False, age=60)
+ link = '<a href="%s/'
+
+ # This hard-codes the URL because it'll fail if it runs against the
+ # 'admin2' custom admin (which doesn't have the Person model).
+ response = self.client.get('/test_admin/admin/admin_views/person/', {})
+ self.assertEqual(response.status_code, 200)
+ self.assertTrue(
+ response.content.index(link % p3.id) < response.content.index(link % p2.id) and
+ response.content.index(link % p2.id) < response.content.index(link % p1.id)
+ )
+
+ def testChangeListSortingModelMeta(self):
+ # Test ordering on Model Meta is respected
+
+ l1 = Language.objects.create(iso='ur', name='Urdu')
+ l2 = Language.objects.create(iso='ar', name='Arabic')
+ link = '<a href="%s/'
+
+ response = self.client.get('/test_admin/admin/admin_views/language/', {})
+ self.assertEqual(response.status_code, 200)
+ self.assertTrue(
+ response.content.index(link % l2.pk) < response.content.index(link % l1.pk)
+ )
+
+ # Test we can override with query string
+ response = self.client.get('/test_admin/admin/admin_views/language/', {'o':'-1'})
+ self.assertEqual(response.status_code, 200)
+ self.assertTrue(
+ response.content.index(link % l1.pk) < response.content.index(link % l2.pk)
+ )
+
+ def testChangeListSortingModelAdmin(self):
+ # Test ordering on Model Admin is respected, and overrides Model Meta
+ 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)
+ )
+
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').
@@ -1956,7 +2031,7 @@ def test_default_redirect(self):
'action' : 'external_mail',
'index': 0,
}
- url = '/test_admin/admin/admin_views/externalsubscriber/?ot=asc&o=1'
+ url = '/test_admin/admin/admin_views/externalsubscriber/?o=1'
response = self.client.post(url, action_data)
self.assertRedirects(response, url)
Please sign in to comment.
Something went wrong with that request. Please try again.