From 4c5e00e397480d3eda1d08bb4295fc2e07656b40 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Mon, 2 Sep 2019 20:20:08 +0300 Subject: [PATCH 01/10] Add OrderingFilterTests.test_ordering_with_nulls to lock down ordering behavior regarding nulls --- tests/models.py | 4 ++-- tests/test_filtering.py | 37 +++++++++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/tests/models.py b/tests/models.py index 9a6f1040a..4831f9479 100644 --- a/tests/models.py +++ b/tests/models.py @@ -41,8 +41,8 @@ def formfield(self, **kwargs): class User(models.Model): username = models.CharField(_('username'), max_length=255) - first_name = SubCharField(max_length=100) - last_name = SubSubCharField(max_length=100) + first_name = SubCharField(max_length=100, null=True, blank=True) + last_name = SubSubCharField(max_length=100, null=True, blank=True) status = models.IntegerField(choices=STATUS_CHOICES, default=0) diff --git a/tests/test_filtering.py b/tests/test_filtering.py index 18f2b4821..dbbe733e8 100644 --- a/tests/test_filtering.py +++ b/tests/test_filtering.py @@ -1871,10 +1871,10 @@ def test_filtering(self): class OrderingFilterTests(TestCase): def setUp(self): - User.objects.create(username='alex', status=1) - User.objects.create(username='jacob', status=2) - User.objects.create(username='aaron', status=2) - User.objects.create(username='carl', status=0) + User.objects.create(username='alex', first_name='Alex', last_name='Allan', status=1) + User.objects.create(username='jacob', first_name='Jacob', last_name='Johnson', status=2) + User.objects.create(username='aaron', first_name='Aaron', last_name='Barrett', status=2) + User.objects.create(username='carl', first_name=None, last_name='Jung', status=0) def test_ordering(self): class F(FilterSet): @@ -1907,6 +1907,35 @@ class Meta: names = f.qs.values_list('username', flat=True) self.assertEqual(list(names), ['aaron', 'alex', 'carl', 'jacob']) + def test_ordering_with_null(self): + class F(FilterSet): + o = OrderingFilter( + fields=('first_name', 'last_name') + ) + + class Meta: + model = User + fields = ['first_name', 'last_name'] + + qs = User.objects.all() + f = F({'o': 'first_name,last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + (None, 'Jung'), + ('Aaron', 'Barrett'), + ('Alex', 'Allan'), + ('Jacob', 'Johnson'), + ]) + + f = F({'o': '-first_name,-last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + ('Jacob', 'Johnson'), + ('Alex', 'Allan'), + ('Aaron', 'Barrett'), + (None, 'Jung'), + ]) + class MiscFilterSetTests(TestCase): From 5d781a2015861f4b06e88dfcb8a2bed17f076b47 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Mon, 2 Sep 2019 20:20:17 +0300 Subject: [PATCH 02/10] Initial push for reworked OrderingFilter Closes https://github.com/carltongibson/django-filter/issues/1021 --- django_filters/filters.py | 269 +++++++++++++++++++++++++++++--------- tests/test_filtering.py | 94 +++++++++++++ tests/test_filters.py | 183 +++++++++++++++++++++++++- 3 files changed, 484 insertions(+), 62 deletions(-) diff --git a/django_filters/filters.py b/django_filters/filters.py index 8f3bd4cb7..e8f1a550b 100644 --- a/django_filters/filters.py +++ b/django_filters/filters.py @@ -1,8 +1,11 @@ -from collections import OrderedDict +import collections +import copy +import itertools from datetime import timedelta from django import forms from django.db.models import Q +from django.db.models.expressions import F, OrderBy from django.db.models.constants import LOOKUP_SEP from django.forms.utils import pretty_name from django.utils.itercompat import is_iterable @@ -26,7 +29,7 @@ RangeField, TimeRangeField ) -from .utils import get_model_field, label_for_filter +from .utils import deprecate, get_model_field, label_for_filter __all__ = [ 'AllValuesFilter', @@ -306,8 +309,8 @@ def departments(request): company = request.user.company return company.department_set.all() - class EmployeeFilter(filters.FilterSet): - department = filters.ModelChoiceFilter(queryset=departments) + class EmployeeFilter(FilterSet): + department = ModelChoiceFilter(queryset=departments) ... The above example restricts the set of departments to those in the logged-in @@ -567,7 +570,7 @@ class LookupChoiceFilter(Filter): ex:: - price = django_filters.LookupChoiceFilter( + price = django_LookupChoiceFilter( field_class=forms.DecimalField, lookup_choices=[ ('exact', 'Equals'), @@ -651,100 +654,248 @@ def filter(self, qs, lookup): 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. + 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. + * ``params`` is a mapping of {param name: ordering descriptor}. + param name is exposed in the choices. Ordering descriptor is + an object containing the following fields: + + ** ``expr`` - model field name or a Django expression + (see https://docs.djangoproject.com/en/latest/ref/models/expressions/). + If neither ``expr`` nor ``exprs`` is specified, ``AssertionError`` is raised. + + ** ``exprs`` - an iterable of model field names or Django expressions + (see https://docs.djangoproject.com/en/latest/ref/models/expressions/). + If neither ``expr`` nor ``exprs`` are specified, ``AssertionError`` is raised. + + ** ``label`` - (optional) customized display label for the corresponding + parameter. If this field is omitted, the label will be derived + from param name. + + ** ``desc_label`` - (optional) customized display label for the + corresponding parameter used for descending search. If this field + is omitted, ``desc_label`` will be derived from ``label``. + + Instead of a complete object, you can provide a simplified ordering + descriptor which can be one of the following: + + ** model field name or a Django expression + + ** an iterable of model field names or Django expressions + + Instead of a mapping, ``params`` also accepts: + + ** the 'list of two-tuples' syntax that retains order + + ** an iterable of strings. In this case model field names are derived + from the corresponding param names. + + * ``fields`` is a mapping of {model field name: param name}, which + can also be expressed in the 'list of two-tuples' syntax, or an iterable + of strings. It is basically an inverted version of ``params`` where + model field name and param name are swapped, which does not allow + exposing several model fields as a single parameter and ordering by + Django expressions. + + This argument is DEPRECATED. You should always use the ``params`` argument. + Using both ``fields`` and ``params`` arguments will result in ``AssertionError``. * ``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. + display labels for ``fields``. It accepts a mapping of + {model field name: human readable label}. Keep in mind that the key is the + model field name, and not the exposed param 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. + Similar to the ``fields`` argument, ``field_labels`` is DEPRECATED. + You should always use the ``params`` argument where a customized + display label can be put into ``label`` field of the corresponding + ordering descriptors. Using both ``field_labels`` and ``params`` + arguments will result in ``AssertionError`` + + * ``choices`` is an optional argument that allows for explicit control + over the exposed options. For example, you can use this argument + 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', {}) + if 'params' in kwargs: + assert 'fields' not in kwargs, "'params' and 'fields' cannot be passed simultaneously" + assert 'field_labels' not in kwargs, "'params' and 'field_labels' cannot be passed simultaneously" + + if 'fields' in kwargs: + deprecate("`fields` argument of OrderingFilter constructor is deprecated in favor of `params`") + if 'field_labels' in kwargs: + deprecate("`field_labels` argument of OrderingFilter constructor is deprecated in favor of `params`") + + if 'params' not in kwargs: + fields = kwargs.get('fields', {}) + fields = self.normalize_fields(fields) + field_labels = kwargs.get('field_labels', {}) + params = self.fields_to_params(fields, field_labels) + else: + params = self.normalize_params(kwargs['params']) - self.param_map = {v: k for k, v in fields.items()} + self._params = params if 'choices' not in kwargs: - kwargs['choices'] = self.build_choices(fields, field_labels) + kwargs['choices'] = self.build_choices(params) + kwargs.pop('fields', None) + kwargs.pop('field_labels', None) + kwargs.pop('params', None) kwargs.setdefault('label', _('Ordering')) kwargs.setdefault('help_text', '') kwargs.setdefault('null_label', None) super().__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) + @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 collections.OrderedDict + if isinstance(fields, dict): + fields = collections.OrderedDict(fields) + else: + + # 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)." - return "-%s" % field_name if descending else field_name + # fields is an iterable of field names + assert all(isinstance(field, str) 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." - def filter(self, qs, value): - if value in EMPTY_VALUES: - return qs + fields = collections.OrderedDict((f, f) if isinstance(f, str) else f for f in fields) + return fields + + @classmethod + def fields_to_params(cls, fields, field_labels): + """ + Convert normalized fields of and field labels into normalized params - ordering = [self.get_ordering_value(param) for param in value] - return qs.order_by(*ordering) + Args: + fields: Normalized fields of {model field name: param name} + field_labels: Field labels of {model field name: label} + Returns: + Normalized params of {model field name: ordering descriptor} + """ + lst = [] + for model_field_name, param_name in fields.items(): + descriptor = {"exprs": (F(model_field_name),)} + if model_field_name in field_labels: + descriptor["label"] = field_labels[model_field_name] + if "-%s" % model_field_name in field_labels: + descriptor["desc_label"] = field_labels["-%s" % model_field_name] + lst.append((param_name, descriptor)) + return collections.OrderedDict(lst) @classmethod - def normalize_fields(cls, fields): + def normalize_params(cls, params): """ - Normalize the fields into an ordered map of {field name: param name} + Normalize the params into an ordered map of {model field name: ordering descriptor} """ - # fields is a mapping, copy into new OrderedDict - if isinstance(fields, dict): - return OrderedDict(fields) + # params is a mapping, copy into new collections.OrderedDict + if isinstance(params, dict): + params = collections.OrderedDict(params) + else: - # 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)." + # convert iterable of values => iterable of pairs (param name, model field name) + assert is_iterable(params), \ + "'params' must be an iterable (e.g., a list, tuple, or mapping)." - # fields is an iterable of field names - assert all(isinstance(field, str) 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." + # params is an iterable of field names + assert all(isinstance(param, str) or + is_iterable(param) and len(param) == 2 + for param in params), \ + "'params' must contain strings or (param name, model field name or object descriptor) pairs." - return OrderedDict([ - (f, f) if isinstance(f, str) else f for f in fields - ]) + params = collections.OrderedDict((f, f) if isinstance(f, str) else f for f in params) - def build_choices(self, fields, labels): + for param_name, descriptor in params.items(): + params[param_name] = cls.normalize_ordering_descriptor(descriptor) + return params + + @classmethod + def normalize_ordering_descriptor(cls, descriptor): + if isinstance(descriptor, str): + # Model field name + return { + "exprs": [F(descriptor)] + } + + if isinstance(descriptor, collections.Mapping): + # An ordering descriptor. Let's normalize it. + descriptor = copy.copy(descriptor) + if 'expr' in descriptor: + assert 'exprs' not in descriptor, \ + "'expr' and 'exprs' cannot be specified simultaneously" + descriptor['exprs'] = (descriptor['expr'],) + del descriptor['expr'] + assert 'exprs' in descriptor, \ + "'expr' or 'exprs' must be specified in the ordering descriptor" + descriptor["exprs"] = [F(field) if isinstance(field, str) else field for field in descriptor["exprs"]] + return descriptor + + if isinstance(descriptor, collections.Sequence): + # A sequence of model field names + return { + "exprs": [F(field) if isinstance(field, str) else field for field in descriptor] + } + + # Assume a Django expression + return { + "exprs": [descriptor] + } + + def build_choices(self, params): + """ + Build choices from params + + Args: + params: Normalized params of {model_field_name: ordering desciptor} + Returns: + List of choices + """ ascending = [ - (param, labels.get(field, _(pretty_name(param)))) - for field, param in fields.items() + (param_name, descriptor.get('label', _(pretty_name(param_name)))) + for param_name, descriptor in params.items() ] descending = [ - ('-%s' % param, labels.get('-%s' % param, self.descending_fmt % label)) - for param, label in ascending + ('-%s' % param_name, descriptor.get('desc_label', self.descending_fmt % label)) + for (param_name, descriptor), (param_name, label) in zip(params.items(), ascending) ] # interleave the ascending and descending choices return [val for pair in zip(ascending, descending) for val in pair] + def filter(self, qs, value): + if value in EMPTY_VALUES: + return qs + return qs.order_by(*itertools.chain(*(self.get_ordering_exprs(param) for param in value))) + + def get_ordering_exprs(self, param_name): + descending = param_name.startswith('-') + param_name = param_name[1:] if descending else param_name + descriptor = self._params.get(param_name) + # For backward compatibility order by param_name if descriptor is not found + exprs = descriptor['exprs'] if descriptor is not None else (F(param_name),) + if descending: + return map(self.reverse_ordering, exprs) + return exprs + + @classmethod + def reverse_ordering(cls, expr): + if isinstance(expr, OrderBy): + return expr.reverse_ordering() + return expr.desc() + class FilterMethod: """ diff --git a/tests/test_filtering.py b/tests/test_filtering.py index dbbe733e8..0d73e76bd 100644 --- a/tests/test_filtering.py +++ b/tests/test_filtering.py @@ -5,6 +5,7 @@ from operator import attrgetter from django import forms +from django.db.models import expressions from django.http import QueryDict from django.test import TestCase, override_settings from django.utils import timezone @@ -1936,6 +1937,99 @@ class Meta: (None, 'Jung'), ]) + def test_ordering_with_params_and_null(self): + class F(FilterSet): + o = OrderingFilter( + params=('first_name', 'last_name') + ) + + class Meta: + model = User + fields = ['first_name', 'last_name'] + + qs = User.objects.all() + f = F({'o': 'first_name,last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + (None, 'Jung'), + ('Aaron', 'Barrett'), + ('Alex', 'Allan'), + ('Jacob', 'Johnson'), + ]) + + f = F({'o': '-first_name,-last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + ('Jacob', 'Johnson'), + ('Alex', 'Allan'), + ('Aaron', 'Barrett'), + (None, 'Jung'), + ]) + + def test_ordering_with_params_and_nulls_last(self): + class F(FilterSet): + o = OrderingFilter( + params={ + 'first_name': {'expr': expressions.F('first_name').asc(nulls_last=True)}, + 'last_name': {'expr': expressions.F('last_name').asc(nulls_last=True)} + } + ) + + class Meta: + model = User + fields = ['first_name', 'last_name'] + + qs = User.objects.all() + f = F({'o': 'first_name,last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + ('Aaron', 'Barrett'), + ('Alex', 'Allan'), + ('Jacob', 'Johnson'), + (None, 'Jung'), + ]) + + f = F({'o': '-first_name,-last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + (None, 'Jung'), + ('Jacob', 'Johnson'), + ('Alex', 'Allan'), + ('Aaron', 'Barrett'), + ]) + + def test_ordering_with_params_and_desc_nulls_last(self): + class F(FilterSet): + o = OrderingFilter( + params={ + 'first_name': {'expr': expressions.F('first_name').desc(nulls_last=True)}, + 'last_name': {'expr': expressions.F('last_name').desc(nulls_last=True)} + } + ) + + class Meta: + model = User + fields = ['first_name', 'last_name'] + + qs = User.objects.all() + f = F({'o': 'first_name,last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + ('Jacob', 'Johnson'), + ('Alex', 'Allan'), + ('Aaron', 'Barrett'), + (None, 'Jung'), + ]) + + f = F({'o': '-first_name,-last_name'}, queryset=qs) + names = f.qs.values_list('first_name', 'last_name') + self.assertEqual(list(names), [ + (None, 'Jung'), + ('Aaron', 'Barrett'), + ('Alex', 'Allan'), + ('Jacob', 'Johnson'), + ]) + class MiscFilterSetTests(TestCase): diff --git a/tests/test_filters.py b/tests/test_filters.py index d63074d7f..1238670c4 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -2,8 +2,10 @@ import mock from collections import OrderedDict from datetime import date, datetime, time, timedelta +import warnings from django import forms +from django.db.models.expressions import F, OrderBy from django.test import TestCase, override_settings from django.utils import translation from django.utils.translation import gettext as _ @@ -1433,19 +1435,19 @@ def test_filtering(self): qs = mock.Mock(spec=['order_by']) f = OrderingFilter() f.filter(qs, ['a', 'b']) - qs.order_by.assert_called_once_with('a', 'b') + qs.order_by.assert_called_once_with(F('a'), F('b')) def test_filtering_descending(self): qs = mock.Mock(spec=['order_by']) f = OrderingFilter() f.filter(qs, ['-a']) - qs.order_by.assert_called_once_with('-a') + qs.order_by.assert_called_once_with(OrderBy(F('a'), descending=True)) def test_filtering_with_fields(self): qs = mock.Mock(spec=['order_by']) f = OrderingFilter(fields={'a': 'b'}) f.filter(qs, ['b', '-b']) - qs.order_by.assert_called_once_with('a', '-a') + qs.order_by.assert_called_once_with(F('a'), OrderBy(F('a'), descending=True)) def test_filtering_skipped_with_none_value(self): qs = mock.Mock(spec=['order_by']) @@ -1580,3 +1582,178 @@ def test_help_text(self): # regression test for #756 - the usual CSV help_text is not relevant to ordering filters. self.assertEqual(OrderingFilter().field.help_text, '') self.assertEqual(OrderingFilter(help_text='a').field.help_text, 'a') + + def test_fields_argument_is_deprecated(self): + with mock.patch.object(warnings, 'warn', spec=['__call__']) as mocked: + OrderingFilter( + fields=['username'], + field_labels={'username': 'BLABLA'}, + ) + msg1 = "`fields` argument of OrderingFilter constructor is deprecated in favor of `params`" + msg2 = "`field_labels` argument of OrderingFilter constructor is deprecated in favor of `params`" + mocked.assert_has_calls([ + (msg1, mock.ANY, mock.ANY), + (msg2, mock.ANY, mock.ANY) + ]) + + def test_fields_and_params(self): + # if `fields` and `params` are passed together, `AssertionError` should be raised + msg = "'params' and 'fields' cannot be passed simultaneously" + with self.assertRaisesMessage(AssertionError, msg): + OrderingFilter( + fields=['username'], + params=['username'] + ) + + def test_field_labels_and_params(self): + # if `field_labels` and `params` are passed together, `AssertionError` should be raised + msg = "'params' and 'field_labels' cannot be passed simultaneously" + with self.assertRaisesMessage(AssertionError, msg): + OrderingFilter( + params=['username'], + field_labels={'username': 'BLABLA'} + ) + + def test_descriptor_without_exprs(self): + # if ordering descriptor does not contain 'expr' or 'exprs', `AssertionError` should be raised + msg = "'expr' or 'exprs' must be specified in the ordering descriptor" + with self.assertRaisesMessage(AssertionError, msg): + OrderingFilter( + params={'user': {}} + ) + + def test_expr_and_exprs(self): + # if both `expr` and `exprs` are specified in the ordering descriptor, `AssertionError` should be raised + msg = "'expr' and 'exprs' cannot be specified simultaneously" + with self.assertRaisesMessage(AssertionError, msg): + OrderingFilter( + params={'user': {"expr": "username", "exprs": ("username",)}} + ) + + def test_params_as_strings(self): + f = OrderingFilter( + params=['username'], + ) + self.assertEqual(f._params, OrderedDict((('username', {'exprs': [F('username')]}),))) + + def test_params_as_list_of_tuples_with_string(self): + f = OrderingFilter( + params=[('user', 'username')] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_list_of_tuples_with_expression(self): + f = OrderingFilter( + params=[('user', F('username'))] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_list_of_tuples_with_descriptor_and_string(self): + f = OrderingFilter( + params=[('user', {'expr': 'username'})] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_list_of_tuples_with_descriptor_and_expression(self): + f = OrderingFilter( + params=[('user', {'expr': F('username')})] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_list_of_tuples_with_descriptor_and_strings(self): + f = OrderingFilter( + params=[('user', {'exprs': ('username',)})] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_list_of_tuples_with_descriptor_and_expressions(self): + f = OrderingFilter( + params=[('user', {'exprs': (F('username'),)})] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_dict_with_string(self): + f = OrderingFilter( + params={'user': 'username'} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_dict_with_expression(self): + f = OrderingFilter( + params={'user': F('username')} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_dict_with_descriptor_and_string(self): + f = OrderingFilter( + params={'user': {'expr': 'username'}} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_dict_with_descriptor_and_expression(self): + f = OrderingFilter( + params={'user': {'expr': F('username')}} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_dict_with_descriptor_and_strings(self): + f = OrderingFilter( + params={'user': {'exprs': ('username',)}} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_params_as_dict_with_descriptor_and_expressions(self): + f = OrderingFilter( + params={'user': {'exprs': (F('username'),)}} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + + def test_choices_unaltered(self): + # provided 'choices' should not be altered when 'fields' is present + f = OrderingFilter( + choices=(('a', 'A'), ('b', 'B')), + fields=(('a', 'c'), ('b', 'd')), + ) + + self.assertSequenceEqual(list(f.field.choices), ( + ('', '---------'), + ('a', 'A'), + ('b', 'B'), + )) + + def test_choices_from_params(self): + f = OrderingFilter( + params=(('c', 'a'), ('d', 'b')), + ) + + self.assertSequenceEqual(list(f.field.choices), ( + ('', '---------'), + ('c', 'C'), + ('-c', 'C (descending)'), + ('d', 'D'), + ('-d', 'D (descending)'), + )) + + def test_params_labels(self): + f = OrderingFilter( + params=(('c', {'expr': 'a', 'label': 'foo'}), ('d', 'b')), + ) + + self.assertSequenceEqual(list(f.field.choices), ( + ('', '---------'), + ('c', 'foo'), + ('-c', 'foo (descending)'), + ('d', 'D'), + ('-d', 'D (descending)'), + )) + + def test_params_labels_descending(self): + f = OrderingFilter( + params=(('username', {'expr': 'username', 'label': 'BLABLA', 'desc_label': 'XYZXYZ'}),) + ) + + self.assertEqual(list(f.field.choices), [ + ('', '---------'), + ('username', 'BLABLA'), + ('-username', 'XYZXYZ'), + ]) From db1986f34f9df24fdeb5e3194a23f3f779d1ed45 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Mon, 2 Sep 2019 20:31:14 +0300 Subject: [PATCH 03/10] Resolve errors reported by lint and isort --- django_filters/filters.py | 4 ++-- tests/test_filters.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/django_filters/filters.py b/django_filters/filters.py index e8f1a550b..85d90f01f 100644 --- a/django_filters/filters.py +++ b/django_filters/filters.py @@ -1,12 +1,12 @@ import collections import copy -import itertools from datetime import timedelta +import itertools from django import forms from django.db.models import Q -from django.db.models.expressions import F, OrderBy from django.db.models.constants import LOOKUP_SEP +from django.db.models.expressions import F, OrderBy from django.forms.utils import pretty_name from django.utils.itercompat import is_iterable from django.utils.timezone import now diff --git a/tests/test_filters.py b/tests/test_filters.py index 1238670c4..1d57668ea 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -1,7 +1,7 @@ -import inspect -import mock from collections import OrderedDict from datetime import date, datetime, time, timedelta +import inspect +import mock import warnings from django import forms @@ -1708,11 +1708,11 @@ def test_params_as_dict_with_descriptor_and_expressions(self): ) self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) - def test_choices_unaltered(self): - # provided 'choices' should not be altered when 'fields' is present + def test_choices_unaltered_with_params(self): + # provided 'choices' should not be altered when 'params' is present f = OrderingFilter( choices=(('a', 'A'), ('b', 'B')), - fields=(('a', 'c'), ('b', 'd')), + params=(('c', 'a'), ('d', 'b')), ) self.assertSequenceEqual(list(f.field.choices), ( From e21ba2d5ecf14d03fe3000b2c98fd5f9a525498a Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Mon, 2 Sep 2019 20:42:42 +0300 Subject: [PATCH 04/10] Run isort --- django_filters/filters.py | 2 +- tests/test_filters.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/django_filters/filters.py b/django_filters/filters.py index 85d90f01f..76c087f5b 100644 --- a/django_filters/filters.py +++ b/django_filters/filters.py @@ -1,7 +1,7 @@ import collections import copy -from datetime import timedelta import itertools +from datetime import timedelta from django import forms from django.db.models import Q diff --git a/tests/test_filters.py b/tests/test_filters.py index 1d57668ea..544754815 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -1,8 +1,8 @@ -from collections import OrderedDict -from datetime import date, datetime, time, timedelta import inspect import mock import warnings +from collections import OrderedDict +from datetime import date, datetime, time, timedelta from django import forms from django.db.models.expressions import F, OrderBy From 25e130cd344554ae9138d46ff5717e2e9ce724fb Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Mon, 2 Sep 2019 21:15:18 +0300 Subject: [PATCH 05/10] Fix tests under Django 1.11 by defining custom F.__eq__ and OrderBy.__eq__ operators --- tests/test_filters.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_filters.py b/tests/test_filters.py index 544754815..dcec3f3da 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -1426,6 +1426,37 @@ class NumberInFilter(BaseRangeFilter, NumberFilter): class OrderingFilterTests(TestCase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + + # Django 1.11 does not define __eq__ operators for F and OrderBy. + # Define them manually when needed + if True or not hasattr(F, '__eq__'): + setattr(F, '__eq__', cls.compareF) + if True or not hasattr(OrderBy, '__eq__'): + setattr(OrderBy, '__eq__', cls.compareOrderBy) + + @staticmethod + def compareF(a, b): + return a.name == b.name + + @staticmethod + def compareOrderBy(a, b): + return ( + a.expression == b.expression and + a.descending == b.descending and + a.nulls_first == b.nulls_first and + a.nulls_last == b.nulls_last) + + @classmethod + def tearDownClass(cls): + if getattr(F, '__eq__') == cls.compareF: + delattr(F, '__eq__') + if getattr(OrderBy, '__eq__') == cls.compareOrderBy: + delattr(OrderBy, '__eq__') + def test_default_field(self): f = OrderingFilter() field = f.field From 36231d524226820191798d7d0c5d3f214e907541 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Mon, 2 Sep 2019 21:25:37 +0300 Subject: [PATCH 06/10] Cover the case when ordering descriptor is specified as a list of model fields --- tests/test_filters.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_filters.py b/tests/test_filters.py index dcec3f3da..b149e2649 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -1673,12 +1673,24 @@ def test_params_as_list_of_tuples_with_string(self): ) self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_list_of_tuples_with_strings(self): + f = OrderingFilter( + params=[('user', ('username',))] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_list_of_tuples_with_expression(self): f = OrderingFilter( params=[('user', F('username'))] ) self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_list_of_tuples_with_expressions(self): + f = OrderingFilter( + params=[('user', (F('username'),))] + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_list_of_tuples_with_descriptor_and_string(self): f = OrderingFilter( params=[('user', {'expr': 'username'})] @@ -1709,12 +1721,24 @@ def test_params_as_dict_with_string(self): ) self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_dict_with_strings(self): + f = OrderingFilter( + params={'user': ('username',)} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_dict_with_expression(self): f = OrderingFilter( params={'user': F('username')} ) self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_dict_with_expressions(self): + f = OrderingFilter( + params={'user': (F('username'),)} + ) + self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + def test_params_as_dict_with_descriptor_and_string(self): f = OrderingFilter( params={'user': {'expr': 'username'}} From 56f81cd6f2a53524fb2c2e387779e576bd1b08e0 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Mon, 2 Sep 2019 21:28:44 +0300 Subject: [PATCH 07/10] Correct a comment --- django_filters/filters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django_filters/filters.py b/django_filters/filters.py index 76c087f5b..c9885cd57 100644 --- a/django_filters/filters.py +++ b/django_filters/filters.py @@ -844,7 +844,7 @@ def normalize_ordering_descriptor(cls, descriptor): return descriptor if isinstance(descriptor, collections.Sequence): - # A sequence of model field names + # A sequence of model field names or Django expressions return { "exprs": [F(field) if isinstance(field, str) else field for field in descriptor] } From 964fa26b1c0a091381ab4f90c2c94293e3160ba7 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Tue, 24 Dec 2019 14:39:09 +0300 Subject: [PATCH 08/10] In the updated OrderingFilter class, rename _params to params to match the style of existing code --- django_filters/filters.py | 4 ++-- tests/test_filters.py | 34 +++++++++++++++++----------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/django_filters/filters.py b/django_filters/filters.py index c9885cd57..2952a9cd4 100644 --- a/django_filters/filters.py +++ b/django_filters/filters.py @@ -739,7 +739,7 @@ def __init__(self, *args, **kwargs): else: params = self.normalize_params(kwargs['params']) - self._params = params + self.params = params if 'choices' not in kwargs: kwargs['choices'] = self.build_choices(params) @@ -883,7 +883,7 @@ def filter(self, qs, value): def get_ordering_exprs(self, param_name): descending = param_name.startswith('-') param_name = param_name[1:] if descending else param_name - descriptor = self._params.get(param_name) + descriptor = self.params.get(param_name) # For backward compatibility order by param_name if descriptor is not found exprs = descriptor['exprs'] if descriptor is not None else (F(param_name),) if descending: diff --git a/tests/test_filters.py b/tests/test_filters.py index b149e2649..2c0367d7a 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -1665,103 +1665,103 @@ def test_params_as_strings(self): f = OrderingFilter( params=['username'], ) - self.assertEqual(f._params, OrderedDict((('username', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('username', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_string(self): f = OrderingFilter( params=[('user', 'username')] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_strings(self): f = OrderingFilter( params=[('user', ('username',))] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_expression(self): f = OrderingFilter( params=[('user', F('username'))] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_expressions(self): f = OrderingFilter( params=[('user', (F('username'),))] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_descriptor_and_string(self): f = OrderingFilter( params=[('user', {'expr': 'username'})] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_descriptor_and_expression(self): f = OrderingFilter( params=[('user', {'expr': F('username')})] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_descriptor_and_strings(self): f = OrderingFilter( params=[('user', {'exprs': ('username',)})] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_list_of_tuples_with_descriptor_and_expressions(self): f = OrderingFilter( params=[('user', {'exprs': (F('username'),)})] ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_string(self): f = OrderingFilter( params={'user': 'username'} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_strings(self): f = OrderingFilter( params={'user': ('username',)} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_expression(self): f = OrderingFilter( params={'user': F('username')} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_expressions(self): f = OrderingFilter( params={'user': (F('username'),)} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_descriptor_and_string(self): f = OrderingFilter( params={'user': {'expr': 'username'}} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_descriptor_and_expression(self): f = OrderingFilter( params={'user': {'expr': F('username')}} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_descriptor_and_strings(self): f = OrderingFilter( params={'user': {'exprs': ('username',)}} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_params_as_dict_with_descriptor_and_expressions(self): f = OrderingFilter( params={'user': {'exprs': (F('username'),)}} ) - self.assertEqual(f._params, OrderedDict((('user', {'exprs': [F('username')]}),))) + self.assertEqual(f.params, OrderedDict((('user', {'exprs': [F('username')]}),))) def test_choices_unaltered_with_params(self): # provided 'choices' should not be altered when 'params' is present From ee0e6baf83f69dfa47b56b1179957687decc6ee6 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Sun, 24 May 2020 14:37:12 +0300 Subject: [PATCH 09/10] Remove `if True` which was probably used for debugging --- tests/test_filters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_filters.py b/tests/test_filters.py index 2c0367d7a..acab43fb8 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -1433,9 +1433,9 @@ def setUpClass(cls): # Django 1.11 does not define __eq__ operators for F and OrderBy. # Define them manually when needed - if True or not hasattr(F, '__eq__'): + if not hasattr(F, '__eq__'): setattr(F, '__eq__', cls.compareF) - if True or not hasattr(OrderBy, '__eq__'): + if not hasattr(OrderBy, '__eq__'): setattr(OrderBy, '__eq__', cls.compareOrderBy) @staticmethod From e19224942ee4ce40c67e2f3f19b63a58c0edb6e5 Mon Sep 17 00:00:00 2001 From: Evgeny Arshinov Date: Sun, 24 May 2020 15:11:56 +0300 Subject: [PATCH 10/10] Use a nullable DateTime model field to test OrderingFilter's handling of nulls --- tests/models.py | 6 +- tests/test_filtering.py | 147 +++++++++++++++++++++------------------- 2 files changed, 81 insertions(+), 72 deletions(-) diff --git a/tests/models.py b/tests/models.py index 4831f9479..ab1c92f47 100644 --- a/tests/models.py +++ b/tests/models.py @@ -41,8 +41,8 @@ def formfield(self, **kwargs): class User(models.Model): username = models.CharField(_('username'), max_length=255) - first_name = SubCharField(max_length=100, null=True, blank=True) - last_name = SubSubCharField(max_length=100, null=True, blank=True) + first_name = SubCharField(max_length=100) + last_name = SubSubCharField(max_length=100) status = models.IntegerField(choices=STATUS_CHOICES, default=0) @@ -51,6 +51,8 @@ class User(models.Model): favorite_books = models.ManyToManyField('Book', related_name='lovers') + last_login = models.DateTimeField(null=True) + def __str__(self): return self.username diff --git a/tests/test_filtering.py b/tests/test_filtering.py index 0d73e76bd..fda5e850c 100644 --- a/tests/test_filtering.py +++ b/tests/test_filtering.py @@ -1872,10 +1872,11 @@ def test_filtering(self): class OrderingFilterTests(TestCase): def setUp(self): - User.objects.create(username='alex', first_name='Alex', last_name='Allan', status=1) - User.objects.create(username='jacob', first_name='Jacob', last_name='Johnson', status=2) - User.objects.create(username='aaron', first_name='Aaron', last_name='Barrett', status=2) - User.objects.create(username='carl', first_name=None, last_name='Jung', status=0) + tz = timezone.utc + User.objects.create(username='alex', status=1, last_login=datetime.datetime(2020, 1, 1, tzinfo=tz)) + User.objects.create(username='jacob', status=2, last_login=datetime.datetime(2020, 2, 1, tzinfo=tz)) + User.objects.create(username='aaron', status=2, last_login=datetime.datetime(2020, 3, 1, tzinfo=tz)) + User.objects.create(username='carl', status=0) def test_ordering(self): class F(FilterSet): @@ -1909,125 +1910,131 @@ class Meta: self.assertEqual(list(names), ['aaron', 'alex', 'carl', 'jacob']) def test_ordering_with_null(self): + tz = timezone.utc + class F(FilterSet): o = OrderingFilter( - fields=('first_name', 'last_name') + fields=('last_login',) ) class Meta: model = User - fields = ['first_name', 'last_name'] + fields = ['last_login'] qs = User.objects.all() - f = F({'o': 'first_name,last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - (None, 'Jung'), - ('Aaron', 'Barrett'), - ('Alex', 'Allan'), - ('Jacob', 'Johnson'), + f = F({'o': 'last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('carl', None), + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), ]) - f = F({'o': '-first_name,-last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - ('Jacob', 'Johnson'), - ('Alex', 'Allan'), - ('Aaron', 'Barrett'), - (None, 'Jung'), + f = F({'o': '-last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), + ('carl', None), ]) def test_ordering_with_params_and_null(self): + tz = timezone.utc + class F(FilterSet): o = OrderingFilter( - params=('first_name', 'last_name') + params=('last_login',) ) class Meta: model = User - fields = ['first_name', 'last_name'] + fields = ['last_login'] qs = User.objects.all() - f = F({'o': 'first_name,last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - (None, 'Jung'), - ('Aaron', 'Barrett'), - ('Alex', 'Allan'), - ('Jacob', 'Johnson'), + f = F({'o': 'last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('carl', None), + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), ]) - f = F({'o': '-first_name,-last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - ('Jacob', 'Johnson'), - ('Alex', 'Allan'), - ('Aaron', 'Barrett'), - (None, 'Jung'), + f = F({'o': '-last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), + ('carl', None), ]) def test_ordering_with_params_and_nulls_last(self): + tz = timezone.utc + class F(FilterSet): o = OrderingFilter( params={ - 'first_name': {'expr': expressions.F('first_name').asc(nulls_last=True)}, - 'last_name': {'expr': expressions.F('last_name').asc(nulls_last=True)} + 'last_login': {'expr': expressions.F('last_login').asc(nulls_last=True)}, } ) class Meta: model = User - fields = ['first_name', 'last_name'] + fields = ['last_login'] qs = User.objects.all() - f = F({'o': 'first_name,last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - ('Aaron', 'Barrett'), - ('Alex', 'Allan'), - ('Jacob', 'Johnson'), - (None, 'Jung'), + f = F({'o': 'last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), + ('carl', None), ]) - f = F({'o': '-first_name,-last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - (None, 'Jung'), - ('Jacob', 'Johnson'), - ('Alex', 'Allan'), - ('Aaron', 'Barrett'), + f = F({'o': '-last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('carl', None), + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), ]) def test_ordering_with_params_and_desc_nulls_last(self): + tz = timezone.utc + class F(FilterSet): o = OrderingFilter( params={ - 'first_name': {'expr': expressions.F('first_name').desc(nulls_last=True)}, - 'last_name': {'expr': expressions.F('last_name').desc(nulls_last=True)} + 'last_login': {'expr': expressions.F('last_login').desc(nulls_last=True)}, } ) class Meta: model = User - fields = ['first_name', 'last_name'] + fields = ['last_login'] qs = User.objects.all() - f = F({'o': 'first_name,last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - ('Jacob', 'Johnson'), - ('Alex', 'Allan'), - ('Aaron', 'Barrett'), - (None, 'Jung'), + f = F({'o': 'last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), + ('carl', None), ]) - f = F({'o': '-first_name,-last_name'}, queryset=qs) - names = f.qs.values_list('first_name', 'last_name') - self.assertEqual(list(names), [ - (None, 'Jung'), - ('Aaron', 'Barrett'), - ('Alex', 'Allan'), - ('Jacob', 'Johnson'), + f = F({'o': '-last_login'}, queryset=qs) + results = f.qs.values_list('username', 'last_login') + self.assertEqual(list(results), [ + ('carl', None), + ('alex', datetime.datetime(2020, 1, 1, tzinfo=tz)), + ('jacob', datetime.datetime(2020, 2, 1, tzinfo=tz)), + ('aaron', datetime.datetime(2020, 3, 1, tzinfo=tz)), ])