diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index b5ed1bde78e1..17338f20ab83 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -320,6 +320,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.block_id, Fields.block_type, Fields.context_key, + Fields.usage_key, Fields.org, Fields.tags, Fields.tags + "." + Fields.tags_taxonomy, diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index eabeab9654ca..f520cd14e40f 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -267,6 +267,13 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) -> } """ + result = { + Fields.collections: { + Fields.collections_display_name: [], + Fields.collections_key: [], + } + } + # Gather the collections associated with this object collections = None try: @@ -279,14 +286,8 @@ def _collections_for_content_object(object_id: UsageKey | LearningContextKey) -> log.warning(f"No component found for {object_id}") if not collections: - return {Fields.collections: {}} + return result - result = { - Fields.collections: { - Fields.collections_display_name: [], - Fields.collections_key: [], - } - } for collection in collections: result[Fields.collections][Fields.collections_display_name].append(collection.title) result[Fields.collections][Fields.collections_key].append(collection.key) diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 085387d336b1..24add6748d7d 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -179,13 +179,19 @@ def library_collection_updated_handler(**kwargs) -> None: log.error("Received null or incorrect data for event") return - # Update collection index synchronously to make sure that search index is updated before - # the frontend invalidates/refetches index. - # See content_library_updated_handler for more details. - update_library_collection_index_doc.apply(args=[ - str(library_collection.library_key), - library_collection.collection_key, - ]) + if library_collection.background: + update_library_collection_index_doc.delay( + str(library_collection.library_key), + library_collection.collection_key, + ) + else: + # Update collection index synchronously to make sure that search index is updated before + # the frontend invalidates/refetches index. + # See content_library_updated_handler for more details. + update_library_collection_index_doc.apply(args=[ + str(library_collection.library_key), + library_collection.collection_key, + ]) @receiver(CONTENT_OBJECT_ASSOCIATIONS_CHANGED) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 4c6227af309f..c89d84490e96 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -219,10 +219,10 @@ def test_reindex_meilisearch(self, mock_meilisearch): doc_vertical["tags"] = {} doc_problem1 = copy.deepcopy(self.doc_problem1) doc_problem1["tags"] = {} - doc_problem1["collections"] = {} + doc_problem1["collections"] = {'display_name': [], 'key': []} doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} - doc_problem2["collections"] = {} + doc_problem2["collections"] = {'display_name': [], 'key': []} doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} @@ -263,7 +263,7 @@ def test_reindex_meilisearch_library_block_error(self, mock_meilisearch): doc_vertical["tags"] = {} doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} - doc_problem2["collections"] = {} + doc_problem2["collections"] = {'display_name': [], 'key': []} orig_from_component = library_api.LibraryXBlockMetadata.from_component @@ -662,7 +662,7 @@ def test_delete_collection(self, mock_meilisearch): doc_problem_without_collection = { "id": self.doc_problem1["id"], - "collections": {}, + "collections": {'display_name': [], 'key': []}, } # Should delete the collection document diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index b9f3779af539..6f45a10daf49 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -80,6 +80,7 @@ from openedx_events.content_authoring.data import ( ContentLibraryData, LibraryBlockData, + LibraryCollectionData, ) from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_CREATED, @@ -88,6 +89,7 @@ LIBRARY_BLOCK_CREATED, LIBRARY_BLOCK_DELETED, LIBRARY_BLOCK_UPDATED, + LIBRARY_COLLECTION_UPDATED, ) from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Collection, Component, MediaType, LearningPackage, PublishableEntity @@ -204,6 +206,15 @@ class ContentLibraryPermissionEntry: access_level = attr.ib(AccessLevel.NO_ACCESS) +@attr.s +class CollectionMetadata: + """ + Class to represent collection metadata in a content library. + """ + key = attr.ib(type=str) + title = attr.ib(type=str) + + @attr.s class LibraryXBlockMetadata: """ @@ -219,9 +230,10 @@ class LibraryXBlockMetadata: published_by = attr.ib("") has_unpublished_changes = attr.ib(False) created = attr.ib(default=None, type=datetime) + collections = attr.ib(type=list[CollectionMetadata], factory=list) @classmethod - def from_component(cls, library_key, component): + def from_component(cls, library_key, component, associated_collections=None): """ Construct a LibraryXBlockMetadata from a Component object. """ @@ -248,6 +260,7 @@ def from_component(cls, library_key, component): last_draft_created=last_draft_created, last_draft_created_by=last_draft_created_by, has_unpublished_changes=component.versioning.has_unpublished_changes, + collections=associated_collections or [], ) @@ -690,7 +703,7 @@ def get_library_components(library_key, text_search=None, block_types=None) -> Q return components -def get_library_block(usage_key) -> LibraryXBlockMetadata: +def get_library_block(usage_key, include_collections=False) -> LibraryXBlockMetadata: """ Get metadata about (the draft version of) one specific XBlock in a library. @@ -713,9 +726,17 @@ def get_library_block(usage_key) -> LibraryXBlockMetadata: if not draft_version: raise ContentLibraryBlockNotFound(usage_key) + if include_collections: + associated_collections = authoring_api.get_entity_collections( + component.learning_package_id, + component.key, + ).values('key', 'title') + else: + associated_collections = None xblock_metadata = LibraryXBlockMetadata.from_component( library_key=usage_key.context_key, component=component, + associated_collections=associated_collections, ) return xblock_metadata @@ -1235,6 +1256,60 @@ def update_library_collection_components( return collection +def set_library_component_collections( + library_key: LibraryLocatorV2, + component: Component, + *, + collection_keys: list[str], + created_by: int | None = None, + # As an optimization, callers may pass in a pre-fetched ContentLibrary instance + content_library: ContentLibrary | None = None, +) -> Component: + """ + It Associates the component with collections for the given collection keys. + + Only collections in queryset are associated with component, all previous component-collections + associations are removed. + + If you've already fetched the ContentLibrary, pass it in to avoid refetching. + + Raises: + * ContentLibraryCollectionNotFound if any of the given collection_keys don't match Collections in the given library. + + Returns the updated Component. + """ + if not content_library: + content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] + assert content_library + assert content_library.learning_package_id + assert content_library.library_key == library_key + + # Note: Component.key matches its PublishableEntity.key + collection_qs = authoring_api.get_collections(content_library.learning_package_id).filter( + key__in=collection_keys + ) + + affected_collections = authoring_api.set_collections( + content_library.learning_package_id, + component, + collection_qs, + created_by=created_by, + ) + + # For each collection, trigger LIBRARY_COLLECTION_UPDATED signal and set background=True to trigger + # collection indexing asynchronously. + for collection in affected_collections: + LIBRARY_COLLECTION_UPDATED.send_event( + library_collection=LibraryCollectionData( + library_key=library_key, + collection_key=collection.key, + background=True, + ) + ) + + return component + + def get_library_collection_usage_key( library_key: LibraryLocatorV2, collection_key: str, diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/serializers.py index 51ba55cd6b48..8e9e5fc2a749 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/serializers.py @@ -134,6 +134,14 @@ class ContentLibraryFilterSerializer(BaseFilterSerializer): type = serializers.ChoiceField(choices=LIBRARY_TYPES, default=None, required=False) +class CollectionMetadataSerializer(serializers.Serializer): + """ + Serializer for CollectionMetadata + """ + key = serializers.CharField() + title = serializers.CharField() + + class LibraryXBlockMetadataSerializer(serializers.Serializer): """ Serializer for LibraryXBlockMetadata @@ -161,6 +169,8 @@ class LibraryXBlockMetadataSerializer(serializers.Serializer): slug = serializers.CharField(write_only=True) tags_count = serializers.IntegerField(read_only=True) + collections = CollectionMetadataSerializer(many=True, required=False) + class LibraryXBlockTypeSerializer(serializers.Serializer): """ @@ -305,3 +315,11 @@ class ContentLibraryCollectionComponentsUpdateSerializer(serializers.Serializer) """ usage_keys = serializers.ListField(child=UsageKeyV2Serializer(), allow_empty=False) + + +class ContentLibraryComponentCollectionsUpdateSerializer(serializers.Serializer): + """ + Serializer for adding/removing Collections to/from a Component. + """ + + collection_keys = serializers.ListField(child=serializers.CharField(), allow_empty=True) diff --git a/openedx/core/djangoapps/content_libraries/signal_handlers.py b/openedx/core/djangoapps/content_libraries/signal_handlers.py index fedee045a9f6..58f45d218e95 100644 --- a/openedx/core/djangoapps/content_libraries/signal_handlers.py +++ b/openedx/core/djangoapps/content_libraries/signal_handlers.py @@ -20,8 +20,8 @@ LIBRARY_COLLECTION_DELETED, LIBRARY_COLLECTION_UPDATED, ) -from openedx_learning.api.authoring import get_collection_components, get_component, get_components -from openedx_learning.api.authoring_models import Collection, CollectionPublishableEntity, Component +from openedx_learning.api.authoring import get_component, get_components +from openedx_learning.api.authoring_models import Collection, CollectionPublishableEntity, Component, PublishableEntity from lms.djangoapps.grades.api import signals as grades_signals @@ -167,9 +167,11 @@ def library_collection_entity_deleted(sender, instance, **kwargs): """ Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for components removed from a collection. """ - # Component.pk matches its entity.pk - component = get_component(instance.entity_id) - _library_collection_component_changed(component) + # Only trigger component updates if CollectionPublishableEntity was cascade deleted due to deletion of a collection. + if isinstance(kwargs.get('origin'), Collection): + # Component.pk matches its entity.pk + component = get_component(instance.entity_id) + _library_collection_component_changed(component) @receiver(m2m_changed, sender=CollectionPublishableEntity, dispatch_uid="library_collection_entities_changed") @@ -177,9 +179,6 @@ def library_collection_entities_changed(sender, instance, action, pk_set, **kwar """ Sends a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for components added/removed/cleared from a collection. """ - if not isinstance(instance, Collection): - return - if action not in ["post_add", "post_remove", "post_clear"]: return @@ -191,18 +190,16 @@ def library_collection_entities_changed(sender, instance, action, pk_set, **kwar log.error("{instance} is not associated with a content library.") return + if isinstance(instance, PublishableEntity): + _library_collection_component_changed(instance.component, library.library_key) + return + + # When action=="post_clear", pk_set==None + # Since the collection instance now has an empty entities set, + # we don't know which ones were removed, so we need to update associations for all library components. + components = get_components(instance.learning_package_id) if pk_set: - components = get_collection_components( - instance.learning_package_id, - instance.key, - ).filter(pk__in=pk_set) - else: - # When action=="post_clear", pk_set==None - # Since the collection instance now has an empty entities set, - # we don't know which ones were removed, so we need to update associations for all library components. - components = get_components( - instance.learning_package_id, - ) + components = components.filter(pk__in=pk_set) for component in components.all(): _library_collection_component_changed(component, library.library_key) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 8041c508dc31..c526e7b1a1f3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -308,6 +308,13 @@ def setUp(self): description="Description for Collection 2", created_by=self.user.id, ) + self.col3 = api.create_library_collection( + self.lib2.library_key, + collection_key="COL3", + title="Collection 3", + description="Description for Collection 3", + created_by=self.user.id, + ) # Create some library blocks in lib1 self.lib1_problem_block = self._add_block_to_library( @@ -316,6 +323,10 @@ def setUp(self): self.lib1_html_block = self._add_block_to_library( self.lib1.library_key, "html", "html1", ) + # Create some library blocks in lib2 + self.lib2_problem_block = self._add_block_to_library( + self.lib2.library_key, "problem", "problem2", + ) def test_create_library_collection(self): event_receiver = mock.Mock() @@ -498,3 +509,55 @@ def test_update_collection_components_from_wrong_library(self): ], ) assert self.lib1_problem_block["id"] in str(exc.exception) + + def test_set_library_component_collections(self): + event_receiver = mock.Mock() + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_receiver) + collection_update_event_receiver = mock.Mock() + LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver) + assert not list(self.col2.entities.all()) + component = api.get_component_from_usage_key(UsageKey.from_string(self.lib2_problem_block["id"])) + + api.set_library_component_collections( + self.lib2.library_key, + component, + collection_keys=[self.col2.key, self.col3.key], + ) + + assert len(authoring_api.get_collection(self.lib2.learning_package_id, self.col2.key).entities.all()) == 1 + assert len(authoring_api.get_collection(self.lib2.learning_package_id, self.col3.key).entities.all()) == 1 + self.assertDictContainsSubset( + { + "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + "sender": None, + "content_object": ContentObjectChangedData( + object_id=self.lib2_problem_block["id"], + changes=["collections"], + ), + }, + event_receiver.call_args_list[0].kwargs, + ) + self.assertDictContainsSubset( + { + "signal": LIBRARY_COLLECTION_UPDATED, + "sender": None, + "library_collection": LibraryCollectionData( + self.lib2.library_key, + collection_key=self.col2.key, + background=True, + ), + }, + collection_update_event_receiver.call_args_list[0].kwargs, + ) + self.assertDictContainsSubset( + { + "signal": LIBRARY_COLLECTION_UPDATED, + "sender": None, + "library_collection": LibraryCollectionData( + self.lib2.library_key, + collection_key=self.col3.key, + background=True, + ), + }, + collection_update_event_receiver.call_args_list[1].kwargs, + ) diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 9455f0de5e61..e77c1b34d277 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -57,6 +57,8 @@ path('blocks//', include([ # Get metadata about a specific XBlock in this library, or delete the block: path('', views.LibraryBlockView.as_view()), + # Update collections for a given component + path('collections/', views.LibraryBlockCollectionsView.as_view(), name='update-collections'), # Get the LTI URL of a specific XBlock path('lti/', views.LibraryBlockLtiUrlView.as_view(), name='lti-url'), # Get the OLX source code of the specified block: diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index 3712af6e597f..6e50559f38f6 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -106,6 +106,7 @@ ContentLibraryPermissionLevelSerializer, ContentLibraryPermissionSerializer, ContentLibraryUpdateSerializer, + ContentLibraryComponentCollectionsUpdateSerializer, LibraryXBlockCreationSerializer, LibraryXBlockMetadataSerializer, LibraryXBlockTypeSerializer, @@ -617,7 +618,7 @@ def get(self, request, usage_key_str): """ key = LibraryUsageLocatorV2.from_string(usage_key_str) api.require_permission_for_library_key(key.lib_key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) - result = api.get_library_block(key) + result = api.get_library_block(key, include_collections=True) return Response(LibraryXBlockMetadataSerializer(result).data) @@ -640,6 +641,41 @@ def delete(self, request, usage_key_str): # pylint: disable=unused-argument return Response({}) +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryBlockCollectionsView(APIView): + """ + View to set collections for a component. + """ + @convert_exceptions + def patch(self, request, usage_key_str) -> Response: + """ + Sets Collections for a Component. + + Collection and Components must all be part of the given library/learning package. + """ + key = LibraryUsageLocatorV2.from_string(usage_key_str) + content_library = api.require_permission_for_library_key( + key.lib_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) + component = api.get_component_from_usage_key(key) + serializer = ContentLibraryComponentCollectionsUpdateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + collection_keys = serializer.validated_data['collection_keys'] + api.set_library_component_collections( + library_key=key.lib_key, + component=component, + collection_keys=collection_keys, + created_by=self.request.user.id, + content_library=content_library, + ) + + return Response({'count': len(collection_keys)}) + + @method_decorator(non_atomic_requests, name="dispatch") @view_auth_classes() class LibraryBlockLtiUrlView(APIView): diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 6fb109d62f09..d37ffb1bcd46 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -140,7 +140,7 @@ optimizely-sdk<5.0 # Date: 2023-09-18 # pinning this version to avoid updates while the library is being developed # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-learning==0.13.1 +openedx-learning==0.15.0 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 2bac3df49c71..6ede5ed12302 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -811,7 +811,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/kernel.in openedx-django-wiki==2.1.0 # via -r requirements/edx/kernel.in -openedx-events==9.14.1 +openedx-events==9.15.0 # via # -r requirements/edx/kernel.in # edx-enterprise @@ -825,7 +825,7 @@ openedx-filters==1.10.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # ora2 -openedx-learning==0.13.1 +openedx-learning==0.15.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index d61db58ae4c2..901a37c05522 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1358,7 +1358,7 @@ openedx-django-wiki==2.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-events==9.14.1 +openedx-events==9.15.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1374,7 +1374,7 @@ openedx-filters==1.10.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # ora2 -openedx-learning==0.13.1 +openedx-learning==0.15.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 32ef31e8d3e5..34e6275592b1 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -970,7 +970,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events==9.14.1 +openedx-events==9.15.0 # via # -r requirements/edx/base.txt # edx-enterprise @@ -984,7 +984,7 @@ openedx-filters==1.10.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning==0.13.1 +openedx-learning==0.15.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 77b3896967fb..5658241be489 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1021,7 +1021,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events==9.14.1 +openedx-events==9.15.0 # via # -r requirements/edx/base.txt # edx-enterprise @@ -1035,7 +1035,7 @@ openedx-filters==1.10.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning==0.13.1 +openedx-learning==0.15.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt