From d355c43a97a3666176300568c8f76a0f72d50273 Mon Sep 17 00:00:00 2001 From: Navin Karkera Date: Tue, 17 Sep 2024 20:18:37 +0530 Subject: [PATCH] feat: associate collection on library block creation Add optional body field to pass collection key to create_library_block api to associate the block to a collection. --- .../core/djangoapps/content_libraries/api.py | 49 ++++++++++++------- .../content_libraries/serializers.py | 2 + .../content_libraries/tests/test_api.py | 41 +++++++++++++--- 3 files changed, 68 insertions(+), 24 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 3dc33aec9616..826517f1535c 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -835,9 +835,11 @@ def validate_can_add_block_to_library( return content_library, usage_key -def create_library_block(library_key, block_type, definition_id, user_id=None): +def create_library_block(library_key, block_type, definition_id, user_id=None, collection_key=None): """ Create a new XBlock in this library of the specified type (e.g. "html"). + + Optionally associates the newly created xblock with given collection_key. """ # It's in the serializer as ``definition_id``, but for our purposes, it's # the block_id. See the comments in ``LibraryXBlockCreationSerializer`` for @@ -846,7 +848,16 @@ def create_library_block(library_key, block_type, definition_id, user_id=None): content_library, usage_key = validate_can_add_block_to_library(library_key, block_type, block_id) - _create_component_for_block(content_library, usage_key, user_id) + component = _create_component_for_block(content_library, usage_key, user_id) + if collection_key: + update_library_collection_components( + library_key, + collection_key, + usage_keys=[usage_key], + created_by=user_id, + content_library=content_library, + component_keys=[component.key] + ) # Now return the metadata about the new block: LIBRARY_BLOCK_CREATED.send_event( @@ -970,6 +981,7 @@ def _create_component_for_block(content_lib, usage_key, user_id=None): key="block.xml", learner_downloadable=False ) + return component def delete_library_block(usage_key, remove_from_parent=True): @@ -1185,6 +1197,8 @@ def update_library_collection_components( remove=False, # As an optimization, callers may pass in a pre-fetched ContentLibrary instance content_library: ContentLibrary | None = None, + # Similarly, users may pass component_keys instead of usage_keys + component_keys: list[str] | None = None, ) -> Collection: """ Associates the Collection with Components for the given UsageKeys. @@ -1206,23 +1220,24 @@ def update_library_collection_components( assert content_library.learning_package_id assert content_library.library_key == library_key - # Fetch the Component.key values for the provided UsageKeys. - component_keys = [] - for usage_key in usage_keys: - # Parse the block_family from the key to use as namespace. - block_type = BlockTypeKey.from_string(str(usage_key)) + # Fetch the Component.key values for the provided UsageKeys if component_keys is not passed + if not component_keys: + component_keys = [] + for usage_key in usage_keys: + # Parse the block_family from the key to use as namespace. + block_type = BlockTypeKey.from_string(str(usage_key)) - try: - component = authoring_api.get_component_by_key( - content_library.learning_package_id, - namespace=block_type.block_family, - type_name=usage_key.block_type, - local_key=usage_key.block_id, - ) - except Component.DoesNotExist as exc: - raise ContentLibraryBlockNotFound(usage_key) from exc + try: + component = authoring_api.get_component_by_key( + content_library.learning_package_id, + namespace=block_type.block_family, + type_name=usage_key.block_type, + local_key=usage_key.block_id, + ) + except Component.DoesNotExist as exc: + raise ContentLibraryBlockNotFound(usage_key) from exc - component_keys.append(component.key) + component_keys.append(component.key) # Note: Component.key matches its PublishableEntity.key entities_qset = PublishableEntity.objects.filter( diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/serializers.py index e9e04646ace4..8c18650a8f19 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/serializers.py @@ -192,6 +192,8 @@ class LibraryXBlockCreationSerializer(serializers.Serializer): # Optional param specified when pasting data from clipboard instead of # creating new block from scratch staged_content = serializers.CharField(required=False) + # Optional param specified when adding a component to a collection on creation + collection_key = serializers.CharField(required=False) class LibraryPasteClipboardSerializer(serializers.Serializer): diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index b02e71b002a3..250954b2b1de 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -6,8 +6,10 @@ import hashlib from unittest import mock +from django.http import HttpRequest from django.test import TestCase +from openedx.core.djangoapps.xblock.api import get_component_from_usage_key from opaque_keys.edx.keys import ( CourseKey, UsageKey, @@ -389,26 +391,37 @@ def test_update_library_collection_wrong_library(self): def test_update_library_collection_components(self): assert not list(self.col1.entities.all()) + problem_usage_key = UsageKey.from_string(self.lib1_problem_block["id"]) + html_usage_key = UsageKey.from_string(self.lib1_html_block["id"]) self.col1 = api.update_library_collection_components( self.lib1.library_key, self.col1.key, - usage_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), - ], + usage_keys=[problem_usage_key, html_usage_key], ) assert len(self.col1.entities.all()) == 2 self.col1 = api.update_library_collection_components( self.lib1.library_key, self.col1.key, - usage_keys=[ - UsageKey.from_string(self.lib1_html_block["id"]), - ], + usage_keys=[html_usage_key], remove=True, ) assert len(self.col1.entities.all()) == 1 + # Pass component key directly + component_key = get_component_from_usage_key(html_usage_key) + with mock.patch( + 'openedx.core.djangoapps.content_libraries.api.authoring_api.get_component_by_key' + ) as mock_get_component_by_key: + self.col1 = api.update_library_collection_components( + self.lib1.library_key, + self.col1.key, + usage_keys=[html_usage_key], + component_keys=[component_key], + ) + mock_get_component_by_key.assert_not_called() + assert len(self.col1.entities.all()) == 2 + def test_update_library_collection_components_event(self): """ Check that a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event is raised for each added/removed component. @@ -472,3 +485,17 @@ def test_update_collection_components_from_wrong_library(self): ], ) assert self.lib1_problem_block["id"] in str(exc.exception) + + def test_create_library_block_under_collection(self): + """Test creation xblock with collection associated on creation""" + fake_request = HttpRequest() + fake_request.LANGUAGE_CODE = "pt-br" + with mock.patch('crum.get_current_request', return_value=fake_request): + # Create Library Block + api.create_library_block( + self.lib2.library_key, + "problem", + "Problem1", + collection_key=self.col2.key, + ) + assert len(self.col2.entities.all()) == 1