Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #13902 -- Fixed admin list_filter so it doesn't show duplicate …

…results when it includes a field spec that involves m2m relationships with an intermediate model. Thanks Iván Raskovsky for the report and patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15526 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 337b6786fdfcc7a8b2f8eced0de6b966ab5553de 1 parent f6aa469
@ramiro ramiro authored
View
29 django/contrib/admin/views/main.py
@@ -169,6 +169,8 @@ def get_ordering(self):
return order_field, order_type
def get_query_set(self):
+ use_distinct = False
+
qs = self.root_query_set
lookup_params = self.params.copy() # a dictionary of the query string
for i in (ALL_VAR, ORDER_VAR, ORDER_TYPE_VAR, SEARCH_VAR, IS_POPUP_VAR):
@@ -181,6 +183,17 @@ def get_query_set(self):
del lookup_params[key]
lookup_params[smart_str(key)] = value
+ if not use_distinct:
+ # Check if it's a relationship that might return more than one
+ # instance
+ field_name = key.split('__', 1)[0]
+ try:
+ f = self.lookup_opts.get_field_by_name(field_name)[0]
+ except models.FieldDoesNotExist:
+ raise IncorrectLookupParameters
+ if hasattr(f, 'rel') and isinstance(f.rel, models.ManyToManyRel):
+ use_distinct = True
+
# if key ends with __in, split parameter into separate values
if key.endswith('__in'):
value = value.split(',')
@@ -246,12 +259,18 @@ def construct_search(field_name):
for bit in self.query.split():
or_queries = [models.Q(**{construct_search(str(field_name)): bit}) for field_name in self.search_fields]
qs = qs.filter(reduce(operator.or_, or_queries))
- for field_name in self.search_fields:
- if '__' in field_name:
- qs = qs.distinct()
- break
+ if not use_distinct:
+ for search_field in self.search_fields:
+ field_name = search_field.split('__', 1)[0]
+ f = self.lookup_opts.get_field_by_name(field_name)[0]
+ if hasattr(f, 'rel') and isinstance(f.rel, models.ManyToManyRel):
+ use_distinct = True
+ break
- return qs
+ if use_distinct:
+ return qs.distinct()
+ else:
+ return qs
def url_for_result(self, result):
return "%s/" % quote(getattr(result, self.pk_attname))
View
42 tests/regressiontests/admin_changelist/models.py
@@ -1,5 +1,4 @@
from django.db import models
-from django.contrib import admin
class Parent(models.Model):
name = models.CharField(max_length=128)
@@ -7,3 +6,44 @@ class Parent(models.Model):
class Child(models.Model):
parent = models.ForeignKey(Parent, editable=False, null=True)
name = models.CharField(max_length=30, blank=True)
+
+class Genre(models.Model):
+ name = models.CharField(max_length=20)
+
+class Band(models.Model):
+ name = models.CharField(max_length=20)
+ nr_of_members = models.PositiveIntegerField()
+ genres = models.ManyToManyField(Genre)
+
+class Musician(models.Model):
+ name = models.CharField(max_length=30)
+
+ def __unicode__(self):
+ return self.name
+
+class Group(models.Model):
+ name = models.CharField(max_length=30)
+ members = models.ManyToManyField(Musician, through='Membership')
+
+ def __unicode__(self):
+ return self.name
+
+class Membership(models.Model):
+ music = models.ForeignKey(Musician)
+ group = models.ForeignKey(Group)
+ role = models.CharField(max_length=15)
+
+class Quartet(Group):
+ pass
+
+class ChordsMusician(Musician):
+ pass
+
+class ChordsBand(models.Model):
+ name = models.CharField(max_length=30)
+ members = models.ManyToManyField(ChordsMusician, through='Invitation')
+
+class Invitation(models.Model):
+ player = models.ForeignKey(ChordsMusician)
+ band = models.ForeignKey(ChordsBand)
+ instrument = models.CharField(max_length=15)
View
111 tests/regressiontests/admin_changelist/tests.py
@@ -5,7 +5,8 @@
from django.template import Context, Template
from django.test import TransactionTestCase
-from regressiontests.admin_changelist.models import Child, Parent
+from models import (Child, Parent, Genre, Band, Musician, Group, Quartet,
+ Membership, ChordsMusician, ChordsBand, Invitation)
class ChangeListTests(TransactionTestCase):
@@ -135,18 +136,122 @@ def test_custom_paginator(self):
cl.get_results(request)
self.assertIsInstance(cl.paginator, CustomPaginator)
+ def test_distinct_for_m2m_in_list_filter(self):
+ """
+ Regression test for #13902: When using a ManyToMany in list_filter,
+ results shouldn't apper more than once. Basic ManyToMany.
+ """
+ blues = Genre.objects.create(name='Blues')
+ band = Band.objects.create(name='B.B. King Review', nr_of_members=11)
+
+ band.genres.add(blues)
+ band.genres.add(blues)
+
+ m = BandAdmin(Band, admin.site)
+ cl = ChangeList(MockFilteredRequestA(), Band, 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)
+
+ cl.get_results(MockFilteredRequestA())
+
+ # There's only one Group instance
+ self.assertEqual(cl.result_count, 1)
+
+ def test_distinct_for_through_m2m_in_list_filter(self):
+ """
+ Regression test for #13902: When using a ManyToMany in list_filter,
+ results shouldn't apper more than once. With an intermediate model.
+ """
+ lead = Musician.objects.create(name='Vox')
+ band = Group.objects.create(name='The Hype')
+ Membership.objects.create(group=band, music=lead, role='lead voice')
+ Membership.objects.create(group=band, music=lead, role='bass player')
+
+ m = GroupAdmin(Group, admin.site)
+ cl = ChangeList(MockFilteredRequestB(), Group, 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)
+
+ cl.get_results(MockFilteredRequestB())
+
+ # There's only one Group 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,
+ results shouldn't apper more than once. Model managed in the
+ admin inherits from the one that defins the relationship.
+ """
+ lead = Musician.objects.create(name='John')
+ four = Quartet.objects.create(name='The Beatles')
+ Membership.objects.create(group=four, music=lead, role='lead voice')
+ Membership.objects.create(group=four, music=lead, role='guitar player')
+
+ m = QuartetAdmin(Quartet, admin.site)
+ cl = ChangeList(MockFilteredRequestB(), Quartet, 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)
+
+ cl.get_results(MockFilteredRequestB())
+
+ # There's only one Quartet instance
+ self.assertEqual(cl.result_count, 1)
+
+ def test_distinct_for_m2m_to_inherited_in_list_filter(self):
+ """
+ Regression test for #13902: When using a ManyToMany in list_filter,
+ results shouldn't apper more than once. Target of the relationship
+ inherits from another.
+ """
+ lead = ChordsMusician.objects.create(name='Player A')
+ three = ChordsBand.objects.create(name='The Chords Trio')
+ Invitation.objects.create(band=three, player=lead, instrument='guitar')
+ Invitation.objects.create(band=three, player=lead, instrument='bass')
+
+ m = ChordsBandAdmin(ChordsBand, admin.site)
+ cl = ChangeList(MockFilteredRequestB(), ChordsBand, 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)
+
+ cl.get_results(MockFilteredRequestB())
+
+ # There's only one ChordsBand instance
+ self.assertEqual(cl.result_count, 1)
+
class ChildAdmin(admin.ModelAdmin):
list_display = ['name', 'parent']
def queryset(self, request):
return super(ChildAdmin, self).queryset(request).select_related("parent__name")
-
class MockRequest(object):
GET = {}
-
class CustomPaginator(Paginator):
def __init__(self, queryset, page_size, orphans=0, allow_empty_first_page=True):
super(CustomPaginator, self).__init__(queryset, 5, orphans=2,
allow_empty_first_page=allow_empty_first_page)
+
+
+class BandAdmin(admin.ModelAdmin):
+ list_filter = ['genres']
+
+class GroupAdmin(admin.ModelAdmin):
+ list_filter = ['members']
+
+class QuartetAdmin(admin.ModelAdmin):
+ list_filter = ['members']
+
+class ChordsBandAdmin(admin.ModelAdmin):
+ list_filter = ['members']
+
+class MockFilteredRequestA(object):
+ GET = { 'genres': 1 }
+
+class MockFilteredRequestB(object):
+ GET = { 'members': 1 }
Please sign in to comment.
Something went wrong with that request. Please try again.