From f07a5f0a21857204465019b4e68f914d31cb396a Mon Sep 17 00:00:00 2001 From: Wiktor Kolodziej Date: Sun, 24 Feb 2013 13:18:21 +0100 Subject: [PATCH] Fixed #11295: If ModelAdmin.queryset returns a filtered QS don't require a 2nd count call Original patch rewritten, added tests and get_filters_params method for ChangeList class. Thanks Alex for the report. --- django/contrib/admin/views/main.py | 29 ++++++++++++++-------- tests/regressiontests/admin_views/tests.py | 28 +++++++++++++++++++++ 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 4b296b3f4f..8bda323d83 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -79,15 +79,23 @@ class ChangeList(object): self.title = title % force_text(self.opts.verbose_name) self.pk_attname = self.lookup_opts.pk.attname - def get_filters(self, request): - lookup_params = self.params.copy() # a dictionary of the query string - use_distinct = False - + def get_filters_params(self, params=None): + """ + Returns all params except IGNORED_PARAMS + """ + if not params: + params = self.params + lookup_params = params.copy() # a dictionary of the query string # Remove all the parameters that are globally and systematically # ignored. for ignored in IGNORED_PARAMS: if ignored in lookup_params: del lookup_params[ignored] + return lookup_params + + def get_filters(self, request): + lookup_params = self.get_filters_params() + use_distinct = False # Normalize the types of keys for key, value in lookup_params.items(): @@ -166,14 +174,13 @@ class ChangeList(object): result_count = paginator.count # Get the total number of objects, with no admin filters applied. - # Perform a slight optimization: Check to see whether any filters were - # given. If not, use paginator.hits to calculate the number of objects, - # because we've already done paginator.hits and the value is cached. - if not self.query_set.query.where: - full_result_count = result_count - else: + # Perform a slight optimization: + # full_result_count is equal to paginator.count if no filters + # were applied + if self.get_filters_params(): full_result_count = self.root_query_set.count() - + else: + full_result_count = result_count can_show_all = result_count <= self.list_max_show_all multi_page = result_count > self.list_per_page diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index cd83b73897..d3cfaa3e24 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -2537,6 +2537,34 @@ class AdminCustomQuerysetTest(TestCase): else: self.assertNotContains(response, 'Primary key = %s' % i) + def test_changelist_view_count_queries(self): + #create 2 Person objects + Person.objects.create(name='person1', gender=1) + Person.objects.create(name='person2', gender=2) + + # 4 queries are expected: 1 for the session, 1 for the user, + # 1 for the count and 1 for the objects on the page + with self.assertNumQueries(4): + resp = self.client.get('/test_admin/admin/admin_views/person/') + self.assertEqual(resp.context['selection_note'], '0 of 2 selected') + self.assertEqual(resp.context['selection_note_all'], 'All 2 selected') + with self.assertNumQueries(4): + extra = {'q': 'not_in_name'} + resp = self.client.get('/test_admin/admin/admin_views/person/', extra) + self.assertEqual(resp.context['selection_note'], '0 of 0 selected') + self.assertEqual(resp.context['selection_note_all'], 'All 0 selected') + with self.assertNumQueries(4): + extra = {'q': 'person'} + resp = self.client.get('/test_admin/admin/admin_views/person/', extra) + self.assertEqual(resp.context['selection_note'], '0 of 2 selected') + self.assertEqual(resp.context['selection_note_all'], 'All 2 selected') + # here one more count(*) query will run, because filters were applied + with self.assertNumQueries(5): + extra = {'gender__exact': '1'} + resp = self.client.get('/test_admin/admin/admin_views/person/', extra) + self.assertEqual(resp.context['selection_note'], '0 of 1 selected') + self.assertEqual(resp.context['selection_note_all'], '1 selected') + def test_change_view(self): for i in self.pks: response = self.client.get('/test_admin/admin/admin_views/emptymodel/%s/' % i)