From 3c79543270c3712b6db3b019463f68153d1540c4 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 10 Sep 2024 11:58:18 -0500 Subject: [PATCH 1/4] refactor: Update description as optional in ContentLibraryCollectionUpdateSerializer --- openedx/core/djangoapps/content_libraries/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/serializers.py index 12a11e96e13c..4a02cb372f3f 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/serializers.py @@ -268,7 +268,7 @@ class ContentLibraryCollectionUpdateSerializer(serializers.Serializer): """ title = serializers.CharField() - description = serializers.CharField() + description = serializers.CharField(allow_blank=True) class ContentLibraryCollectionCreateSerializer(ContentLibraryCollectionUpdateSerializer): From 8118b77fa9f5740baa88aa4a6835725984c1104d Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 10 Sep 2024 12:36:33 -0500 Subject: [PATCH 2/4] refactor: Create collection Rest API to auto-generate key --- .../content_libraries/serializers.py | 8 ----- .../content_libraries/views_collections.py | 34 +++++++++++++------ 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/serializers.py index 4a02cb372f3f..2062f96d93ae 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/serializers.py @@ -271,14 +271,6 @@ class ContentLibraryCollectionUpdateSerializer(serializers.Serializer): description = serializers.CharField(allow_blank=True) -class ContentLibraryCollectionCreateSerializer(ContentLibraryCollectionUpdateSerializer): - """ - Serializer for adding a Collection in a Content Library - """ - - key = serializers.CharField() - - class UsageKeyV2Serializer(serializers.Serializer): """ Serializes a UsageKeyV2. diff --git a/openedx/core/djangoapps/content_libraries/views_collections.py b/openedx/core/djangoapps/content_libraries/views_collections.py index 482b6e2bb9f1..2f40a1788628 100644 --- a/openedx/core/djangoapps/content_libraries/views_collections.py +++ b/openedx/core/djangoapps/content_libraries/views_collections.py @@ -5,6 +5,8 @@ from __future__ import annotations from django.db.models import QuerySet +from django.utils.text import slugify +from django.db import transaction from rest_framework.decorators import action from rest_framework.response import Response @@ -21,7 +23,6 @@ from openedx.core.djangoapps.content_libraries.serializers import ( ContentLibraryCollectionSerializer, ContentLibraryCollectionComponentsUpdateSerializer, - ContentLibraryCollectionCreateSerializer, ContentLibraryCollectionUpdateSerializer, ) @@ -109,17 +110,30 @@ def create(self, request, *args, **kwargs) -> Response: Create a Collection that belongs to a Content Library """ content_library = self.get_content_library() - create_serializer = ContentLibraryCollectionCreateSerializer(data=request.data) + create_serializer = ContentLibraryCollectionUpdateSerializer(data=request.data) create_serializer.is_valid(raise_exception=True) - collection = api.create_library_collection( - library_key=content_library.library_key, - content_library=content_library, - collection_key=create_serializer.validated_data["key"], - title=create_serializer.validated_data["title"], - description=create_serializer.validated_data["description"], - created_by=request.user.id, - ) + title = create_serializer.validated_data['title'] + key = slugify(title) + + attempt = 0 + collection = None + while not collection: + modified_key = key if attempt == 0 else key + '-' + str(attempt) + try: + # Add transaction here to avoid TransactionManagementError on retry + with transaction.atomic(): + collection = api.create_library_collection( + library_key=content_library.library_key, + content_library=content_library, + collection_key=modified_key, + title=title, + description=create_serializer.validated_data["description"], + created_by=request.user.id, + ) + except api.LibraryCollectionAlreadyExists: + attempt += 1 + serializer = self.get_serializer(collection) return Response(serializer.data) From b6ca7c5144c487fb02452e774c822c007a16f6cb Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 10 Sep 2024 12:57:14 -0500 Subject: [PATCH 3/4] test: For updates in create collection rest api --- .../tests/test_views_collections.py | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) 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 ad027f2412bc..42ac858b144a 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py @@ -161,17 +161,16 @@ def test_create_library_collection(self): """ Test creating a Content Library Collection """ - post_data = { - "key": "COL4", + 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 + post_data["key"] = 'collection-4' self.assertDictContainsEntries(resp.data, post_data) created_collection = Collection.objects.get(id=resp.data["id"]) @@ -182,8 +181,7 @@ def test_create_library_collection(self): self._add_user_by_email(self.lib1.library_key, reader.email, access_level="read") with self.as_user(reader): - post_data = { - "key": "COL5", + post_data = { "title": "Collection 5", "description": "Description for Collection 5", } @@ -193,6 +191,31 @@ def test_create_library_collection(self): assert resp.status_code == 403 + def test_create_collection_same_key(self): + """ + Test collection creation with same key + """ + post_data = { + "title": "Same Collection", + "description": "Description for Collection 4", + } + self.client.post( + URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data, format="json" + ) + + for i in range(0, 100): + resp = self.client.post( + URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data, format="json" + ) + expected_data = { + "key": f"same-collection-{i + 1}", + "title": "Same Collection", + "description": "Description for Collection 4", + } + + assert resp.status_code == 200 + self.assertDictContainsEntries(resp.data, expected_data) + def test_create_invalid_library_collection(self): """ Test creating an invalid Content Library Collection From 4078c2f1042bbc4a1404957ed944021e1600823c Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 10 Sep 2024 14:17:59 -0500 Subject: [PATCH 4/4] style: Fix lint in test_views_collections.py --- .../content_libraries/tests/test_views_collections.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 42ac858b144a..bc600759b5b3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py @@ -161,7 +161,7 @@ def test_create_library_collection(self): """ Test creating a Content Library Collection """ - post_data = { + post_data = { "title": "Collection 4", "description": "Description for Collection 4", } @@ -181,7 +181,7 @@ def test_create_library_collection(self): self._add_user_by_email(self.lib1.library_key, reader.email, access_level="read") with self.as_user(reader): - post_data = { + post_data = { "title": "Collection 5", "description": "Description for Collection 5", } @@ -203,7 +203,7 @@ def test_create_collection_same_key(self): URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data, format="json" ) - for i in range(0, 100): + for i in range(100): resp = self.client.post( URL_LIB_COLLECTIONS.format(lib_key=self.lib1.library_key), post_data, format="json" ) @@ -213,7 +213,7 @@ def test_create_collection_same_key(self): "description": "Description for Collection 4", } - assert resp.status_code == 200 + assert resp.status_code == 200 self.assertDictContainsEntries(resp.data, expected_data) def test_create_invalid_library_collection(self):