From ff5517988adec04d364521fdaf4a36a3f88942ef Mon Sep 17 00:00:00 2001 From: Haki Benita Date: Sat, 16 Dec 2017 20:50:11 +0200 Subject: [PATCH] Fixed #28933 -- Improved the efficiency of ModelAdmin.date_hierarchy queries. --- django/contrib/admin/views/main.py | 33 +++++++++++ tests/admin_changelist/admin.py | 1 + tests/admin_changelist/test_date_hierarchy.py | 55 +++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 tests/admin_changelist/test_date_hierarchy.py diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 5141b9e2f1..761ed3a754 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -1,5 +1,7 @@ from collections import OrderedDict +from datetime import datetime, timedelta +from django.conf import settings from django.contrib.admin import FieldListFilter from django.contrib.admin.exceptions import ( DisallowedModelAdminLookup, DisallowedModelAdminToField, @@ -18,6 +20,7 @@ from django.db import models from django.db.models.expressions import F, OrderBy from django.urls import reverse from django.utils.http import urlencode +from django.utils.timezone import make_aware from django.utils.translation import gettext # Changelist settings @@ -136,6 +139,36 @@ class ChangeList: if spec and spec.has_output(): filter_specs.append(spec) + if self.date_hierarchy: + # Create bounded lookup parameters so that the query is more + # efficient. + year = lookup_params.pop('%s__year' % self.date_hierarchy, None) + if year is not None: + month = lookup_params.pop('%s__month' % self.date_hierarchy, None) + day = lookup_params.pop('%s__day' % self.date_hierarchy, None) + try: + from_date = datetime( + int(year), + int(month if month is not None else 1), + int(day if day is not None else 1), + ) + except ValueError as e: + raise IncorrectLookupParameters(e) from e + if settings.USE_TZ: + from_date = make_aware(from_date) + if day: + to_date = from_date + timedelta(days=1) + elif month: + # In this branch, from_date will always be the first of a + # month, so advancing 32 days gives the next month. + to_date = (from_date + timedelta(days=32)).replace(day=1) + else: + to_date = from_date.replace(year=from_date.year + 1) + lookup_params.update({ + '%s__gte' % self.date_hierarchy: from_date, + '%s__lt' % self.date_hierarchy: to_date, + }) + # At this point, all the parameters used by the various ListFilters # have been removed from lookup_params, which now only contains other # parameters passed via the query string. We now loop through the diff --git a/tests/admin_changelist/admin.py b/tests/admin_changelist/admin.py index 915dc32ef0..1e686bd6fb 100644 --- a/tests/admin_changelist/admin.py +++ b/tests/admin_changelist/admin.py @@ -16,6 +16,7 @@ class CustomPaginator(Paginator): class EventAdmin(admin.ModelAdmin): + date_hierarchy = 'date' list_display = ['event_date_func'] def event_date_func(self, event): diff --git a/tests/admin_changelist/test_date_hierarchy.py b/tests/admin_changelist/test_date_hierarchy.py new file mode 100644 index 0000000000..96590ccc82 --- /dev/null +++ b/tests/admin_changelist/test_date_hierarchy.py @@ -0,0 +1,55 @@ +from datetime import datetime + +from django.contrib.admin.options import IncorrectLookupParameters +from django.test import RequestFactory, TestCase +from django.utils.timezone import make_aware + +from .admin import EventAdmin, site as custom_site +from .models import Event + + +class DateHierarchyTests(TestCase): + factory = RequestFactory() + + def assertDateParams(self, query, expected_from_date, expected_to_date): + query = {'date__%s' % field: val for field, val in query.items()} + request = self.factory.get('/', query) + changelist = EventAdmin(Event, custom_site).get_changelist_instance(request) + _, _, lookup_params, _ = changelist.get_filters(request) + self.assertEqual(lookup_params['date__gte'], expected_from_date) + self.assertEqual(lookup_params['date__lt'], expected_to_date) + + def test_bounded_params(self): + tests = ( + ({'year': 2017}, datetime(2017, 1, 1), datetime(2018, 1, 1)), + ({'year': 2017, 'month': 2}, datetime(2017, 2, 1), datetime(2017, 3, 1)), + ({'year': 2017, 'month': 12}, datetime(2017, 12, 1), datetime(2018, 1, 1)), + ({'year': 2017, 'month': 12, 'day': 15}, datetime(2017, 12, 15), datetime(2017, 12, 16)), + ({'year': 2017, 'month': 12, 'day': 31}, datetime(2017, 12, 31), datetime(2018, 1, 1)), + ({'year': 2017, 'month': 2, 'day': 28}, datetime(2017, 2, 28), datetime(2017, 3, 1)), + ) + for query, expected_from_date, expected_to_date in tests: + with self.subTest(query=query): + self.assertDateParams(query, expected_from_date, expected_to_date) + + def test_bounded_params_with_time_zone(self): + with self.settings(USE_TZ=True, TIME_ZONE='Asia/Jerusalem'): + self.assertDateParams( + {'year': 2017, 'month': 2, 'day': 28}, + make_aware(datetime(2017, 2, 28)), + make_aware(datetime(2017, 3, 1)), + ) + + def test_invalid_params(self): + tests = ( + {'year': 'x'}, + {'year': 2017, 'month': 'x'}, + {'year': 2017, 'month': 12, 'day': 'x'}, + {'year': 2017, 'month': 13}, + {'year': 2017, 'month': 12, 'day': 32}, + {'year': 2017, 'month': 0}, + {'year': 2017, 'month': 12, 'day': 0}, + ) + for invalid_query in tests: + with self.subTest(query=invalid_query), self.assertRaises(IncorrectLookupParameters): + self.assertDateParams(invalid_query, None, None)