Ticket #14396: 14396_list_select_related.diff

django/contrib/admin/validation.py

@@ -4 +4 @@
     _get_foreign_key)
 from django.contrib.admin.options import flatten_fieldsets, BaseModelAdmin
 from django.contrib.admin.options import HORIZONTAL, VERTICAL
+from django.contrib.admin.util import lookup_field
 
 
 __all__ = ['validate']
@@ -132 +133 @@
                             raise ImproperlyConfigured("%s.readonly_fields[%d], %r is not a callable or an attribute of %r or found in the model %r."
                                 % (cls.__name__, idx, field, cls.__name__, model._meta.object_name))
 
-    # list_select_related = False
     # save_as = False
     # save_on_top = False
-    for attr in ('list_select_related', 'save_as', 'save_on_top'):
+    for attr in ('save_as', 'save_on_top'):
         if not isinstance(getattr(cls, attr), bool):
             raise ImproperlyConfigured("'%s.%s' should be a boolean."
                     % (cls.__name__, attr))
 
+    # list_select_related = False
+    if not isinstance(cls.list_select_related, (bool, list, tuple)):
+        raise ImproperlyConfigured("'%s.list_select_related' should be a boolean, a list or a tuple."
+                                   % cls.__name__)
 
     # inlines = []
     if hasattr(cls, 'inlines'):
@@ -169 +173 @@
     fk = _get_foreign_key(parent_model, cls.model, fk_name=cls.fk_name, can_fail=True)
 
     # extra = 3
-    if not isinstance(cls.extra, int):
+    if not isinstance(getattr(cls, 'extra'), int):
         raise ImproperlyConfigured("'%s.extra' should be a integer."
                 % cls.__name__)
 

django/contrib/admin/views/main.py

@@ -3 +3 @@
 from django.contrib.admin.util import quote
 from django.core.paginator import Paginator, InvalidPage
 from django.db import models
+from django.db.models.query import QuerySet
 from django.utils.encoding import force_unicode, smart_str
 from django.utils.translation import ugettext
 from django.utils.http import urlencode
@@ -201 +202 @@
         # with a relationship and the provided queryset doesn't already have
         # select_related defined.
         if not qs.query.select_related:
-            if self.list_select_related:
+            if isinstance(self.list_select_related, (list, tuple)):
+                qs = qs.select_related(*self.list_select_related)
+            elif self.list_select_related:
                 qs = qs.select_related()
             else:
                 for field_name in self.list_display:
@@ -210 +213 @@
                     except models.FieldDoesNotExist:
                         pass
                     else:
-                        if isinstance(f.rel, models.ManyToOneRel):
-                            qs = qs.select_related()
-                            break
+                        if isinstance(f.rel, (models.ManyToOneRel, models.OneToOneRel)):
+                            qs = qs.select_related(field_name)
 
         # Set ordering.
         if self.order_field:

tests/regressiontests/admin_changelist/tests.py

@@ -2 +2 @@
 from django.contrib.admin.views.main import ChangeList
 from django.template import Context, Template
 from django.test import TransactionTestCase
-from regressiontests.admin_changelist.models import Child, Parent
+from regressiontests.admin_changelist.models import Child, Parent, NullableChild
 
 class ChangeListTests(TransactionTestCase):
     def test_select_related_preserved(self):
@@ -16 +16 @@
                 m.list_select_related, m.list_per_page, m.list_editable, m)
         self.assertEqual(cl.query_set.query.select_related, {'parent': {'name': {}}})
 
+    def test_select_related_nullable(self):
+        """
+        Regression test for #14396: allow list_select_related to explicitly 
+        specify relations to follow.
+        """
+        m = NullableChildAdmin(NullableChild, admin.site)
+        cl = ChangeList(MockRequest(), NullableChild, m.list_display, 
+                        m.list_display_links, m.list_filter, m.date_hierarchy, 
+                        m.search_fields, m.list_select_related, m.list_per_page, 
+                        m.list_editable, m)
+        self.assertEqual(cl.query_set.query.select_related, {'parent': {}})
+
     def test_result_list_html(self):
         """
         Verifies that inclusion tag result_list generates a table when with
@@ -76 +88 @@
     def queryset(self, request):
         return super(ChildAdmin, self).queryset(request).select_related("parent__name")
 
+class NullableChildAdmin(admin.ModelAdmin):
+    list_select_related = ('parent',)
+
 class MockRequest(object):
     GET = {}

tests/regressiontests/admin_changelist/models.py

@@ -6 +6 @@
 
 class Child(models.Model):
     parent = models.ForeignKey(Parent, editable=False)
-    name = models.CharField(max_length=30, blank=True)
- No newline at end of file
+    name = models.CharField(max_length=30, blank=True)
+
+class NullableChild(models.Model):
+    parent = models.ForeignKey(Parent, null=True, blank=True)
+    name = models.CharField(max_length=30, blank=True)

tests/regressiontests/modeladmin/tests.py

@@ -963 +963 @@
 
         self.assertRaisesRegexp(
             ImproperlyConfigured,
-            "'ValidationTestModelAdmin.list_select_related' should be a boolean.",
+            "'ValidationTestModelAdmin.list_select_related' should be a boolean, a list or a tuple.",
             validate,
             ValidationTestModelAdmin,
             ValidationTestModel,
@@ -974 +974 @@
 
         validate(ValidationTestModelAdmin, ValidationTestModel)
 
+        class ValidationTestModelAdmin(ModelAdmin):
+            list_select_related = ('field1', 'field2')
+
+        validate(ValidationTestModelAdmin, ValidationTestModel)
+
     def test_save_as_validation(self):
 
         class ValidationTestModelAdmin(ModelAdmin):

docs/ref/contrib/admin/index.txt

@@ -476 +476 @@
 objects on the admin change list page. This can save you a bunch of database
 queries.
 
-The value should be either ``True`` or ``False``. Default is ``False``.
+The value should be one of:
 
+* ``True`` or ``False``. If ``True``, the standard ``select_related()`` call 
+  will be used. 
+
+* A list or tuple of field names. In this case, the value is used as the 
+  argument to :meth:`~django.db.models.QuerySet.select_related` which determines 
+  which relations should be followed.
+
+The default is ``False``.
+
 Note that Django will use :meth:`~django.db.models.QuerySet.select_related`,
 regardless of this setting, if one of the ``list_display`` fields is a
-``ForeignKey``.
+``ForeignKey`` or ``OneToOneField``.
 
 .. attribute:: ModelAdmin.inlines