Author: carljm
Date: 2011-04-22 21:40:06 -0700 (Fri, 22 Apr 2011)
New Revision: 16094
Modified:
django/branches/releases/1.3.X/django/contrib/admin/views/main.py
django/branches/releases/1.3.X/tests/regressiontests/admin_changelist/tests.py
Log:
[1.3.X] Fixed #15819 - Fixed 1.3 regression from r15526 causing duplicate
search results in admin with search_fields traversing to non-M2M related
models. Thanks to Adam Kochanowski for the report and Ryan Kaskel for the patch.
Backport of r16093 from trunk.
Modified: django/branches/releases/1.3.X/django/contrib/admin/views/main.py
===================================================================
--- django/branches/releases/1.3.X/django/contrib/admin/views/main.py
2011-04-23 03:55:21 UTC (rev 16093)
+++ django/branches/releases/1.3.X/django/contrib/admin/views/main.py
2011-04-23 04:40:06 UTC (rev 16094)
@@ -26,6 +26,15 @@
# Text to display within change-list table cells if the value is blank.
EMPTY_CHANGELIST_VALUE = ugettext_lazy('(None)')
+def field_needs_distinct(field):
+ if ((hasattr(field, 'rel') and
+ isinstance(field.rel, models.ManyToManyRel)) or
+ (isinstance(field, models.related.RelatedObject) and
+ not field.field.unique)):
+ return True
+ return False
+
+
class ChangeList(object):
def __init__(self, request, model, list_display, list_display_links,
list_filter, date_hierarchy, search_fields, list_select_related, list_per_page,
list_editable, model_admin):
self.model = model
@@ -189,8 +198,7 @@
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
+ use_distinct = field_needs_distinct(f)
# if key ends with __in, split parameter into separate values
if key.endswith('__in'):
@@ -264,7 +272,7 @@
for search_spec in orm_lookups:
field_name = search_spec.split('__', 1)[0]
f = self.lookup_opts.get_field_by_name(field_name)[0]
- if hasattr(f, 'rel') and isinstance(f.rel,
models.ManyToManyRel):
+ if field_needs_distinct(f):
use_distinct = True
break
Modified:
django/branches/releases/1.3.X/tests/regressiontests/admin_changelist/tests.py
===================================================================
---
django/branches/releases/1.3.X/tests/regressiontests/admin_changelist/tests.py
2011-04-23 03:55:21 UTC (rev 16093)
+++
django/branches/releases/1.3.X/tests/regressiontests/admin_changelist/tests.py
2011-04-23 04:40:06 UTC (rev 16094)
@@ -1,6 +1,6 @@
from django.contrib import admin
from django.contrib.admin.options import IncorrectLookupParameters
-from django.contrib.admin.views.main import ChangeList
+from django.contrib.admin.views.main import ChangeList, SEARCH_VAR
from django.core.paginator import Paginator
from django.template import Context, Template
from django.test import TransactionTestCase
@@ -148,12 +148,14 @@
band.genres.add(blues)
m = BandAdmin(Band, admin.site)
- cl = ChangeList(MockFilteredRequestA(blues.pk), Band, m.list_display,
+ request = MockFilterRequest('genres', blues.pk)
+
+ cl = ChangeList(request, 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(blues.pk))
+ cl.get_results(request)
# There's only one Group instance
self.assertEqual(cl.result_count, 1)
@@ -169,12 +171,14 @@
Membership.objects.create(group=band, music=lead, role='bass player')
m = GroupAdmin(Group, admin.site)
- cl = ChangeList(MockFilteredRequestB(lead.pk), Group, m.list_display,
+ request = MockFilterRequest('members', lead.pk)
+
+ cl = ChangeList(request, 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(lead.pk))
+ cl.get_results(request)
# There's only one Group instance
self.assertEqual(cl.result_count, 1)
@@ -191,12 +195,14 @@
Membership.objects.create(group=four, music=lead, role='guitar player')
m = QuartetAdmin(Quartet, admin.site)
- cl = ChangeList(MockFilteredRequestB(lead.pk), Quartet, m.list_display,
+ request = MockFilterRequest('members', lead.pk)
+
+ cl = ChangeList(request, 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(lead.pk))
+ cl.get_results(request)
# There's only one Quartet instance
self.assertEqual(cl.result_count, 1)
@@ -213,16 +219,59 @@
Invitation.objects.create(band=three, player=lead, instrument='bass')
m = ChordsBandAdmin(ChordsBand, admin.site)
- cl = ChangeList(MockFilteredRequestB(lead.pk), ChordsBand,
m.list_display,
+ request = MockFilterRequest('members', lead.pk)
+
+ cl = ChangeList(request, 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(lead.pk))
+ cl.get_results(request)
# There's only one ChordsBand instance
self.assertEqual(cl.result_count, 1)
+ def test_distinct_for_non_unique_related_object_in_list_filter(self):
+ """
+ Regressions tests for #15819: If a field listed in list_filters
+ is a non-unique related object, distinct() must be called.
+ """
+ parent = Parent.objects.create(name='Mary')
+ # Two children with the same name
+ Child.objects.create(parent=parent, name='Daniel')
+ Child.objects.create(parent=parent, name='Daniel')
+
+ m = ParentAdmin(Parent, admin.site)
+ request = MockFilterRequest('child__name', 'Daniel')
+
+ cl = ChangeList(request, Parent, 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)
+
+ # Make sure distinct() was called
+ self.assertEqual(cl.query_set.count(), 1)
+
+ def test_distinct_for_non_unique_related_object_in_search_fields(self):
+ """
+ Regressions tests for #15819: If a field listed in search_fields
+ is a non-unique related object, distinct() must be called.
+ """
+ parent = Parent.objects.create(name='Mary')
+ Child.objects.create(parent=parent, name='Danielle')
+ Child.objects.create(parent=parent, name='Daniel')
+
+ m = ParentAdmin(Parent, admin.site)
+ request = MockSearchRequest('daniel')
+
+ cl = ChangeList(request, Parent, 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)
+
+ # Make sure distinct() was called
+ self.assertEqual(cl.query_set.count(), 1)
+
def test_pagination(self):
"""
Regression tests for #12893: Pagination in admins changelist doesn't
@@ -254,6 +303,11 @@
self.assertEqual(cl.paginator.page_range, [1, 2, 3])
+class ParentAdmin(admin.ModelAdmin):
+ list_filter = ['child__name']
+ search_fields = ['child__name']
+
+
class ChildAdmin(admin.ModelAdmin):
list_display = ['name', 'parent']
list_per_page = 10
@@ -288,10 +342,10 @@
class ChordsBandAdmin(admin.ModelAdmin):
list_filter = ['members']
-class MockFilteredRequestA(object):
- def __init__(self, pk):
- self.GET = { 'genres' : pk }
+class MockFilterRequest(object):
+ def __init__(self, filter, q):
+ self.GET = {filter: q}
-class MockFilteredRequestB(object):
- def __init__(self, pk):
- self.GET = { 'members': pk }
+class MockSearchRequest(object):
+ def __init__(self, q):
+ self.GET = {SEARCH_VAR: q}
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
