made it compatible with the old ordering template and added rendering tests for them

rest_framework/filters.py

@@ -275,33 +275,39 @@ class OrderingFilter(BaseFilterBackend):
         return queryset
 
     def get_template_context(self, request, queryset, view):
-        current = self.get_ordering(request, queryset, view)
+        # current and options should be deleted with the next major update
-        if not current:
+        currents = self.get_ordering(request, queryset, view)
-            current = None
+        current = None if not currents else currents[0]
         options = []
+        options_plus = []
         context = {
             'request': request,
             'current': current,
+            'currents': currents,
             'param': self.ordering_param,
         }
         for key, label in self.get_valid_fields(queryset, view, context):
-            options.append((key, '%s - %s' % (label, _('ascending')), *self.plus_key(current, key)))
+            for sign, order_label in [('', _('ascending')), ('-', _('descending'))]:
-            options.append(('-' + key, '%s - %s' % (label, _('descending')), *self.plus_key(current, '-' + key)))
+                item = (sign + key, '%s - %s' % (label, order_label))
+                options.append(item)
+                options_plus.append(item + self.plus_key(currents, sign + key))
+
         context['options'] = options
+        context['options_plus'] = options_plus
         return context
 
     @staticmethod
-    def plus_key(current, key):
+    def plus_key(currents, key):
         priority = None
         plus_key = None
-        if current:
+        if currents:
-            if len(current) > 1:
+            if len(currents) > 1:
                 try:
-                    priority = current.index(key) + 1
+                    priority = currents.index(key) + 1
                 except ValueError:
                     pass
 
-            for_plus = current.copy()
+            for_plus = currents.copy()
             if key in for_plus:
                 # for click on minus
                 for_plus.remove(key)

rest_framework/static/rest_framework/css/default.css

@@ -87,4 +87,4 @@ pre {
 
 #filtersModal .list-group-item:hover .glyphicon-plus[data-plus-ordering] {
     visibility: visible;
-}
+}

rest_framework/templates/rest_framework/filters/ordering.html

@@ -2,8 +2,8 @@
 {% load i18n %}
 <h2>{% trans "Ordering" %}</h2>
 <div class="list-group">
-    {% for key, label, priority, plus_key in options %}
+    {% for key, label, priority, plus_key in options_plus %}
-        {% if key in current %}
+        {% if key in currents %}
             <a href="{% add_query_param request param key %}" class="list-group-item active">
                 <span class="glyphicon glyphicon-minus" style="float: right" aria-hidden="true"
                       data-plus-ordering="{% if plus_key %}{% add_query_param request param plus_key %}{% else %}{% remove_query_param request param %}{% endif %}"></span>
@@ -14,7 +14,7 @@
             </a>
         {% else %}
             <a href="{% add_query_param request param key %}" class="list-group-item">
-                {% if current %}
+                {% if currents %}
                     <span class="glyphicon glyphicon-plus" style="float: right" aria-hidden="true"
                           data-plus-ordering="{% add_query_param request param plus_key %}"></span>
                 {% endif %}

tests/test_filters.py

@@ -6,12 +6,15 @@ from django.core.exceptions import ImproperlyConfigured
 from django.db import models
 from django.db.models import CharField, Transform
 from django.db.models.functions import Concat, Upper
+from django.template import Context, Template
 from django.test import TestCase
 from django.test.utils import override_settings
 
 from rest_framework import filters, generics, serializers
 from rest_framework.compat import coreschema
+from rest_framework.request import Request
 from rest_framework.test import APIRequestFactory
+from rest_framework.viewsets import ViewSet
 
 factory = APIRequestFactory()
 
@@ -712,43 +715,97 @@ class OrderingFilterTests(TestCase):
             view(request)
 
 
+class OrderingViewSet(ViewSet):
+    ordering_fields = [
+        ('id', 'ID'),
+        ('slug', 'Slug'),
+        ('author', 'Author'),
+    ]
+
+
+oldOrderingTemplate = Template("""
+{% load rest_framework %}
+{% load i18n %}
+<h2>{% trans "Ordering" %}</h2>
+<div class="list-group">
+    {% for key, label in options %}
+        {% if key == current %}
+            <a href="{% add_query_param request param key %}" class="list-group-item active">
+                <span class="glyphicon glyphicon-ok" style="float: right" aria-hidden="true"></span> {{ label }}
+            </a>
+        {% else %}
+            <a href="{% add_query_param request param key %}" class="list-group-item">{{ label }}</a>
+        {% endif %}
+    {% endfor %}
+</div>
+""")
+
+
 class OrderingFilterPlusGlyphTests(TestCase):
-    def setUp(self):
+    def create_objects(self, currents):
-        self.filter = filters.OrderingFilter()
+        self.ordering_filter = filters.OrderingFilter()
+        self.request = Request(factory.get('/', {'ordering': ','.join(currents)} if currents else None))
+        self.view = OrderingViewSet()
+
+    def assertPlusResult(self, currents, expected):
+        self.create_objects(currents)
+
+        context = self.ordering_filter.get_template_context(self.request, None, self.view)
+        for key, expected_prio_and_plus_key in expected.items():
+            prio_and_plus_key = [x[2:] for x in context['options_plus'] if x[0] == key][0]
+            self.assertEqual(prio_and_plus_key, expected_prio_and_plus_key)
+
+        self.assertOldTemplateWillRenderProperly(context)
+
+    def assertOldTemplateWillRenderProperly(self, context_dict):
+        context = Context(context_dict)
+        result = oldOrderingTemplate.render(context)
+
+        for key, label in context_dict['options']:
+            self.assertIn('ordering=' + key, result)
+            self.assertIn(label, result)
+
+        if context_dict['current']:
+            self.assertIn('class="glyphicon glyphicon-ok"', result)
 
     def test_no_ordering_defined(self):
-        self.assertEqual((None, None), self.filter.plus_key(None, 'id'))
+        current_ordering = None
-        self.assertEqual((None, None), self.filter.plus_key(None, '-id'))
+        expected = {
+            'id': (None, None),
+            '-id': (None, None),
+        }
+        self.assertPlusResult(current_ordering, expected)
 
-    def test_one_ordering_param_same_key(self):
+    def test_one_ordering_param(self):
-        # there is no priority output at all
+        current_ordering = ['id']
-        self.assertEqual((None, None), self.filter.plus_key(['id'], 'id'))
+        expected = {
-        self.assertEqual((None, None), self.filter.plus_key(['-id'], '-id'))
+            'id': (None, None),
+            '-id': (None, '-id'),
+            'slug': (None, 'id,slug'),
+            '-slug': (None, 'id,-slug'),
+        }
+        self.assertPlusResult(current_ordering, expected)
 
-        self.assertEqual((None, '-id'), self.filter.plus_key(['id'], '-id'))
+        current_ordering = ['-id']
-        self.assertEqual((None, 'id'), self.filter.plus_key(['-id'], 'id'))
+        expected = {
+            'id': (None, 'id'),
+            '-id': (None, None),
+            'slug': (None, '-id,slug'),
+            '-slug': (None, '-id,-slug'),
+        }
+        self.assertPlusResult(current_ordering, expected)
 
-    def test_one_ordering_param_different_key(self):
+    def test_two_ordering_params(self):
-        # there is no priority output at all
+        current_ordering = ['id', 'slug']
-        self.assertEqual((None, 'id,slug'), self.filter.plus_key(['id'], 'slug'))
+        expected = {
-        self.assertEqual((None, 'id,-slug'), self.filter.plus_key(['id'], '-slug'))
+            'id': (1, 'slug'),
-
+            '-id': (None, 'slug,-id'),
-        self.assertEqual((None, '-id,slug'), self.filter.plus_key(['-id'], 'slug'))
+            'slug': (2, 'id'),
-        self.assertEqual((None, '-id,-slug'), self.filter.plus_key(['-id'], '-slug'))
+            '-slug': (None, 'id,-slug'),
-
+            'author': (None, 'id,slug,author'),
-    def test_two_ordering_param_exist_key(self):
+            '-author': (None, 'id,slug,-author'),
-        self.assertEqual((1, 'slug'), self.filter.plus_key(['id', 'slug'], 'id'))
+        }
-        self.assertEqual((None, 'slug,-id'), self.filter.plus_key(['id', 'slug'], '-id'))
+        self.assertPlusResult(current_ordering, expected)
-
-        self.assertEqual((2, 'id'), self.filter.plus_key(['id', 'slug'], 'slug'))
-        self.assertEqual((None, 'id,-slug'), self.filter.plus_key(['id', 'slug'], '-slug'))
-
-        self.assertEqual((None, 'id,-slug'), self.filter.plus_key(['id', 'slug'], '-slug'))
-        self.assertEqual((2, 'id'), self.filter.plus_key(['id', 'slug'], 'slug'))
-
-    def test_two_ordering_param_other_key(self):
-        self.assertEqual((None, 'id,slug,author'), self.filter.plus_key(['id', 'slug'], 'author'))
-        self.assertEqual((None, 'id,slug,-author'), self.filter.plus_key(['id', 'slug'], '-author'))
 
 
 class SensitiveOrderingFilterModel(models.Model):