From 971afe6095cfdf562eb6afd8277d04781f06c2e4 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Fri, 13 Dec 2024 23:47:46 +0530 Subject: [PATCH] feat: api to restore soft-deleted component [FC-0076] (#35993) Adds API to handle restoring soft-deleted library blocks. --- .../djangoapps/content/search/documents.py | 5 ++- .../content/search/tests/test_handlers.py | 10 +++++ .../core/djangoapps/content_libraries/api.py | 40 +++++++++++++++++++ .../content_libraries/tests/test_api.py | 29 ++++++++++++++ .../core/djangoapps/content_libraries/urls.py | 1 + .../djangoapps/content_libraries/views.py | 16 ++++++++ .../djangoapps/content_tagging/handlers.py | 18 --------- .../content_tagging/tests/test_tasks.py | 25 +++++++++--- 8 files changed, 120 insertions(+), 24 deletions(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 0dd02683ceea..40fe4529272b 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -306,7 +306,10 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) -> If the object is in no collections, returns: { - "collections": {}, + "collections": { + "display_name": [], + "key": [], + }, } """ diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py index 3577cbfc5692..dc274d182696 100644 --- a/openedx/core/djangoapps/content/search/tests/test_handlers.py +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -185,3 +185,13 @@ def test_create_delete_library_block(self, meilisearch_client): meilisearch_client.return_value.index.return_value.delete_document.assert_called_with( "lborgalib_aproblemproblem1-ca3186e9" ) + + # Restore the Library Block + library_api.restore_library_block(problem.usage_key) + meilisearch_client.return_value.index.return_value.update_documents.assert_any_call([doc_problem]) + meilisearch_client.return_value.index.return_value.update_documents.assert_any_call( + [{'id': doc_problem['id'], 'collections': {'display_name': [], 'key': []}}] + ) + meilisearch_client.return_value.index.return_value.update_documents.assert_any_call( + [{'id': doc_problem['id'], 'tags': {}}] + ) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 5195826468c2..c51c707fc470 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1133,6 +1133,46 @@ def delete_library_block(usage_key, remove_from_parent=True): ) +def restore_library_block(usage_key): + """ + Restore the specified library block. + """ + component = get_component_from_usage_key(usage_key) + library_key = usage_key.context_key + affected_collections = authoring_api.get_entity_collections(component.learning_package_id, component.key) + + # Set draft version back to the latest available component version id. + authoring_api.set_draft_version(component.pk, component.versioning.latest.pk) + + LIBRARY_BLOCK_CREATED.send_event( + library_block=LibraryBlockData( + library_key=library_key, + usage_key=usage_key + ) + ) + + # Add tags and collections back to index + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData( + object_id=str(usage_key), + changes=["collections", "tags"], + ), + ) + + # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger + # collection indexing asynchronously. + # + # To restore the component in the collections + for collection in affected_collections: + LIBRARY_COLLECTION_UPDATED.send_event( + library_collection=LibraryCollectionData( + library_key=library_key, + collection_key=collection.key, + background=True, + ) + ) + + def get_library_block_static_asset_files(usage_key) -> list[LibraryXBlockStaticFile]: """ Given an XBlock in a content library, list all the static asset files diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 7be3e592ba9d..203cc7a9397a 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -591,6 +591,35 @@ def test_delete_library_block(self): event_receiver.call_args_list[0].kwargs, ) + def test_restore_library_block(self): + api.update_library_collection_components( + self.lib1.library_key, + self.col1.key, + usage_keys=[ + UsageKey.from_string(self.lib1_problem_block["id"]), + UsageKey.from_string(self.lib1_html_block["id"]), + ], + ) + + event_receiver = mock.Mock() + LIBRARY_COLLECTION_UPDATED.connect(event_receiver) + + api.restore_library_block(UsageKey.from_string(self.lib1_problem_block["id"])) + + assert event_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_COLLECTION_UPDATED, + "sender": None, + "library_collection": LibraryCollectionData( + self.lib1.library_key, + collection_key=self.col1.key, + background=True, + ), + }, + event_receiver.call_args_list[0].kwargs, + ) + def test_add_component_and_revert(self): # Add component and publish api.update_library_collection_components( diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 857126eef7c9..1272d79b3873 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -57,6 +57,7 @@ path('blocks//', include([ # Get metadata about a specific XBlock in this library, or delete the block: path('', views.LibraryBlockView.as_view()), + path('restore/', views.LibraryBlockRestore.as_view()), # Update collections for a given component path('collections/', views.LibraryBlockCollectionsView.as_view(), name='update-collections'), # Get the LTI URL of a specific XBlock diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index 3dc7f538df86..94197f508775 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -644,6 +644,22 @@ def delete(self, request, usage_key_str): # pylint: disable=unused-argument return Response({}) +@view_auth_classes() +class LibraryBlockRestore(APIView): + """ + View to restore soft-deleted library xblocks. + """ + @convert_exceptions + def post(self, request, usage_key_str) -> Response: + """ + Restores a soft-deleted library block that belongs to a Content Library + """ + 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.restore_library_block(key) + return Response(None, status=status.HTTP_204_NO_CONTENT) + + @method_decorator(non_atomic_requests, name="dispatch") @view_auth_classes() class LibraryBlockCollectionsView(APIView): diff --git a/openedx/core/djangoapps/content_tagging/handlers.py b/openedx/core/djangoapps/content_tagging/handlers.py index cc86f7e0dcd6..86cbb7167cbe 100644 --- a/openedx/core/djangoapps/content_tagging/handlers.py +++ b/openedx/core/djangoapps/content_tagging/handlers.py @@ -20,7 +20,6 @@ XBLOCK_DUPLICATED, LIBRARY_BLOCK_CREATED, LIBRARY_BLOCK_UPDATED, - LIBRARY_BLOCK_DELETED, ) from .api import copy_object_tags @@ -30,7 +29,6 @@ update_course_tags, update_xblock_tags, update_library_block_tags, - delete_library_block_tags, ) from .toggles import CONTENT_TAGGING_AUTO @@ -119,22 +117,6 @@ def auto_tag_library_block(**kwargs): ) -@receiver(LIBRARY_BLOCK_DELETED) -def delete_tag_library_block(**kwargs): - """ - Delete tags associated with a Library XBlock whenever the block is deleted. - """ - library_block_data = kwargs.get("library_block", None) - if not library_block_data or not isinstance(library_block_data, LibraryBlockData): - log.error("Received null or incorrect data for event") - return - - try: - delete_library_block_tags(str(library_block_data.usage_key)) - except Exception as err: # pylint: disable=broad-except - log.error(f"Failed to delete library block tags: {err}") - - @receiver(XBLOCK_DUPLICATED) def duplicate_tags(**kwargs): """ diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index c14adfcce13a..d0e10ecfb7ae 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -14,7 +14,9 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase -from openedx.core.djangoapps.content_libraries.api import create_library, create_library_block, delete_library_block +from openedx.core.djangoapps.content_libraries.api import ( + create_library, create_library_block, delete_library_block, restore_library_block +) from .. import api from ..models.base import TaxonomyOrg @@ -267,7 +269,7 @@ def test_waffle_disabled_create_delete_xblock(self): # Still no tags assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, None) - def test_create_delete_library_block(self): + def test_create_delete_restore_library_block(self): # Create library library = create_library( org=self.orgA, @@ -287,11 +289,17 @@ def test_create_delete_library_block(self): # Check if the tags are created in the Library Block with the user's preferred language assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, 'Português (Brasil)') - # Delete the XBlock + # Soft delete the XBlock delete_library_block(library_block.usage_key) - # Check if the tags are deleted - assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, None) + # Check that the tags are not deleted + assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, 'Português (Brasil)') + + # Restore the XBlock + restore_library_block(library_block.usage_key) + + # Check if the tags are still present in the Library Block with the user's preferred language + assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, 'Português (Brasil)') @override_waffle_flag(CONTENT_TAGGING_AUTO, active=False) def test_waffle_disabled_create_delete_library_block(self): @@ -319,3 +327,10 @@ def test_waffle_disabled_create_delete_library_block(self): # Still no tags assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, None) + + # Restore the XBlock + with patch('crum.get_current_request', return_value=fake_request): + restore_library_block(library_block.usage_key) + + # Still no tags + assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, None)