From dc4144ec956dabfb25b8ee1479b50be40722e159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Thu, 10 Apr 2025 17:15:53 -0500 Subject: [PATCH] feat: add restore container API, delete index when deleting container (#36464) --- openedx/core/djangoapps/content/search/api.py | 9 ++-- .../djangoapps/content/search/documents.py | 3 +- .../djangoapps/content/search/handlers.py | 18 ++++++- .../core/djangoapps/content/search/tasks.py | 19 ++++++- .../content/search/tests/test_api.py | 11 ++++ .../content_libraries/api/containers.py | 26 +++++++--- .../content_libraries/rest_api/containers.py | 20 ++++++++ .../content_libraries/tests/base.py | 5 ++ .../tests/test_containers.py | 50 +++++++++++++++++++ .../core/djangoapps/content_libraries/urls.py | 2 + 10 files changed, 148 insertions(+), 15 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index d9bcc55cd9..31db56bbf2 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -17,8 +17,7 @@ from django.core.paginator import Paginator from meilisearch import Client as MeilisearchClient from meilisearch.errors import MeilisearchApiError, MeilisearchError from meilisearch.models.task import TaskInfo -from opaque_keys import OpaqueKey -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import UsageKey, OpaqueKey from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryCollectionLocator from openedx_learning.api import authoring as authoring_api from common.djangoapps.student.roles import GlobalStaff @@ -613,14 +612,14 @@ def upsert_xblock_index_doc(usage_key: UsageKey, recursive: bool = True) -> None _update_index_docs(docs) -def delete_index_doc(usage_key: UsageKey) -> None: +def delete_index_doc(key: OpaqueKey) -> None: """ Deletes the document for the given XBlock from the search index Args: - usage_key (UsageKey): The usage key of the XBlock to be removed from the index + key (OpaqueKey): The opaque key of the XBlock/Container to be removed from the index """ - doc = searchable_doc_for_key(usage_key) + doc = searchable_doc_for_key(key) _delete_index_doc(doc[Fields.id]) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index c5821af34e..dbb55cb755 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -8,8 +8,7 @@ from hashlib import blake2b from django.core.exceptions import ObjectDoesNotExist from django.utils.text import slugify -from opaque_keys import OpaqueKey -from opaque_keys.edx.keys import LearningContextKey, UsageKey +from opaque_keys.edx.keys import LearningContextKey, UsageKey, OpaqueKey from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Collection diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index f932c4e71c..ba7e58eb80 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -46,6 +46,7 @@ from .api import ( ) from .tasks import ( delete_library_block_index_doc, + delete_library_container_index_doc, delete_xblock_index_doc, update_content_library_index_docs, update_library_collection_index_doc, @@ -238,7 +239,6 @@ def content_object_associations_changed_handler(**kwargs) -> None: @receiver(LIBRARY_CONTAINER_CREATED) -@receiver(LIBRARY_CONTAINER_DELETED) @receiver(LIBRARY_CONTAINER_UPDATED) @only_if_meilisearch_enabled def library_container_updated_handler(**kwargs) -> None: @@ -263,3 +263,19 @@ def library_container_updated_handler(**kwargs) -> None: str(library_container.library_key), library_container.container_key, ]) + + +@receiver(LIBRARY_CONTAINER_DELETED) +@only_if_meilisearch_enabled +def library_container_deleted(**kwargs) -> None: + """ + Delete the index for the content library container + """ + library_container = kwargs.get("library_container", None) + if not library_container or not isinstance(library_container, LibraryContainerData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + # Update content library index synchronously to make sure that search index is updated before + # the frontend invalidates/refetches results. This is only a single document update so is very fast. + delete_library_container_index_doc.apply(args=[str(library_container.container_key)]) diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index f23ca9aa30..1c66050415 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -11,7 +11,11 @@ from celery_utils.logged_task import LoggedTask from edx_django_utils.monitoring import set_code_owner_attribute from meilisearch.errors import MeilisearchError from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 +from opaque_keys.edx.locator import ( + LibraryContainerLocator, + LibraryLocatorV2, + LibraryUsageLocatorV2, +) from . import api @@ -124,3 +128,16 @@ def update_library_container_index_doc(library_key_str: str, container_key_str: log.info("Updating content index documents for container %s in library%s", container_key, library_key) api.upsert_library_container_index_doc(container_key) + + +@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) +@set_code_owner_attribute +def delete_library_container_index_doc(container_key_str: str) -> None: + """ + Celery task to delete the content index document for a library block + """ + container_key = LibraryContainerLocator.from_string(container_key_str) + + log.info("Deleting content index document for library block with id: %s", container_key) + + api.delete_index_doc(container_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 5786451d1f..63317d020f 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -867,6 +867,17 @@ class TestSearchApi(ModuleStoreTestCase): doc_problem_without_collection, ]) + @override_settings(MEILISEARCH_ENABLED=True) + def test_delete_index_container(self, mock_meilisearch): + """ + Test delete a container index. + """ + library_api.delete_container(self.unit.container_key) + + mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with( + self.unit_dict["id"], + ) + @override_settings(MEILISEARCH_ENABLED=True) def test_index_library_container_metadata(self, mock_meilisearch): """ diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index bf519901ae..8a80cec352 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -44,6 +44,7 @@ __all__ = [ "library_container_locator", "update_container", "delete_container", + "restore_container", "update_container_children", "get_containers_contains_component", ] @@ -121,7 +122,7 @@ def library_container_locator( ) -def _get_container(container_key: LibraryContainerLocator) -> Container: +def _get_container(container_key: LibraryContainerLocator, isDeleted=False) -> Container: """ Internal method to fetch the Container object from its LibraryContainerLocator @@ -135,7 +136,7 @@ def _get_container(container_key: LibraryContainerLocator) -> Container: learning_package.id, key=container_key.container_id, ) - if container and container.versioning.draft: + if container and (isDeleted or container.versioning.draft): return container raise ContentLibraryContainerNotFound @@ -234,10 +235,7 @@ def delete_container( No-op if container doesn't exist or has already been soft-deleted. """ - try: - container = _get_container(container_key) - except ContentLibraryContainerNotFound: - return + container = _get_container(container_key) authoring_api.soft_delete_draft(container.pk) @@ -251,6 +249,22 @@ def delete_container( # TODO: trigger a LIBRARY_COLLECTION_UPDATED for each collection the container was in +def restore_container(container_key: LibraryContainerLocator) -> None: + """ + Restore the specified library container. + """ + container = _get_container(container_key, isDeleted=True) + + authoring_api.set_draft_version(container.pk, container.versioning.latest.pk) + + LIBRARY_CONTAINER_CREATED.send_event( + library_container=LibraryContainerData( + library_key=container_key.library_key, + container_key=str(container_key), + ) + ) + + def get_container_children( container_key: LibraryContainerLocator, published=False, diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 95e468b4a4..3c65fc006b 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -273,3 +273,23 @@ class LibraryContainerChildrenView(GenericAPIView): container_key, action=authoring_api.ChildrenEntitiesAction.REPLACE, ) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerRestore(GenericAPIView): + """ + View to restore soft-deleted library containers. + """ + @convert_exceptions + def post(self, request, container_key: LibraryContainerLocator) -> Response: + """ + Restores a soft-deleted library container + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + api.restore_container(container_key) + return Response(None, status=HTTP_204_NO_CONTENT) diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 6adb8184ea..3c07aabe06 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -34,6 +34,7 @@ URL_LIB_BLOCK_ASSETS = URL_LIB_BLOCK + 'assets/' # List the static asset files URL_LIB_BLOCK_ASSET_FILE = URL_LIB_BLOCK + 'assets/{file_name}' # Get, delete, or upload a specific static asset file URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library 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_LTI_PREFIX = URL_PREFIX + 'lti/1.3/' URL_LIB_LTI_JWKS = URL_LIB_LTI_PREFIX + 'pub/jwks/' @@ -386,6 +387,10 @@ class ContentLibrariesRestApiTest(APITransactionTestCase): """ Delete a container (unit etc.) """ return self._api('delete', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response) + def _restore_container(self, container_key: str, expect_response=204): + """ Restore a deleted a container (unit etc.) """ + return self._api('post', URL_LIB_CONTAINER_RESTORE.format(container_key=container_key), None, expect_response) + def _get_container_components(self, container_key: str, expect_response=200): """ Get container components""" return self._api( diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 52546396f2..46d206af50 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -330,3 +330,53 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): }, update_receiver.call_args_list[0].kwargs, ) + + def test_restore_unit(self): + """ + Test restore a deleted unit. + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) + + # Create a unit: + create_date = datetime(2024, 9, 8, 7, 6, 5, tzinfo=timezone.utc) + with freeze_time(create_date): + container_data = self._create_container(lib["id"], "unit", slug="u1", display_name="Test Unit") + + # Delete the unit + self._delete_container(container_data["container_key"]) + + create_receiver = mock.Mock() + LIBRARY_CONTAINER_CREATED.connect(create_receiver) + + # Restore container + self._restore_container(container_data["container_key"]) + new_container_data = self._get_container(container_data["container_key"]) + expected_data = { + "container_key": "lct:CL-TEST:containers:unit:u1", + "container_type": "unit", + "display_name": "Test Unit", + "last_published": None, + "published_by": "", + "last_draft_created": "2024-09-08T07:06:05Z", + "last_draft_created_by": 'Bob', + 'has_unpublished_changes': True, + 'created': '2024-09-08T07:06:05Z', + 'modified': '2024-09-08T07:06:05Z', + 'collections': [], + } + + self.assertDictContainsEntries(new_container_data, expected_data) + + assert create_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_CREATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + create_receiver.call_args_list[0].kwargs, + ) diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 99e98a2cc4..c46cc8641c 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -82,6 +82,8 @@ urlpatterns = [ path('', containers.LibraryContainerView.as_view()), # update components under container path('children/', containers.LibraryContainerChildrenView.as_view()), + # Restore a soft-deleted container + path('restore/', containers.LibraryContainerRestore.as_view()), # Update collections for a given container # path('collections/', views.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'), # path('publish/', views.LibraryContainerPublishView.as_view()),