Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #11295: If ModelAdmin.queryset returns a filtered QS don't requ…

…ire a 2nd count call

Original patch rewritten, added tests and get_filters_params method for ChangeList class.
Thanks Alex for the report.
  • Loading branch information...
commit f07a5f0a21857204465019b4e68f914d31cb396a 1 parent e4ee3d8
Wiktor Kolodziej authored
29  django/contrib/admin/views/main.py
@@ -79,15 +79,23 @@ def __init__(self, request, model, list_display, list_display_links,
79 79
         self.title = title % force_text(self.opts.verbose_name)
80 80
         self.pk_attname = self.lookup_opts.pk.attname
81 81
 
82  
-    def get_filters(self, request):
83  
-        lookup_params = self.params.copy() # a dictionary of the query string
84  
-        use_distinct = False
85  
-
  82
+    def get_filters_params(self, params=None):
  83
+        """
  84
+        Returns all params except IGNORED_PARAMS
  85
+        """
  86
+        if not params:
  87
+            params = self.params
  88
+        lookup_params = params.copy() # a dictionary of the query string
86 89
         # Remove all the parameters that are globally and systematically
87 90
         # ignored.
88 91
         for ignored in IGNORED_PARAMS:
89 92
             if ignored in lookup_params:
90 93
                 del lookup_params[ignored]
  94
+        return lookup_params
  95
+
  96
+    def get_filters(self, request):
  97
+        lookup_params = self.get_filters_params()
  98
+        use_distinct = False
91 99
 
92 100
         # Normalize the types of keys
93 101
         for key, value in lookup_params.items():
@@ -166,14 +174,13 @@ def get_results(self, request):
166 174
         result_count = paginator.count
167 175
 
168 176
         # Get the total number of objects, with no admin filters applied.
169  
-        # Perform a slight optimization: Check to see whether any filters were
170  
-        # given. If not, use paginator.hits to calculate the number of objects,
171  
-        # because we've already done paginator.hits and the value is cached.
172  
-        if not self.query_set.query.where:
173  
-            full_result_count = result_count
174  
-        else:
  177
+        # Perform a slight optimization:
  178
+        # full_result_count is equal to paginator.count if no filters
  179
+        # were applied
  180
+        if self.get_filters_params():
175 181
             full_result_count = self.root_query_set.count()
176  
-
  182
+        else:
  183
+            full_result_count = result_count
177 184
         can_show_all = result_count <= self.list_max_show_all
178 185
         multi_page = result_count > self.list_per_page
179 186
 
28  tests/regressiontests/admin_views/tests.py
@@ -2537,6 +2537,34 @@ def test_changelist_view(self):
2537 2537
             else:
2538 2538
                 self.assertNotContains(response, 'Primary key = %s' % i)
2539 2539
 
  2540
+    def test_changelist_view_count_queries(self):
  2541
+        #create 2 Person objects
  2542
+        Person.objects.create(name='person1', gender=1)
  2543
+        Person.objects.create(name='person2', gender=2)
  2544
+
  2545
+        # 4 queries are expected: 1 for the session, 1 for the user,
  2546
+        # 1 for the count and 1 for the objects on the page
  2547
+        with self.assertNumQueries(4):
  2548
+            resp = self.client.get('/test_admin/admin/admin_views/person/')
  2549
+            self.assertEqual(resp.context['selection_note'], '0 of 2 selected')
  2550
+            self.assertEqual(resp.context['selection_note_all'], 'All 2 selected')
  2551
+        with self.assertNumQueries(4):
  2552
+            extra = {'q': 'not_in_name'}
  2553
+            resp = self.client.get('/test_admin/admin/admin_views/person/', extra)
  2554
+            self.assertEqual(resp.context['selection_note'], '0 of 0 selected')
  2555
+            self.assertEqual(resp.context['selection_note_all'], 'All 0 selected')
  2556
+        with self.assertNumQueries(4):
  2557
+            extra = {'q': 'person'}
  2558
+            resp = self.client.get('/test_admin/admin/admin_views/person/', extra)
  2559
+            self.assertEqual(resp.context['selection_note'], '0 of 2 selected')
  2560
+            self.assertEqual(resp.context['selection_note_all'], 'All 2 selected')
  2561
+        # here one more count(*) query will run, because filters were applied
  2562
+        with self.assertNumQueries(5):
  2563
+            extra = {'gender__exact': '1'}
  2564
+            resp = self.client.get('/test_admin/admin/admin_views/person/', extra)
  2565
+            self.assertEqual(resp.context['selection_note'], '0 of 1 selected')
  2566
+            self.assertEqual(resp.context['selection_note_all'], '1 selected')
  2567
+
2540 2568
     def test_change_view(self):
2541 2569
         for i in self.pks:
2542 2570
             response = self.client.get('/test_admin/admin/admin_views/emptymodel/%s/' % i)

0 notes on commit f07a5f0

Please sign in to comment.
Something went wrong with that request. Please try again.