Ticket #13902: 13902_distinct_in_changelist5.patch

django/contrib/admin/views/main.py

@@ -169 +169 @@
         return order_field, order_type
 
     def get_query_set(self):
+        is_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 +183 @@
                 del lookup_params[key]
                 lookup_params[smart_str(key)] = value
 
+            if not is_distinct:
+                # Check if it's a relationship that might return more than one
+                # instance
+                try:
+                    f = self.lookup_opts.get_field_by_name(key.split('__',1)[0])
+                except models.FieldDoesNotExist:
+                    raise IncorrectLookupParameters
+                if (isinstance(f[0], models.related.RelatedObject) or 
+                    isinstance(f[0].rel, (models.ManyToOneRel,
+                                         models.ManyToManyRel))):
+                    is_distinct = True
+
             # if key ends with __in, split parameter into separate values
             if key.endswith('__in'):
                 lookup_params[key] = value.split(',')
@@ -244 +258 @@
             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 is_distinct:
+                for field_name in self.search_fields:
+                    f = self.lookup_opts.get_field_by_name(field_name.split('__',1)[0])
+                    if (isinstance(f[0], models.related.RelatedObject) or 
+                        isinstance(f[0].rel, (models.ManyToOneRel,
+                                             models.ManyToManyRel))):
+                        is_distinct = True
+                        break
 
-        return qs
+        if is_distinct:
+            return qs.distinct()
+        else:
+            return qs
 
     def url_for_result(self, result):
         return "%s/" % quote(getattr(result, self.pk_attname))

tests/regressiontests/admin_changelist/models.py

@@ -7 +7 @@
 class Child(models.Model):
     parent = models.ForeignKey(Parent, editable=False, null=True)
     name = models.CharField(max_length=30, blank=True)
+    multiple_m2m = models.ManyToManyField(Parent, related_name='multiple_m2m',
+                                          through='Through')
+
+class Through(models.Model):
+    parent = models.ForeignKey(Parent)
+    child = models.ForeignKey(Child)

tests/regressiontests/admin_changelist/tests.py

@@ -5 +5 @@
 from django.template import Context, Template
 from django.test import TransactionTestCase
 
-from regressiontests.admin_changelist.models import Child, Parent
+from regressiontests.admin_changelist.models import Child, Parent, Through
 
 
 class ChangeListTests(TransactionTestCase):
+    def setUp(self):
+        self.new_parent = Parent.objects.create(name='parent')
+        self.new_child = Child.objects.create(name='name', parent=self.new_parent)
+
     def test_select_related_preserved(self):
         """
         Regression test for #10348: ChangeList.get_query_set() shouldn't
@@ -35 +39 @@
         template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}')
         context = Context({'cl': cl})
         table_output = template.render(context)
-        row_html = '<tbody><tr class="row1"><td><input type="checkbox" class="action-select" value="1" name="_selected_action" /></td><th><a href="1/">name</a></th><td>(None)</td></tr></tbody>'
+        row_html = '<tbody><tr class="row1"><td><input type="checkbox" class="action-select" value="2" name="_selected_action" /></td><th><a href="2/">name</a></th><td>(None)</td></tr>'
         self.assertFalse(table_output.find(row_html) == -1,
             'Failed to find expected row element: %s' % table_output)
 
@@ -45 +49 @@
         Verifies that inclusion tag result_list generates a table when with
         default ModelAdmin settings.
         """
-        new_parent = Parent.objects.create(name='parent')
-        new_child = Child.objects.create(name='name', parent=new_parent)
+        new_parent = self.new_parent
+        new_child = self.new_child
         request = MockRequest()
         m = ChildAdmin(Child, admin.site)
         cl = ChangeList(request, Child, m.list_display, m.list_display_links,
@@ -69 +73 @@
         when list_editable is enabled are rendered in a div outside the
         table.
         """
-        new_parent = Parent.objects.create(name='parent')
-        new_child = Child.objects.create(name='name', parent=new_parent)
+        new_parent = self.new_parent
+        new_child = self.new_child
         request = MockRequest()
         m = ChildAdmin(Child, admin.site)
 
@@ -136 +140 @@
         self.assertIsInstance(cl.paginator, CustomPaginator)
 
 
+    def test_distinct(self):
+        """
+        Regression test for #13902: When using a ManyToMany in list_filter,
+        results may apper more than once
+        """
+        new_parent = self.new_parent
+        new_child = self.new_child
+        relation1 = Through.objects.create(parent=new_parent, child=new_child)
+        relation2 = Through.objects.create(parent=new_parent, child=new_child)
+
+        m = ChildAdmin(Child, admin.site)
+        cl = ChangeList(MockFilteredRequest(), Child, 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(MockFilteredRequest())
+
+        # There's only one Child 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")
+    list_filter = ['multiple_m2m', ]
 
 
 class MockRequest(object):
     GET = {}
 
+class MockFilteredRequest(object):
+    GET = {'multiple_m2m__id__exact': 1, }
 
 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)
+