feat: adds APIs to update or delete a container

openedx/core/djangoapps/content_libraries/api/containers.py

@@ -16,24 +16,33 @@
 from openedx_events.content_authoring.data import LibraryContainerData
 from openedx_events.content_authoring.signals import (
     LIBRARY_CONTAINER_CREATED,
+    LIBRARY_CONTAINER_DELETED,
+    LIBRARY_CONTAINER_UPDATED,
 )
 from openedx_learning.api import authoring as authoring_api
-from openedx_learning.api import authoring_models
+from openedx_learning.api.authoring_models import Container
 
 from ..models import ContentLibrary
 from .libraries import PublishableItem
 
+
 # The public API is only the following symbols:
 __all__ = [
+    "ContentLibraryContainerNotFound",
     "ContainerMetadata",
     "ContainerType",
     "get_container",
     "create_container",
     "get_container_children",
     "library_container_locator",
+    "update_container",
+    "delete_container",
 ]
 
 
+ContentLibraryContainerNotFound = Container.DoesNotExist
+
+
 class ContainerType(Enum):
     Unit = "unit"
 
@@ -47,7 +56,7 @@ class ContainerMetadata(PublishableItem):
     container_type: ContainerType
 
     @classmethod
-    def from_container(cls, library_key, container: authoring_models.Container, associated_collections=None):
+    def from_container(cls, library_key, container: Container, associated_collections=None):
         """
         Construct a ContainerMetadata object from a Container object.
         """
@@ -89,7 +98,7 @@ def from_container(cls, library_key, container: authoring_models.Container, asso
 
 def library_container_locator(
     library_key: LibraryLocatorV2,
-    container: authoring_models.Container,
+    container: Container,
 ) -> LibraryContainerLocator:
     """
     Returns a LibraryContainerLocator for the given library + container.
@@ -106,9 +115,11 @@ def library_container_locator(
     )
 
 
-def get_container(container_key: LibraryContainerLocator) -> ContainerMetadata:
+def _get_container(container_key: LibraryContainerLocator) -> Container:
     """
-    Get a container (a Section, Subsection, or Unit).
+    Internal method to fetch the Container object from its LibraryContainerLocator
+
+    Raises ContentLibraryContainerNotFound if no container found, or if the container has been soft deleted.
     """
     assert isinstance(container_key, LibraryContainerLocator)
     content_library = ContentLibrary.objects.get_by_key(container_key.library_key)
@@ -118,6 +129,16 @@ def get_container(container_key: LibraryContainerLocator) -> ContainerMetadata:
         learning_package.id,
         key=container_key.container_id,
     )
+    if container and container.versioning.draft:
+        return container
+    raise ContentLibraryContainerNotFound
+
+
+def get_container(container_key: LibraryContainerLocator) -> ContainerMetadata:
+    """
+    Get a container (a Section, Subsection, or Unit).
+    """
+    container = _get_container(container_key)
     container_meta = ContainerMetadata.from_container(container_key.library_key, container)
     assert container_meta.container_type.value == container_key.container_type
     return container_meta
@@ -170,6 +191,60 @@ def create_container(
     return ContainerMetadata.from_container(library_key, container)
 
 
+def update_container(
+    container_key: LibraryContainerLocator,
+    display_name: str,
+    user_id: int | None,
+) -> ContainerMetadata:
+    """
+    Update a container (e.g. a Unit) title.
+    """
+    container = _get_container(container_key)
+    library_key = container_key.library_key
+
+    assert container.unit
+    unit_version = authoring_api.create_next_unit_version(
+        container.unit,
+        title=display_name,
+        created=datetime.now(),
+        created_by=user_id,
+    )
+
+    LIBRARY_CONTAINER_UPDATED.send_event(
+        library_container=LibraryContainerData(
+            library_key=library_key,
+            container_key=str(container_key),
+        )
+    )
+
+    return ContainerMetadata.from_container(library_key, unit_version.container)
+
+
+def delete_container(
+    container_key: LibraryContainerLocator,
+) -> None:
+    """
+    Delete a container (e.g. a Unit) (soft delete).
+
+    No-op if container doesn't exist or has already been soft-deleted.
+    """
+    try:
+        container = _get_container(container_key)
+    except ContentLibraryContainerNotFound:
+        return
+
+    authoring_api.soft_delete_draft(container.pk)
+
+    LIBRARY_CONTAINER_DELETED.send_event(
+        library_container=LibraryContainerData(
+            library_key=container_key.library_key,
+            container_key=str(container_key),
+        )
+    )
+
+    # TODO: trigger a LIBRARY_COLLECTION_UPDATED for each collection the container was in
+
+
 def get_container_children(
     container_key: LibraryContainerLocator,
     published=False,

openedx/core/djangoapps/content_libraries/api/libraries.py

@@ -97,7 +97,6 @@
 from openedx_learning.api import authoring as authoring_api
 from openedx_learning.api.authoring_models import (
     Collection,
-    Container,
     Component,
     ComponentVersion,
     MediaType,
@@ -128,7 +127,6 @@
     # Exceptions - maybe move them to a new file?
     "ContentLibraryNotFound",
     "ContentLibraryCollectionNotFound",
-    "ContentLibraryContainerNotFound",
     "ContentLibraryBlockNotFound",
     "LibraryAlreadyExists",
     "LibraryCollectionAlreadyExists",
@@ -181,8 +179,6 @@
 
 ContentLibraryCollectionNotFound = Collection.DoesNotExist
 
-ContentLibraryContainerNotFound = Container.DoesNotExist
-
 
 class ContentLibraryBlockNotFound(XBlockNotFoundError):
     """ XBlock not found in the content library """

openedx/core/djangoapps/content_libraries/rest_api/containers.py

@@ -14,6 +14,7 @@
 from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator
 from rest_framework.generics import GenericAPIView
 from rest_framework.response import Response
+from rest_framework.status import HTTP_204_NO_CONTENT
 
 from openedx.core.djangoapps.content_libraries import api, permissions
 from openedx.core.lib.api.view_utils import view_auth_classes
@@ -62,7 +63,7 @@ def post(self, request, lib_key_str):
 @view_auth_classes()
 class LibraryContainerView(GenericAPIView):
     """
-    View to get data about a specific container (a section, subsection, or unit)
+    View to retrieve or update data about a specific container (a section, subsection, or unit)
     """
     serializer_class = serializers.LibraryContainerMetadataSerializer
 
@@ -81,3 +82,45 @@ def get(self, request, container_key: LibraryContainerLocator):
         )
         container = api.get_container(container_key)
         return Response(serializers.LibraryContainerMetadataSerializer(container).data)
+
+    @convert_exceptions
+    @swagger_auto_schema(
+        request_body=serializers.LibraryContainerUpdateSerializer,
+        responses={200: serializers.LibraryContainerMetadataSerializer}
+    )
+    def patch(self, request, container_key: LibraryContainerLocator):
+        """
+        Update a Container.
+        """
+        api.require_permission_for_library_key(
+            container_key.library_key,
+            request.user,
+            permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
+        )
+        serializer = serializers.LibraryContainerUpdateSerializer(data=request.data)
+        serializer.is_valid(raise_exception=True)
+
+        container = api.update_container(
+            container_key,
+            display_name=serializer.validated_data['display_name'],
+            user_id=request.user.id,
+        )
+
+        return Response(serializers.LibraryContainerMetadataSerializer(container).data)
+
+    @convert_exceptions
+    def delete(self, request, container_key: LibraryContainerLocator):
+        """
+        Delete a Container (soft delete).
+        """
+        api.require_permission_for_library_key(
+            container_key.library_key,
+            request.user,
+            permissions.CAN_EDIT_THIS_CONTENT_LIBRARY,
+        )
+
+        api.delete_container(
+            container_key,
+        )
+
+        return Response({}, status=HTTP_204_NO_CONTENT)

openedx/core/djangoapps/content_libraries/rest_api/serializers.py

@@ -270,6 +270,13 @@ def to_internal_value(self, data):
         return result
 
 
+class LibraryContainerUpdateSerializer(serializers.Serializer):
+    """
+    Serializer for updating metadata for Containers like Sections, Subsections, Units
+    """
+    display_name = serializers.CharField()
+
+
 class ContentLibraryBlockImportTaskSerializer(serializers.ModelSerializer):
     """
     Serializer for a Content Library block import task.

openedx/core/djangoapps/content_libraries/rest_api/utils.py

@@ -34,6 +34,9 @@ def wrapped_fn(*args, **kwargs):
         except api.ContentLibraryCollectionNotFound:
             log.exception("Collection not found in content library")
             raise NotFound  # lint-amnesty, pylint: disable=raise-missing-from
+        except api.ContentLibraryContainerNotFound:
+            log.exception("Container not found in content library")
+            raise NotFound  # lint-amnesty, pylint: disable=raise-missing-from
         except api.LibraryCollectionAlreadyExists as exc:
             log.exception(str(exc))
             raise ValidationError(str(exc))  # lint-amnesty, pylint: disable=raise-missing-from

openedx/core/djangoapps/content_libraries/tests/base.py

@@ -363,3 +363,12 @@ def _create_container(self, lib_key, container_type, slug: str | None, display_n
     def _get_container(self, container_key: str, expect_response=200):
         """ Get a container (unit etc.) """
         return self._api('get', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response)
+
+    def _update_container(self, container_key: str, display_name: str, expect_response=200):
+        """ Update a container (unit etc.) """
+        data = {"display_name": display_name}
+        return self._api('patch', URL_LIB_CONTAINER.format(container_key=container_key), data, expect_response)
+
+    def _delete_container(self, container_key: str, expect_response=204):
+        """ Delete a container (unit etc.) """
+        return self._api('delete', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response)

openedx/core/djangoapps/content_libraries/tests/test_containers.py

@@ -9,9 +9,14 @@
 
 from opaque_keys.edx.locator import LibraryLocatorV2
 from openedx_events.content_authoring.data import LibraryContainerData
-from openedx_events.content_authoring.signals import LIBRARY_CONTAINER_CREATED
+from openedx_events.content_authoring.signals import (
+    LIBRARY_CONTAINER_CREATED,
+    LIBRARY_CONTAINER_DELETED,
+    LIBRARY_CONTAINER_UPDATED,
+)
 from openedx_events.tests.utils import OpenEdxEventsTestMixin
 
+from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest
 from openedx.core.djangolib.testing.utils import skip_unless_cms
 
@@ -38,6 +43,8 @@ class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest):
     """
     ENABLED_OPENEDX_EVENTS = [
         LIBRARY_CONTAINER_CREATED.event_type,
+        LIBRARY_CONTAINER_DELETED.event_type,
+        LIBRARY_CONTAINER_UPDATED.event_type,
     ]
 
     def test_unit_crud(self):
@@ -50,6 +57,12 @@ def test_unit_crud(self):
         create_receiver = mock.Mock()
         LIBRARY_CONTAINER_CREATED.connect(create_receiver)
 
+        update_receiver = mock.Mock()
+        LIBRARY_CONTAINER_UPDATED.connect(update_receiver)
+
+        delete_receiver = mock.Mock()
+        LIBRARY_CONTAINER_DELETED.connect(delete_receiver)
+
         # Create a unit:
         create_date = datetime(2024, 9, 8, 7, 6, 5, tzinfo=timezone.utc)
         with freeze_time(create_date):
@@ -87,7 +100,69 @@ def test_unit_crud(self):
         # make sure it contains the same data when we read it back:
         self.assertDictContainsEntries(unit_as_read, expected_data)
 
-    # TODO: test that a regular user with read-only permissions on the library cannot create units
+        # Update the unit:
+        modified_date = datetime(2024, 10, 9, 8, 7, 6, tzinfo=timezone.utc)
+        with freeze_time(modified_date):
+            container_data = self._update_container("lct:CL-TEST:containers:unit:u1", display_name="Unit ABC")
+        expected_data['last_draft_created'] = expected_data['modified'] = '2024-10-09T08:07:06Z'
+        expected_data['display_name'] = 'Unit ABC'
+        self.assertDictContainsEntries(container_data, expected_data)
+
+        assert update_receiver.call_count == 1
+        self.assertDictContainsSubset(
+            {
+                "signal": LIBRARY_CONTAINER_UPDATED,
+                "sender": None,
+                "library_container": LibraryContainerData(
+                    lib_key,
+                    container_key="lct:CL-TEST:containers:unit:u1",
+                ),
+            },
+            update_receiver.call_args_list[0].kwargs,
+        )
+
+        # Re-fetch the unit
+        unit_as_re_read = self._get_container(container_data["container_key"])
+        # make sure it contains the same data when we read it back:
+        self.assertDictContainsEntries(unit_as_re_read, expected_data)
+
+        # Delete the unit
+        self._delete_container(container_data["container_key"])
+        self._get_container(container_data["container_key"], expect_response=404)
+        assert delete_receiver.call_count == 1
+        self.assertDictContainsSubset(
+            {
+                "signal": LIBRARY_CONTAINER_DELETED,
+                "sender": None,
+                "library_container": LibraryContainerData(
+                    lib_key,
+                    container_key="lct:CL-TEST:containers:unit:u1",
+                ),
+            },
+            delete_receiver.call_args_list[0].kwargs,
+        )
+
+    def test_unit_permissions(self):
+        """
+        Test that a regular user with read-only permissions on the library cannot create, update, or delete units.
+        """
+        lib = self._create_library(slug="containers2", title="Container Test Library 2", description="Unit permissions")
+        container_data = self._create_container(lib["id"], "unit", slug="u2", display_name="Test Unit")
+
+        random_user = UserFactory.create(username="Random", email="[email protected]")
+        with self.as_user(random_user):
+            self._create_container(lib["id"], "unit", slug="u3", display_name="Test Unit", expect_response=403)
+            self._get_container(container_data["container_key"], expect_response=403)
+            self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403)
+            self._delete_container(container_data["container_key"], expect_response=403)
+
+        # Granting read-only permissions on the library should only allow retrieval, nothing else.
+        self._add_user_by_email(lib["id"], random_user.email, access_level="read")
+        with self.as_user(random_user):
+            self._create_container(lib["id"], "unit", slug="u2", display_name="Test Unit", expect_response=403)
+            self._get_container(container_data["container_key"], expect_response=200)
+            self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403)
+            self._delete_container(container_data["container_key"], expect_response=403)
 
     def test_unit_gets_auto_slugs(self):
         """

openedx/core/djangoapps/content_libraries/urls.py

@@ -78,7 +78,7 @@
         ])),
         # Containers are Sections, Subsections, and Units
         path('containers/<lib_container_key:container_key>/', include([
-            # Get metadata about a specific container in this library, or delete the container:
+            # Get metadata about a specific container in this library, update or delete the container:
             path('', containers.LibraryContainerView.as_view()),
             # Update collections for a given container
             # path('collections/', views.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'),