diff --git a/cms/envs/common.py b/cms/envs/common.py index 59eca17b8133..5d4dea7c2359 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1879,6 +1879,7 @@ "openedx_learning.apps.authoring.components", "openedx_learning.apps.authoring.contents", "openedx_learning.apps.authoring.publishing", + "openedx_learning.apps.authoring.units", ] diff --git a/lms/envs/common.py b/lms/envs/common.py index 37a508606c1b..6b9d516eeb5e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3369,6 +3369,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring "openedx_learning.apps.authoring.components", "openedx_learning.apps.authoring.contents", "openedx_learning.apps.authoring.publishing", + "openedx_learning.apps.authoring.units", ] diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index dd8cfcdc891d..a597ae51e865 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -18,7 +18,7 @@ from meilisearch.errors import MeilisearchApiError, MeilisearchError from meilisearch.models.task import TaskInfo from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryCollectionLocator +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryCollectionLocator from openedx_learning.api import authoring as authoring_api from common.djangoapps.student.roles import GlobalStaff from rest_framework.request import Request @@ -40,6 +40,7 @@ meili_id_from_opaque_key, searchable_doc_for_course_block, searchable_doc_for_collection, + searchable_doc_for_container, searchable_doc_for_library_block, searchable_doc_for_usage_key, searchable_doc_collections, @@ -475,6 +476,31 @@ def index_collection_batch(batch, num_done, library_key) -> int: status_cb(f"Error indexing collection batch {p}: {err}") return num_done + ############## Containers ############## + def index_container_batch(batch, num_done, library_key) -> int: + docs = [] + for container in batch: + try: + container_key = lib_api.library_container_locator( + library_key, + container, + ) + doc = searchable_doc_for_container(container_key) + # TODO: when we add container tags + # doc.update(searchable_doc_tags_for_container(container_key)) + docs.append(doc) + except Exception as err: # pylint: disable=broad-except + status_cb(f"Error indexing container {container.key}: {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(index_name).add_documents(docs)) + except (TypeError, KeyError, MeilisearchError) as err: + status_cb(f"Error indexing container batch {p}: {err}") + return num_done + for lib_key in lib_keys: status_cb(f"{num_contexts_done + 1}/{num_contexts}. Now indexing blocks in library {lib_key}") lib_docs = index_library(lib_key) @@ -497,6 +523,22 @@ def index_collection_batch(batch, num_done, library_key) -> int: IncrementalIndexCompleted.objects.get_or_create(context_key=lib_key) status_cb(f"{num_collections_done}/{num_collections} collections indexed for library {lib_key}") + # Similarly, batch process Containers (units, sections, etc) in pages of 100 + containers = authoring_api.get_containers(library.learning_package_id) + num_containers = containers.count() + num_containers_done = 0 + status_cb(f"{num_containers_done}/{num_containers}. Now indexing containers in library {lib_key}") + paginator = Paginator(containers, 100) + for p in paginator.page_range: + num_containers_done = index_container_batch( + paginator.page(p).object_list, + num_containers_done, + lib_key, + ) + status_cb(f"{num_containers_done}/{num_containers} containers indexed for library {lib_key}") + if incremental: + IncrementalIndexCompleted.objects.get_or_create(context_key=lib_key) + num_contexts_done += 1 ############## Courses ############## @@ -732,6 +774,30 @@ def update_library_components_collections( _update_index_docs(docs) +def upsert_library_container_index_doc(container_key: LibraryContainerLocator) -> None: + """ + Creates, updates, or deletes the document for the given Library Container in the search index. + + TODO: add support for indexing a container's components, like upsert_library_collection_index_doc does. + """ + doc = searchable_doc_for_container(container_key) + + # Soft-deleted/disabled containers are removed from the index + # and their components updated. + if doc.get('_disabled'): + + _delete_index_doc(doc[Fields.id]) + + # Hard-deleted containers are also deleted from the index + elif not doc.get(Fields.type): + + _delete_index_doc(doc[Fields.id]) + + # Otherwise, upsert the container. + else: + _update_index_docs([doc]) + + def upsert_content_library_index_docs(library_key: LibraryLocatorV2) -> None: """ Creates or updates the documents for the given Content Library in the search index diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 3d739781fe71..14c7f712dcfd 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -10,7 +10,7 @@ from django.core.exceptions import ObjectDoesNotExist from opaque_keys.edx.keys import LearningContextKey, UsageKey from openedx_learning.api import authoring as authoring_api -from opaque_keys.edx.locator import LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator from rest_framework.exceptions import NotFound from openedx.core.djangoapps.content.search.models import SearchAccess @@ -100,6 +100,7 @@ class DocType: """ course_block = "course_block" library_block = "library_block" + library_container = "library_container" collection = "collection" @@ -546,3 +547,58 @@ def searchable_doc_for_collection( doc['_disabled'] = True return doc + + +def searchable_doc_for_container( + container_key: LibraryContainerLocator, +) -> dict: + """ + Generate a dictionary document suitable for ingestion into a search engine + like Meilisearch or Elasticsearch, so that the given collection can be + found using faceted search. + + If no container is found for the given container key, the returned document + will contain only basic information derived from the container key, and no + Fields.type value will be included in the returned dict. + """ + doc = { + Fields.id: meili_id_from_opaque_key(container_key), + Fields.context_key: str(container_key.library_key), + Fields.org: str(container_key.org), + # In the future, this may be either course_container or library_container + Fields.type: DocType.library_container, + # To check if it is "unit", "section", "subsection", etc.. + Fields.block_type: container_key.container_type, + Fields.usage_key: str(container_key), # Field name isn't exact but this is the closest match + Fields.block_id: container_key.container_id, # Field name isn't exact but this is the closest match + Fields.access_id: _meili_access_id_from_context_key(container_key.library_key), + } + + try: + container = lib_api.get_container(container_key) + except lib_api.ContentLibraryCollectionNotFound: + # Container not found, so we can only return the base doc + pass + + if container: + # TODO: check if there's a more efficient way to load these num_children counts? + draft_num_children = len(lib_api.get_container_children(container_key, published=False)) + + doc.update({ + Fields.display_name: container.display_name, + Fields.created: container.created.timestamp(), + Fields.modified: container.modified.timestamp(), + Fields.num_children: draft_num_children, + }) + library = lib_api.get_library(container_key.library_key) + if library: + doc[Fields.breadcrumbs] = [{"display_name": library.title}] + + if container.published_version_num is not None: + published_num_children = len(lib_api.get_container_children(container_key, published=True)) + doc[Fields.published] = { + # Fields.published_display_name: container_published.title, TODO: set the published title + Fields.published_num_children: published_num_children, + } + + return doc diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 24add6748d7d..4565165e87ea 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -14,6 +14,7 @@ ContentObjectChangedData, LibraryBlockData, LibraryCollectionData, + LibraryContainerData, XBlockData, ) from openedx_events.content_authoring.signals import ( @@ -25,6 +26,9 @@ LIBRARY_COLLECTION_CREATED, LIBRARY_COLLECTION_DELETED, LIBRARY_COLLECTION_UPDATED, + LIBRARY_CONTAINER_CREATED, + LIBRARY_CONTAINER_DELETED, + LIBRARY_CONTAINER_UPDATED, XBLOCK_CREATED, XBLOCK_DELETED, XBLOCK_UPDATED, @@ -45,6 +49,7 @@ delete_xblock_index_doc, update_content_library_index_docs, update_library_collection_index_doc, + update_library_container_index_doc, upsert_library_block_index_doc, upsert_xblock_index_doc, ) @@ -225,3 +230,31 @@ def content_object_associations_changed_handler(**kwargs) -> None: upsert_block_tags_index_docs(usage_key) if not content_object.changes or "collections" in content_object.changes: upsert_block_collections_index_docs(usage_key) + + +@receiver(LIBRARY_CONTAINER_CREATED) +@receiver(LIBRARY_CONTAINER_DELETED) +@receiver(LIBRARY_CONTAINER_UPDATED) +@only_if_meilisearch_enabled +def library_container_updated_handler(**kwargs) -> None: + """ + Create or update the index for the content library container + """ + library_container = kwargs.get("library_container", None) + if not library_container or not isinstance(library_container, LibraryContainerData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + if library_container.background: + update_library_container_index_doc.delay( + str(library_container.library_key), + library_container.container_key, + ) + else: + # Update container 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_container_index_doc.apply(args=[ + str(library_container.library_key), + library_container.container_key, + ]) diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index 98390a12f3b3..f23ca9aa304e 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -11,7 +11,7 @@ from edx_django_utils.monitoring import set_code_owner_attribute from meilisearch.errors import MeilisearchError from opaque_keys.edx.keys import UsageKey -from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from . import api @@ -110,3 +110,17 @@ def update_library_components_collections(library_key_str: str, collection_key: log.info("Updating document.collections for library %s collection %s components", library_key, collection_key) api.update_library_components_collections(library_key, collection_key) + + +@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) +@set_code_owner_attribute +def update_library_container_index_doc(library_key_str: str, container_key_str: str) -> None: + """ + Celery task to update the content index document for a library container + """ + library_key = LibraryLocatorV2.from_string(library_key_str) + container_key = LibraryContainerLocator.from_string(container_key_str) + + log.info("Updating content index documents for container %s in library%s", container_key, library_key) + + api.upsert_library_container_index_doc(container_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 8063307d61e9..2decf2374fac 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -208,6 +208,35 @@ def setUp(self): "breadcrumbs": [{"display_name": "Library"}], } + # Create a unit: + with freeze_time(created_date): + self.unit = library_api.create_container( + library_key=self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit-1", + title="Unit 1", + user_id=None, + ) + self.unit_key = "lct:org1:lib:unit:unit-1" + self.unit_dict = { + "id": "lctorg1libunitunit-1-e4527f7c", + "block_id": "unit-1", + "block_type": "unit", + "usage_key": self.unit_key, + "type": "library_container", + "display_name": "Unit 1", + # description is not set for containers + "num_children": 0, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + @override_settings(MEILISEARCH_ENABLED=False) def test_reindex_meilisearch_disabled(self, mock_meilisearch): with self.assertRaises(RuntimeError): @@ -231,14 +260,16 @@ def test_reindex_meilisearch(self, mock_meilisearch): doc_problem2["collections"] = {'display_name': [], 'key': []} doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} + doc_unit = copy.deepcopy(self.unit_dict) api.rebuild_index() - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 3 + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), call([doc_collection]), + call([doc_unit]), ], any_order=True, ) @@ -259,14 +290,16 @@ def test_reindex_meilisearch_incremental(self, mock_meilisearch): doc_problem2["collections"] = {"display_name": [], "key": []} doc_collection = copy.deepcopy(self.collection_dict) doc_collection["tags"] = {} + doc_unit = copy.deepcopy(self.unit_dict) api.rebuild_index(incremental=True) - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 3 + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), call([doc_collection]), + call([doc_unit]), ], any_order=True, ) @@ -280,13 +313,13 @@ def simulated_interruption(message): with pytest.raises(Exception, match="Simulated interruption"): api.rebuild_index(simulated_interruption, incremental=True) - # two more calls due to collections - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 5 + # three more calls due to collections and containers + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 7 assert IncrementalIndexCompleted.objects.all().count() == 1 api.rebuild_index(incremental=True) assert IncrementalIndexCompleted.objects.all().count() == 0 # one missing course indexed - assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 6 + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 8 @override_settings(MEILISEARCH_ENABLED=True) def test_reset_meilisearch_index(self, mock_meilisearch): @@ -340,6 +373,22 @@ def test_reindex_meilisearch_collection_error(self, mock_meilisearch): f"Error indexing collection {self.collection}: Failed to generate document" ) + @override_settings(MEILISEARCH_ENABLED=True) + @patch( + "openedx.core.djangoapps.content.search.api.searchable_doc_for_container", + Mock(side_effect=Exception("Failed to generate document")), + ) + def test_reindex_meilisearch_container_error(self, mock_meilisearch): + + mock_logger = Mock() + api.rebuild_index(mock_logger) + assert call( + [self.unit_dict] + ) not in mock_meilisearch.return_value.index.return_value.add_documents.mock_calls + mock_logger.assert_any_call( + "Error indexing container unit-1: Failed to generate document" + ) + @override_settings(MEILISEARCH_ENABLED=True) def test_reindex_meilisearch_library_block_error(self, mock_meilisearch): diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 3bab2795b9f5..38b1d607ab05 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -22,6 +22,7 @@ searchable_doc_tags_for_collection, searchable_doc_collections, searchable_doc_for_collection, + searchable_doc_for_container, searchable_doc_for_library_block, ) from ..models import SearchAccess @@ -30,6 +31,7 @@ searchable_doc_tags = lambda x: x searchable_doc_tags_for_collection = lambda x: x searchable_doc_for_collection = lambda x: x + searchable_doc_for_container = lambda x: x searchable_doc_for_library_block = lambda x: x SearchAccess = {} @@ -494,6 +496,41 @@ def test_collection_with_published_library(self): } } + def test_draft_container(self): + """ + Test creating a search document for a draft-only container + """ + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + container_meta = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Unit, + slug="unit1", + title="A Unit in the Search Index", + user_id=None, + ) + + doc = searchable_doc_for_container(container_meta.container_key) + + assert doc == { + "id": "lctedx2012_fallunitunit1-edd13a0c", + "block_id": "unit1", + "block_type": "unit", + "usage_key": "lct:edX:2012_Fall:unit:unit1", + "type": "library_container", + "org": "edX", + "display_name": "A Unit in the Search Index", + # description is not set for containers + "num_children": 0, + "context_key": "lib:edX:2012_Fall", + "access_id": self.library_access_id, + "breadcrumbs": [{"display_name": "some content_library"}], + "created": 1680674828.0, + "modified": 1680674828.0, + # "tags" should be here but we haven't implemented them yet + # "published" is not set since we haven't published it yet + } + def test_mathjax_plain_text_conversion_for_search(self): """ Test how an HTML block with mathjax equations gets converted to plain text in search description. diff --git a/openedx/core/djangoapps/content_libraries/api/__init__.py b/openedx/core/djangoapps/content_libraries/api/__init__.py new file mode 100644 index 000000000000..d4d9fe047fb9 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/__init__.py @@ -0,0 +1,7 @@ +""" +Python API for working with content libraries +""" +from .containers import * +from .libraries import * +from .blocks import * +from . import permissions diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py new file mode 100644 index 000000000000..383a8d8fbd07 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -0,0 +1,30 @@ +""" +Content libraries API methods related to XBlocks/Components. + +These methods don't enforce permissions (only the REST APIs do). +""" +# pylint: disable=unused-import + +# TODO: move all the API methods related to blocks and assets in here from 'libraries.py' +# TODO: use __all__ to limit what symbols are public. + +from .libraries import ( + LibraryXBlockMetadata, + LibraryXBlockStaticFile, + LibraryXBlockType, + get_library_components, + get_library_block, + set_library_block_olx, + library_component_usage_key, + get_component_from_usage_key, + validate_can_add_block_to_library, + create_library_block, + import_staged_content_from_user_clipboard, + get_or_create_olx_media_type, + delete_library_block, + restore_library_block, + get_library_block_static_asset_files, + add_library_block_static_asset_file, + delete_library_block_static_asset_file, + publish_component_changes, +) diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py new file mode 100644 index 000000000000..5b24b6540b10 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -0,0 +1,265 @@ +""" +API for containers (Sections, Subsections, Units) in Content Libraries +""" +from __future__ import annotations +from dataclasses import dataclass +from datetime import datetime +from enum import Enum +from uuid import uuid4 + +from django.utils.text import slugify +from opaque_keys.edx.locator import ( + LibraryLocatorV2, + LibraryContainerLocator, +) + +from openedx_events.content_authoring.data import LibraryContainerData +from openedx_events.content_authoring.signals import ( + LIBRARY_CONTAINER_CREATED, + LIBRARY_CONTAINER_DELETED, + LIBRARY_CONTAINER_UPDATED, +) +from openedx_learning.api import authoring as authoring_api +from openedx_learning.api.authoring_models import Container + +from ..models import ContentLibrary +from .libraries import PublishableItem + + +# The public API is only the following symbols: +__all__ = [ + "ContentLibraryContainerNotFound", + "ContainerMetadata", + "ContainerType", + "get_container", + "create_container", + "get_container_children", + "library_container_locator", + "update_container", + "delete_container", +] + + +ContentLibraryContainerNotFound = Container.DoesNotExist + + +class ContainerType(Enum): + Unit = "unit" + + +@dataclass(frozen=True, kw_only=True) +class ContainerMetadata(PublishableItem): + """ + Class that represents the metadata about a Container (e.g. Unit) in a content library. + """ + container_key: LibraryContainerLocator + container_type: ContainerType + + @classmethod + def from_container(cls, library_key, container: Container, associated_collections=None): + """ + Construct a ContainerMetadata object from a Container object. + """ + last_publish_log = container.versioning.last_publish_log + container_key = library_container_locator( + library_key, + container=container, + ) + container_type = ContainerType(container_key.container_type) + + published_by = "" + if last_publish_log and last_publish_log.published_by: + published_by = last_publish_log.published_by.username + + draft = container.versioning.draft + published = container.versioning.published + last_draft_created = draft.created if draft else None + if draft and draft.publishable_entity_version.created_by: + last_draft_created_by = draft.publishable_entity_version.created_by.username + else: + last_draft_created_by = "" + + return cls( + container_key=container_key, + container_type=container_type, + display_name=draft.title, + created=container.created, + modified=draft.created, + draft_version_num=draft.version_num, + published_version_num=published.version_num if published else None, + last_published=None if last_publish_log is None else last_publish_log.published_at, + published_by=published_by, + last_draft_created=last_draft_created, + last_draft_created_by=last_draft_created_by, + has_unpublished_changes=authoring_api.contains_unpublished_changes(container.pk), + collections=associated_collections or [], + ) + + +def library_container_locator( + library_key: LibraryLocatorV2, + container: Container, +) -> LibraryContainerLocator: + """ + Returns a LibraryContainerLocator for the given library + container. + + Currently only supports Unit-type containers; will support other container types in future. + """ + assert container.unit is not None + container_type = ContainerType.Unit + + return LibraryContainerLocator( + library_key, + container_type=container_type.value, + container_id=container.publishable_entity.key, + ) + + +def _get_container(container_key: LibraryContainerLocator) -> Container: + """ + Internal method to fetch the Container object from its LibraryContainerLocator + + Raises ContentLibraryContainerNotFound if no container found, or if the container has been soft deleted. + """ + assert isinstance(container_key, LibraryContainerLocator) + content_library = ContentLibrary.objects.get_by_key(container_key.library_key) + learning_package = content_library.learning_package + assert learning_package is not None + container = authoring_api.get_container_by_key( + learning_package.id, + key=container_key.container_id, + ) + if container and container.versioning.draft: + return container + raise ContentLibraryContainerNotFound + + +def get_container(container_key: LibraryContainerLocator) -> ContainerMetadata: + """ + Get a container (a Section, Subsection, or Unit). + """ + container = _get_container(container_key) + container_meta = ContainerMetadata.from_container(container_key.library_key, container) + assert container_meta.container_type.value == container_key.container_type + return container_meta + + +def create_container( + library_key: LibraryLocatorV2, + container_type: ContainerType, + slug: str | None, + title: str, + user_id: int | None, +) -> ContainerMetadata: + """ + Create a container (e.g. a Unit) in the specified content library. + + It will initially be empty. + """ + assert isinstance(library_key, LibraryLocatorV2) + content_library = ContentLibrary.objects.get_by_key(library_key) + assert content_library.learning_package_id # Should never happen but we made this a nullable field so need to check + if slug is None: + # Automatically generate a slug. Append a random suffix so it should be unique. + slug = slugify(title, allow_unicode=True) + '-' + uuid4().hex[-6:] + # Make sure the slug is valid by first creating a key for the new container: + container_key = LibraryContainerLocator( + library_key=library_key, + container_type=container_type.value, + container_id=slug, + ) + # Then try creating the actual container: + match container_type: + case ContainerType.Unit: + container, _initial_version = authoring_api.create_unit_and_version( + content_library.learning_package_id, + key=slug, + title=title, + created=datetime.now(), + created_by=user_id, + ) + case _: + raise ValueError(f"Invalid container type: {container_type}") + + LIBRARY_CONTAINER_CREATED.send_event( + library_container=LibraryContainerData( + library_key=library_key, + container_key=str(container_key), + ) + ) + + return ContainerMetadata.from_container(library_key, container) + + +def update_container( + container_key: LibraryContainerLocator, + display_name: str, + user_id: int | None, +) -> ContainerMetadata: + """ + Update a container (e.g. a Unit) title. + """ + container = _get_container(container_key) + library_key = container_key.library_key + + assert container.unit + unit_version = authoring_api.create_next_unit_version( + container.unit, + title=display_name, + created=datetime.now(), + created_by=user_id, + ) + + LIBRARY_CONTAINER_UPDATED.send_event( + library_container=LibraryContainerData( + library_key=library_key, + container_key=str(container_key), + ) + ) + + return ContainerMetadata.from_container(library_key, unit_version.container) + + +def delete_container( + container_key: LibraryContainerLocator, +) -> None: + """ + Delete a container (e.g. a Unit) (soft delete). + + No-op if container doesn't exist or has already been soft-deleted. + """ + try: + container = _get_container(container_key) + except ContentLibraryContainerNotFound: + return + + authoring_api.soft_delete_draft(container.pk) + + LIBRARY_CONTAINER_DELETED.send_event( + library_container=LibraryContainerData( + library_key=container_key.library_key, + container_key=str(container_key), + ) + ) + + # TODO: trigger a LIBRARY_COLLECTION_UPDATED for each collection the container was in + + +def get_container_children( + container_key: LibraryContainerLocator, + published=False, +) -> list[authoring_api.ContainerEntityListEntry]: + """ + Get the entities contained in the given container (e.g. the components/xblocks in a unit) + """ + assert isinstance(container_key, LibraryContainerLocator) + content_library = ContentLibrary.objects.get_by_key(container_key.library_key) + learning_package = content_library.learning_package + assert learning_package is not None + container = authoring_api.get_container_by_key( + learning_package.id, + key=container_key.container_id, + ) + child_entities = authoring_api.get_entities_in_container(container, published=published) + # TODO: convert the return type to list[ContainerMetadata | LibraryXBlockMetadata] ? + return child_entities diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api/libraries.py similarity index 95% rename from openedx/core/djangoapps/content_libraries/api.py rename to openedx/core/djangoapps/content_libraries/api/libraries.py index 36ace3f84fb2..13d41921e860 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -62,7 +62,7 @@ import requests from django.conf import settings -from django.contrib.auth.models import AbstractUser, Group +from django.contrib.auth.models import AbstractUser, AnonymousUser, Group from django.core.exceptions import ObjectDoesNotExist, PermissionDenied from django.core.validators import validate_unicode_slug from django.db import IntegrityError, transaction @@ -116,12 +116,60 @@ from openedx.core.types import User as UserType from xmodule.modulestore.django import modulestore -from . import permissions, tasks -from .constants import ALL_RIGHTS_RESERVED -from .models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask +from .. import permissions, tasks +from ..constants import ALL_RIGHTS_RESERVED +from ..models import ContentLibrary, ContentLibraryPermission, ContentLibraryBlockImportTask log = logging.getLogger(__name__) +# The public API is only the following symbols: +__all__ = [ + # Exceptions - maybe move them to a new file? + "ContentLibraryNotFound", + "ContentLibraryCollectionNotFound", + "ContentLibraryBlockNotFound", + "LibraryAlreadyExists", + "LibraryCollectionAlreadyExists", + "LibraryBlockAlreadyExists", + "BlockLimitReachedError", + "IncompatibleTypesError", + "InvalidNameError", + "LibraryPermissionIntegrityError", + # Library Models + "ContentLibrary", # Should this be public or not? + "ContentLibraryMetadata", + "AccessLevel", + "ContentLibraryPermissionEntry", + "CollectionMetadata", + # Library API methods + "user_can_create_library", + "get_libraries_for_user", + "get_metadata", + "require_permission_for_library_key", + "get_library", + "create_library", + "get_library_team", + "get_library_user_permissions", + "set_library_user_permissions", + "set_library_group_permissions", + "update_library", + "delete_library", + "get_allowed_block_types", + "publish_changes", + "revert_changes", + # Collections - TODO: move to a new file + "create_library_collection", + "update_library_collection", + "update_library_collection_components", + "set_library_component_collections", + "get_library_collection_usage_key", + "get_library_collection_from_usage_key", + # Import - TODO: move to a new file + "EdxModulestoreImportClient", + "EdxApiImportClient", + "import_blocks_create_task", +] + # Exceptions # ========== @@ -229,18 +277,25 @@ class CollectionMetadata: @dataclass(frozen=True) -class LibraryXBlockMetadata: +class LibraryItem: """ - Class that represents the metadata about an XBlock in a content library. + Common fields for anything that can be found in a content library. """ - usage_key: LibraryUsageLocatorV2 created: datetime modified: datetime + display_name: str + + +@dataclass(frozen=True, kw_only=True) +class PublishableItem(LibraryItem): + """ + Common fields for anything that can be found in a content library that has + draft/publish support. + """ draft_version_num: int published_version_num: int | None = None - display_name: str = "" last_published: datetime | None = None - # THe username of the user who last published this. + # The username of the user who last published this. published_by: str = "" last_draft_created: datetime | None = None # The username of the user who created the last draft. @@ -248,6 +303,14 @@ class LibraryXBlockMetadata: has_unpublished_changes: bool = False collections: list[CollectionMetadata] = field(default_factory=list) + +@dataclass(frozen=True, kw_only=True) +class LibraryXBlockMetadata(PublishableItem): + """ + Class that represents the metadata about an XBlock in a content library. + """ + usage_key: LibraryUsageLocatorV2 + @classmethod def from_component(cls, library_key, component, associated_collections=None): """ @@ -416,6 +479,7 @@ def get_library(library_key: LibraryLocatorV2) -> ContentLibraryMetadata: """ ref = ContentLibrary.objects.get_by_key(library_key) learning_package = ref.learning_package + assert learning_package is not None # Shouldn't happen - this is just for the type checker num_blocks = authoring_api.get_all_drafts(learning_package.id).count() last_publish_log = authoring_api.get_last_publish(learning_package.id) last_draft_log = authoring_api.get_entities_with_unpublished_changes(learning_package.id) \ @@ -455,7 +519,7 @@ def get_library(library_key: LibraryLocatorV2) -> ContentLibraryMetadata: return ContentLibraryMetadata( key=library_key, title=learning_package.title, - description=ref.learning_package.description, + description=learning_package.description, num_blocks=num_blocks, version=version, last_published=None if last_publish_log is None else last_publish_log.published_at, @@ -557,6 +621,8 @@ def get_library_user_permissions(library_key: LibraryLocatorV2, user: UserType) Fetch the specified user's access information. Will return None if no permissions have been granted. """ + if isinstance(user, AnonymousUser): + return None # Mostly here for the type checker ref = ContentLibrary.objects.get_by_key(library_key) grant = ref.permission_grants.filter(user=user).first() if grant is None: @@ -574,6 +640,8 @@ def set_library_user_permissions(library_key: LibraryLocatorV2, user: UserType, access_level should be one of the AccessLevel values defined above. """ + if isinstance(user, AnonymousUser): + raise TypeError("Invalid user type") # Mostly here for the type checker ref = ContentLibrary.objects.get_by_key(library_key) current_grant = get_library_user_permissions(library_key, user) if current_grant and current_grant.access_level == AccessLevel.ADMIN_LEVEL: @@ -633,6 +701,8 @@ def update_library( return content_lib = ContentLibrary.objects.get_by_key(library_key) + learning_package_id = content_lib.learning_package_id + assert learning_package_id is not None with transaction.atomic(): # We need to make updates to both the ContentLibrary and its linked @@ -643,12 +713,12 @@ def update_library( if allow_public_read is not None: content_lib.allow_public_read = allow_public_read if library_license is not None: - content_lib.library_license = library_license + content_lib.license = library_license content_lib.save() if learning_pkg_changed: authoring_api.update_learning_package( - content_lib.learning_package_id, + learning_package_id, title=title, description=description, ) @@ -675,7 +745,8 @@ def delete_library(library_key: LibraryLocatorV2) -> None: # TODO: We should eventually detach the LearningPackage and delete it # asynchronously, especially if we need to delete a bunch of stuff # on the filesystem for it. - learning_package.delete() + if learning_package: + learning_package.delete() CONTENT_LIBRARY_DELETED.send_event( content_library=ContentLibraryData( @@ -709,6 +780,7 @@ def get_library_components( """ lib = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] learning_package = lib.learning_package + assert learning_package is not None components = authoring_api.get_components( learning_package.id, draft=True, @@ -860,7 +932,8 @@ def validate_can_add_block_to_library( content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] # If adding a component would take us over our max, return an error. - component_count = authoring_api.get_all_drafts(content_library.learning_package.id).count() + assert content_library.learning_package_id is not None + component_count = authoring_api.get_all_drafts(content_library.learning_package_id).count() if component_count + 1 > settings.MAX_BLOCKS_PER_CONTENT_LIBRARY: raise BlockLimitReachedError( _("Library cannot have more than {} Components").format( @@ -1356,7 +1429,7 @@ def publish_changes(library_key: LibraryLocatorV2, user_id: int | None = None): Publish all pending changes to the specified library. """ learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package - + assert learning_package is not None # shouldn't happen but it's technically possible. authoring_api.publish_all_drafts(learning_package.id, published_by=user_id) CONTENT_LIBRARY_UPDATED.send_event( @@ -1398,6 +1471,7 @@ def revert_changes(library_key: LibraryLocatorV2) -> None: last published version. """ learning_package = ContentLibrary.objects.get_by_key(library_key).learning_package + assert learning_package is not None # shouldn't happen but it's technically possible. authoring_api.reset_drafts_to_published(learning_package.id) CONTENT_LIBRARY_UPDATED.send_event( @@ -1652,6 +1726,7 @@ def get_library_collection_from_usage_key( library_key = collection_usage_key.library_key collection_key = collection_usage_key.collection_id content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] + assert content_library.learning_package_id is not None # shouldn't happen but it's technically possible. try: return authoring_api.get_collection( content_library.learning_package_id, diff --git a/openedx/core/djangoapps/content_libraries/api/permissions.py b/openedx/core/djangoapps/content_libraries/api/permissions.py new file mode 100644 index 000000000000..6064b80d6f9e --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/api/permissions.py @@ -0,0 +1,14 @@ +""" +Public permissions that are part of the content libraries API +""" +# pylint: disable=unused-import + +from ..permissions import ( + CAN_CREATE_CONTENT_LIBRARY, + CAN_DELETE_THIS_CONTENT_LIBRARY, + CAN_EDIT_THIS_CONTENT_LIBRARY, + CAN_EDIT_THIS_CONTENT_LIBRARY_TEAM, + CAN_LEARN_FROM_THIS_CONTENT_LIBRARY, + CAN_VIEW_THIS_CONTENT_LIBRARY, + CAN_VIEW_THIS_CONTENT_LIBRARY_TEAM +) diff --git a/openedx/core/djangoapps/content_libraries/models.py b/openedx/core/djangoapps/content_libraries/models.py index 61e28b944851..415821145605 100644 --- a/openedx/core/djangoapps/content_libraries/models.py +++ b/openedx/core/djangoapps/content_libraries/models.py @@ -36,6 +36,7 @@ import contextlib import logging +from typing import ClassVar import uuid from django.contrib.auth import get_user_model @@ -67,11 +68,11 @@ User = get_user_model() -class ContentLibraryManager(models.Manager): +class ContentLibraryManager(models.Manager["ContentLibrary"]): """ Custom manager for ContentLibrary class. """ - def get_by_key(self, library_key): + def get_by_key(self, library_key) -> "ContentLibrary": """ Get the ContentLibrary for the given LibraryLocatorV2 key. """ @@ -92,7 +93,7 @@ class ContentLibrary(models.Model): .. no_pii: """ - objects: ContentLibraryManager[ContentLibrary] = ContentLibraryManager() + objects: ClassVar[ContentLibraryManager] = ContentLibraryManager() id = models.AutoField(primary_key=True) # Every Library is uniquely and permanently identified by an 'org' and a diff --git a/openedx/core/djangoapps/content_libraries/rest_api/__init__.py b/openedx/core/djangoapps/content_libraries/rest_api/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py new file mode 100644 index 000000000000..6c24c35394b9 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -0,0 +1,19 @@ +""" +Content Library REST APIs related to XBlocks/Components and their static assets +""" +# pylint: disable=unused-import + +# TODO: move the block and block asset related views from 'libraries' into this file +from .libraries import ( + LibraryBlockAssetListView, + LibraryBlockAssetView, + LibraryBlockCollectionsView, + LibraryBlockLtiUrlView, + LibraryBlockOlxView, + LibraryBlockPublishView, + LibraryBlockRestore, + LibraryBlocksView, + LibraryBlockView, + LibraryComponentAssetView, + LibraryComponentDraftAssetView, +) diff --git a/openedx/core/djangoapps/content_libraries/views_collections.py b/openedx/core/djangoapps/content_libraries/rest_api/collections.py similarity index 96% rename from openedx/core/djangoapps/content_libraries/views_collections.py rename to openedx/core/djangoapps/content_libraries/rest_api/collections.py index 21c4b12dd3da..f1b63b2c1845 100644 --- a/openedx/core/djangoapps/content_libraries/views_collections.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/collections.py @@ -1,7 +1,6 @@ """ Collections API Views """ - from __future__ import annotations from django.db.models import QuerySet @@ -17,10 +16,10 @@ from openedx_learning.api import authoring as authoring_api from openedx_learning.api.authoring_models import Collection -from openedx.core.djangoapps.content_libraries import api, permissions -from openedx.core.djangoapps.content_libraries.models import ContentLibrary -from openedx.core.djangoapps.content_libraries.views import convert_exceptions -from openedx.core.djangoapps.content_libraries.serializers import ( +from .. import api, permissions +from ..models import ContentLibrary +from .utils import convert_exceptions +from .serializers import ( ContentLibraryCollectionSerializer, ContentLibraryCollectionComponentsUpdateSerializer, ContentLibraryCollectionUpdateSerializer, diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py new file mode 100644 index 000000000000..ad23a51d55b3 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -0,0 +1,126 @@ +""" +REST API views for containers (sections, subsections, units) in content libraries +""" +from __future__ import annotations + +import logging + +from django.contrib.auth import get_user_model +from django.db.transaction import non_atomic_requests +from django.utils.decorators import method_decorator +from django.utils.translation import gettext as _ +from drf_yasg.utils import swagger_auto_schema + +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator +from rest_framework.generics import GenericAPIView +from rest_framework.response import Response +from rest_framework.status import HTTP_204_NO_CONTENT + +from openedx.core.djangoapps.content_libraries import api, permissions +from openedx.core.lib.api.view_utils import view_auth_classes +from . import serializers +from .utils import convert_exceptions + +User = get_user_model() +log = logging.getLogger(__name__) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainersView(GenericAPIView): + """ + Views to work with Containers in a specific content library. + """ + serializer_class = serializers.LibraryContainerMetadataSerializer + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.LibraryContainerMetadataSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def post(self, request, lib_key_str): + """ + Create a new Container in this content library + """ + library_key = LibraryLocatorV2.from_string(lib_key_str) + api.require_permission_for_library_key(library_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY) + serializer = serializers.LibraryContainerMetadataSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + container_type = serializer.validated_data['container_type'] + container = api.create_container( + library_key, + container_type, + title=serializer.validated_data['display_name'], + slug=serializer.validated_data.get('slug'), + user_id=request.user.id, + ) + + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + +@method_decorator(non_atomic_requests, name="dispatch") +@view_auth_classes() +class LibraryContainerView(GenericAPIView): + """ + View to retrieve or update data about a specific container (a section, subsection, or unit) + """ + serializer_class = serializers.LibraryContainerMetadataSerializer + + @convert_exceptions + @swagger_auto_schema( + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def get(self, request, container_key: LibraryContainerLocator): + """ + Get information about a container + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, + ) + container = api.get_container(container_key) + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + @convert_exceptions + @swagger_auto_schema( + request_body=serializers.LibraryContainerUpdateSerializer, + responses={200: serializers.LibraryContainerMetadataSerializer} + ) + def patch(self, request, container_key: LibraryContainerLocator): + """ + Update a Container. + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + serializer = serializers.LibraryContainerUpdateSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + + container = api.update_container( + container_key, + display_name=serializer.validated_data['display_name'], + user_id=request.user.id, + ) + + return Response(serializers.LibraryContainerMetadataSerializer(container).data) + + @convert_exceptions + def delete(self, request, container_key: LibraryContainerLocator): + """ + Delete a Container (soft delete). + """ + api.require_permission_for_library_key( + container_key.library_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, + ) + + api.delete_container( + container_key, + ) + + return Response({}, status=HTTP_204_NO_CONTENT) diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py similarity index 96% rename from openedx/core/djangoapps/content_libraries/views.py rename to openedx/core/djangoapps/content_libraries/rest_api/libraries.py index a1b7a2602450..5e370dc40e33 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/libraries.py @@ -62,8 +62,6 @@ to Learning Core) atomic: https://github.com/openedx/edx-platform/pull/30456 """ - -from functools import wraps import itertools import json import logging @@ -86,7 +84,6 @@ from pylti1p3.exception import LtiException, OIDCException import edx_api_doc_tools as apidocs -from opaque_keys import InvalidKeyError from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_learning.api import authoring from organizations.api import ensure_organization @@ -105,7 +102,7 @@ user_can_create_organizations, ) from openedx.core.djangoapps.content_libraries import api, permissions -from openedx.core.djangoapps.content_libraries.serializers import ( +from openedx.core.djangoapps.content_libraries.rest_api.serializers import ( ContentLibraryBlockImportTaskCreateSerializer, ContentLibraryBlockImportTaskSerializer, ContentLibraryFilterSerializer, @@ -129,50 +126,14 @@ from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.types.http import RestRequest -from .models import ContentLibrary, LtiGradedResource, LtiProfile +from .utils import convert_exceptions +from ..models import ContentLibrary, LtiGradedResource, LtiProfile User = get_user_model() log = logging.getLogger(__name__) -def convert_exceptions(fn): - """ - Catch any Content Library API exceptions that occur and convert them to - DRF exceptions so DRF will return an appropriate HTTP response - """ - - @wraps(fn) - def wrapped_fn(*args, **kwargs): - try: - return fn(*args, **kwargs) - except InvalidKeyError as exc: - log.exception(str(exc)) - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryNotFound: - log.exception("Content library not found") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryBlockNotFound: - log.exception("XBlock not found in content library") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.ContentLibraryCollectionNotFound: - log.exception("Collection not found in content library") - raise NotFound # lint-amnesty, pylint: disable=raise-missing-from - except api.LibraryCollectionAlreadyExists as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.LibraryBlockAlreadyExists as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.InvalidNameError as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - except api.BlockLimitReachedError as exc: - log.exception(str(exc)) - raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from - return wrapped_fn - - class LibraryApiPaginationDocs: """ API docs for query params related to paginating ContentLibraryMetadata objects. diff --git a/openedx/core/djangoapps/content_libraries/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py similarity index 83% rename from openedx/core/djangoapps/content_libraries/serializers.py rename to openedx/core/djangoapps/content_libraries/rest_api/serializers.py index c2a26220d4c7..67b8ec00e56f 100644 --- a/openedx/core/djangoapps/content_libraries/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -10,6 +10,7 @@ from opaque_keys import InvalidKeyError from openedx_learning.api.authoring_models import Collection +from openedx.core.djangoapps.content_libraries.api.containers import ContainerMetadata, ContainerType from openedx.core.djangoapps.content_libraries.constants import ( ALL_RIGHTS_RESERVED, LICENSE_OPTIONS, @@ -19,7 +20,7 @@ ContentLibrary ) from openedx.core.lib.api.serializers import CourseKeyField -from . import permissions +from .. import permissions DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%SZ' @@ -230,6 +231,52 @@ class LibraryXBlockStaticFilesSerializer(serializers.Serializer): files = LibraryXBlockStaticFileSerializer(many=True) +class LibraryContainerMetadataSerializer(serializers.Serializer): + """ + Serializer for Containers like Sections, Subsections, Units + + Converts from ContainerMetadata to JSON-compatible data + """ + container_key = serializers.CharField(read_only=True) + container_type = serializers.CharField() + display_name = serializers.CharField() + last_published = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + published_by = serializers.CharField(read_only=True) + last_draft_created = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + last_draft_created_by = serializers.CharField(read_only=True) + has_unpublished_changes = serializers.BooleanField(read_only=True) + created = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + modified = serializers.DateTimeField(format=DATETIME_FORMAT, read_only=True) + tags_count = serializers.IntegerField(read_only=True) + collections = CollectionMetadataSerializer(many=True, required=False, read_only=True) + + # When creating a new container in a library, the slug becomes the ID part of + # the definition key and usage key: + slug = serializers.CharField(write_only=True, required=False) + + def to_representation(self, instance: ContainerMetadata): + """ Convert to JSON-serializable data types """ + data = super().to_representation(instance) + data["container_type"] = instance.container_type.value # Force to a string, not an enum value instance + return data + + def to_internal_value(self, data): + """ + Convert JSON-ish data back to native python types. + Returns a dictionary, not a ContainerMetadata instance. + """ + result = super().to_internal_value(data) + result["container_type"] = ContainerType(data["container_type"]) + return result + + +class LibraryContainerUpdateSerializer(serializers.Serializer): + """ + Serializer for updating metadata for Containers like Sections, Subsections, Units + """ + display_name = serializers.CharField() + + class ContentLibraryBlockImportTaskSerializer(serializers.ModelSerializer): """ Serializer for a Content Library block import task. diff --git a/openedx/core/djangoapps/content_libraries/rest_api/url_converters.py b/openedx/core/djangoapps/content_libraries/rest_api/url_converters.py new file mode 100644 index 000000000000..c8c4b1c6ebda --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/url_converters.py @@ -0,0 +1,23 @@ +""" +URL pattern converters +https://docs.djangoproject.com/en/5.1/topics/http/urls/#registering-custom-path-converters +""" +from opaque_keys import InvalidKeyError +from opaque_keys.edx.locator import LibraryContainerLocator + + +class LibraryContainerLocatorConverter: + """ + Converter that matches library container IDs like: + lct:CL-TEST:containers:unit:u1 + """ + regex = r'[\w-]+(:[\w\-.]+)+' + + def to_python(self, value: str) -> LibraryContainerLocator: + try: + return LibraryContainerLocator.from_string(value) + except InvalidKeyError as exc: + raise ValueError from exc + + def to_url(self, value: LibraryContainerLocator) -> str: + return str(value) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/utils.py b/openedx/core/djangoapps/content_libraries/rest_api/utils.py new file mode 100644 index 000000000000..99825fbe75f8 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/rest_api/utils.py @@ -0,0 +1,52 @@ +""" +REST API utilities for content libraries +""" +from functools import wraps +import logging + +from opaque_keys import InvalidKeyError +from rest_framework.exceptions import NotFound, ValidationError + +from .. import api + +log = logging.getLogger(__name__) + + +def convert_exceptions(fn): + """ + Catch any Content Library API exceptions that occur and convert them to + DRF exceptions so DRF will return an appropriate HTTP response + """ + + @wraps(fn) + def wrapped_fn(*args, **kwargs): + try: + return fn(*args, **kwargs) + except InvalidKeyError as exc: + log.exception(str(exc)) + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryNotFound: + log.exception("Content library not found") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryBlockNotFound: + log.exception("XBlock not found in content library") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryCollectionNotFound: + log.exception("Collection not found in content library") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.ContentLibraryContainerNotFound: + log.exception("Container not found in content library") + raise NotFound # lint-amnesty, pylint: disable=raise-missing-from + except api.LibraryCollectionAlreadyExists as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + except api.LibraryBlockAlreadyExists as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + except api.InvalidNameError as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + except api.BlockLimitReachedError as exc: + log.exception(str(exc)) + raise ValidationError(str(exc)) # lint-amnesty, pylint: disable=raise-missing-from + return wrapped_fn diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 77de1c028aa7..d8030afae7f0 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -22,6 +22,7 @@ URL_LIB_LINKS = URL_LIB_DETAIL + 'links/' # Get the list of links in this library, or add a new one URL_LIB_COMMIT = URL_LIB_DETAIL + 'commit/' # Commit (POST) or revert (DELETE) all pending changes to this library URL_LIB_BLOCKS = URL_LIB_DETAIL + 'blocks/' # Get the list of XBlocks in this library, or add a new one +URL_LIB_CONTAINERS = URL_LIB_DETAIL + 'containers/' # Create a new container in this library URL_LIB_TEAM = URL_LIB_DETAIL + 'team/' # Get the list of users/groups authorized to use this library URL_LIB_TEAM_USER = URL_LIB_TEAM + 'user/{username}/' # Add/edit/remove a user's permission to use this library URL_LIB_TEAM_GROUP = URL_LIB_TEAM + 'group/{group_name}/' # Add/edit/remove a group's permission to use this library @@ -31,6 +32,7 @@ URL_LIB_BLOCK_OLX = URL_LIB_BLOCK + 'olx/' # Get or set the OLX of the specified XBlock URL_LIB_BLOCK_ASSETS = URL_LIB_BLOCK + 'assets/' # List the static asset files of the specified XBlock URL_LIB_BLOCK_ASSET_FILE = URL_LIB_BLOCK + 'assets/{file_name}' # Get, delete, or upload a specific static asset file +URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library URL_LIB_LTI_PREFIX = URL_PREFIX + 'lti/1.3/' URL_LIB_LTI_JWKS = URL_LIB_LTI_PREFIX + 'pub/jwks/' @@ -350,3 +352,23 @@ def _get_library_block_fields(self, block_key, version=None, expect_response=200 def _set_library_block_fields(self, block_key, new_fields, expect_response=200): """ Set the fields of a specific block in the library. This API is only used by the MFE editors. """ return self._api('post', URL_BLOCK_FIELDS_URL.format(block_key=block_key), new_fields, expect_response) + + def _create_container(self, lib_key, container_type, slug: str | None, display_name: str, expect_response=200): + """ Create a container (unit etc.) """ + data = {"container_type": container_type, "display_name": display_name} + if slug: + data["slug"] = slug + return self._api('post', URL_LIB_CONTAINERS.format(lib_key=lib_key), data, expect_response) + + def _get_container(self, container_key: str, expect_response=200): + """ Get a container (unit etc.) """ + return self._api('get', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response) + + def _update_container(self, container_key: str, display_name: str, expect_response=200): + """ Update a container (unit etc.) """ + data = {"display_name": display_name} + return self._api('patch', URL_LIB_CONTAINER.format(container_key=container_key), data, expect_response) + + def _delete_container(self, container_key: str, expect_response=204): + """ Delete a container (unit etc.) """ + return self._api('delete', URL_LIB_CONTAINER.format(container_key=container_key), None, expect_response) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 203cc7a9397a..d4be6fdf6bd9 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -65,11 +65,11 @@ def test_import_blocks_from_course_without_course(self): with self.assertRaises(ValueError): self.client.import_blocks_from_course('foobar', lambda *_: None) - @mock.patch('openedx.core.djangoapps.content_libraries.api.create_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block_static_asset_files') - @mock.patch('openedx.core.djangoapps.content_libraries.api.publish_changes') - @mock.patch('openedx.core.djangoapps.content_libraries.api.set_library_block_olx') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.create_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block_static_asset_files') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.publish_changes') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.set_library_block_olx') def test_import_blocks_from_course_on_block_with_olx( self, mock_set_library_block_olx, @@ -101,9 +101,9 @@ def test_import_blocks_from_course_on_block_with_olx( mock.ANY, 'fake-olx') mock_publish_changes.assert_called_once() - @mock.patch('openedx.core.djangoapps.content_libraries.api.create_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block_static_asset_files') - @mock.patch('openedx.core.djangoapps.content_libraries.api.set_library_block_olx') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.create_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block_static_asset_files') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.set_library_block_olx') def test_import_block_when_called_twice_same_block_but_different_course( self, mock_set_library_block_olx, @@ -138,7 +138,7 @@ def test_import_block_when_called_twice_same_block_but_different_course( mock_set_library_block_olx.assert_called_once() -@mock.patch('openedx.core.djangoapps.content_libraries.api.OAuthAPIClient') +@mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.OAuthAPIClient') class EdxApiImportClientTest(TestCase): """ Tests for EdxApiImportClient. @@ -195,11 +195,11 @@ def mock_oauth_client_response(self, mock_oauth_client, *, content=None, excepti return mock_response, mock_content return mock_response - @mock.patch('openedx.core.djangoapps.content_libraries.api.add_library_block_static_asset_file') - @mock.patch('openedx.core.djangoapps.content_libraries.api.create_library_block') - @mock.patch('openedx.core.djangoapps.content_libraries.api.get_library_block_static_asset_files') - @mock.patch('openedx.core.djangoapps.content_libraries.api.publish_changes') - @mock.patch('openedx.core.djangoapps.content_libraries.api.set_library_block_olx') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.add_library_block_static_asset_file') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.create_library_block') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.get_library_block_static_asset_files') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.publish_changes') + @mock.patch('openedx.core.djangoapps.content_libraries.api.libraries.set_library_block_olx') def test_import_block_when_url_is_from_studio( self, mock_set_library_block_olx, diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py new file mode 100644 index 000000000000..23a519899a85 --- /dev/null +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -0,0 +1,180 @@ +""" +Tests for Learning-Core-based Content Libraries +""" +from datetime import datetime, timezone + +import ddt +from freezegun import freeze_time +from unittest import mock + +from opaque_keys.edx.locator import LibraryLocatorV2 +from openedx_events.content_authoring.data import LibraryContainerData +from openedx_events.content_authoring.signals import ( + LIBRARY_CONTAINER_CREATED, + LIBRARY_CONTAINER_DELETED, + LIBRARY_CONTAINER_UPDATED, +) +from openedx_events.tests.utils import OpenEdxEventsTestMixin + +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest +from openedx.core.djangolib.testing.utils import skip_unless_cms + + +@skip_unless_cms +@ddt.ddt +class ContainersTestCase(OpenEdxEventsTestMixin, ContentLibrariesRestApiTest): + """ + Tests for containers (Sections, Subsections, Units) in Content Libraries. + + These tests use the REST API, which in turn relies on the Python API. + Some tests may use the python API directly if necessary to provide + coverage of any code paths not accessible via the REST API. + + In general, these tests should + (1) Use public APIs only - don't directly create data using other methods, + which results in a less realistic test and ties the test suite too + closely to specific implementation details. + (Exception: users can be provisioned using a user factory) + (2) Assert that fields are present in responses, but don't assert that the + entire response has some specific shape. That way, things like adding + new fields to an API response, which are backwards compatible, won't + break any tests, but backwards-incompatible API changes will. + """ + ENABLED_OPENEDX_EVENTS = [ + LIBRARY_CONTAINER_CREATED.event_type, + LIBRARY_CONTAINER_DELETED.event_type, + LIBRARY_CONTAINER_UPDATED.event_type, + ] + + def test_unit_crud(self): + """ + Test Create, Read, Update, and Delete of a Unit + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + lib_key = LibraryLocatorV2.from_string(lib["id"]) + + create_receiver = mock.Mock() + LIBRARY_CONTAINER_CREATED.connect(create_receiver) + + update_receiver = mock.Mock() + LIBRARY_CONTAINER_UPDATED.connect(update_receiver) + + delete_receiver = mock.Mock() + LIBRARY_CONTAINER_DELETED.connect(delete_receiver) + + # Create a unit: + create_date = datetime(2024, 9, 8, 7, 6, 5, tzinfo=timezone.utc) + with freeze_time(create_date): + container_data = self._create_container(lib["id"], "unit", slug="u1", display_name="Test Unit") + expected_data = { + "container_key": "lct:CL-TEST:containers:unit:u1", + "container_type": "unit", + "display_name": "Test Unit", + "last_published": None, + "published_by": "", + "last_draft_created": "2024-09-08T07:06:05Z", + "last_draft_created_by": 'Bob', + 'has_unpublished_changes': True, + 'created': '2024-09-08T07:06:05Z', + 'modified': '2024-09-08T07:06:05Z', + 'collections': [], + } + + self.assertDictContainsEntries(container_data, expected_data) + assert create_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_CREATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + create_receiver.call_args_list[0].kwargs, + ) + + # Fetch the unit: + unit_as_read = self._get_container(container_data["container_key"]) + # make sure it contains the same data when we read it back: + self.assertDictContainsEntries(unit_as_read, expected_data) + + # Update the unit: + modified_date = datetime(2024, 10, 9, 8, 7, 6, tzinfo=timezone.utc) + with freeze_time(modified_date): + container_data = self._update_container("lct:CL-TEST:containers:unit:u1", display_name="Unit ABC") + expected_data['last_draft_created'] = expected_data['modified'] = '2024-10-09T08:07:06Z' + expected_data['display_name'] = 'Unit ABC' + self.assertDictContainsEntries(container_data, expected_data) + + assert update_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_UPDATED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + update_receiver.call_args_list[0].kwargs, + ) + + # Re-fetch the unit + unit_as_re_read = self._get_container(container_data["container_key"]) + # make sure it contains the same data when we read it back: + self.assertDictContainsEntries(unit_as_re_read, expected_data) + + # Delete the unit + self._delete_container(container_data["container_key"]) + self._get_container(container_data["container_key"], expect_response=404) + assert delete_receiver.call_count == 1 + self.assertDictContainsSubset( + { + "signal": LIBRARY_CONTAINER_DELETED, + "sender": None, + "library_container": LibraryContainerData( + lib_key, + container_key="lct:CL-TEST:containers:unit:u1", + ), + }, + delete_receiver.call_args_list[0].kwargs, + ) + + def test_unit_permissions(self): + """ + Test that a regular user with read-only permissions on the library cannot create, update, or delete units. + """ + lib = self._create_library(slug="containers2", title="Container Test Library 2", description="Unit permissions") + container_data = self._create_container(lib["id"], "unit", slug="u2", display_name="Test Unit") + + random_user = UserFactory.create(username="Random", email="random@example.com") + with self.as_user(random_user): + self._create_container(lib["id"], "unit", slug="u3", display_name="Test Unit", expect_response=403) + self._get_container(container_data["container_key"], expect_response=403) + self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403) + self._delete_container(container_data["container_key"], expect_response=403) + + # Granting read-only permissions on the library should only allow retrieval, nothing else. + self._add_user_by_email(lib["id"], random_user.email, access_level="read") + with self.as_user(random_user): + self._create_container(lib["id"], "unit", slug="u2", display_name="Test Unit", expect_response=403) + self._get_container(container_data["container_key"], expect_response=200) + self._update_container(container_data["container_key"], display_name="Unit ABC", expect_response=403) + self._delete_container(container_data["container_key"], expect_response=403) + + def test_unit_gets_auto_slugs(self): + """ + Test that we can create units by specifying only a title, and they get + unique slugs assigned automatically. + """ + lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") + + # Create two units, specifying their titles but not their slugs/keys: + container1_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + container2_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + # Notice the container IDs below are slugified from the title: "alpha-bravo-NNNNN" + assert container1_data["container_key"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") + assert container2_data["container_key"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") + assert container1_data["container_key"] != container2_data["container_key"] diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 7295e3bab0b5..bcb1573a27c0 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -150,10 +150,10 @@ def test_library_org_validation(self): self._create_library(slug="existing-org-1", title="Library in an existing org", org="CL-TEST") @patch( - "openedx.core.djangoapps.content_libraries.views.user_can_create_organizations", + "openedx.core.djangoapps.content_libraries.rest_api.libraries.user_can_create_organizations", ) @patch( - "openedx.core.djangoapps.content_libraries.views.get_allowed_organizations_for_libraries", + "openedx.core.djangoapps.content_libraries.rest_api.libraries.get_allowed_organizations_for_libraries", ) @override_settings(ORGANIZATIONS_AUTOCREATE=False) def test_library_org_no_autocreate(self, mock_get_allowed_organizations, mock_can_create_organizations): @@ -198,7 +198,10 @@ def test_library_org_no_autocreate(self, mock_get_allowed_organizations, mock_ca assert mock_get_allowed_organizations.call_count == 2 @skip("This endpoint shouldn't support num_blocks and has_unpublished_*.") - @patch("openedx.core.djangoapps.content_libraries.views.LibraryRootView.pagination_class.page_size", new=2) + @patch( + "openedx.core.djangoapps.content_libraries.rest_api.libraries.LibraryRootView.pagination_class.page_size", + new=2, + ) def test_list_library(self): """ Test the /libraries API and its pagination @@ -496,7 +499,10 @@ def test_library_blocks_studio_view(self): assert 'resources' in fragment assert 'Hello world!' in fragment['content'] - @patch("openedx.core.djangoapps.content_libraries.views.LibraryBlocksView.pagination_class.page_size", new=2) + @patch( + "openedx.core.djangoapps.content_libraries.rest_api.libraries.LibraryBlocksView.pagination_class.page_size", + new=2, + ) def test_list_library_blocks(self): """ Test the /libraries/{lib_key_str}/blocks API and its pagination diff --git a/openedx/core/djangoapps/content_libraries/urls.py b/openedx/core/djangoapps/content_libraries/urls.py index 1272d79b3873..1656a8589616 100644 --- a/openedx/core/djangoapps/content_libraries/urls.py +++ b/openedx/core/djangoapps/content_libraries/urls.py @@ -2,26 +2,29 @@ URL configuration for Studio's Content Libraries REST API """ -from django.urls import include, path, re_path +from django.urls import include, path, re_path, register_converter from rest_framework import routers -from . import views -from . import views_collections +from .rest_api import blocks, collections, containers, libraries, url_converters # Django application name. app_name = 'openedx.core.djangoapps.content_libraries' +# URL converters + +register_converter(url_converters.LibraryContainerLocatorConverter, "lib_container_key") + # Router for importing blocks from courseware. import_blocks_router = routers.DefaultRouter() -import_blocks_router.register(r'tasks', views.LibraryImportTaskViewSet, basename='import-block-task') +import_blocks_router.register(r'tasks', libraries.LibraryImportTaskViewSet, basename='import-block-task') library_collections_router = routers.DefaultRouter() library_collections_router.register( - r'collections', views_collections.LibraryCollectionsView, basename="library-collections" + r'collections', collections.LibraryCollectionsView, basename="library-collections" ) # These URLs are only used in Studio. The LMS already provides all the @@ -31,61 +34,71 @@ urlpatterns = [ path('api/libraries/v2/', include([ # list of libraries / create a library: - path('', views.LibraryRootView.as_view()), + path('', libraries.LibraryRootView.as_view()), path('/', include([ # get data about a library, update a library, or delete a library: - path('', views.LibraryDetailsView.as_view()), + path('', libraries.LibraryDetailsView.as_view()), # Get the list of XBlock types that can be added to this library - path('block_types/', views.LibraryBlockTypesView.as_view()), + path('block_types/', libraries.LibraryBlockTypesView.as_view()), # Get the list of XBlocks in this library, or add a new one: - path('blocks/', views.LibraryBlocksView.as_view()), - # Commit (POST) or revert (DELETE) all pending changes to this library: - path('commit/', views.LibraryCommitView.as_view()), + path('blocks/', blocks.LibraryBlocksView.as_view()), + # Add a new container (unit etc.) to this library: + path('containers/', containers.LibraryContainersView.as_view()), + # Publish (POST) or revert (DELETE) all pending changes to this library: + path('commit/', libraries.LibraryCommitView.as_view()), # Get the list of users/groups who have permission to view/edit/administer this library: - path('team/', views.LibraryTeamView.as_view()), + path('team/', libraries.LibraryTeamView.as_view()), # Add/Edit (PUT) or remove (DELETE) a user's permission to use this library - path('team/user//', views.LibraryTeamUserView.as_view()), + path('team/user//', libraries.LibraryTeamUserView.as_view()), # Add/Edit (PUT) or remove (DELETE) a group's permission to use this library - path('team/group//', views.LibraryTeamGroupView.as_view()), + path('team/group//', libraries.LibraryTeamGroupView.as_view()), # Import blocks into this library. path('import_blocks/', include(import_blocks_router.urls)), # Paste contents of clipboard into library - path('paste_clipboard/', views.LibraryPasteClipboardView.as_view()), + path('paste_clipboard/', libraries.LibraryPasteClipboardView.as_view()), # Library Collections path('', include(library_collections_router.urls)), ])), path('blocks//', include([ # Get metadata about a specific XBlock in this library, or delete the block: - path('', views.LibraryBlockView.as_view()), - path('restore/', views.LibraryBlockRestore.as_view()), + path('', blocks.LibraryBlockView.as_view()), + # Restore a soft-deleted block + path('restore/', blocks.LibraryBlockRestore.as_view()), # Update collections for a given component - path('collections/', views.LibraryBlockCollectionsView.as_view(), name='update-collections'), + path('collections/', blocks.LibraryBlockCollectionsView.as_view(), name='update-collections'), # Get the LTI URL of a specific XBlock - path('lti/', views.LibraryBlockLtiUrlView.as_view(), name='lti-url'), + path('lti/', blocks.LibraryBlockLtiUrlView.as_view(), name='lti-url'), # Get the OLX source code of the specified block: - path('olx/', views.LibraryBlockOlxView.as_view()), + path('olx/', blocks.LibraryBlockOlxView.as_view()), # CRUD for static asset files associated with a block in the library: - path('assets/', views.LibraryBlockAssetListView.as_view()), - path('assets/', views.LibraryBlockAssetView.as_view()), - path('publish/', views.LibraryBlockPublishView.as_view()), + path('assets/', blocks.LibraryBlockAssetListView.as_view()), + path('assets/', blocks.LibraryBlockAssetView.as_view()), + path('publish/', blocks.LibraryBlockPublishView.as_view()), # Future: discard changes for just this one block - # Future: set a block's tags (tags are stored in a Tag bundle and linked in) + ])), + # Containers are Sections, Subsections, and Units + path('containers//', include([ + # Get metadata about a specific container in this library, update or delete the container: + path('', containers.LibraryContainerView.as_view()), + # Update collections for a given container + # path('collections/', views.LibraryContainerCollectionsView.as_view(), name='update-collections-ct'), + # path('publish/', views.LibraryContainerPublishView.as_view()), ])), re_path(r'^lti/1.3/', include([ - path('login/', views.LtiToolLoginView.as_view(), name='lti-login'), - path('launch/', views.LtiToolLaunchView.as_view(), name='lti-launch'), - path('pub/jwks/', views.LtiToolJwksView.as_view(), name='lti-pub-jwks'), + path('login/', libraries.LtiToolLoginView.as_view(), name='lti-login'), + path('launch/', libraries.LtiToolLaunchView.as_view(), name='lti-launch'), + path('pub/jwks/', libraries.LtiToolJwksView.as_view(), name='lti-pub-jwks'), ])), ])), path('library_assets/', include([ path( 'component_versions//', - views.LibraryComponentAssetView.as_view(), + blocks.LibraryComponentAssetView.as_view(), name='library-assets', ), path( 'blocks//', - views.LibraryComponentDraftAssetView.as_view(), + blocks.LibraryComponentDraftAssetView.as_view(), name='library-draft-assets', ), ]) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 96f6886b622b..9163ebd58aa0 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -12,7 +12,7 @@ import openedx_tagging.core.tagging.api as oel_tagging from django.db.models import Exists, OuterRef, Q, QuerySet from django.utils.timezone import now -from opaque_keys.edx.keys import CourseKey, LibraryCollectionKey +from opaque_keys.edx.keys import CourseKey, LibraryItemKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy from openedx_tagging.core.tagging.models.utils import TAGS_CSV_SEPARATOR @@ -230,7 +230,7 @@ def generate_csv_rows(object_id, buffer) -> Iterator[str]: """ content_key = get_content_key_from_string(object_id) - if isinstance(content_key, (UsageKey, LibraryCollectionKey)): + if isinstance(content_key, (UsageKey, LibraryItemKey)): raise ValueError("The object_id must be a CourseKey or a LibraryLocatorV2.") all_object_tags, taxonomies = get_all_object_tags(content_key) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index b693f7ee0f56..d85c87d62b17 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -5,8 +5,8 @@ import ddt from django.test.testcases import TestCase from fs.osfs import OSFS -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey -from opaque_keys.edx.locator import LibraryLocatorV2 +from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.locator import LibraryLocatorV2, LibraryCollectionLocator from openedx_tagging.core.tagging.models import ObjectTag from organizations.models import Organization from .test_objecttag_export_helpers import TestGetAllObjectTagsMixin, TaggedCourseMixin @@ -381,7 +381,7 @@ def test_copy_cross_org_tags(self): self._test_copy_object_tags(src_key, dst_key, expected_tags) def test_tag_collection(self): - collection_key = LibraryCollectionKey.from_string("lib-collection:orgA:libX:1") + collection_key = LibraryCollectionLocator.from_string("lib-collection:orgA:libX:1") api.tag_object( object_id=str(collection_key), diff --git a/openedx/core/djangoapps/content_tagging/types.py b/openedx/core/djangoapps/content_tagging/types.py index 9ffb090d61e3..3684e5705376 100644 --- a/openedx/core/djangoapps/content_tagging/types.py +++ b/openedx/core/djangoapps/content_tagging/types.py @@ -5,11 +5,11 @@ from typing import Dict, List, Union -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey +from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryItemKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy -ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, LibraryCollectionKey] +ContentKey = Union[LibraryLocatorV2, CourseKey, UsageKey, LibraryItemKey] ContextKey = Union[LibraryLocatorV2, CourseKey] TagValuesByTaxonomyIdDict = Dict[int, List[str]] diff --git a/openedx/core/djangoapps/content_tagging/utils.py b/openedx/core/djangoapps/content_tagging/utils.py index 39dd925c1acd..419b960927fd 100644 --- a/openedx/core/djangoapps/content_tagging/utils.py +++ b/openedx/core/djangoapps/content_tagging/utils.py @@ -5,7 +5,7 @@ from edx_django_utils.cache import RequestCache from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryCollectionKey +from opaque_keys.edx.keys import CourseKey, UsageKey, LibraryItemKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import Taxonomy from organizations.models import Organization @@ -28,11 +28,11 @@ def get_content_key_from_string(key_str: str) -> ContentKey: return UsageKey.from_string(key_str) except InvalidKeyError: try: - return LibraryCollectionKey.from_string(key_str) + return LibraryItemKey.from_string(key_str) except InvalidKeyError as usage_key_error: raise ValueError( "object_id must be one of the following " - "keys: CourseKey, LibraryLocatorV2, UsageKey or LibCollectionKey" + "keys: CourseKey, LibraryLocatorV2, UsageKey or LibraryItemKey" ) from usage_key_error @@ -44,8 +44,8 @@ def get_context_key_from_key(content_key: ContentKey) -> ContextKey: if isinstance(content_key, (CourseKey, LibraryLocatorV2)): return content_key - # If the content key is a LibraryCollectionKey, return the LibraryLocatorV2 - if isinstance(content_key, LibraryCollectionKey): + # If the content key is a LibraryItemKey, return the LibraryLocatorV2 + if isinstance(content_key, LibraryItemKey): return content_key.library_key # If the content key is a UsageKey, return the context key diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 2972b1876f57..fed7e47c5447 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -112,7 +112,7 @@ numpy<2.0.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.18.3 +openedx-learning==0.19.1 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3ae198a322fe..24ede4f1f253 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -477,7 +477,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/kernel.in edx-name-affirmation==3.0.1 # via -r requirements/edx/kernel.in -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/kernel.in # edx-bulk-grades @@ -820,7 +820,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/kernel.in -openedx-learning==0.18.3 +openedx-learning==0.19.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 50bb602efab9..c9acc77e898b 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -766,7 +766,7 @@ edx-name-affirmation==3.0.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -1383,7 +1383,7 @@ openedx-forum==0.1.9 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-learning==0.18.3 +openedx-learning==0.19.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 5b1a92fe3ca8..afe2bedf5954 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -561,7 +561,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/base.txt edx-name-affirmation==3.0.1 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/base.txt # edx-bulk-grades @@ -992,7 +992,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning==0.18.3 +openedx-learning==0.19.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index a17b9db4c868..55fa2f29082d 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -75,7 +75,7 @@ edx-event-bus-kafka>=5.6.0 # Kafka implementation of event bus edx-event-bus-redis edx-milestones edx-name-affirmation -edx-opaque-keys +edx-opaque-keys>=2.12.0 edx-organizations edx-proctoring>=2.0.1 # using hash to support django42 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 32034f3163e5..795e66c61adc 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -588,7 +588,7 @@ edx-milestones==0.6.0 # via -r requirements/edx/base.txt edx-name-affirmation==3.0.1 # via -r requirements/edx/base.txt -edx-opaque-keys[django]==2.11.0 +edx-opaque-keys[django]==2.12.0 # via # -r requirements/edx/base.txt # edx-bulk-grades @@ -1050,7 +1050,7 @@ openedx-filters==2.0.1 # ora2 openedx-forum==0.1.9 # via -r requirements/edx/base.txt -openedx-learning==0.18.3 +openedx-learning==0.19.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt