diff --git a/rest_framework/filters.py b/rest_framework/filters.py index 5ee37563c..ba600c03c 100644 --- a/rest_framework/filters.py +++ b/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) - if not current: - current = None + # current and options should be deleted with the next major update + currents = self.get_ordering(request, queryset, view) + 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))) - options.append(('-' + key, '%s - %s' % (label, _('descending')), *self.plus_key(current, '-' + key))) + for sign, order_label in [('', _('ascending')), ('-', _('descending'))]: + 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 len(current) > 1: + if currents: + 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) diff --git a/rest_framework/static/rest_framework/css/default.css b/rest_framework/static/rest_framework/css/default.css index 99fca1f15..e0c85076c 100644 --- a/rest_framework/static/rest_framework/css/default.css +++ b/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; -} \ No newline at end of file +} diff --git a/rest_framework/templates/rest_framework/filters/ordering.html b/rest_framework/templates/rest_framework/filters/ordering.html index e094c6ce9..95abd50c2 100644 --- a/rest_framework/templates/rest_framework/filters/ordering.html +++ b/rest_framework/templates/rest_framework/filters/ordering.html @@ -2,8 +2,8 @@ {% load i18n %}

{% trans "Ordering" %}

- {% for key, label, priority, plus_key in options %} - {% if key in current %} + {% for key, label, priority, plus_key in options_plus %} + {% if key in currents %} @@ -14,7 +14,7 @@ {% else %} - {% if current %} + {% if currents %} {% endif %} diff --git a/tests/test_filters.py b/tests/test_filters.py index 684add679..22dfd93ae 100644 --- a/tests/test_filters.py +++ b/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 %} +

{% trans "Ordering" %}

+
+ {% for key, label in options %} + {% if key == current %} + + {{ label }} + + {% else %} + {{ label }} + {% endif %} + {% endfor %} +
+""") + + class OrderingFilterPlusGlyphTests(TestCase): - def setUp(self): - self.filter = filters.OrderingFilter() + def create_objects(self, currents): + 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')) - self.assertEqual((None, None), self.filter.plus_key(None, '-id')) + current_ordering = None + expected = { + 'id': (None, None), + '-id': (None, None), + } + self.assertPlusResult(current_ordering, expected) - def test_one_ordering_param_same_key(self): - # there is no priority output at all - self.assertEqual((None, None), self.filter.plus_key(['id'], 'id')) - self.assertEqual((None, None), self.filter.plus_key(['-id'], '-id')) + def test_one_ordering_param(self): + current_ordering = ['id'] + expected = { + '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')) - self.assertEqual((None, 'id'), self.filter.plus_key(['-id'], 'id')) + current_ordering = ['-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): - # there is no priority output at all - self.assertEqual((None, 'id,slug'), self.filter.plus_key(['id'], 'slug')) - self.assertEqual((None, 'id,-slug'), self.filter.plus_key(['id'], '-slug')) - - self.assertEqual((None, '-id,slug'), self.filter.plus_key(['-id'], 'slug')) - self.assertEqual((None, '-id,-slug'), self.filter.plus_key(['-id'], '-slug')) - - def test_two_ordering_param_exist_key(self): - self.assertEqual((1, 'slug'), self.filter.plus_key(['id', 'slug'], 'id')) - self.assertEqual((None, 'slug,-id'), self.filter.plus_key(['id', 'slug'], '-id')) - - 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')) + def test_two_ordering_params(self): + current_ordering = ['id', 'slug'] + expected = { + 'id': (1, 'slug'), + '-id': (None, 'slug,-id'), + 'slug': (2, 'id'), + '-slug': (None, 'id,-slug'), + 'author': (None, 'id,slug,author'), + '-author': (None, 'id,slug,-author'), + } + self.assertPlusResult(current_ordering, expected) class SensitiveOrderingFilterModel(models.Model):