Skip to content

Commit

Permalink
feat: set collections for a library component [FC-0062] (#35600)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people authored Oct 15, 2024
1 parent 17122eb commit 70df3de
Show file tree
Hide file tree
Showing 15 changed files with 248 additions and 49 deletions.
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content/search/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 8 additions & 7 deletions openedx/core/djangoapps/content/search/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down
20 changes: 13 additions & 7 deletions openedx/core/djangoapps/content/search/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions openedx/core/djangoapps/content/search/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"] = {}

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
79 changes: 77 additions & 2 deletions openedx/core/djangoapps/content_libraries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
from openedx_events.content_authoring.data import (
ContentLibraryData,
LibraryBlockData,
LibraryCollectionData,
)
from openedx_events.content_authoring.signals import (
CONTENT_LIBRARY_CREATED,
Expand All @@ -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
Expand Down Expand Up @@ -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:
"""
Expand All @@ -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.
"""
Expand All @@ -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 [],
)


Expand Down Expand Up @@ -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.
Expand All @@ -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

Expand Down Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions openedx/core/djangoapps/content_libraries/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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)
35 changes: 16 additions & 19 deletions openedx/core/djangoapps/content_libraries/signal_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -167,19 +167,18 @@ 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")
def library_collection_entities_changed(sender, instance, action, pk_set, **kwargs):
"""
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

Expand All @@ -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)
Loading

0 comments on commit 70df3de

Please sign in to comment.