Commit d084176c authored by Samuel Paccoud's avatar Samuel Paccoud Committed by Tim Graham
Browse files

Fixed #16609 -- Fixed duplicate admin results when searching nested M2M relations. 

This was fixed earlier but only when the M2M relation was at the first
level on the object. This commit fixes the issue even when the M2M is
at deeper levels, such as behind a foreign key.
parent 4dcc6493

django/contrib/admin/utils.py

+15 −4
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ from django.core.urlresolvers import NoReverseMatch, reverse 
from django.db import models

from django.db.models.constants import LOOKUP_SEP

from django.db.models.deletion import Collector

from django.db.models.sql.constants import QUERY_TERMS

from django.forms.forms import pretty_name

from django.utils import formats, six, timezone

from django.utils.encoding import force_str, force_text, smart_text
@@ -22,9 +23,19 @@ def lookup_needs_distinct(opts, lookup_path): 
    """

    Returns True if 'distinct()' should be used to query the given lookup path.

    """

    field_name = lookup_path.split('__', 1)[0]

    lookup_fields = lookup_path.split('__')

    # Remove the last item of the lookup path if it is a query term

    if lookup_fields[-1] in QUERY_TERMS:

        lookup_fields = lookup_fields[:-1]

    # Now go through the fields (following all relations) and look for an m2m

    for field_name in lookup_fields:

        field = opts.get_field(field_name)

    if hasattr(field, 'get_path_info') and any(path.m2m for path in field.get_path_info()):

        if hasattr(field, 'get_path_info'):

            # This field is a relation, update opts to follow the relation

            path_info = field.get_path_info()

            opts = path_info[-1].to_opts

            if any(path.m2m for path in path_info):

                # This field is a m2m relation so we know we need to call distinct

                return True

    return False

tests/admin_changelist/admin.py

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





class ConcertAdmin(admin.ModelAdmin):

    list_filter = ['group__members']

    search_fields = ['group__members__name']





class QuartetAdmin(admin.ModelAdmin):

    list_filter = ['members']

tests/admin_changelist/models.py

+5 −0
Original line number Diff line number Diff line
@@ -44,6 +44,11 @@ class Group(models.Model): 
        return self.name





class Concert(models.Model):

    name = models.CharField(max_length=30)

    group = models.ForeignKey(Group)





class Membership(models.Model):

    music = models.ForeignKey(Musician)

    group = models.ForeignKey(Group)

tests/admin_changelist/tests.py

+53 −5
Original line number Diff line number Diff line
@@ -17,17 +17,17 @@ from django.test.client import RequestFactory 
from django.utils import formats, six



from .admin import (

    BandAdmin, ChildAdmin, ChordsBandAdmin, CustomPaginationAdmin,

    CustomPaginator, DynamicListDisplayChildAdmin,

    BandAdmin, ChildAdmin, ChordsBandAdmin, ConcertAdmin,

    CustomPaginationAdmin, CustomPaginator, DynamicListDisplayChildAdmin,

    DynamicListDisplayLinksChildAdmin, DynamicListFilterChildAdmin,

    DynamicSearchFieldsChildAdmin, FilteredChildAdmin, GroupAdmin,

    InvitationAdmin, NoListDisplayLinksParentAdmin, ParentAdmin, QuartetAdmin,

    SwallowAdmin, site as custom_site,

)

from .models import (

    Band, Child, ChordsBand, ChordsMusician, CustomIdUser, Event, Genre, Group,

    Invitation, Membership, Musician, OrderedObject, Parent, Quartet, Swallow,

    UnorderedObject,

    Band, Child, ChordsBand, ChordsMusician, Concert, CustomIdUser, Event,

    Genre, Group, Invitation, Membership, Musician, OrderedObject, Parent,

    Quartet, Swallow, UnorderedObject,

)
@@ -259,6 +259,31 @@ class ChangeListTests(TestCase): 
        # There's only one Group instance

        self.assertEqual(cl.result_count, 1)



    def test_distinct_for_through_m2m_at_second_level_in_list_filter(self):

        """

        When using a ManyToMany in list_filter at the second level behind a

        ForeignKey, distinct() must be called and results shouldn't appear more

        than once.

        """

        lead = Musician.objects.create(name='Vox')

        band = Group.objects.create(name='The Hype')

        Concert.objects.create(name='Woodstock', group=band)

        Membership.objects.create(group=band, music=lead, role='lead voice')

        Membership.objects.create(group=band, music=lead, role='bass player')



        m = ConcertAdmin(Concert, admin.site)

        request = self.factory.get('/concert/', data={'group__members': lead.pk})



        cl = ChangeList(request, Concert, 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)



        cl.get_results(request)



        # There's only one Concert instance

        self.assertEqual(cl.result_count, 1)



    def test_distinct_for_inherited_m2m_in_list_filter(self):

        """

        Regression test for #13902: When using a ManyToMany in list_filter,
@@ -348,6 +373,29 @@ class ChangeListTests(TestCase): 
        # Make sure distinct() was called

        self.assertEqual(cl.queryset.count(), 1)



    def test_distinct_for_many_to_many_at_second_level_in_search_fields(self):

        """

        When using a ManyToMany in search_fields at the second level behind a

        ForeignKey, distinct() must be called and results shouldn't appear more

        than once.

        """

        lead = Musician.objects.create(name='Vox')

        band = Group.objects.create(name='The Hype')

        Concert.objects.create(name='Woodstock', group=band)

        Membership.objects.create(group=band, music=lead, role='lead voice')

        Membership.objects.create(group=band, music=lead, role='bass player')



        m = ConcertAdmin(Concert, admin.site)

        request = self.factory.get('/concert/', data={SEARCH_VAR: 'vox'})



        cl = ChangeList(request, Concert, 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)



        # There's only one Concert instance

        self.assertEqual(cl.queryset.count(), 1)



    def test_pagination(self):

        """

        Regression tests for #12893: Pagination in admins changelist doesn't