From 334c0fee51e29bfd6a9f53ffc50210cd1c612da2 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 17 Apr 2025 22:05:44 -0700 Subject: [PATCH] feat: REST API to publish the changes to a container in a library (#36543) * feat: REST API to publish the changes to a container * fix: trigger LIBRARY_CONTAINER_UPDATED when component published for components in containers. --------- Co-authored-by: Jillian Vogel --- .../content_libraries/api/blocks.py | 31 ++++--- .../content_libraries/api/containers.py | 52 ++++++++++- .../content_libraries/rest_api/blocks.py | 8 ++ .../content_libraries/rest_api/containers.py | 22 +++++ .../content_libraries/tests/base.py | 5 ++ .../tests/test_containers.py | 89 ++++++++++++++++++- .../core/djangoapps/content_libraries/urls.py | 3 +- 7 files changed, 193 insertions(+), 17 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 738fec4b41..1bfb054f42 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -48,7 +48,6 @@ from openedx.core.djangoapps.xblock.api import ( from openedx.core.types import User as UserType from ..models import ContentLibrary -from ..permissions import CAN_EDIT_THIS_CONTENT_LIBRARY from .exceptions import ( BlockLimitReachedError, ContentLibraryBlockNotFound, @@ -67,7 +66,6 @@ from .containers import ( from .libraries import ( library_collection_locator, library_component_usage_key, - require_permission_for_library_key, PublishableItem, ) @@ -891,18 +889,14 @@ def publish_component_changes(usage_key: LibraryUsageLocatorV2, user: UserType): """ Publish all pending changes in a single component. """ - content_library = require_permission_for_library_key( - usage_key.lib_key, - user, - CAN_EDIT_THIS_CONTENT_LIBRARY - ) - learning_package = content_library.learning_package - - assert learning_package component = get_component_from_usage_key(usage_key) - drafts_to_publish = authoring_api.get_all_drafts(learning_package.id).filter( - entity__key=component.key - ) + library_key = usage_key.context_key + content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] + learning_package = content_library.learning_package + assert learning_package + # The core publishing API is based on draft objects, so find the draft that corresponds to this component: + drafts_to_publish = authoring_api.get_all_drafts(learning_package.id).filter(entity__key=component.key) + # Publish the component and update anything that needs to be updated (e.g. search index): authoring_api.publish_from_drafts(learning_package.id, draft_qset=drafts_to_publish, published_by=user.id) LIBRARY_BLOCK_UPDATED.send_event( library_block=LibraryBlockData( @@ -911,6 +905,17 @@ def publish_component_changes(usage_key: LibraryUsageLocatorV2, user: UserType): ) ) + # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger + # container indexing asynchronously. + affected_containers = get_containers_contains_component(usage_key) + for container in affected_containers: + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + container_key=container.container_key, + background=True, + ) + ) + def _component_exists(usage_key: UsageKeyV2) -> bool: """ diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 71b582f15b..800cbc0585 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -6,14 +6,21 @@ from __future__ import annotations from dataclasses import dataclass from datetime import datetime from enum import Enum +import logging from uuid import uuid4 from django.utils.text import slugify from opaque_keys.edx.keys import UsageKeyV2 from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 -from openedx_events.content_authoring.data import ContentObjectChangedData, LibraryCollectionData, LibraryContainerData +from openedx_events.content_authoring.data import ( + ContentObjectChangedData, + LibraryBlockData, + LibraryCollectionData, + LibraryContainerData, +) from openedx_events.content_authoring.signals import ( CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + LIBRARY_BLOCK_UPDATED, LIBRARY_COLLECTION_UPDATED, LIBRARY_CONTAINER_CREATED, LIBRARY_CONTAINER_DELETED, @@ -27,7 +34,7 @@ from openedx.core.djangoapps.xblock.api import get_component_from_usage_key from ..models import ContentLibrary from .exceptions import ContentLibraryContainerNotFound -from .libraries import LibraryXBlockMetadata, PublishableItem +from .libraries import LibraryXBlockMetadata, PublishableItem, library_component_usage_key # The public API is only the following symbols: __all__ = [ @@ -46,8 +53,11 @@ __all__ = [ "restore_container", "update_container_children", "get_containers_contains_component", + "publish_container_changes", ] +log = logging.getLogger(__name__) + class ContainerType(Enum): Unit = "unit" @@ -400,3 +410,41 @@ def get_containers_contains_component( ContainerMetadata.from_container(usage_key.context_key, container) for container in containers ] + + +def publish_container_changes(container_key: LibraryContainerLocator, user_id: int | None) -> None: + """ + Publish all unpublished changes in a container and all its child + containers/blocks. + """ + container = get_container_from_key(container_key) + library_key = container_key.library_key + content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] + learning_package = content_library.learning_package + assert learning_package + # The core publishing API is based on draft objects, so find the draft that corresponds to this container: + drafts_to_publish = authoring_api.get_all_drafts(learning_package.id).filter(entity__pk=container.pk) + # Publish the container, which will also auto-publish any unpublished child components: + publish_log = authoring_api.publish_from_drafts( + learning_package.id, + draft_qset=drafts_to_publish, + published_by=user_id, + ) + # Update anything that needs to be updated (e.g. search index): + for record in publish_log.records.select_related("entity", "entity__container", "entity__component").all(): + if hasattr(record.entity, "component"): + # This is a child component like an XBLock in a Unit that was published: + usage_key = library_component_usage_key(library_key, record.entity.component) + LIBRARY_BLOCK_UPDATED.send_event( + library_block=LibraryBlockData(library_key=library_key, usage_key=usage_key) + ) + elif hasattr(record.entity, "container"): + # This is a child container like a Unit, or is the same "container" we published above. + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData(container_key=container_key) + ) + else: + log.warning( + f"PublishableEntity {record.entity.pk} / {record.entity.key} was modified during publish operation " + "but is of unknown type." + ) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py index b4f23f93c6..6ab35c47c6 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -233,7 +233,15 @@ class LibraryBlockPublishView(APIView): @convert_exceptions def post(self, request, usage_key_str): + """ + Publish the draft changes made to this component. + """ key = LibraryUsageLocatorV2.from_string(usage_key_str) + api.require_permission_for_library_key( + key.lib_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) api.publish_component_changes(key, request.user) return Response({}) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 6c76bf3a2d..cd1b7602fb 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -328,3 +328,25 @@ class LibraryContainerCollectionsView(GenericAPIView): ) return Response({'count': len(collection_keys)}) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerPublishView(GenericAPIView): + """ + View to publish a container, or revert to last published. + """ + @convert_exceptions + def post(self, request: RestRequest, container_key: LibraryContainerLocator) -> Response: + """ + Publish the container and its children + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + api.publish_container_changes(container_key, request.user.id) + # If we need to in the future, we could return a list of all the child containers/components that were + # auto-published as a result. + return Response({}) diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 195805f94e..b81382ab00 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -36,6 +36,7 @@ URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a containe URL_LIB_CONTAINER_COMPONENTS = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container URL_LIB_CONTAINER_RESTORE = URL_LIB_CONTAINER + 'restore/' # Restore a deleted container URL_LIB_CONTAINER_COLLECTIONS = URL_LIB_CONTAINER + 'collections/' # Handle associated collections +URL_LIB_CONTAINER_PUBLISH = URL_LIB_CONTAINER + 'publish/' # Publish changes to the specified container + children URL_LIB_LTI_PREFIX = URL_PREFIX + 'lti/1.3/' URL_LIB_LTI_JWKS = URL_LIB_LTI_PREFIX + 'pub/jwks/' @@ -455,3 +456,7 @@ class ContentLibrariesRestApiTest(APITransactionTestCase): {'collection_keys': collection_keys}, expect_response ) + + def _publish_container(self, container_key, expect_response=200): + """ Publish all changes in the specified container + children """ + return self._api('post', URL_LIB_CONTAINER_PUBLISH.format(container_key=container_key), None, expect_response) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 10f7f269c6..9b97ddfdfa 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -7,9 +7,10 @@ from unittest import mock import ddt from freezegun import freeze_time -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_events.content_authoring.data import LibraryContainerData from openedx_events.content_authoring.signals import ( + LIBRARY_BLOCK_UPDATED, LIBRARY_CONTAINER_CREATED, LIBRARY_CONTAINER_DELETED, LIBRARY_CONTAINER_UPDATED, @@ -43,6 +44,7 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): break any tests, but backwards-incompatible API changes will. """ ENABLED_OPENEDX_EVENTS = [ + LIBRARY_BLOCK_UPDATED.event_type, LIBRARY_CONTAINER_CREATED.event_type, LIBRARY_CONTAINER_DELETED.event_type, LIBRARY_CONTAINER_UPDATED.event_type, @@ -413,3 +415,88 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): # Verify the collections assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.key}] + + def test_publish_container(self): # pylint: disable=too-many-statements + """ + Test that we can publish the changes to a specific container + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + + # Create two containers and add some components + container1 = self._create_container(lib["id"], "unit", display_name="Alpha Unit", slug=None) + container2 = self._create_container(lib["id"], "unit", display_name="Bravo Unit", slug=None) + problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) + html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) + html_block2 = self._add_block_to_library(lib["id"], "html", "Html2", can_stand_alone=False) + self._add_container_components(container1["id"], children_ids=[problem_block["id"], html_block["id"]]) + self._add_container_components(container2["id"], children_ids=[html_block["id"], html_block2["id"]]) + # At first everything is unpublished: + c1_before = self._get_container(container1["id"]) + assert c1_before["has_unpublished_changes"] + c1_components_before = self._get_container_components(container1["id"]) + assert len(c1_components_before) == 2 + assert c1_components_before[0]["id"] == problem_block["id"] + assert c1_components_before[0]["has_unpublished_changes"] + assert c1_components_before[0]["published_by"] is None + assert c1_components_before[1]["id"] == html_block["id"] + assert c1_components_before[1]["has_unpublished_changes"] + assert c1_components_before[1]["published_by"] is None + c2_before = self._get_container(container2["id"]) + assert c2_before["has_unpublished_changes"] + + # Set up event receivers after the initial mock data setup is complete: + updated_container_receiver = mock.Mock() + updated_block_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(updated_container_receiver) + LIBRARY_BLOCK_UPDATED.connect(updated_block_receiver) + + # 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 + c1_components_after = self._get_container_components(container1["id"]) + assert len(c1_components_after) == 2 + assert c1_components_after[0]["id"] == problem_block["id"] + assert c1_components_after[0]["has_unpublished_changes"] is False + assert c1_components_after[0]["published_by"] == self.user.username + assert c1_components_after[1]["id"] == html_block["id"] + assert c1_components_after[1]["has_unpublished_changes"] is False + assert c1_components_after[1]["published_by"] == self.user.username + + # and container 2 is still unpublished, except for the shared HTML block that is also in container 1: + c2_after = self._get_container(container2["id"]) + assert c2_after["has_unpublished_changes"] + c2_components_after = self._get_container_components(container2["id"]) + assert len(c2_components_after) == 2 + assert c2_components_after[0]["id"] == html_block["id"] + assert c2_components_after[0]["has_unpublished_changes"] is False # published since it's also in container 1 + assert c2_components_after[0]["published_by"] == self.user.username + assert c2_components_after[1]["id"] == html_block2["id"] + assert c2_components_after[1]["has_unpublished_changes"] # unaffected + assert c2_components_after[1]["published_by"] is None + + # Make sure that the right events were sent out. + # First, there should be one container updated event: + assert len(updated_container_receiver.call_args_list) == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "library_container": LibraryContainerData( + container_key=LibraryContainerLocator.from_string(container1["id"]), + ), + }, + updated_container_receiver.call_args_list[0].kwargs, + ) + + # Second, two XBlock updated events: + assert len(updated_block_receiver.call_args_list) == 2 + updated_block_ids = set( + call.kwargs["library_block"].usage_key for call in updated_block_receiver.call_args_list + ) + assert updated_block_ids == { + LibraryUsageLocatorV2.from_string(problem_block["id"]), + LibraryUsageLocatorV2.from_string(html_block["id"]), + } + assert all(call.kwargs["signal"] == LIBRARY_BLOCK_UPDATED for call in updated_block_receiver.call_args_list) diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index adbc89b2cc..2b9cd59af7 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -84,7 +84,8 @@ urlpatterns = [ path('restore/', containers.LibraryContainerRestore.as_view()), # Update collections for a given container path('collections/', containers.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'), - # path('publish/', views.LibraryContainerPublishView.as_view()), + # Publish a container (or reset to last published) + path('publish/', containers.LibraryContainerPublishView.as_view()), ])), re_path(r'^lti/1.3/', include([ path('login/', libraries.LtiToolLoginView.as_view(), name='lti-login'),