feat: set collections for a library component [FC-0062] (#35600)

* feat: add & remove collections to component

Co-authored-by: Rômulo Penido <romulo.penido@gmail.com>
Co-authored-by: Chris Chávez <xnpiochv@gmail.com>
This commit is contained in:
Navin Karkera
2024-10-15 20:41:54 +05:30
committed by GitHub
parent 17122eb442
commit 70df3deea6
15 changed files with 248 additions and 49 deletions

View File

@@ -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,

View File

@@ -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)

View File

@@ -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)

View File

@@ -219,10 +219,10 @@ class TestSearchApi(ModuleStoreTestCase):
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 @@ class TestSearchApi(ModuleStoreTestCase):
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 @@ class TestSearchApi(ModuleStoreTestCase):
doc_problem_without_collection = {
"id": self.doc_problem1["id"],
"collections": {},
"collections": {'display_name': [], 'key': []},
}
# Should delete the collection document

View File

@@ -80,6 +80,7 @@ from opaque_keys import InvalidKeyError
from openedx_events.content_authoring.data import (
ContentLibraryData,
LibraryBlockData,
LibraryCollectionData,
)
from openedx_events.content_authoring.signals import (
CONTENT_LIBRARY_CREATED,
@@ -88,6 +89,7 @@ from openedx_events.content_authoring.signals import (
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 @@ class LibraryXBlockMetadata:
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,

View File

@@ -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)

View File

@@ -20,8 +20,8 @@ from openedx_events.content_authoring.signals import (
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)

View File

@@ -308,6 +308,13 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe
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 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe
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 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe
],
)
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,
)

View File

@@ -57,6 +57,8 @@ urlpatterns = [
path('blocks/<str:usage_key_str>/', 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:

View File

@@ -106,6 +106,7 @@ from openedx.core.djangoapps.content_libraries.serializers import (
ContentLibraryPermissionLevelSerializer,
ContentLibraryPermissionSerializer,
ContentLibraryUpdateSerializer,
ContentLibraryComponentCollectionsUpdateSerializer,
LibraryXBlockCreationSerializer,
LibraryXBlockMetadataSerializer,
LibraryXBlockTypeSerializer,
@@ -617,7 +618,7 @@ class LibraryBlockView(APIView):
"""
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 @@ class LibraryBlockView(APIView):
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):

View File

@@ -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.

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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