From 616afd409d45d1a566f4dff95dca9068d93fbdfb Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 10 Nov 2025 21:10:31 -0500 Subject: [PATCH 1/2] fix: Call `LIBRARY_CONTAINER_PUBLISHED` for parent of containers --- .../djangoapps/content_libraries/tasks.py | 10 ++++ .../content_libraries/tests/test_events.py | 47 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index 93d9fef725ec..d424e8a1a8f5 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -127,6 +127,16 @@ def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None elif hasattr(record.entity, "container"): container_key = api.library_container_locator(library_key, record.entity.container) affected_containers.add(container_key) + + try: + # We do need to notify listeners that the parent container(s) have changed, + # e.g. so the search index can update the "has_unpublished_changes" + for parent_container in api.get_containers_contains_item(container_key): + affected_containers.add(parent_container.container_key) + # TODO: should this be a CONTAINER_CHILD_PUBLISHED event instead of CONTAINER_PUBLISHED ? + except api.ContentLibraryContainerNotFound: + # The deleted children remains in the entity, so, in this case, the container may not be found. + pass else: log.warning( f"PublishableEntity {record.entity.pk} / {record.entity.key} was modified during publish operation " diff --git a/openedx/core/djangoapps/content_libraries/tests/test_events.py b/openedx/core/djangoapps/content_libraries/tests/test_events.py index 88d426d3ef06..79a9a99a7e92 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_events.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_events.py @@ -449,6 +449,53 @@ def test_publish_container(self) -> None: c2_after = self._get_container(container2["id"]) assert c2_after["has_unpublished_changes"] + def test_publish_child_container(self): + """ + Test the events that get emitted when we publish the changes to a container that is child of another container + """ + # Create some containers + container1 = self._create_container(self.lib1_key, "unit", display_name="Alpha Unit", slug=None) + container2 = self._create_container(self.lib1_key, "subsection", display_name="Bravo Subsection", slug=None) + + # Add one container as child + self._add_container_children(container2["id"], children_ids=[container1["id"]]) + + # At first everything is unpublished: + c1_before = self._get_container(container1["id"]) + assert c1_before["has_unpublished_changes"] + c2_before = self._get_container(container2["id"]) + assert c2_before["has_unpublished_changes"] + + # clear event log after the initial mock data setup is complete: + self.clear_events() + + # Now publish only Container 1 + self._publish_container(container1["id"]) + + # Now it is published: + c1_after = self._get_container(container1["id"]) + assert c1_after["has_unpublished_changes"] is False + + # And publish events were emitted: + self.expect_new_events( + { # An event for container 1 being published: + "signal": LIBRARY_CONTAINER_PUBLISHED, + "library_container": LibraryContainerData( + container_key=LibraryContainerLocator.from_string(container1["id"]), + ), + }, + { # An event for parent (container 2): + "signal": LIBRARY_CONTAINER_PUBLISHED, + "library_container": LibraryContainerData( + container_key=LibraryContainerLocator.from_string(container2["id"]), + ), + }, + ) + + # note that container 2 is still unpublished + c2_after = self._get_container(container2["id"]) + assert c2_after["has_unpublished_changes"] + def test_restore_unit(self) -> None: """ Test restoring a deleted unit via the "restore" API. From fb65b9ea01b23e6577c5f9f9f902029f161118a9 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 11 Nov 2025 13:50:56 -0500 Subject: [PATCH 2/2] style: Nits on the code --- .../djangoapps/content_libraries/tasks.py | 1 - .../content_libraries/tests/test_events.py | 28 +++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index d424e8a1a8f5..d4e6196e03a2 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -133,7 +133,6 @@ def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None # e.g. so the search index can update the "has_unpublished_changes" for parent_container in api.get_containers_contains_item(container_key): affected_containers.add(parent_container.container_key) - # TODO: should this be a CONTAINER_CHILD_PUBLISHED event instead of CONTAINER_PUBLISHED ? except api.ContentLibraryContainerNotFound: # The deleted children remains in the entity, so, in this case, the container may not be found. pass diff --git a/openedx/core/djangoapps/content_libraries/tests/test_events.py b/openedx/core/djangoapps/content_libraries/tests/test_events.py index 79a9a99a7e92..975cfbafb4d9 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_events.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_events.py @@ -454,46 +454,46 @@ def test_publish_child_container(self): Test the events that get emitted when we publish the changes to a container that is child of another container """ # Create some containers - container1 = self._create_container(self.lib1_key, "unit", display_name="Alpha Unit", slug=None) - container2 = self._create_container(self.lib1_key, "subsection", display_name="Bravo Subsection", slug=None) + unit = self._create_container(self.lib1_key, "unit", display_name="Alpha Unit", slug=None) + subsection = self._create_container(self.lib1_key, "subsection", display_name="Bravo Subsection", slug=None) # Add one container as child - self._add_container_children(container2["id"], children_ids=[container1["id"]]) + self._add_container_children(subsection["id"], children_ids=[unit["id"]]) # At first everything is unpublished: - c1_before = self._get_container(container1["id"]) + c1_before = self._get_container(unit["id"]) assert c1_before["has_unpublished_changes"] - c2_before = self._get_container(container2["id"]) + c2_before = self._get_container(subsection["id"]) assert c2_before["has_unpublished_changes"] # clear event log after the initial mock data setup is complete: self.clear_events() - # Now publish only Container 1 - self._publish_container(container1["id"]) + # Now publish only the unit + self._publish_container(unit["id"]) # Now it is published: - c1_after = self._get_container(container1["id"]) + c1_after = self._get_container(unit["id"]) assert c1_after["has_unpublished_changes"] is False # And publish events were emitted: self.expect_new_events( - { # An event for container 1 being published: + { # An event for the unit being published: "signal": LIBRARY_CONTAINER_PUBLISHED, "library_container": LibraryContainerData( - container_key=LibraryContainerLocator.from_string(container1["id"]), + container_key=LibraryContainerLocator.from_string(unit["id"]), ), }, - { # An event for parent (container 2): + { # An event for parent (subsection): "signal": LIBRARY_CONTAINER_PUBLISHED, "library_container": LibraryContainerData( - container_key=LibraryContainerLocator.from_string(container2["id"]), + container_key=LibraryContainerLocator.from_string(subsection["id"]), ), }, ) - # note that container 2 is still unpublished - c2_after = self._get_container(container2["id"]) + # note that subsection is still unpublished + c2_after = self._get_container(subsection["id"]) assert c2_after["has_unpublished_changes"] def test_restore_unit(self) -> None: