From 5434ce231d75004bdbe5cf2b7b24ce67a2a6e737 Mon Sep 17 00:00:00 2001 From: Luke Plant Date: Thu, 2 Jun 2011 16:18:47 +0000 Subject: [PATCH] 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 --- django/contrib/admin/media/css/base.css | 28 ++++++ django/contrib/admin/media/css/rtl.css | 8 ++ .../admin/media/img/admin/icon_cog.gif | Bin 0 -> 402 bytes .../templates/admin/change_list_results.html | 67 ++++++++++++- .../contrib/admin/templatetags/admin_list.py | 93 ++++++++++++++---- django/contrib/admin/views/main.py | 81 ++++++++------- docs/ref/contrib/admin/index.txt | 6 +- docs/releases/1.4.txt | 8 ++ tests/regressiontests/admin_filters/tests.py | 4 +- tests/regressiontests/admin_views/models.py | 14 ++- tests/regressiontests/admin_views/tests.py | 85 +++++++++++++++- 11 files changed, 323 insertions(+), 71 deletions(-) create mode 100644 django/contrib/admin/media/img/admin/icon_cog.gif diff --git a/django/contrib/admin/media/css/base.css b/django/contrib/admin/media/css/base.css index c5e385d508..26e2cbf14a 100644 --- a/django/contrib/admin/media/css/base.css +++ b/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 { diff --git a/django/contrib/admin/media/css/rtl.css b/django/contrib/admin/media/css/rtl.css index 975c035e0f..ec2bb2a9da 100644 --- a/django/contrib/admin/media/css/rtl.css +++ b/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 { diff --git a/django/contrib/admin/media/img/admin/icon_cog.gif b/django/contrib/admin/media/img/admin/icon_cog.gif new file mode 100644 index 0000000000000000000000000000000000000000..d86a9a74a9ffadab986dd358307f1c6b98b9f5ce GIT binary patch literal 402 zcmV;D0d4+ANk%w1VGsZi0Hr$sRa;?ITVhyVWm#WlV`^_@YjA0Ab!u;PYjb#Qb9!xb zdUkt(czl9;gNc5Gi+_cSe}#;IhKz!UkAjGhhmMtqkClj!my45`ke8m5nVy!LpqZVa zo1LMTps1aoqMo9qpQ5IvsjQ}{s-&v0r>wB3t+lGIvaYhXwzs*pxxTfzzqYx(wYtE% zyS})+!o0n`yS~4>zrwq~#l6ABzQM!4!NbPJ#l^?U$;!*l&dtuw&i?=a{{R6000930 z0RI30A^8LW002J#EC2ui01yBW000KBz@Knp8!aFp wEgQxu0YU*EDI?V~GS=1~(b5ko1Pj;;1St>Q#L)~aEDS#Z=t}SK@j^iWJAOf}xc~qF literal 0 HcmV?d00001 diff --git a/django/contrib/admin/templates/admin/change_list_results.html b/django/contrib/admin/templates/admin/change_list_results.html index b3fa224e6b..e916433cf2 100644 --- a/django/contrib/admin/templates/admin/change_list_results.html +++ b/django/contrib/admin/templates/admin/change_list_results.html @@ -1,3 +1,5 @@ +{% load adminmedia %} +{% load i18n %} {% if result_hidden_fields %}
{# DIV for HTML validation #} {% for item in result_hidden_fields %}{{ item }}{% endfor %} @@ -8,10 +10,18 @@ -{% for header in result_headers %}{% endfor %} +{% for header in result_headers %} +{% endfor %} @@ -24,4 +34,53 @@
-{% if header.sortable %}{% endif %} -{{ header.text|capfirst }} -{% if header.sortable %}{% endif %} + {% if header.sortable %}{% endif %} + {{ header.text|capfirst }} + {% if header.sortable %} + {% if header.sort_pos > 0 %} + {% if header.sort_pos == 1 %} {% endif %} + {{ header.sort_pos }} + {% endif %} + + {% endif %} +
+ +{# Sorting popup: #} + + + {% endif %} diff --git a/django/contrib/admin/templatetags/admin_list.py b/django/contrib/admin/templatetags/admin_list.py index b72c0be431..884b45051a 100644 --- a/django/contrib/admin/templatetags/admin_list.py +++ b/django/contrib/admin/templatetags/admin_list.py @@ -7,6 +7,7 @@ from django.contrib.admin.views.main import (ALL_VAR, EMPTY_CHANGELIST_VALUE, 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') diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 50d576b883..1d48140fc5 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -3,6 +3,7 @@ import operator 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 @@ class ChangeList(object): 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 @@ class ChangeList(object): 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 @@ class ChangeList(object): 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): diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index e5aecb4622..3ba3b1b0eb 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/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 diff --git a/docs/releases/1.4.txt b/docs/releases/1.4.txt index 2a2090a9b1..d0c02b0036 100644 --- a/docs/releases/1.4.txt +++ b/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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/regressiontests/admin_filters/tests.py b/tests/regressiontests/admin_filters/tests.py index cccfe34077..5095abbb9f 100644 --- a/tests/regressiontests/admin_filters/tests.py +++ b/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,) diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py index 854fb60e70..98873966eb 100644 --- a/tests/regressiontests/admin_views/models.py +++ b/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) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 7c6cdd4944..4361dc992e 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -32,7 +32,7 @@ from django.utils import unittest # 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 @@ class AdminViewBasicTest(TestCase): 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 @@ class AdminViewBasicTest(TestCase): 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 @@ class AdminViewBasicTest(TestCase): 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 @@ class AdminViewBasicTest(TestCase): "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 = '