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 8 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
84 changes: 79 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 Down Expand Up @@ -150,6 +155,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 +329,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 +346,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 +417,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 +489,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 +1067,69 @@ def revert_changes(library_key):
)


def update_collection_contents(
library: ContentLibraryMetadata,
collection_pk: int,
usage_keys: list[UsageKeyV2],
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:
* Collection.DoesNotExist if no Collection with the given pk is found in the given library.
* Component.DoesNotExist if any of the given usage_keys don't correspond to a Component in the
library/learning_package.
"""
learning_package = library.learning_package
collections_qset = authoring_api.get_learning_package_collections(
learning_package.id,
).filter(pk=collection_pk)

collection = collections_qset.first()
if not collection:
raise Collection.DoesNotExist()

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

# Can raise Component.DoesNotExist
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,
)
component_keys.append(component.key)

# Note: Component.key matches its PublishableEntity.key
contents_qset = learning_package.publishable_entities.filter(
key__in=component_keys,
)
if not contents_qset.count():
raise Component.DoesNotExist()

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

# Emit a CONTENT_OBJECT_TAGS_CHANGED event for each of the objects added/removed
object_keys = contents_qset.values_list("key", flat=True)
for object_key in object_keys:
CONTENT_OBJECT_TAGS_CHANGED.send_event(
content_object=ContentObjectData(object_id=object_key),
)

Copy link
Member

@rpenido rpenido Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I get why we are dispatching CONTENT_OBJECT_TAGS_CHANGED here. Maybe you mean LIBRARY_COLLECTION_UPDATED?

Note: There is a discussion about this event here.

Edit: Now I saw that this came from the issue description. But I don't see any benefit from using this event. Currently, it updates only the tag data (ref), not the metadata of the document (that we would need here)

Copy link
Member Author

@pomegranited pomegranited Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rpenido Yep, that's the current behaviour, but we're planning to update it with openedx/modular-learning#230. I've made a start at that here: jill/collection-components-rest...open-craft:edx-platform:jill/collection-components-search

But will see how that discussion shakes out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rpenido I will rename CONTENT_OBJECT_TAGS_CHANGED to CONTENT_OBJECT_ASSOCIATIONS_CHANGED, and add a field indicating whether it was the tag or collection that changed when I do openedx/modular-learning#230.

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