diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 4e1af641..c087dfe9 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -1367,23 +1367,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( diff --git a/tests/openedx_learning/apps/authoring/units/test_api.py b/tests/openedx_learning/apps/authoring/units/test_api.py index 1e7c0ea1..0e83c2cf 100644 --- a/tests/openedx_learning/apps/authoring/units/test_api.py +++ b/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): """