Rework DateRangeFilter (#852)

* Replace DateRangeFilter.options w/ choices/filters

- Replaces the 'options' data structure with standard form field choices
  and a 'filters' dict, which contains the date filters separately.
- Choices now use descriptive string values instead of integers.

* Ensure DateRangeFilter choices and filters match

* Add assertion for removed DateRangeFilter.options

* Remove duplicate code in DateRangeFilterTests

* Mock dates in DateRangeFilterTests

* Add 'yesterday' test for DateRangeFilter

* Fix 'yesterday' filtering for DateRangeFilter

django_filters/filters.py

@@ -398,51 +398,66 @@ def _truncate(dt):
 
 
 class DateRangeFilter(ChoiceFilter):
-    options = {
-        '': (_('Any date'), lambda qs, name: qs),
-        1: (_('Today'), lambda qs, name: qs.filter(**{
+    choices = [
+        ('today', _('Today')),
+        ('yesterday', _('Yesterday')),
+        ('week', _('Past 7 days')),
+        ('month', _('This month')),
+        ('year', _('This year')),
+    ]
+
+    filters = {
+        'today': lambda qs, name: qs.filter(**{
             '%s__year' % name: now().year,
             '%s__month' % name: now().month,
             '%s__day' % name: now().day
-        })),
-        2: (_('Past 7 days'), lambda qs, name: qs.filter(**{
+        }),
+        'yesterday': lambda qs, name: qs.filter(**{
+            '%s__year' % name: (now() - timedelta(days=1)).year,
+            '%s__month' % name: (now() - timedelta(days=1)).month,
+            '%s__day' % name: (now() - timedelta(days=1)).day,
+        }),
+        'week': lambda qs, name: qs.filter(**{
             '%s__gte' % name: _truncate(now() - timedelta(days=7)),
             '%s__lt' % name: _truncate(now() + timedelta(days=1)),
-        })),
-        3: (_('This month'), lambda qs, name: qs.filter(**{
+        }),
+        'month': lambda qs, name: qs.filter(**{
             '%s__year' % name: now().year,
             '%s__month' % name: now().month
-        })),
-        4: (_('This year'), lambda qs, name: qs.filter(**{
-            '%s__year' % name: now().year,
-        })),
-        5: (_('Yesterday'), lambda qs, name: qs.filter(**{
+        }),
+        'year': lambda qs, name: qs.filter(**{
             '%s__year' % name: now().year,
-            '%s__month' % name: now().month,
-            '%s__day' % name: (now() - timedelta(days=1)).day,
-        })),
+        }),
     }
 
-    def __init__(self, *args, **kwargs):
-        kwargs['choices'] = [
-            (key, value[0]) for key, value in self.options.items()]
+    def __init__(self, choices=None, filters=None, *args, **kwargs):
+        if choices is not None:
+            self.choices = choices
+        if filters is not None:
+            self.filters = filters
 
-        # empty/null choices not relevant
-        kwargs.setdefault('empty_label', None)
+        unique = set([x[0] for x in self.choices]) ^ set(self.filters)
+        assert not unique, \
+            "Keys must be present in both 'choices' and 'filters'. Missing keys: " \
+            "'%s'" % ', '.join(sorted(unique))
+
+        # TODO: remove assertion in 2.1
+        assert not hasattr(self, 'options'), \
+            "The 'options' attribute has been replaced by 'choices' and 'filters'. " \
+            "See: https://django-filter.readthedocs.io/en/master/guide/migration.html"
+
+        # null choice not relevant
         kwargs.setdefault('null_label', None)
-        super().__init__(*args, **kwargs)
+        super().__init__(choices=self.choices, *args, **kwargs)
 
     def filter(self, qs, value):
-        try:
-            value = int(value)
-        except (ValueError, TypeError):
-            value = ''
+        if not value:
+            return qs
 
-        assert value in self.options
-        qs = self.options[value][1](qs, self.field_name)
-        if self.distinct:
-            qs = qs.distinct()
-        return qs
+        assert value in self.filters
+
+        qs = self.filters[value](qs, self.field_name)
+        return qs.distinct() if self.distinct else qs
 
 
 class DateFromToRangeFilter(RangeFilter):

tests/test_filtering.py

@@ -1,11 +1,12 @@
+import contextlib
 import datetime
 import mock
 import unittest
 
 from django import forms
 from django.test import TestCase, override_settings
 from django.utils import timezone
-from django.utils.timezone import now
+from django.utils.timezone import make_aware, now
 
 from django_filters.filters import (
     AllValuesFilter,
@@ -762,17 +763,19 @@ class Meta:
                                  lambda o: o.title)
 
 
-# TODO:
-# year & month filtering could be better. The problem is that the test dates
-# are relative to today, which is always changing. So, two_weeks_ago is not a
-# valid date for 'this month' during the first half of the month, but is during
-# the second half. Similary, five_days_ago is not during 'this year' when the
-# tests are ran on January 1. All we can test is what is absolutely never valid
-# eg, a date from two_years_ago is never a valid date for 'this year'.
 class DateRangeFilterTests(TestCase):
 
-    def setUp(self):
-        today = now().date()
+    class CommentFilter(FilterSet):
+        date = DateRangeFilter()
+
+        class Meta:
+            model = Comment
+            fields = ['date']
+
+    @contextlib.contextmanager
+    def relative_to(self, today):
+        today = make_aware(today)
+        yesterday = today - datetime.timedelta(days=1)
         five_days_ago = today - datetime.timedelta(days=5)
         two_weeks_ago = today - datetime.timedelta(days=14)
         two_months_ago = today - datetime.timedelta(days=62)
@@ -784,61 +787,37 @@ def setUp(self):
         Comment.objects.create(date=two_years_ago, author=alex, time=time)
         Comment.objects.create(date=five_days_ago, author=alex, time=time)
         Comment.objects.create(date=today, author=alex, time=time)
+        Comment.objects.create(date=yesterday, author=alex, time=time)
         Comment.objects.create(date=two_months_ago, author=alex, time=time)
 
-    def test_filtering_for_year(self):
-        class F(FilterSet):
-            date = DateRangeFilter()
-
-            class Meta:
-                model = Comment
-                fields = ['date']
-
-        f = F({'date': '4'})  # this year
+        with mock.patch('django_filters.filters.now') as mock_now:
+            mock_now.return_value = today
+            yield
 
-        # assert what is NOT valid for now.
-        # self.assertQuerysetEqual(f.qs, [1, 3, 4, 5], lambda o: o.pk, False)
-        self.assertNotIn(2, f.qs.values_list('pk', flat=True))
+    def test_filtering_for_year(self):
+        f = self.CommentFilter({'date': 'year'})
+        with self.relative_to(datetime.datetime(now().year, 4, 1)):
+            self.assertQuerysetEqual(f.qs, [1, 3, 4, 5, 6], lambda o: o.pk, False)
 
     def test_filtering_for_month(self):
-        class F(FilterSet):
-            date = DateRangeFilter()
-
-            class Meta:
-                model = Comment
-                fields = ['date']
-
-        f = F({'date': '3'})  # this month
-
-        # assert what is NOT valid for now.
-        # self.assertQuerysetEqual(f.qs, [1, 3, 4], lambda o: o.pk, False)
-        self.assertNotIn(2, f.qs.values_list('pk', flat=True))
-        self.assertNotIn(5, f.qs.values_list('pk', flat=True))
+        f = self.CommentFilter({'date': 'month'})
+        with self.relative_to(datetime.datetime(now().year, 4, 21)):
+            self.assertQuerysetEqual(f.qs, [1, 3, 4, 5], lambda o: o.pk, False)
 
     def test_filtering_for_week(self):
-        class F(FilterSet):
-            date = DateRangeFilter()
-
-            class Meta:
-                model = Comment
-                fields = ['date']
+        f = self.CommentFilter({'date': 'week'})
+        with self.relative_to(datetime.datetime(now().year, 1, 1)):
+            self.assertQuerysetEqual(f.qs, [3, 4, 5], lambda o: o.pk, False)
 
-        f = F({'date': '2'})  # this week
-        self.assertQuerysetEqual(f.qs, [3, 4], lambda o: o.pk, False)
+    def test_filtering_for_yesterday(self):
+        f = self.CommentFilter({'date': 'yesterday'})
+        with self.relative_to(datetime.datetime(now().year, 1, 1)):
+            self.assertQuerysetEqual(f.qs, [5], lambda o: o.pk, False)
 
     def test_filtering_for_today(self):
-        class F(FilterSet):
-            date = DateRangeFilter()
-
-            class Meta:
-                model = Comment
-                fields = ['date']
-
-        f = F({'date': '1'})  # today
-        self.assertQuerysetEqual(f.qs, [4], lambda o: o.pk, False)
-
-    # it will be difficult to test for TZ related issues, where "today" means
-    # different things to both user and server.
+        f = self.CommentFilter({'date': 'today'})
+        with self.relative_to(datetime.datetime(now().year, 1, 1)):
+            self.assertQuerysetEqual(f.qs, [4], lambda o: o.pk, False)
 
 
 class DateFromToRangeFilterTests(TestCase):

tests/test_filters.py

@@ -922,9 +922,15 @@ def test_filtering_ignores_lookup_expr(self):
 class DateRangeFilterTests(TestCase):
 
     def test_creating(self):
-        f = DateRangeFilter()
-        self.assertIn('choices', f.extra)
-        self.assertEqual(len(DateRangeFilter.options), len(f.extra['choices']))
+        f = DateRangeFilter(empty_label=None)
+        self.assertEqual(len(f.choices), 5)
+        self.assertIs(f.choices, f.extra['choices'])
+
+        f = DateRangeFilter(empty_label=None, choices=[], filters=[])
+        self.assertEqual(f.choices, [])
+        self.assertEqual(f.filters, [])
+        self.assertEqual(len(f.choices), 0)
+        self.assertIs(f.choices, f.extra['choices'])
 
     def test_default_field(self):
         f = DateRangeFilter()
@@ -943,14 +949,29 @@ def test_filtering_skipped_with_out_of_range_value(self):
         qs = mock.Mock(spec=[])
         f = DateRangeFilter()
         with self.assertRaises(AssertionError):
-            f.filter(qs, 999)
+            f.filter(qs, 'tomorrow')
+
+    def test_choices_and_filters_mismatch(self):
+        msg = "Keys must be present in both 'choices' and 'filters'. Missing keys: 'a, b'"
+        with self.assertRaisesMessage(AssertionError, msg):
+            DateRangeFilter(choices=[('a', 'a')], filters={'b': None})
+
+    def test_options_removed(self):
+        msg = "The 'options' attribute has been replaced by 'choices' and 'filters'. " \
+              "See: https://django-filter.readthedocs.io/en/master/guide/migration.html"
+
+        class F(DateRangeFilter):
+            options = None
+
+        with self.assertRaisesMessage(AssertionError, msg):
+            F()
 
     def test_filtering_for_this_year(self):
         qs = mock.Mock(spec=['filter'])
         with mock.patch('django_filters.filters.now') as mock_now:
             now_dt = mock_now.return_value
             f = DateRangeFilter()
-            f.filter(qs, '4')
+            f.filter(qs, 'year')
             qs.filter.assert_called_once_with(
                 None__year=now_dt.year)
 
@@ -959,7 +980,7 @@ def test_filtering_for_this_month(self):
         with mock.patch('django_filters.filters.now') as mock_now:
             now_dt = mock_now.return_value
             f = DateRangeFilter()
-            f.filter(qs, '3')
+            f.filter(qs, 'month')
             qs.filter.assert_called_once_with(
                 None__year=now_dt.year, None__month=now_dt.month)
 
@@ -971,7 +992,7 @@ def test_filtering_for_7_days(self):
             mock_d1, mock_d2 = mock.MagicMock(), mock.MagicMock()
             mock_truncate.side_effect = [mock_d1, mock_d2]
             f = DateRangeFilter()
-            f.filter(qs, '2')
+            f.filter(qs, 'week')
             self.assertEqual(
                 mock_td.call_args_list,
                 [mock.call(days=7), mock.call(days=1)]
@@ -983,7 +1004,7 @@ def test_filtering_for_today(self):
         with mock.patch('django_filters.filters.now') as mock_now:
             now_dt = mock_now.return_value
             f = DateRangeFilter()
-            f.filter(qs, '1')
+            f.filter(qs, 'today')
             qs.filter.assert_called_once_with(
                 None__year=now_dt.year,
                 None__month=now_dt.month,
@@ -994,10 +1015,10 @@ def test_filtering_for_yesterday(self):
         with mock.patch('django_filters.filters.now') as mock_now:
             now_dt = mock_now.return_value
             f = DateRangeFilter()
-            f.filter(qs, '5')
+            f.filter(qs, 'yesterday')
             qs.filter.assert_called_once_with(
-                None__year=now_dt.year,
-                None__month=now_dt.month,
+                None__year=(now_dt - timedelta(days=1)).year,
+                None__month=(now_dt - timedelta(days=1)).month,
                 None__day=(now_dt - timedelta(days=1)).day,
             )