refactor: Use convert_exceptions + update tests

openedx/core/djangoapps/content_libraries/collections/rest_api/v1/tests/test_views.py

@@ -5,6 +5,7 @@
 from __future__ import annotations
 
 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
@@ -33,23 +34,24 @@ def setUp(self):
         self.lib2 = ContentLibrary.objects.get(slug="test-lib-col-2")
 
         # Create Content Library Collections
-        self.col1 = Collection.objects.create(
+        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",
-            created_by=self.user,
         )
-        self.col2 = Collection.objects.create(
+
+        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",
-            created_by=self.user,
         )
-        self.col3 = Collection.objects.create(
+        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",
-            created_by=self.user,
         )
 
     def test_get_library_collection(self):
@@ -94,6 +96,12 @@ def test_get_invalid_library_collection(self):
 
         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
@@ -120,11 +128,15 @@ 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))
+        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
@@ -181,6 +193,14 @@ def test_create_invalid_library_collection(self):
 
         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
@@ -243,6 +263,14 @@ def test_update_invalid_library_collection(self):
 
         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

openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py

@@ -19,6 +19,7 @@
 )
 
 from openedx.core.djangoapps.content_libraries import api, permissions
+from openedx.core.djangoapps.content_libraries.utils import convert_exceptions
 from openedx.core.djangoapps.content_libraries.serializers import (
     ContentLibraryCollectionSerializer,
     ContentLibraryCollectionCreateOrUpdateSerializer,
@@ -39,18 +40,15 @@ def _verify_and_fetch_library_collection(self, library_key, collection_id, user,
         """
         Verify that the collection belongs to the library and the user has the correct permissions
         """
-        try:
-            library_obj = api.require_permission_for_library_key(library_key, user, permission)
-        except api.ContentLibraryNotFound as exc:
-            raise Http404 from exc
-
+        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(
                 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
@@ -74,6 +72,7 @@ def retrieve(self, request, *args, **kwargs):
         serializer = self.get_serializer(collection)
         return Response(serializer.data)
 
+    @convert_exceptions
     def list(self, request, *args, **kwargs):
         """
         List Collections that belong to Content Library
@@ -85,17 +84,15 @@ def list(self, request, *args, **kwargs):
         # 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)
-        try:
-            content_library = api.require_permission_for_library_key(
-                library_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
-            )
-        except api.ContentLibraryNotFound as exc:
-            raise Http404 from exc
+        content_library = api.require_permission_for_library_key(
+            library_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY
+        )
 
         collections = authoring_api.get_learning_package_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
@@ -107,12 +104,9 @@ def create(self, request, *args, **kwargs):
         # 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)
-        try:
-            content_library = api.require_permission_for_library_key(
-                library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY
-            )
-        except api.ContentLibraryNotFound as exc:
-            raise Http404 from exc
+        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)
@@ -134,6 +128,7 @@ def create(self, request, *args, **kwargs):
 
         return Response(serializer.data)
 
+    @convert_exceptions
     def partial_update(self, request, *args, **kwargs):
         """
         Update a Collection that belongs to a Content Library
@@ -170,6 +165,7 @@ def partial_update(self, request, *args, **kwargs):
 
         return Response(serializer.data)
 
+    @convert_exceptions
     def destroy(self, request, *args, **kwargs):
         """
         Deletes a Collection that belongs to a Content Library

openedx/core/djangoapps/content_libraries/utils.py

@@ -0,0 +1,44 @@
+""" 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.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

openedx/core/djangoapps/content_libraries/views.py

@@ -63,7 +63,6 @@
         https://github.com/openedx/edx-platform/pull/30456
 """
 
-from functools import wraps
 import itertools
 import json
 import logging
@@ -120,40 +119,13 @@
 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()
 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 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.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
-
-
 class LibraryApiPaginationDocs:
     """
     API docs for query params related to paginating ContentLibraryMetadata objects.