Commit 9ed971f4 authored by Marc Tamlyn's avatar Marc Tamlyn
Browse files

Merge pull request #1245 from oinopion/list_select_related

Fixed #19080 -- Fine-grained control over select_related in admin
parents 31fd64ad 0fd9f7c9
Loading
Loading
Loading
Loading
+8 −2
Original line number Diff line number Diff line
@@ -310,8 +310,14 @@ class ModelAdminValidator(BaseValidator):
                                % (cls.__name__, idx, field))

    def validate_list_select_related(self, cls, model):
        " Validate that list_select_related is a boolean. "
        check_type(cls, 'list_select_related', bool)
        " Validate that list_select_related is a boolean, a list or a tuple. "
        list_select_related = getattr(cls, 'list_select_related', None)
        if list_select_related:
            types = (bool, tuple, list)
            if not isinstance(list_select_related, types):
                raise ImproperlyConfigured("'%s.list_select_related' should be "
                                           "either a bool, a tuple or a list" %
                                           cls.__name__)

    def validate_list_per_page(self, cls, model):
        " Validate that list_per_page is an integer. "
+27 −17
Original line number Diff line number Diff line
@@ -356,36 +356,46 @@ class ChangeList(six.with_metaclass(RenameChangeListMethods)):
            # ValueError, ValidationError, or ?.
            raise IncorrectLookupParameters(e)

        # Use select_related() if one of the list_display options is a field
        # 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:
                qs = qs.select_related()
            else:
                for field_name in self.list_display:
                    try:
                        field = self.lookup_opts.get_field(field_name)
                    except models.FieldDoesNotExist:
                        pass
                    else:
                        if isinstance(field.rel, models.ManyToOneRel):
                            qs = qs.select_related()
                            break
            qs = self.apply_select_related(qs)

        # Set ordering.
        ordering = self.get_ordering(request, qs)
        qs = qs.order_by(*ordering)

        # Apply search results
        qs, search_use_distinct = self.model_admin.get_search_results(request, qs, self.query)
        qs, search_use_distinct = self.model_admin.get_search_results(
            request, qs, self.query)

        # Remove duplicates from results, if neccesary
        # Remove duplicates from results, if necessary
        if filters_use_distinct | search_use_distinct:
            return qs.distinct()
        else:
            return qs

    def apply_select_related(self, qs):
        if self.list_select_related is True:
            return qs.select_related()

        if self.list_select_related is False:
            if self.has_related_field_in_list_display():
                return qs.select_related()

        if self.list_select_related:
            return qs.select_related(*self.list_select_related)
        return qs

    def has_related_field_in_list_display(self):
        for field_name in self.list_display:
            try:
                field = self.lookup_opts.get_field(field_name)
            except models.FieldDoesNotExist:
                pass
            else:
                if isinstance(field.rel, models.ManyToOneRel):
                    return True
        return False

    def url_for_result(self, result):
        pk = getattr(result, self.pk_attname)
        return reverse('admin:%s_%s_change' % (self.opts.app_label,
+17 −5
Original line number Diff line number Diff line
@@ -812,12 +812,24 @@ subclass::
    the list of 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``.
    .. versionchanged:: dev

    Note that Django will use
    :meth:`~django.db.models.query.QuerySet.select_related`,
    regardless of this setting if one of the ``list_display`` fields is a
    ``ForeignKey``.
    The value should be either a boolean, a list or a tuple. Default is
    ``False``.

    When value is ``True``, ``select_related()`` will always be called. When
    value is set to ``False``, Django will look at ``list_display`` and call
    ``select_related()`` if any ``ForeignKey`` is present.

    If you need more fine-grained control, use a tuple (or list) as value for
    ``list_select_related``. Empty tuple will prevent Django from calling
    ``select_related`` at all. Any other tuple will be passed directly to
    ``select_related`` as parameters. For example::

        class ArticleAdmin(admin.ModelAdmin):
            list_select_related = ('author', 'category')

    will call ``select_related('author', 'category')``.

.. attribute:: ModelAdmin.ordering

+5 −0
Original line number Diff line number Diff line
@@ -67,6 +67,11 @@ class ChordsBandAdmin(admin.ModelAdmin):
    list_filter = ['members']


class InvitationAdmin(admin.ModelAdmin):
    list_display = ('band', 'player')
    list_select_related = ('player',)


class DynamicListDisplayChildAdmin(admin.ModelAdmin):
    list_display = ('parent', 'name', 'age')

+27 −4
Original line number Diff line number Diff line
@@ -18,7 +18,7 @@ from .admin import (ChildAdmin, QuartetAdmin, BandAdmin, ChordsBandAdmin,
    GroupAdmin, ParentAdmin, DynamicListDisplayChildAdmin,
    DynamicListDisplayLinksChildAdmin, CustomPaginationAdmin,
    FilteredChildAdmin, CustomPaginator, site as custom_site,
    SwallowAdmin, DynamicListFilterChildAdmin)
    SwallowAdmin, DynamicListFilterChildAdmin, InvitationAdmin)
from .models import (Event, Child, Parent, Genre, Band, Musician, Group,
    Quartet, Membership, ChordsMusician, ChordsBand, Invitation, Swallow,
    UnorderedObject, OrderedObject, CustomIdUser)
@@ -47,8 +47,31 @@ class ChangeListTests(TestCase):
        request = self.factory.get('/child/')
        cl = ChangeList(request, Child, 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_max_show_all, m.list_editable, m)
        self.assertEqual(cl.queryset.query.select_related, {'parent': {'name': {}}})
                        m.list_select_related, m.list_per_page,
                        m.list_max_show_all, m.list_editable, m)
        self.assertEqual(cl.queryset.query.select_related, {
            'parent': {'name': {}}
        })

    def test_select_related_as_tuple(self):
        ia = InvitationAdmin(Invitation, admin.site)
        request = self.factory.get('/invitation/')
        cl = ChangeList(request, Child, ia.list_display, ia.list_display_links,
                        ia.list_filter, ia.date_hierarchy, ia.search_fields,
                        ia.list_select_related, ia.list_per_page,
                        ia.list_max_show_all, ia.list_editable, ia)
        self.assertEqual(cl.queryset.query.select_related, {'player': {}})

    def test_select_related_as_empty_tuple(self):
        ia = InvitationAdmin(Invitation, admin.site)
        ia.list_select_related = ()
        request = self.factory.get('/invitation/')
        cl = ChangeList(request, Child, ia.list_display, ia.list_display_links,
                        ia.list_filter, ia.date_hierarchy, ia.search_fields,
                        ia.list_select_related, ia.list_per_page,
                        ia.list_max_show_all, ia.list_editable, ia)
        self.assertEqual(cl.queryset.query.select_related, False)


    def test_result_list_empty_changelist_value(self):
        """
Loading