Commit 6a3d9182 authored by Carl Meyer's avatar Carl Meyer
Browse files

[1.3.X] Fixed #15819 - Fixed 1.3 regression from r15526 causing duplicate... 

[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.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.3.X@16094 bcc190cf-cafb-0310-a4f2-bffc1f526a37
parent fe2713dd

django/contrib/admin/views/main.py

+11 −3
Original line number Diff line number Diff line
@@ -26,6 +26,15 @@ ERROR_FLAG = 'e' 
# 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 @@ class ChangeList(object): 
                    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 @@ class ChangeList(object): 
                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

tests/regressiontests/admin_changelist/tests.py

+69 −15
Original line number Diff line number Diff line 
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 @@ class ChangeListTests(TransactionTestCase): 
        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 @@ class ChangeListTests(TransactionTestCase): 
        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 @@ class ChangeListTests(TransactionTestCase): 
        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 @@ class ChangeListTests(TransactionTestCase): 
        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 @@ class ChangeListTests(TransactionTestCase): 
        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 QuartetAdmin(admin.ModelAdmin): 
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}