Rework ordering into a filter

django_filters/filters.py

@@ -2,6 +2,7 @@
 from __future__ import unicode_literals
 
 import warnings
+from collections import OrderedDict
 from datetime import timedelta
 
 from django import forms
@@ -10,14 +11,15 @@
 from django.db.models.constants import LOOKUP_SEP
 from django.conf import settings
 from django.utils import six
+from django.utils.itercompat import is_iterable
 from django.utils.timezone import now
 from django.utils.translation import ugettext_lazy as _
 
 from .fields import (
     Lookup, LookupTypeField, BaseCSVField, BaseRangeField, RangeField,
     DateRangeField, DateTimeRangeField, TimeRangeField, IsoDateTimeField
 )
-from .utils import deprecate
+from .utils import deprecate, pretty_name
 
 
 __all__ = [
@@ -39,6 +41,7 @@
     'MultipleChoiceFilter',
     'NumberFilter',
     'NumericRangeFilter',
+    'OrderingFilter',
     'RangeFilter',
     'TimeFilter',
     'TimeRangeFilter',
@@ -489,6 +492,102 @@ def __init__(self, *args, **kwargs):
         super(BaseRangeFilter, self).__init__(*args, **kwargs)
 
 
+class OrderingFilter(BaseCSVFilter, ChoiceFilter):
+    """
+    Enable queryset ordering. As an extension of ``ChoiceFilter`` it accepts
+    two additional arguments that are used to build the ordering choices.
+
+    * ``fields`` is a mapping of {model field name: parameter name}. The
+      parameter names are exposed in the choices and mask/alias the field
+      names used in the ``order_by()`` call. Similar to field ``choices``,
+      ``fields`` accepts the 'list of two-tuples' syntax that retains order.
+      ``fields`` may also just be an iterable of strings. In this case, the
+      field names simply double as the exposed parameter names.
+
+    * ``field_labels`` is an optional argument that allows you to customize
+      the display label for the corresponding parameter. It accepts a mapping
+      of {field name: human readable label}. Keep in mind that the key is the
+      field name, and not the exposed parameter name.
+
+    Additionally, you can just provide your own ``choices`` if you require
+    explicit control over the exposed options. For example, when you might
+    want to disable descending sort options.
+
+    This filter is also CSV-based, and accepts multiple ordering params. The
+    default select widget does not enable the use of this, but it is useful
+    for APIs.
+
+    """
+    descending_fmt = _('%s (descending)')
+
+    def __init__(self, *args, **kwargs):
+        """
+        ``fields`` may be either a mapping or an iterable.
+        ``field_labels`` must be a map of field names to display labels
+        """
+        fields = kwargs.pop('fields', {})
+        fields = self.normalize_fields(fields)
+        field_labels = kwargs.pop('field_labels', {})
+
+        self.param_map = {v: k for k, v in fields.items()}
+
+        if 'choices' not in kwargs:
+            kwargs['choices'] = self.build_choices(fields, field_labels)
+
+        kwargs.setdefault('label', _('Ordering'))
+        super(OrderingFilter, self).__init__(*args, **kwargs)
+
+    def get_ordering_value(self, param):
+        descending = param.startswith('-')
+        param = param[1:] if descending else param
+        field_name = self.param_map.get(param, param)
+
+        return "-%s" % field_name if descending else field_name
+
+    def filter(self, qs, value):
+        if value in EMPTY_VALUES:
+            return qs
+
+        ordering = [self.get_ordering_value(param) for param in value]
+        return qs.order_by(*ordering)
+
+    @classmethod
+    def normalize_fields(cls, fields):
+        """
+        Normalize the fields into an ordered map of {field name: param name}
+        """
+        # fields is a mapping, copy into new OrderedDict
+        if isinstance(fields, dict):
+            return OrderedDict(fields)
+
+        # convert iterable of values => iterable of pairs (field name, param name)
+        assert is_iterable(fields), \
+            "'fields' must be an iterable (e.g., a list, tuple, or mapping)."
+
+        # fields is an iterable of field names
+        assert all(isinstance(field, six.string_types) or
+                   is_iterable(field) and len(field) == 2  # may need to be wrapped in parens
+                   for field in fields), \
+            "'fields' must contain strings or (field name, param name) pairs."
+
+        return OrderedDict([
+            (f, f) if isinstance(f, six.string_types) else f for f in fields
+        ])
+
+    def build_choices(self, fields, labels):
+        ascending = [
+            (param, labels.get(field, pretty_name(param)))
+            for field, param in fields.items()
+        ]
+        descending = [
+            ('-%s' % pair[0], self.descending_fmt % pair[1])
+            for pair in ascending
+        ]
+
+        # interleave the ascending and descending choices
+        return [val for pair in zip(ascending, descending) for val in pair]
+
+
 class MethodFilter(Filter):
     """
     This filter will allow you to run a method that exists on the filterset class

django_filters/filterset.py

@@ -2,12 +2,10 @@
 from __future__ import unicode_literals
 
 import copy
-import re
 from collections import OrderedDict
 
 from django import forms
 from django.forms.forms import NON_FIELD_ERRORS
-from django.core.validators import EMPTY_VALUES
 from django.db import models
 from django.db.models.constants import LOOKUP_SEP
 from django.db.models.fields.related import ForeignObjectRel
@@ -19,8 +17,8 @@
 from .filters import (Filter, CharFilter, BooleanFilter, BaseInFilter, BaseRangeFilter,
                       ChoiceFilter, DateFilter, DateTimeFilter, TimeFilter, ModelChoiceFilter,
                       ModelMultipleChoiceFilter, NumberFilter, UUIDFilter,
-                      DurationFilter)
-from .utils import try_dbfield, get_all_model_fields, get_model_field, resolve_field, deprecate
+                      DurationFilter, OrderingFilter)
+from .utils import try_dbfield, get_all_model_fields, get_model_field, resolve_field, pretty_name, deprecate
 
 
 ORDER_BY_FIELD = 'o'
@@ -205,6 +203,11 @@ def __new__(cls, name, bases, attrs):
             raise TypeError("Meta.fields contains a field that isn't defined "
                             "on this FilterSet: {}".format(not_defined))
 
+        # check key existence instead of setdefault - prevents unnecessary filter construction
+        order_by_field = new_class._meta.order_by_field
+        if opts.order_by and order_by_field not in filters:
+            filters[order_by_field] = new_class.get_ordering_filter(opts, filters)
+
         new_class.declared_filters = declared_filters
         new_class.base_filters = filters
         return new_class
@@ -330,23 +333,6 @@ def qs(self):
                 if value is not None:  # valid & clean data
                     qs = filter_.filter(qs, value)
 
-            if self._meta.order_by:
-                order_field = self.form.fields[self._meta.order_by_field]
-                data = self.form[self._meta.order_by_field].data
-                ordered_value = None
-                try:
-                    ordered_value = order_field.clean(data)
-                except forms.ValidationError:
-                    pass
-
-                # With a None-queryset, ordering must be enforced (#84).
-                if (ordered_value in EMPTY_VALUES and
-                        self.strict == STRICTNESS.RETURN_NO_RESULTS):
-                    ordered_value = self.form.fields[self._meta.order_by_field].choices[0][0]
-
-                if ordered_value:
-                    qs = qs.order_by(*self.get_order_by(ordered_value))
-
             self._qs = qs
 
         return self._qs
@@ -357,7 +343,7 @@ def form(self):
             fields = OrderedDict([
                 (name, filter_.field)
                 for name, filter_ in six.iteritems(self.filters)])
-            fields[self._meta.order_by_field] = self.ordering_field
+
             Form = type(str('%sForm' % self.__class__.__name__),
                         (self._meta.form,), fields)
             if self._meta.together:
@@ -368,49 +354,48 @@ def form(self):
                 self._form = Form(prefix=self.form_prefix)
         return self._form
 
-    def get_ordering_field(self):
-        if self._meta.order_by:
-            if isinstance(self._meta.order_by, (list, tuple)):
-                if isinstance(self._meta.order_by[0], (list, tuple)):
-                    # e.g. (('field', 'Display name'), ...)
-                    choices = [(f[0], f[1]) for f in self._meta.order_by]
-                else:
-                    choices = []
-                    for f in self._meta.order_by:
-                        if f[0] == '-':
-                            label = _('%s (descending)' % capfirst(f[1:]))
-                        else:
-                            label = capfirst(f)
-                        choices.append((f, label))
+    @classmethod
+    def get_ordering_filter(cls, opts, filters):
+        assert not isinstance(opts.fields, dict), \
+            "'order_by' is not compatible with the 'fields' dict syntax. Use OrderingFilter instead."
+
+        def display_text(name, fltr):
+            """
+            ``name`` is the filter's attribute name on the FilterSet
+            ``text`` is the current display text, which is either the ``name``
+                     or an explicitly assigned label.
+            """
+            # TODO: use `fltr._label` in label-improvements branch
+            text = fltr.label or name.lstrip('-')
+            if name.startswith('-'):
+                text = _('%s (descending)' % text)
+
+            return pretty_name(text)
+
+        if isinstance(opts.order_by, (list, tuple)):
+
+            # e.g. (('field', 'Display name'), ...)
+            if isinstance(opts.order_by[0], (list, tuple)):
+                choices = [(f[0], f[1]) for f in opts.order_by]
+                fields = {filters.get(f[0].lstrip('-')).name: f[0] for f in opts.order_by}
+                return OrderingFilter(choices=choices, fields=fields)
+
+            # e.g. ('field1', 'field2', ...)
             else:
-                # add asc and desc field names
-                # use the filter's label if provided
-                choices = []
-                for f, fltr in self.filters.items():
-                    choices.extend([
-                        (f, fltr.label or capfirst(f)),
-                        ("-%s" % (f), _('%s (descending)' % (fltr.label or capfirst(f))))
-                    ])
-            return forms.ChoiceField(label=_("Ordering"), required=False,
-                                     choices=choices)
+                # (filter name, filter instance)
+                order_by = [(f, filters.get(f.lstrip('-'))) for f in opts.order_by]
 
-    @property
-    def ordering_field(self):
-        if not hasattr(self, '_ordering_field'):
-            self._ordering_field = self.get_ordering_field()
-        return self._ordering_field
-
-    def get_order_by(self, order_choice):
-        re_ordering_field = re.compile(r'(?P<inverse>\-?)(?P<field>.*)')
-        m = re.match(re_ordering_field, order_choice)
-        inverted = m.group('inverse')
-        filter_api_name = m.group('field')
-
-        _filter = self.filters.get(filter_api_name, None)
-
-        if _filter and filter_api_name != _filter.name:
-            return [inverted + _filter.name]
-        return [order_choice]
+                # preference filter label over attribute name
+                choices = [(f, display_text(f, fltr)) for f, fltr in order_by]
+                fields = {fltr.name: f for f, fltr in order_by}
+                return OrderingFilter(choices=choices, fields=fields)
+
+        # opts.order_by = True
+        order_by = filters.items()
+
+        fields = [(fltr.name, f) for f, fltr in order_by]
+        labels = {f: display_text(f, fltr) for f, fltr in order_by}
+        return OrderingFilter(fields=fields, field_labels=labels)
 
     @classmethod
     def filters_for_model(cls, model, opts):

django_filters/utils.py

@@ -9,6 +9,11 @@
 from django.db.models.fields.related import ForeignObjectRel
 from django.utils import six, timezone
 
+try:
+    from django.forms.utils import pretty_name
+except ImportError:  # Django 1.8
+    from django.forms.forms import pretty_name
+
 from .compat import remote_field, remote_model
 from .exceptions import FieldLookupError