feat: add restore container API, delete index when deleting container (#36464)
This commit is contained in:
@@ -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])
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)])
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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):
|
||||
"""
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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()),
|
||||
|
||||
Reference in New Issue
Block a user