Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 38 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
bf17dbb
feat: add & remove collections to component
navinkarkera Oct 3, 2024
e5d3b69
refactor: set collections in a component
navinkarkera Oct 4, 2024
40a63e4
refactor: async collections indexing
navinkarkera Oct 4, 2024
d18d5ba
fix: lint issues
navinkarkera Oct 4, 2024
ac59d53
test: test set collections for component
navinkarkera Oct 7, 2024
fa8ad9e
chore: fix docs
navinkarkera Oct 7, 2024
0b7a782
chore: temporarily update openedx-learning package
navinkarkera Oct 7, 2024
e007398
fix: lint issues
navinkarkera Oct 7, 2024
0a438d0
chore: add usage_key to filterable attribute
navinkarkera Oct 8, 2024
4c8ab8a
feat: add collections to component api
navinkarkera Oct 9, 2024
86b8cdd
fix: lint issues
navinkarkera Oct 9, 2024
cd9eec9
fix: remove collection-entity through model post_delete signal handler
navinkarkera Oct 10, 2024
97ad3ba
fix: use lazy field to control collection update task
navinkarkera Oct 10, 2024
baa10fc
chore: temporarily point openedx-events to dev branch
navinkarkera Oct 10, 2024
e553ec0
fix: use get_components in m2m_changed signal handler
navinkarkera Oct 10, 2024
758334f
fix: circuvent meilisearch bug
rpenido Oct 10, 2024
39e825e
feat: add & remove collections to component
navinkarkera Oct 3, 2024
22d9bce
refactor: set collections in a component
navinkarkera Oct 4, 2024
d0c304c
refactor: async collections indexing
navinkarkera Oct 4, 2024
5d31959
fix: lint issues
navinkarkera Oct 4, 2024
acfb0fc
test: test set collections for component
navinkarkera Oct 7, 2024
ae1c7c5
chore: fix docs
navinkarkera Oct 7, 2024
f38ca21
chore: temporarily update openedx-learning package
navinkarkera Oct 7, 2024
68206c2
fix: lint issues
navinkarkera Oct 7, 2024
cedbea7
chore: add usage_key to filterable attribute
navinkarkera Oct 8, 2024
8fef447
feat: add collections to component api
navinkarkera Oct 9, 2024
5dd1a91
fix: lint issues
navinkarkera Oct 9, 2024
dac0f1e
fix: remove collection-entity through model post_delete signal handler
navinkarkera Oct 10, 2024
a0f439f
fix: use lazy field to control collection update task
navinkarkera Oct 10, 2024
b41edf8
chore: temporarily point openedx-events to dev branch
navinkarkera Oct 10, 2024
4248639
Merge branch 'navin/component-collection-api' into rpenido/navin/comp…
navinkarkera Oct 11, 2024
c4ce75f
fix: use get_components in m2m_changed signal handler
navinkarkera Oct 10, 2024
557b176
Merge pull request #695 from open-craft/rpenido/navin/component-colle…
navinkarkera Oct 11, 2024
ddb343d
fix: update components if collection was deleted
navinkarkera Oct 11, 2024
9d2bfe3
refactor: rename lazy field to background
navinkarkera Oct 11, 2024
6d4cb36
chore: update openedx-events version
navinkarkera Oct 12, 2024
9bea9e8
chore: update openedx-learning version
navinkarkera Oct 15, 2024
e8f5bfb
Merge branch 'master' into navin/component-collection-api
ChrisChV Oct 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(
ChrisChV marked this conversation as resolved.
Show resolved Hide resolved
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
Loading