fix: get_containers_with_entity() was returning duplicate results (#304)

Author: bradenmacdonald

openedx_learning/apps/authoring/publishing/api.py

@@ -1370,23 +1370,16 @@ def get_containers_with_entity(
         ignore_pinned: if true, ignore any pinned references to the entity.
     """
     if ignore_pinned:
-        # TODO: Do we need to run distinct() on this? Will fix in https://github.com/openedx/openedx-learning/issues/303
         qs = Container.objects.filter(
             # Note: these two conditions must be in the same filter() call, or the query won't be correct.
             publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_id=publishable_entity_pk,  # pylint: disable=line-too-long # noqa: E501
             publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_version_id=None,  # pylint: disable=line-too-long # noqa: E501
-        ).order_by("pk")  # Ordering is mostly for consistent test cases.
+        )
     else:
         qs = Container.objects.filter(
             publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_id=publishable_entity_pk,  # pylint: disable=line-too-long # noqa: E501
-        ).order_by("pk")  # Ordering is mostly for consistent test cases.
-    # Could alternately do this query in two steps. Not sure which is more efficient; depends on how the DB plans it.
-    # # Find all the EntityLists that contain the given entity:
-    # lists = EntityList.objects.filter(entitylistrow__entity_id=publishable_entity_pk).values_list('pk', flat=True)
-    # qs = Container.objects.filter(
-    #     publishable_entity__draft__version__containerversion__entity_list__in=lists
-    # )
-    return qs
+        )
+    return qs.order_by("pk").distinct()  # Ordering is mostly for consistent test cases.
 
 
 def get_container_children_count(

tests/openedx_learning/apps/authoring/units/test_api.py

@@ -968,6 +968,11 @@ def test_units_containing(self):
         # Units 5/6 don't contain it
         _unit5_no = self.create_unit_with_components([self.component_2_v1, self.component_2], key="u5")
         _unit6_no = self.create_unit_with_components([], key="u6")
+        # To test unique results, unit 7 ✅ contains several copies of component 1. Also tests matching against
+        # components that aren't in the first position.
+        unit7_several = self.create_unit_with_components([
+            self.component_2, self.component_1, self.component_1_v1, self.component_1,
+        ], key="u7")
 
         # No need to publish anything as the get_containers_with_entity() API only considers drafts (for now).
 
@@ -980,6 +985,7 @@ def test_units_containing(self):
             unit1_1pinned,
             unit2_1pinned_v2,
             unit4_unpinned,
+            unit7_several,  # This should only appear once, not several times.
         ]
 
         # Test retrieving only "unpinned", for cases like potential deletion of a component, where we wouldn't care
@@ -990,7 +996,7 @@ def test_units_containing(self):
                 c.unit for c in
                 authoring_api.get_containers_with_entity(self.component_1.pk, ignore_pinned=True).select_related("unit")
             ]
-        assert result2 == [unit4_unpinned]
+        assert result2 == [unit4_unpinned, unit7_several]
 
     def test_add_remove_container_children(self):
         """