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: Add REST endpoints to update Components in a Collections (temp) #674

Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f58bb67
feat: Add Library Collections REST endpoints
yusuf-musleh Aug 14, 2024
f490007
test: Add tests for Collections REST APIs
yusuf-musleh Aug 22, 2024
17933a1
chore: Add missing __init__ files
yusuf-musleh Aug 23, 2024
743291c
feat: Verify collection belongs to library
yusuf-musleh Aug 23, 2024
3da44d5
feat: Add events emitting for Collections
yusuf-musleh Aug 26, 2024
59514cc
chore: fix pylint errors
yusuf-musleh Aug 26, 2024
250434f
chore: update openedx-learning
pomegranited Aug 23, 2024
f4521b5
feat: add/remove components to/from a collection
pomegranited Aug 28, 2024
129bcf2
refactor: use "components" not "contents"
pomegranited Aug 29, 2024
e8013eb
refactor: use oel_collections.get_collections
pomegranited Aug 29, 2024
94890db
fix: pylint/type error
pomegranited Aug 29, 2024
6ded8a9
test: adds tests for api.update_collection_components
pomegranited Aug 29, 2024
0d8079c
test: fixes failing test
pomegranited Aug 29, 2024
fe43b72
feat: Add Library Collections REST endpoints
yusuf-musleh Aug 14, 2024
f5900be
test: Add tests for Collections REST APIs
yusuf-musleh Aug 22, 2024
97739c6
chore: Add missing __init__ files
yusuf-musleh Aug 23, 2024
2996a2e
feat: Verify collection belongs to library
yusuf-musleh Aug 23, 2024
fbfd983
feat: Add events emitting for Collections
yusuf-musleh Aug 26, 2024
cf9e906
chore: fix pylint errors
yusuf-musleh Aug 26, 2024
5bf9422
refactor: Use convert_exceptions + update tests
yusuf-musleh Aug 29, 2024
ea24e2f
Merge remote-tracking branch 'opencraft/yusuf-musleh/collections-crud…
pomegranited Aug 29, 2024
8034861
refactor: index collections within each library
pomegranited Aug 29, 2024
55338e1
test: fix flaky test
pomegranited Aug 30, 2024
23f5d5b
refactor: adapt to oel_collection.update_collection_components changes
pomegranited Aug 30, 2024
0a1732e
Merge branch 'yusuf-musleh/collections-crud-rest-api' into jill/colle…
pomegranited Sep 2, 2024
addde52
refactor: moved code around after merging base
pomegranited Sep 2, 2024
51f5c1a
refactor: simplify the REST API params and validation
pomegranited Sep 3, 2024
7a8e4c6
chore: uses openedx-learning==0.11.3
pomegranited Sep 3, 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
12 changes: 12 additions & 0 deletions docs/hooks/events.rst
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,15 @@ Content Authoring Events
* - `CONTENT_OBJECT_TAGS_CHANGED <https://github.com/openedx/openedx-events/blob/c0eb4ba1a3d7d066d58e5c87920b8ccb0645f769/openedx_events/content_authoring/signals.py#L207>`_
- org.openedx.content_authoring.content.object.tags.changed.v1
- 2024-03-31

* - `LIBRARY_COLLECTION_CREATED <https://github.com/openedx/openedx-events/blob/main/openedx_events/content_authoring/signals.py#L219>`_
- org.openedx.content_authoring.content.library.collection.created.v1
- 2024-08-23

* - `LIBRARY_COLLECTION_UPDATED <https://github.com/openedx/openedx-events/blob/main/openedx_events/content_authoring/signals.py#L230>`_
- org.openedx.content_authoring.content.library.collection.updated.v1
- 2024-08-23

* - `LIBRARY_COLLECTION_DELETED <https://github.com/openedx/openedx-events/blob/main/openedx_events/content_authoring/signals.py#L241>`_
- org.openedx.content_authoring.content.library.collection.deleted.v1
- 2024-08-23
76 changes: 36 additions & 40 deletions openedx/core/djangoapps/content/search/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,16 +296,12 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None:
status_cb("Counting courses...")
num_courses = CourseOverview.objects.count()

# Get the list of collections
status_cb("Counting collections...")
num_collections = authoring_api.get_collections().count()

# Some counters so we can track our progress as indexing progresses:
num_contexts = num_courses + num_libraries + num_collections
num_contexts = num_courses + num_libraries
num_contexts_done = 0 # How many courses/libraries we've indexed
num_blocks_done = 0 # How many individual components/XBlocks we've indexed

status_cb(f"Found {num_courses} courses, {num_libraries} libraries and {num_collections} collections.")
status_cb(f"Found {num_courses} courses, {num_libraries} libraries.")
with _using_temp_index(status_cb) as temp_index_name:
############## Configure the index ##############

Expand Down Expand Up @@ -390,10 +386,43 @@ def index_library(lib_key: str) -> list:
status_cb(f"Error indexing library {lib_key}: {err}")
return docs

############## Collections ##############
def index_collection_batch(batch, num_done) -> int:
docs = []
for collection in batch:
try:
doc = searchable_doc_for_collection(collection)
# Uncomment below line once collections are tagged.
# doc.update(searchable_doc_tags(collection.id))
docs.append(doc)
except Exception as err: # pylint: disable=broad-except
status_cb(f"Error indexing collection {collection}: {err}")
num_done += 1

if docs:
try:
# Add docs in batch of 100 at once (usually faster than adding one at a time):
_wait_for_meili_task(client.index(temp_index_name).add_documents(docs))
except (TypeError, KeyError, MeilisearchError) as err:
status_cb(f"Error indexing collection batch {p}: {err}")
return num_done

for lib_key in lib_keys:
status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing library {lib_key}")
status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing blocks in library {lib_key}")
lib_docs = index_library(lib_key)
num_blocks_done += len(lib_docs)

# To reduce memory usage on large instances, split up the Collections into pages of 100 collections:
library = lib_api.get_library(lib_key)
collections = authoring_api.get_collections(library.learning_package.id, enabled=True)
num_collections = collections.count()
num_collections_done = 0
status_cb(f"{num_collections_done + 1}/{num_collections}. Now indexing collections in library {lib_key}")
paginator = Paginator(collections, 100)
for p in paginator.page_range:
num_collections_done = index_collection_batch(paginator.page(p).object_list, num_collections_done)
status_cb(f"{num_collections_done}/{num_collections} collections indexed for library {lib_key}")

num_contexts_done += 1

############## Courses ##############
Expand Down Expand Up @@ -430,39 +459,6 @@ def add_with_children(block):
num_contexts_done += 1
num_blocks_done += len(course_docs)

############## Collections ##############
status_cb("Indexing collections...")

def index_collection_batch(batch, num_contexts_done) -> int:
docs = []
for collection in batch:
status_cb(
f"{num_contexts_done + 1}/{num_contexts}. "
f"Now indexing collection {collection.title} ({collection.id})"
)
try:
doc = searchable_doc_for_collection(collection)
# Uncomment below line once collections are tagged.
# doc.update(searchable_doc_tags(collection.id))
docs.append(doc)
except Exception as err: # pylint: disable=broad-except
status_cb(f"Error indexing collection {collection}: {err}")
finally:
num_contexts_done += 1

if docs:
try:
# Add docs in batch of 100 at once (usually faster than adding one at a time):
_wait_for_meili_task(client.index(temp_index_name).add_documents(docs))
except (TypeError, KeyError, MeilisearchError) as err:
status_cb(f"Error indexing collection batch {p}: {err}")
return num_contexts_done

# To reduce memory usage on large instances, split up the Collections into pages of 100 collections:
paginator = Paginator(authoring_api.get_collections(enabled=True), 100)
for p in paginator.page_range:
num_contexts_done = index_collection_batch(paginator.page(p).object_list, num_contexts_done)

status_cb(f"Done! {num_blocks_done} blocks indexed across {num_contexts_done} courses, collections and libraries.")


Expand Down
94 changes: 89 additions & 5 deletions openedx/core/djangoapps/content_libraries/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,20 @@
from django.utils.translation import gettext as _
from edx_rest_api_client.client import OAuthAPIClient
from lxml import etree
from opaque_keys.edx.keys import UsageKey, UsageKeyV2
from opaque_keys.edx.keys import BlockTypeKey, UsageKey, UsageKeyV2
from opaque_keys.edx.locator import (
LibraryLocatorV2,
LibraryUsageLocatorV2,
LibraryLocator as LibraryLocatorV1
)
from opaque_keys import InvalidKeyError
from openedx_events.content_authoring.data import ContentLibraryData, LibraryBlockData
from openedx_events.content_authoring.data import (
ContentLibraryData,
ContentObjectData,
LibraryBlockData,
)
from openedx_events.content_authoring.signals import (
CONTENT_OBJECT_TAGS_CHANGED,
CONTENT_LIBRARY_CREATED,
CONTENT_LIBRARY_DELETED,
CONTENT_LIBRARY_UPDATED,
Expand All @@ -86,7 +91,7 @@
LIBRARY_BLOCK_UPDATED,
)
from openedx_learning.api import authoring as authoring_api
from openedx_learning.api.authoring_models import Component, MediaType
from openedx_learning.api.authoring_models import Collection, Component, MediaType, LearningPackage
from organizations.models import Organization
from xblock.core import XBlock
from xblock.exceptions import XBlockNotFoundError
Expand All @@ -111,6 +116,8 @@

ContentLibraryNotFound = ContentLibrary.DoesNotExist

ContentLibraryCollectionNotFound = Collection.DoesNotExist


class ContentLibraryBlockNotFound(XBlockNotFoundError):
""" XBlock not found in the content library """
Expand Down Expand Up @@ -150,6 +157,7 @@ class ContentLibraryMetadata:
Class that represents the metadata about a content library.
"""
key = attr.ib(type=LibraryLocatorV2)
learning_package = attr.ib(type=LearningPackage)
title = attr.ib("")
description = attr.ib("")
num_blocks = attr.ib(0)
Expand Down Expand Up @@ -323,13 +331,14 @@ def get_metadata(queryset, text_search=None):
has_unpublished_changes=False,
has_unpublished_deletes=False,
license=lib.license,
learning_package=lib.learning_package,
)
for lib in queryset
]
return libraries


def require_permission_for_library_key(library_key, user, permission):
def require_permission_for_library_key(library_key, user, permission) -> ContentLibrary:
"""
Given any of the content library permission strings defined in
openedx.core.djangoapps.content_libraries.permissions,
Expand All @@ -339,10 +348,12 @@ def require_permission_for_library_key(library_key, user, permission):
Raises django.core.exceptions.PermissionDenied if the user doesn't have
permission.
"""
library_obj = ContentLibrary.objects.get_by_key(library_key)
library_obj = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined]
if not user.has_perm(permission, obj=library_obj):
raise PermissionDenied

return library_obj


def get_library(library_key):
"""
Expand Down Expand Up @@ -408,6 +419,7 @@ def get_library(library_key):
license=ref.license,
created=learning_package.created,
updated=learning_package.updated,
learning_package=learning_package
)


Expand Down Expand Up @@ -479,6 +491,7 @@ def create_library(
allow_public_learning=ref.allow_public_learning,
allow_public_read=ref.allow_public_read,
license=library_license,
learning_package=ref.learning_package
)


Expand Down Expand Up @@ -1056,6 +1069,77 @@ def revert_changes(library_key):
)


def update_collection_components(
library: ContentLibraryMetadata,
collection_pk: int,
usage_keys: list[UsageKeyV2],
created_by: int | None = None,
remove=False,
) -> int:
"""
Associates the Collection with Components for the given UsageKeys.

By default the Components are added to the Collection.
If remove=True, the Components are removed from the Collection.

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)

# Fetch the Component.key values for the provided UsageKeys.
component_keys = []
for usage_key in usage_keys:
# Parse the block_family from the key to use as namespace.
block_type = BlockTypeKey.from_string(str(usage_key))

try:
component = authoring_api.get_component_by_key(
learning_package.id,
namespace=block_type.block_family,
type_name=usage_key.block_type,
local_key=usage_key.block_id,
)
except Component.DoesNotExist as exc:
raise ContentLibraryBlockNotFound(usage_key) from exc

component_keys.append(component.key)

# Note: Component.key matches its PublishableEntity.key
entities_qset = learning_package.publishable_entities.filter(
key__in=component_keys,
)

if remove:
count = authoring_api.remove_from_collections(
collections_qset,
entities_qset,
)
else:
count = authoring_api.add_to_collections(
collections_qset,
entities_qset,
created_by=created_by,
)

# Emit a CONTENT_OBJECT_TAGS_CHANGED event for each of the objects added/removed
for usage_key in usage_keys:
CONTENT_OBJECT_TAGS_CHANGED.send_event(
content_object=ContentObjectData(object_id=usage_key),
)

return count


# V1/V2 Compatibility Helpers
# (Should be removed as part of
# https://github.com/openedx/edx-platform/issues/32457)
Expand Down
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content_libraries/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ def ready(self):
Import signal handler's module to ensure they are registered.
"""
from . import signal_handlers # pylint: disable=unused-import
from .collections import handlers # pylint: disable=unused-import
Empty file.
57 changes: 57 additions & 0 deletions openedx/core/djangoapps/content_libraries/collections/handlers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
"""
Signal handlers for Content Library Collections.
"""

import logging

from django.dispatch import receiver
from openedx_events.content_authoring.data import LibraryCollectionData
from openedx_events.content_authoring.signals import (
LIBRARY_COLLECTION_CREATED,
LIBRARY_COLLECTION_UPDATED,
LIBRARY_COLLECTION_DELETED,
)


log = logging.getLogger(__name__)


@receiver(LIBRARY_COLLECTION_CREATED)
def library_collection_created_handler(**kwargs):
"""
Content Library Collection Created signal handler
"""
library_collection_data = kwargs.get("library_collection", None)
if not library_collection_data or not isinstance(library_collection_data, LibraryCollectionData):
log.error("Received null or incorrect data for event")
return

log.info("Received Collection Created Signal")

# TODO: Implement handler logic


@receiver(LIBRARY_COLLECTION_UPDATED)
def library_collection_updated_handler(**kwargs):
"""
Content Library Collection Updated signal handler
"""
library_collection_data = kwargs.get("library_collection", None)
if not library_collection_data or not isinstance(library_collection_data, LibraryCollectionData):
log.error("Received null or incorrect data for event")
return

log.info("Received Collection Updated Signal")


@receiver(LIBRARY_COLLECTION_DELETED)
def library_collection_deleted_handler(**kwargs):
"""
Content Library Collection Deleted signal handler
"""
library_collection_data = kwargs.get("library_collection", None)
if not library_collection_data or not isinstance(library_collection_data, LibraryCollectionData):
log.error("Received null or incorrect data for event")
return

log.info("Received Collection Deleted Signal")
Loading
Loading