Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #19080 -- Fine-grained control over select_related in admin

  • Loading branch information...
commit 0fd9f7c95f748764867dc148a2bacef807466d85 1 parent 4f4e924
Tomasz Paczkowski authored June 04, 2013
10  django/contrib/admin/validation.py
@@ -310,8 +310,14 @@ def validate_list_filter(self, cls, model):
310 310
                                 % (cls.__name__, idx, field))
311 311
 
312 312
     def validate_list_select_related(self, cls, model):
313  
-        " Validate that list_select_related is a boolean. "
314  
-        check_type(cls, 'list_select_related', bool)
  313
+        " Validate that list_select_related is a boolean, a list or a tuple. "
  314
+        list_select_related = getattr(cls, 'list_select_related', None)
  315
+        if list_select_related:
  316
+            types = (bool, tuple, list)
  317
+            if not isinstance(list_select_related, types):
  318
+                raise ImproperlyConfigured("'%s.list_select_related' should be "
  319
+                                           "either a bool, a tuple or a list" %
  320
+                                           cls.__name__)
315 321
 
316 322
     def validate_list_per_page(self, cls, model):
317 323
         " Validate that list_per_page is an integer. "
44  django/contrib/admin/views/main.py
@@ -356,36 +356,46 @@ def get_queryset(self, request):
356 356
             # ValueError, ValidationError, or ?.
357 357
             raise IncorrectLookupParameters(e)
358 358
 
359  
-        # Use select_related() if one of the list_display options is a field
360  
-        # with a relationship and the provided queryset doesn't already have
361  
-        # select_related defined.
362 359
         if not qs.query.select_related:
363  
-            if self.list_select_related:
364  
-                qs = qs.select_related()
365  
-            else:
366  
-                for field_name in self.list_display:
367  
-                    try:
368  
-                        field = self.lookup_opts.get_field(field_name)
369  
-                    except models.FieldDoesNotExist:
370  
-                        pass
371  
-                    else:
372  
-                        if isinstance(field.rel, models.ManyToOneRel):
373  
-                            qs = qs.select_related()
374  
-                            break
  360
+            qs = self.apply_select_related(qs)
375 361
 
376 362
         # Set ordering.
377 363
         ordering = self.get_ordering(request, qs)
378 364
         qs = qs.order_by(*ordering)
379 365
 
380 366
         # Apply search results
381  
-        qs, search_use_distinct = self.model_admin.get_search_results(request, qs, self.query)
  367
+        qs, search_use_distinct = self.model_admin.get_search_results(
  368
+            request, qs, self.query)
382 369
 
383  
-        # Remove duplicates from results, if neccesary
  370
+        # Remove duplicates from results, if necessary
384 371
         if filters_use_distinct | search_use_distinct:
385 372
             return qs.distinct()
386 373
         else:
387 374
             return qs
388 375
 
  376
+    def apply_select_related(self, qs):
  377
+        if self.list_select_related is True:
  378
+            return qs.select_related()
  379
+
  380
+        if self.list_select_related is False:
  381
+            if self.has_related_field_in_list_display():
  382
+                return qs.select_related()
  383
+
  384
+        if self.list_select_related:
  385
+            return qs.select_related(*self.list_select_related)
  386
+        return qs
  387
+
  388
+    def has_related_field_in_list_display(self):
  389
+        for field_name in self.list_display:
  390
+            try:
  391
+                field = self.lookup_opts.get_field(field_name)
  392
+            except models.FieldDoesNotExist:
  393
+                pass
  394
+            else:
  395
+                if isinstance(field.rel, models.ManyToOneRel):
  396
+                    return True
  397
+        return False
  398
+
389 399
     def url_for_result(self, result):
390 400
         pk = getattr(result, self.pk_attname)
391 401
         return reverse('admin:%s_%s_change' % (self.opts.app_label,
22  docs/ref/contrib/admin/index.txt
@@ -812,12 +812,24 @@ subclass::
812 812
     the list of objects on the admin change list page. This can save you a
813 813
     bunch of database queries.
814 814
 
815  
-    The value should be either ``True`` or ``False``. Default is ``False``.
  815
+    .. versionchanged:: dev
816 816
 
817  
-    Note that Django will use
818  
-    :meth:`~django.db.models.query.QuerySet.select_related`,
819  
-    regardless of this setting if one of the ``list_display`` fields is a
820  
-    ``ForeignKey``.
  817
+    The value should be either a boolean, a list or a tuple. Default is
  818
+    ``False``.
  819
+
  820
+    When value is ``True``, ``select_related()`` will always be called. When
  821
+    value is set to ``False``, Django will look at ``list_display`` and call
  822
+    ``select_related()`` if any ``ForeignKey`` is present.
  823
+
  824
+    If you need more fine-grained control, use a tuple (or list) as value for
  825
+    ``list_select_related``. Empty tuple will prevent Django from calling
  826
+    ``select_related`` at all. Any other tuple will be passed directly to
  827
+    ``select_related`` as parameters. For example::
  828
+
  829
+        class ArticleAdmin(admin.ModelAdmin):
  830
+            list_select_related = ('author', 'category')
  831
+
  832
+    will call ``select_related('author', 'category')``.
821 833
 
822 834
 .. attribute:: ModelAdmin.ordering
823 835
 
5  tests/admin_changelist/admin.py
@@ -67,6 +67,11 @@ class ChordsBandAdmin(admin.ModelAdmin):
67 67
     list_filter = ['members']
68 68
 
69 69
 
  70
+class InvitationAdmin(admin.ModelAdmin):
  71
+    list_display = ('band', 'player')
  72
+    list_select_related = ('player',)
  73
+
  74
+
70 75
 class DynamicListDisplayChildAdmin(admin.ModelAdmin):
71 76
     list_display = ('parent', 'name', 'age')
72 77
 
31  tests/admin_changelist/tests.py
@@ -18,7 +18,7 @@
18 18
     GroupAdmin, ParentAdmin, DynamicListDisplayChildAdmin,
19 19
     DynamicListDisplayLinksChildAdmin, CustomPaginationAdmin,
20 20
     FilteredChildAdmin, CustomPaginator, site as custom_site,
21  
-    SwallowAdmin, DynamicListFilterChildAdmin)
  21
+    SwallowAdmin, DynamicListFilterChildAdmin, InvitationAdmin)
22 22
 from .models import (Event, Child, Parent, Genre, Band, Musician, Group,
23 23
     Quartet, Membership, ChordsMusician, ChordsBand, Invitation, Swallow,
24 24
     UnorderedObject, OrderedObject, CustomIdUser)
@@ -46,9 +46,32 @@ def test_select_related_preserved(self):
46 46
         m = ChildAdmin(Child, admin.site)
47 47
         request = self.factory.get('/child/')
48 48
         cl = ChangeList(request, Child, m.list_display, m.list_display_links,
49  
-                m.list_filter, m.date_hierarchy, m.search_fields,
50  
-                m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m)
51  
-        self.assertEqual(cl.queryset.query.select_related, {'parent': {'name': {}}})
  49
+                        m.list_filter, m.date_hierarchy, m.search_fields,
  50
+                        m.list_select_related, m.list_per_page,
  51
+                        m.list_max_show_all, m.list_editable, m)
  52
+        self.assertEqual(cl.queryset.query.select_related, {
  53
+            'parent': {'name': {}}
  54
+        })
  55
+
  56
+    def test_select_related_as_tuple(self):
  57
+        ia = InvitationAdmin(Invitation, admin.site)
  58
+        request = self.factory.get('/invitation/')
  59
+        cl = ChangeList(request, Child, ia.list_display, ia.list_display_links,
  60
+                        ia.list_filter, ia.date_hierarchy, ia.search_fields,
  61
+                        ia.list_select_related, ia.list_per_page,
  62
+                        ia.list_max_show_all, ia.list_editable, ia)
  63
+        self.assertEqual(cl.queryset.query.select_related, {'player': {}})
  64
+
  65
+    def test_select_related_as_empty_tuple(self):
  66
+        ia = InvitationAdmin(Invitation, admin.site)
  67
+        ia.list_select_related = ()
  68
+        request = self.factory.get('/invitation/')
  69
+        cl = ChangeList(request, Child, ia.list_display, ia.list_display_links,
  70
+                        ia.list_filter, ia.date_hierarchy, ia.search_fields,
  71
+                        ia.list_select_related, ia.list_per_page,
  72
+                        ia.list_max_show_all, ia.list_editable, ia)
  73
+        self.assertEqual(cl.queryset.query.select_related, False)
  74
+
52 75
 
53 76
     def test_result_list_empty_changelist_value(self):
54 77
         """
6  tests/modeladmin/tests.py
@@ -1161,9 +1161,11 @@ def test_list_select_related_validation(self):
1161 1161
         class ValidationTestModelAdmin(ModelAdmin):
1162 1162
             list_select_related = 1
1163 1163
 
1164  
-        six.assertRaisesRegex(self,
  1164
+        six.assertRaisesRegex(
  1165
+            self,
1165 1166
             ImproperlyConfigured,
1166  
-            "'ValidationTestModelAdmin.list_select_related' should be a bool.",
  1167
+            "'ValidationTestModelAdmin.list_select_related' should be either a "
  1168
+            "bool, a tuple or a list",
1167 1169
             ValidationTestModelAdmin.validate,
1168 1170
             ValidationTestModel,
1169 1171
         )

0 notes on commit 0fd9f7c

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