diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index f77d848f64..1a4f76edfb 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -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,10 +23,20 @@ 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] - field = opts.get_field(field_name) - if hasattr(field, 'get_path_info') and any(path.m2m for path in field.get_path_info()): - return True + 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'): + # 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 diff --git a/tests/admin_changelist/admin.py b/tests/admin_changelist/admin.py index 926a45d518..bae857f30c 100644 --- a/tests/admin_changelist/admin.py +++ b/tests/admin_changelist/admin.py @@ -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'] diff --git a/tests/admin_changelist/models.py b/tests/admin_changelist/models.py index 76249b2cd3..2b8d2bc7f7 100644 --- a/tests/admin_changelist/models.py +++ b/tests/admin_changelist/models.py @@ -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) diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py index 7342e45830..ec8018e8af 100644 --- a/tests/admin_changelist/tests.py +++ b/tests/admin_changelist/tests.py @@ -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