From addde52ead87bd57e2fa05f85757d5c23962876d Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Mon, 2 Sep 2024 13:01:23 +0930 Subject: [PATCH] refactor: moved code around after merging base --- .../core/djangoapps/content_libraries/apps.py | 1 - .../content_libraries/collections/__init__.py | 0 .../content_libraries/collections/handlers.py | 57 --- .../collections/rest_api/__init__.py | 0 .../collections/rest_api/v1/__init__.py | 0 .../collections/rest_api/v1/tests/__init__.py | 0 .../rest_api/v1/tests/test_views.py | 421 ------------------ .../collections/rest_api/v1/views.py | 208 --------- .../tests/test_views_collections.py | 133 ++++++ .../djangoapps/content_libraries/utils.py | 48 -- .../djangoapps/content_libraries/views.py | 2 +- .../content_libraries/views_collections.py | 34 +- 12 files changed, 166 insertions(+), 738 deletions(-) delete mode 100644 openedx/core/djangoapps/content_libraries/collections/__init__.py delete mode 100644 openedx/core/djangoapps/content_libraries/collections/handlers.py delete mode 100644 openedx/core/djangoapps/content_libraries/collections/rest_api/__init__.py delete mode 100644 openedx/core/djangoapps/content_libraries/collections/rest_api/v1/__init__.py delete mode 100644 openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/__init__.py delete mode 100644 openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py delete mode 100644 openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py delete mode 100644 openedx/core/djangoapps/content_libraries/utils.py diff --git a/openedx/core/djangoapps/content_libraries/apps.py b/openedx/core/djangoapps/content_libraries/apps.py index aea920714ce0..52c3e5179721 100644 --- a/openedx/core/djangoapps/content_libraries/apps.py +++ b/openedx/core/djangoapps/content_libraries/apps.py @@ -37,4 +37,3 @@ def ready(self): Import signal handler's module to ensure they are registered. """ from . import signal_handlers # pylint: disable=unused-import - from .collections import handlers # pylint: disable=unused-import diff --git a/openedx/core/djangoapps/content_libraries/collections/__init__.py b/openedx/core/djangoapps/content_libraries/collections/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/openedx/core/djangoapps/content_libraries/collections/handlers.py b/openedx/core/djangoapps/content_libraries/collections/handlers.py deleted file mode 100644 index c2c67ad403e5..000000000000 --- a/openedx/core/djangoapps/content_libraries/collections/handlers.py +++ /dev/null @@ -1,57 +0,0 @@ -""" -Signal handlers for Content Library Collections. -""" - -import logging - -from django.dispatch import receiver -from openedx_events.content_authoring.data import LibraryCollectionData -from openedx_events.content_authoring.signals import ( - LIBRARY_COLLECTION_CREATED, - LIBRARY_COLLECTION_UPDATED, - LIBRARY_COLLECTION_DELETED, -) - - -log = logging.getLogger(__name__) - - -@receiver(LIBRARY_COLLECTION_CREATED) -def library_collection_created_handler(**kwargs): - """ - Content Library Collection Created signal handler - """ - library_collection_data = kwargs.get("library_collection", None) - if not library_collection_data or not isinstance(library_collection_data, LibraryCollectionData): - log.error("Received null or incorrect data for event") - return - - log.info("Received Collection Created Signal") - - # TODO: Implement handler logic - - -@receiver(LIBRARY_COLLECTION_UPDATED) -def library_collection_updated_handler(**kwargs): - """ - Content Library Collection Updated signal handler - """ - library_collection_data = kwargs.get("library_collection", None) - if not library_collection_data or not isinstance(library_collection_data, LibraryCollectionData): - log.error("Received null or incorrect data for event") - return - - log.info("Received Collection Updated Signal") - - -@receiver(LIBRARY_COLLECTION_DELETED) -def library_collection_deleted_handler(**kwargs): - """ - Content Library Collection Deleted signal handler - """ - library_collection_data = kwargs.get("library_collection", None) - if not library_collection_data or not isinstance(library_collection_data, LibraryCollectionData): - log.error("Received null or incorrect data for event") - return - - log.info("Received Collection Deleted Signal") diff --git a/openedx/core/djangoapps/content_libraries/collections/rest_api/__init__.py b/openedx/core/djangoapps/content_libraries/collections/rest_api/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/__init__.py b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/__init__.py b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py deleted file mode 100644 index b9321c136383..000000000000 --- a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py +++ /dev/null @@ -1,421 +0,0 @@ -""" -Tests Library Collections REST API views -""" - -from __future__ import annotations -import ddt - -from openedx_learning.api.authoring_models import Collection -from openedx_learning.api.authoring import create_collection -from opaque_keys.edx.locator import LibraryLocatorV2 - -from openedx.core.djangolib.testing.utils import skip_unless_cms -from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest -from openedx.core.djangoapps.content_libraries.models import ContentLibrary -from common.djangoapps.student.tests.factories import UserFactory - -URL_PREFIX = '/api/libraries/v2/{lib_key}/' -URL_LIB_COLLECTIONS = URL_PREFIX + 'collections/' -URL_LIB_COLLECTION = URL_LIB_COLLECTIONS + '{collection_id}/' -URL_LIB_COLLECTION_COMPONENTS = URL_LIB_COLLECTION + 'components/' - - -@ddt.ddt -@skip_unless_cms # Content Library Collections REST API is only available in Studio -class ContentLibraryCollectionsViewsTest(ContentLibrariesRestApiTest): - """ - Tests for Content Library Collection REST API Views - """ - - def setUp(self): - super().setUp() - - # Create Content Libraries - self._create_library("test-lib-col-1", "Test Library 1") - self._create_library("test-lib-col-2", "Test Library 2") - self.lib1 = ContentLibrary.objects.get(slug="test-lib-col-1") - self.lib2 = ContentLibrary.objects.get(slug="test-lib-col-2") - - # Create Content Library Collections - self.col1 = create_collection( - learning_package_id=self.lib1.learning_package.id, - title="Collection 1", - created_by=self.user.id, - description="Description for Collection 1", - ) - self.col2 = create_collection( - learning_package_id=self.lib1.learning_package.id, - title="Collection 2", - created_by=self.user.id, - description="Description for Collection 2", - ) - self.col3 = create_collection( - learning_package_id=self.lib2.learning_package.id, - title="Collection 3", - created_by=self.user.id, - description="Description for Collection 3", - ) - - # Create some library blocks - self.lib1_problem_block = self._add_block_to_library( - self.lib1.library_key, "problem", "problem1", - ) - self.lib1_html_block = self._add_block_to_library( - self.lib1.library_key, "html", "html1", - ) - self.lib2_problem_block = self._add_block_to_library( - self.lib2.library_key, "problem", "problem2", - ) - self.lib2_html_block = self._add_block_to_library( - self.lib2.library_key, "html", "html2", - ) - - def test_get_library_collection(self): - """ - Test retrieving a Content Library Collection - """ - resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_id=self.col3.id) - ) - - # Check that correct Content Library Collection data retrieved - expected_collection = { - "title": "Collection 3", - "description": "Description for Collection 3", - } - assert resp.status_code == 200 - self.assertDictContainsEntries(resp.data, expected_collection) - - # Check that a random user without permissions cannot access Content Library Collection - random_user = UserFactory.create(username="Random", email="random@example.com") - with self.as_user(random_user): - resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_id=self.col3.id) - ) - assert resp.status_code == 403 - - def test_get_invalid_library_collection(self): - """ - Test retrieving a an invalid Content Library Collection or one that does not exist - """ - # Fetch collection that belongs to a different library, it should fail - resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_id=self.col3.id) - ) - - assert resp.status_code == 404 - - # Fetch collection with invalid ID provided, it should fail - resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_id=123) - ) - - assert resp.status_code == 404 - - # Fetch collection with invalid library_key provided, it should fail - resp = self.client.get( - URL_LIB_COLLECTION.format(lib_key=123, collection_id=123) - ) - assert resp.status_code == 404 - - def test_list_library_collections(self): - """ - Test listing Content Library Collections - """ - resp = self.client.get(URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key)) - - # Check that the correct collections are listed - assert resp.status_code == 200 - assert len(resp.data) == 2 - expected_collections = [ - {"title": "Collection 1", "description": "Description for Collection 1"}, - {"title": "Collection 2", "description": "Description for Collection 2"}, - ] - for collection, expected in zip(resp.data, expected_collections): - self.assertDictContainsEntries(collection, expected) - - # Check that a random user without permissions cannot access Content Library Collections - random_user = UserFactory.create(username="Random", email="random@example.com") - with self.as_user(random_user): - resp = self.client.get(URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key)) - assert resp.status_code == 403 - - def test_list_invalid_library_collections(self): - """ - Test listing invalid Content Library Collections - """ - invalid_key = LibraryLocatorV2.from_string("lib:DoesNotExist:NE1") - resp = self.client.get(URL_LIB_COLLECTIONS.format(lib_key=invalid_key)) - - assert resp.status_code == 404 - - non_existing_key = LibraryLocatorV2.from_string("lib:DoesNotExist:NE1") - resp = self.client.get(URL_LIB_COLLECTIONS.format(lib_key=non_existing_key)) - - assert resp.status_code == 404 - - # List collections with invalid library_key provided, it should fail - resp = resp = self.client.get(URL_LIB_COLLECTIONS.format(lib_key=123)) - assert resp.status_code == 404 - - def test_create_library_collection(self): - """ - Test creating a Content Library Collection - """ - post_data = { - "title": "Collection 4", - "description": "Description for Collection 4", - } - resp = self.client.post( - URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data, format="json" - ) - - # Check that the new Content Library Collection is returned in response and created in DB - assert resp.status_code == 200 - self.assertDictContainsEntries(resp.data, post_data) - - created_collection = Collection.objects.get(id=resp.data["id"]) - self.assertIsNotNone(created_collection) - - # Check that user with read only access cannot create new Content Library Collection - reader = UserFactory.create(username="Reader", email="reader@example.com") - self._add_user_by_email(self.lib1.library_key, reader.email, access_level="read") - - with self.as_user(reader): - post_data = { - "title": "Collection 5", - "description": "Description for Collection 5", - } - resp = self.client.post( - URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data, format="json" - ) - - assert resp.status_code == 403 - - def test_create_invalid_library_collection(self): - """ - Test creating an invalid Content Library Collection - """ - post_data_missing_title = { - "description": "Description for Collection 4", - } - resp = self.client.post( - URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data_missing_title, format="json" - ) - - assert resp.status_code == 400 - - post_data_missing_desc = { - "title": "Collection 4", - } - resp = self.client.post( - URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data_missing_desc, format="json" - ) - - assert resp.status_code == 400 - - # Create collection with invalid library_key provided, it should fail - resp = self.client.post( - URL_LIB_COLLECTIONS.format(lib_key=123), - {**post_data_missing_title, **post_data_missing_desc}, - format="json" - ) - assert resp.status_code == 404 - - def test_update_library_collection(self): - """ - Test updating a Content Library Collection - """ - patch_data = { - "title": "Collection 3 Updated", - } - resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_id=self.col3.id), - patch_data, - format="json" - ) - - # Check that updated Content Library Collection is returned in response and updated in DB - assert resp.status_code == 200 - self.assertDictContainsEntries(resp.data, patch_data) - - created_collection = Collection.objects.get(id=resp.data["id"]) - self.assertIsNotNone(created_collection) - self.assertEqual(created_collection.title, patch_data["title"]) - - # Check that user with read only access cannot update a Content Library Collection - reader = UserFactory.create(username="Reader", email="reader@example.com") - self._add_user_by_email(self.lib1.library_key, reader.email, access_level="read") - - with self.as_user(reader): - patch_data = { - "title": "Collection 3 should not update", - } - resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_id=self.col3.id), - patch_data, - format="json" - ) - - assert resp.status_code == 403 - - def test_update_invalid_library_collection(self): - """ - Test updating an invalid Content Library Collection or one that does not exist - """ - patch_data = { - "title": "Collection 3 Updated", - } - # Update collection that belongs to a different library, it should fail - resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_id=self.col3.id), - patch_data, - format="json" - ) - - assert resp.status_code == 404 - - # Update collection with invalid ID provided, it should fail - resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=self.lib1.library_key, collection_id=123), - patch_data, - format="json" - ) - - assert resp.status_code == 404 - - # Update collection with invalid library_key provided, it should fail - resp = self.client.patch( - URL_LIB_COLLECTION.format(lib_key=123, collection_id=self.col3.id), - patch_data, - format="json" - ) - assert resp.status_code == 404 - - def test_delete_library_collection(self): - """ - Test deleting a Content Library Collection - - Note: Currently not implemented and should return a 405 - """ - resp = self.client.delete( - URL_LIB_COLLECTION.format(lib_key=self.lib2.library_key, collection_id=self.col3.id) - ) - - assert resp.status_code == 405 - - def test_get_components(self): - """ - Retrieving components is not supported by the REST API; - use Meilisearch instead. - """ - resp = self.client.get( - URL_LIB_COLLECTION_COMPONENTS.format( - lib_key=self.lib1.library_key, - collection_id=self.col1.id, - ), - ) - assert resp.status_code == 405 - - def test_update_components(self): - """ - Test adding and removing components from a collection. - """ - # Add two components to col1 - resp = self.client.patch( - URL_LIB_COLLECTION_COMPONENTS.format( - lib_key=self.lib1.library_key, - collection_id=self.col1.id, - ), - data={ - "usage_keys": [ - self.lib1_problem_block["id"], - self.lib1_html_block["id"], - ] - } - ) - assert resp.status_code == 200 - assert resp.data == {"count": 2} - - # Remove one of the added components from col1 - resp = self.client.delete( - URL_LIB_COLLECTION_COMPONENTS.format( - lib_key=self.lib1.library_key, - collection_id=self.col1.id, - ), - data={ - "usage_keys": [ - self.lib1_problem_block["id"], - ] - } - ) - assert resp.status_code == 200 - assert resp.data == {"count": 1} - - @ddt.data("patch", "delete") - def test_update_components_wrong_collection(self, method): - """ - Collection must belong to the requested library. - """ - resp = getattr(self.client, method)( - URL_LIB_COLLECTION_COMPONENTS.format( - lib_key=self.lib2.library_key, - collection_id=self.col1.id, - ), - data={ - "usage_keys": [ - self.lib1_problem_block["id"], - ] - } - ) - assert resp.status_code == 404 - - @ddt.data("patch", "delete") - def test_update_components_missing_data(self, method): - """ - List of usage keys must contain at least one item. - """ - resp = getattr(self.client, method)( - URL_LIB_COLLECTION_COMPONENTS.format( - lib_key=self.lib2.library_key, - collection_id=self.col3.id, - ), - ) - assert resp.status_code == 400 - assert resp.data == { - "usage_keys": ["This field is required."], - } - - @ddt.data("patch", "delete") - def test_update_components_from_another_library(self, method): - """ - Adding/removing components from another library raises a 404. - """ - resp = getattr(self.client, method)( - URL_LIB_COLLECTION_COMPONENTS.format( - lib_key=self.lib2.library_key, - collection_id=self.col3.id, - ), - data={ - "usage_keys": [ - self.lib1_problem_block["id"], - self.lib1_html_block["id"], - ] - } - ) - assert resp.status_code == 404 - - @ddt.data("patch", "delete") - def test_update_components_permissions(self, method): - """ - Check that a random user without permissions cannot update a Content Library Collection's components. - """ - random_user = UserFactory.create(username="Random", email="random@example.com") - with self.as_user(random_user): - resp = getattr(self.client, method)( - URL_LIB_COLLECTION_COMPONENTS.format( - lib_key=self.lib1.library_key, - collection_id=self.col1.id, - ), - ) - assert resp.status_code == 403 diff --git a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py deleted file mode 100644 index 86456ca3ee3b..000000000000 --- a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py +++ /dev/null @@ -1,208 +0,0 @@ -""" -Collections API Views -""" - -from __future__ import annotations - -from django.http import Http404 - -from rest_framework.decorators import action -from rest_framework.response import Response -from rest_framework.status import HTTP_405_METHOD_NOT_ALLOWED -from rest_framework.viewsets import ModelViewSet - -from openedx_learning.api.authoring_models import Collection -from openedx_learning.api import authoring as authoring_api -from opaque_keys.edx.locator import LibraryLocatorV2 -from openedx_events.content_authoring.data import LibraryCollectionData -from openedx_events.content_authoring.signals import ( - LIBRARY_COLLECTION_CREATED, - LIBRARY_COLLECTION_UPDATED, -) - -from openedx.core.djangoapps.content_libraries import api, permissions -from openedx.core.djangoapps.content_libraries.serializers import ( - ContentLibraryCollectionSerializer, - ContentLibraryCollectionComponentsUpdateSerializer, - ContentLibraryCollectionCreateOrUpdateSerializer, -) -from openedx.core.djangoapps.content_libraries.utils import convert_exceptions - - -class LibraryCollectionsView(ModelViewSet): - """ - Views to get, create and update Library Collections. - - **Warning** These APIs are UNSTABLE, and may change once we implement the ability to store - units/sections/subsections in the library. - """ - - serializer_class = ContentLibraryCollectionSerializer - - def _verify_and_fetch_library_collection(self, library_key, collection_id, user, permission) -> Collection | None: - """ - Verify that the collection belongs to the library and the user has the correct permissions - """ - library_obj = api.require_permission_for_library_key(library_key, user, permission) - collection = None - if library_obj.learning_package_id: - collection = authoring_api.get_collections( - library_obj.learning_package_id - ).filter(id=collection_id).first() - return collection - - @convert_exceptions - def retrieve(self, request, *args, **kwargs): - """ - Retrieve the Content Library Collection - """ - lib_key_str = kwargs.pop('lib_key_str', None) - if not lib_key_str: - raise Http404 - - pk = kwargs.pop("pk", None) - library_key = LibraryLocatorV2.from_string(lib_key_str) - - # Check if user has permissions to view this collection by checking if - # user has permission to view the Content Library it belongs to - collection = self._verify_and_fetch_library_collection( - library_key, pk, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY - ) - - if not collection: - raise Http404 - - serializer = self.get_serializer(collection) - return Response(serializer.data) - - @convert_exceptions - def list(self, request, *args, **kwargs): - """ - List Collections that belong to Content Library - """ - lib_key_str = kwargs.pop('lib_key_str', None) - if not lib_key_str: - raise Http404 - - # Check if user has permissions to view collections by checking if user - # has permission to view the Content Library they belong to - library_key = LibraryLocatorV2.from_string(lib_key_str) - content_library = api.require_permission_for_library_key( - library_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY - ) - - collections = authoring_api.get_collections(content_library.learning_package.id) - serializer = self.get_serializer(collections, many=True) - return Response(serializer.data) - - @convert_exceptions - def create(self, request, *args, **kwargs): - """ - Create a Collection that belongs to a Content Library - """ - lib_key_str = kwargs.pop('lib_key_str', None) - if not lib_key_str: - raise Http404 - - # Check if user has permissions to create a collection in the Content Library - # by checking if user has permission to edit the Content Library - library_key = LibraryLocatorV2.from_string(lib_key_str) - content_library = api.require_permission_for_library_key( - library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY - ) - - create_serializer = ContentLibraryCollectionCreateOrUpdateSerializer(data=request.data) - create_serializer.is_valid(raise_exception=True) - collection = authoring_api.create_collection( - content_library.learning_package.id, - create_serializer.validated_data["title"], - request.user.id, - create_serializer.validated_data["description"] - ) - serializer = self.get_serializer(collection) - - # Emit event for library content collection creation - LIBRARY_COLLECTION_CREATED.send_event( - library_collection=LibraryCollectionData( - library_key=library_key, - collection_id=collection.id - ) - ) - - return Response(serializer.data) - - @convert_exceptions - def partial_update(self, request, *args, **kwargs): - """ - Update a Collection that belongs to a Content Library - """ - lib_key_str = kwargs.pop('lib_key_str', None) - if not lib_key_str: - raise Http404 - - pk = kwargs.pop('pk', None) - library_key = LibraryLocatorV2.from_string(lib_key_str) - # Check if user has permissions to update a collection in the Content Library - # by checking if user has permission to edit the Content Library - collection = self._verify_and_fetch_library_collection( - library_key, pk, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY - ) - - if not collection: - raise Http404 - - update_serializer = ContentLibraryCollectionCreateOrUpdateSerializer( - collection, data=request.data, partial=True - ) - update_serializer.is_valid(raise_exception=True) - updated_collection = authoring_api.update_collection(pk, **update_serializer.validated_data) - serializer = self.get_serializer(updated_collection) - - # Emit event for library content collection updated - LIBRARY_COLLECTION_UPDATED.send_event( - library_collection=LibraryCollectionData( - library_key=library_key, - collection_id=collection.id - ) - ) - - return Response(serializer.data) - - @convert_exceptions - def destroy(self, request, *args, **kwargs): - """ - Deletes a Collection that belongs to a Content Library - - Note: (currently not allowed) - """ - # TODO: Implement the deletion logic and emit event signal - - return Response(None, status=HTTP_405_METHOD_NOT_ALLOWED) - - @convert_exceptions - @action(detail=True, methods=['delete', 'patch'], url_path='components', url_name='components-update') - def update_components(self, request, lib_key_str, pk=None): - """ - Adds (PATCH) or removes (DELETE) Components to/from a Collection. - - Collection and Components must all be part of the given library/learning package. - """ - library_key = LibraryLocatorV2.from_string(lib_key_str) - collection = self._verify_and_fetch_library_collection( - library_key, pk, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY - ) - if not collection: - raise Http404 - - serializer = ContentLibraryCollectionComponentsUpdateSerializer(data=request.data) - serializer.is_valid(raise_exception=True) - - usage_keys = serializer.validated_data["usage_keys"] - api.update_collection_components( - collection, - usage_keys=usage_keys, - created_by=self.request.user.id, - remove=(request.method == "DELETE"), - ) - - return Response({'count': len(usage_keys)}) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py index 1b4ce1a1b2fa..a03c7dd65484 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py @@ -3,6 +3,7 @@ """ from __future__ import annotations +import ddt from openedx_learning.api.authoring_models import Collection from openedx_learning.api.authoring import create_collection @@ -16,8 +17,10 @@ URL_PREFIX = '/api/libraries/v2/{lib_key}/' URL_LIB_COLLECTIONS = URL_PREFIX + 'collections/' URL_LIB_COLLECTION = URL_LIB_COLLECTIONS + '{collection_id}/' +URL_LIB_COLLECTION_COMPONENTS = URL_LIB_COLLECTION + 'components/' +@ddt.ddt @skip_unless_cms # Content Library Collections REST API is only available in Studio class ContentLibraryCollectionsViewsTest(ContentLibrariesRestApiTest): """ @@ -54,6 +57,20 @@ def setUp(self): description="Description for Collection 3", ) + # Create some library blocks + self.lib1_problem_block = self._add_block_to_library( + self.lib1.library_key, "problem", "problem1", + ) + self.lib1_html_block = self._add_block_to_library( + self.lib1.library_key, "html", "html1", + ) + self.lib2_problem_block = self._add_block_to_library( + self.lib2.library_key, "problem", "problem2", + ) + self.lib2_html_block = self._add_block_to_library( + self.lib2.library_key, "html", "html2", + ) + def test_get_library_collection(self): """ Test retrieving a Content Library Collection @@ -282,3 +299,119 @@ def test_delete_library_collection(self): ) assert resp.status_code == 405 + + def test_get_components(self): + """ + Retrieving components is not supported by the REST API; + use Meilisearch instead. + """ + resp = self.client.get( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib1.library_key, + collection_id=self.col1.id, + ), + ) + assert resp.status_code == 405 + + def test_update_components(self): + """ + Test adding and removing components from a collection. + """ + # Add two components to col1 + resp = self.client.patch( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib1.library_key, + collection_id=self.col1.id, + ), + data={ + "usage_keys": [ + self.lib1_problem_block["id"], + self.lib1_html_block["id"], + ] + } + ) + assert resp.status_code == 200 + assert resp.data == {"count": 2} + + # Remove one of the added components from col1 + resp = self.client.delete( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib1.library_key, + collection_id=self.col1.id, + ), + data={ + "usage_keys": [ + self.lib1_problem_block["id"], + ] + } + ) + assert resp.status_code == 200 + assert resp.data == {"count": 1} + + @ddt.data("patch", "delete") + def test_update_components_wrong_collection(self, method): + """ + Collection must belong to the requested library. + """ + resp = getattr(self.client, method)( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib2.library_key, + collection_id=self.col1.id, + ), + data={ + "usage_keys": [ + self.lib1_problem_block["id"], + ] + } + ) + assert resp.status_code == 404 + + @ddt.data("patch", "delete") + def test_update_components_missing_data(self, method): + """ + List of usage keys must contain at least one item. + """ + resp = getattr(self.client, method)( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib2.library_key, + collection_id=self.col3.id, + ), + ) + assert resp.status_code == 400 + assert resp.data == { + "usage_keys": ["This field is required."], + } + + @ddt.data("patch", "delete") + def test_update_components_from_another_library(self, method): + """ + Adding/removing components from another library raises a 404. + """ + resp = getattr(self.client, method)( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib2.library_key, + collection_id=self.col3.id, + ), + data={ + "usage_keys": [ + self.lib1_problem_block["id"], + self.lib1_html_block["id"], + ] + } + ) + assert resp.status_code == 404 + + @ddt.data("patch", "delete") + def test_update_components_permissions(self, method): + """ + Check that a random user without permissions cannot update a Content Library Collection's components. + """ + random_user = UserFactory.create(username="Random", email="random@example.com") + with self.as_user(random_user): + resp = getattr(self.client, method)( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib1.library_key, + collection_id=self.col1.id, + ), + ) + assert resp.status_code == 403 diff --git a/openedx/core/djangoapps/content_libraries/utils.py b/openedx/core/djangoapps/content_libraries/utils.py deleted file mode 100644 index da21d9057c92..000000000000 --- a/openedx/core/djangoapps/content_libraries/utils.py +++ /dev/null @@ -1,48 +0,0 @@ -""" Utils used for the content libraries. """ - -from functools import wraps -import logging - -from rest_framework.exceptions import NotFound, ValidationError - -from opaque_keys import InvalidKeyError - -from openedx.core.djangoapps.content_libraries import api - - -log = logging.getLogger(__name__) - - -def convert_exceptions(fn): - """ - Catch any Content Library API exceptions that occur and convert them to - DRF exceptions so DRF will return an appropriate HTTP response - """ - - @wraps(fn) - def wrapped_fn(*args, **kwargs): - try: - return fn(*args, **kwargs) - except InvalidKeyError as exc: - log.exception(str(exc)) - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryNotFound: - log.exception("Content library not found") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryBlockNotFound: - log.exception("XBlock not found in content library") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryCollectionNotFound: - log.exception("Collection not found in content library") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - - except api.LibraryBlockAlreadyExists as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.InvalidNameError as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.BlockLimitReachedError as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - return wrapped_fn diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index bdb36d9f1aae..bb1c4903f478 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -63,6 +63,7 @@ https://github.com/openedx/edx-platform/pull/30456 """ +from functools import wraps import itertools import json import logging @@ -120,7 +121,6 @@ from openedx.core.djangoapps.xblock import api as xblock_api from .models import ContentLibrary, LtiGradedResource, LtiProfile -from .utils import convert_exceptions User = get_user_model() diff --git a/openedx/core/djangoapps/content_libraries/views_collections.py b/openedx/core/djangoapps/content_libraries/views_collections.py index 5a51470077ae..2317ee3764e1 100644 --- a/openedx/core/djangoapps/content_libraries/views_collections.py +++ b/openedx/core/djangoapps/content_libraries/views_collections.py @@ -6,6 +6,7 @@ from django.http import Http404 +from rest_framework.decorators import action from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet from rest_framework.status import HTTP_405_METHOD_NOT_ALLOWED @@ -22,6 +23,7 @@ from openedx.core.djangoapps.content_libraries.views import convert_exceptions from openedx.core.djangoapps.content_libraries.serializers import ( ContentLibraryCollectionSerializer, + ContentLibraryCollectionComponentsUpdateSerializer, ContentLibraryCollectionCreateOrUpdateSerializer, ) @@ -43,7 +45,7 @@ def _verify_and_fetch_library_collection(self, library_key, collection_id, user, library_obj = api.require_permission_for_library_key(library_key, user, permission) collection = None if library_obj.learning_package_id: - collection = authoring_api.get_learning_package_collections( + collection = authoring_api.get_collections( library_obj.learning_package_id ).filter(id=collection_id).first() return collection @@ -88,7 +90,7 @@ def list(self, request, *args, **kwargs): library_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY ) - collections = authoring_api.get_learning_package_collections(content_library.learning_package.id) + collections = authoring_api.get_collections(content_library.learning_package.id) serializer = self.get_serializer(collections, many=True) return Response(serializer.data) @@ -175,3 +177,31 @@ def destroy(self, request, *args, **kwargs): # TODO: Implement the deletion logic and emit event signal return Response(None, status=HTTP_405_METHOD_NOT_ALLOWED) + + @convert_exceptions + @action(detail=True, methods=['delete', 'patch'], url_path='components', url_name='components-update') + def update_components(self, request, lib_key_str, pk=None): + """ + Adds (PATCH) or removes (DELETE) Components to/from a Collection. + + Collection and Components must all be part of the given library/learning package. + """ + library_key = LibraryLocatorV2.from_string(lib_key_str) + collection = self._verify_and_fetch_library_collection( + library_key, pk, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) + if not collection: + raise Http404 + + serializer = ContentLibraryCollectionComponentsUpdateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + usage_keys = serializer.validated_data["usage_keys"] + api.update_collection_components( + collection, + usage_keys=usage_keys, + created_by=self.request.user.id, + remove=(request.method == "DELETE"), + ) + + return Response({'count': len(usage_keys)})