From 23f5d5b3e2725393bdef995e4e6c7621470de9e8 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 30 Aug 2024 14:17:43 +0930 Subject: [PATCH] refactor: adapt to oel_collection.update_collection_components changes --- .../core/djangoapps/content_libraries/api.py | 33 ++++++-------- .../collections/rest_api/v1/views.py | 18 ++++---- .../content_libraries/tests/test_api.py | 43 ++++--------------- 3 files changed, 29 insertions(+), 65 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 79f76c36d5ed..e3d4adc606d5 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -91,7 +91,7 @@ LIBRARY_BLOCK_UPDATED, ) from openedx_learning.api import authoring as authoring_api -from openedx_learning.api.authoring_models import Collection, Component, MediaType, LearningPackage +from openedx_learning.api.authoring_models import Collection, Component, MediaType, LearningPackage, PublishableEntity from organizations.models import Organization from xblock.core import XBlock from xblock.exceptions import XBlockNotFoundError @@ -1070,12 +1070,11 @@ def revert_changes(library_key): def update_collection_components( - library: ContentLibraryMetadata, - collection_pk: int, + collection: Collection, usage_keys: list[UsageKeyV2], created_by: int | None = None, remove=False, -) -> int: +) -> Collection: """ Associates the Collection with Components for the given UsageKeys. @@ -1085,17 +1084,9 @@ def update_collection_components( Raises: * ContentLibraryCollectionNotFound if no Collection with the given pk is found in the given library. * ContentLibraryBlockNotFound if any of the given usage_keys don't match Components in the given library. - """ - if not usage_keys: - return 0 - - learning_package = library.learning_package - collections_qset = authoring_api.get_collections(learning_package.id).filter(pk=collection_pk) - - collection = collections_qset.first() - if not collection: - raise ContentLibraryCollectionNotFound(collection_pk) + Returns the updated Collection. + """ # Fetch the Component.key values for the provided UsageKeys. component_keys = [] for usage_key in usage_keys: @@ -1104,7 +1095,7 @@ def update_collection_components( try: component = authoring_api.get_component_by_key( - learning_package.id, + collection.learning_package_id, namespace=block_type.block_family, type_name=usage_key.block_type, local_key=usage_key.block_id, @@ -1115,18 +1106,18 @@ def update_collection_components( component_keys.append(component.key) # Note: Component.key matches its PublishableEntity.key - entities_qset = learning_package.publishable_entities.filter( + entities_qset = PublishableEntity.objects.filter( key__in=component_keys, ) if remove: - count = authoring_api.remove_from_collections( - collections_qset, + collection = authoring_api.remove_from_collection( + collection.pk, entities_qset, ) else: - count = authoring_api.add_to_collections( - collections_qset, + collection = authoring_api.add_to_collection( + collection.pk, entities_qset, created_by=created_by, ) @@ -1137,7 +1128,7 @@ def update_collection_components( content_object=ContentObjectData(object_id=usage_key), ) - return count + return collection # V1/V2 Compatibility Helpers 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 index df2e2340155f..86456ca3ee3b 100644 --- a/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_libraries/collections/rest_api/v1/views.py @@ -188,21 +188,21 @@ def update_components(self, request, lib_key_str, pk=None): Collection and Components must all be part of the given library/learning package. """ library_key = LibraryLocatorV2.from_string(lib_key_str) - library_obj = api.require_permission_for_library_key( - library_key, - request.user, - permissions.CAN_EDIT_THIS_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 serializer = ContentLibraryCollectionComponentsUpdateSerializer(data=request.data) serializer.is_valid(raise_exception=True) - count = api.update_collection_components( - library_obj, - collection_pk=pk, - usage_keys=serializer.validated_data["usage_keys"], + 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': count}) + return Response({'count': len(usage_keys)}) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 0c373f285055..f3226876131e 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -305,27 +305,24 @@ def setUp(self): ) def test_update_collection_components(self): - count = api.update_collection_components( - library=self.lib1, - collection_pk=self.col1.pk, + assert not list(self.col1.entities.all()) + + self.col1 = api.update_collection_components( + collection=self.col1, usage_keys=[ UsageKey.from_string(self.lib1_problem_block["id"]), UsageKey.from_string(self.lib1_html_block["id"]), ], ) - assert count == 2 assert len(self.col1.entities.all()) == 2 - count = api.update_collection_components( - library=self.lib1, - collection_pk=self.col1.pk, + self.col1 = api.update_collection_components( + collection=self.col1, usage_keys=[ UsageKey.from_string(self.lib1_html_block["id"]), ], remove=True, ) - assert count == 1 - self.col1.refresh_from_db() assert len(self.col1.entities.all()) == 1 def test_update_collection_components_event(self): @@ -335,8 +332,7 @@ def test_update_collection_components_event(self): event_receiver = mock.Mock() CONTENT_OBJECT_TAGS_CHANGED.connect(event_receiver) api.update_collection_components( - library=self.lib1, - collection_pk=self.col1.pk, + collection=self.col1, usage_keys=[ UsageKey.from_string(self.lib1_problem_block["id"]), UsageKey.from_string(self.lib1_html_block["id"]), @@ -365,33 +361,10 @@ def test_update_collection_components_event(self): event_receiver.call_args_list[1].kwargs, ) - def test_update_no_components(self): - """ - Calling update_collection_components with no usage_keys should not fail, just return 0. - """ - assert not api.update_collection_components( - library=self.lib1, - collection_pk=self.col1.pk, - usage_keys=[], - ) - def test_update_collection_components_from_wrong_library(self): with self.assertRaises(api.ContentLibraryBlockNotFound) as exc: api.update_collection_components( - library=self.lib2, - collection_pk=self.col2.pk, - usage_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), - ], - ) - assert self.lib1_problem_block["id"] in str(exc.exception) - - def test_update_collection_components_from_wrong_collection(self): - with self.assertRaises(api.ContentLibraryCollectionNotFound) as exc: - api.update_collection_components( - library=self.lib2, - collection_pk=self.col1.pk, + collection=self.col2, usage_keys=[ UsageKey.from_string(self.lib1_problem_block["id"]), UsageKey.from_string(self.lib1_html_block["id"]),