diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index 9782a37057c1..68891554f546 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -354,7 +354,7 @@ def summarize_by_downstream_context(cls, downstream_context_key: CourseKey) -> Q @classmethod def update_or_create( cls, - upstream_container: Container | None, + upstream_container_id: int | None, /, upstream_container_key: LibraryContainerLocator, upstream_context_key: str, @@ -377,8 +377,8 @@ def update_or_create( 'version_synced': version_synced, 'version_declined': version_declined, } - if upstream_container: - new_values['upstream_container'] = upstream_container + if upstream_container_id: + new_values['upstream_container_id'] = upstream_container_id try: link = cls.objects.get(downstream_usage_key=downstream_usage_key) has_changes = False diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 39c2649118f5..fc3e4f296bde 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -267,7 +267,7 @@ def put(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response fetch_customizable_fields_from_block(downstream=downstream, user=request.user) else: assert isinstance(link.upstream_key, LibraryContainerLocator) - fetch_customizable_fields_from_container(downstream=downstream, user=request.user) + fetch_customizable_fields_from_container(downstream=downstream) except BadDownstream as exc: logger.exception( "'%s' is an invalid downstream; refusing to set its upstream to '%s'", diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 21270a188aba..1ad400f3a5eb 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -87,7 +87,7 @@ from common.djangoapps.xblock_django.api import deprecated_xblocks from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService from openedx.core import toggles as core_toggles -from openedx.core.djangoapps.content_libraries.api import get_container_from_key +from openedx.core.djangoapps.content_libraries.api import get_container from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled from openedx.core.djangoapps.credit.api import get_credit_requirements, is_credit_course from openedx.core.djangoapps.discussions.config.waffle import ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND @@ -2402,7 +2402,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime | """ upstream_container_key = LibraryContainerLocator.from_string(xblock.upstream) try: - lib_component = get_container_from_key(upstream_container_key) + lib_component = get_container(upstream_container_key).container_pk except ObjectDoesNotExist: log.error(f"Library component not found for {upstream_container_key}") lib_component = None diff --git a/cms/lib/xblock/upstream_sync_container.py b/cms/lib/xblock/upstream_sync_container.py index 44e3b429ec05..4e8302323808 100644 --- a/cms/lib/xblock/upstream_sync_container.py +++ b/cms/lib/xblock/upstream_sync_container.py @@ -45,7 +45,7 @@ def sync_from_upstream_container( user, permission=lib_api.permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) - upstream_meta = lib_api.get_container(link.upstream_key, user) + upstream_meta = lib_api.get_container(link.upstream_key) upstream_children = lib_api.get_container_children(link.upstream_key, published=True) _update_customizable_fields(upstream=upstream_meta, downstream=downstream, only_fetch=False) _update_non_customizable_fields(upstream=upstream_meta, downstream=downstream) @@ -54,7 +54,7 @@ def sync_from_upstream_container( return upstream_children -def fetch_customizable_fields_from_container(*, downstream: XBlock, user: User) -> None: +def fetch_customizable_fields_from_container(*, downstream: XBlock) -> None: """ Fetch upstream-defined value of customizable fields and save them on the downstream. @@ -62,7 +62,7 @@ def fetch_customizable_fields_from_container(*, downstream: XBlock, user: User) Basically, this sets the value of "upstream_display_name" on the downstream block. """ - upstream = lib_api.get_container(LibraryContainerLocator.from_string(downstream.upstream), user) + upstream = lib_api.get_container(LibraryContainerLocator.from_string(downstream.upstream)) _update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=True) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 134899a819c2..28b5e74450a6 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -71,6 +71,7 @@ class Fields: # The "content" field is a dictionary of arbitrary data, depending on the block_type. # It comes from each XBlock's index_dictionary() method (if present) plus some processing. # Text (html) blocks have an "html_content" key in here, capa has "capa_content" and "problem_types", and so on. + # Containers store their list of child usage keys here. content = "content" # Collections use this field to communicate how many entities/components they contain. @@ -87,6 +88,7 @@ class Fields: published = "published" published_display_name = "display_name" published_description = "description" + published_content = "content" published_num_children = "num_children" # Note: new fields or values can be added at any time, but if they need to be indexed for filtering or keyword @@ -347,13 +349,10 @@ def _collections_for_content_object(object_id: OpaqueKey) -> dict: collections = authoring_api.get_entity_collections( component.learning_package_id, component.key, - ) + ).values('key', 'title') elif isinstance(object_id, LibraryContainerLocator): - container = lib_api.get_container_from_key(object_id) - collections = authoring_api.get_entity_collections( - container.publishable_entity.learning_package_id, - container.key, - ) + container = lib_api.get_container(object_id, include_collections=True) + collections = container.collections else: log.warning(f"Unexpected key type for {object_id}") @@ -364,8 +363,8 @@ def _collections_for_content_object(object_id: OpaqueKey) -> dict: return result for collection in collections: - result[Fields.collections][Fields.collections_display_name].append(collection.title) - result[Fields.collections][Fields.collections_key].append(collection.key) + result[Fields.collections][Fields.collections_display_name].append(collection["title"]) + result[Fields.collections][Fields.collections_key].append(collection["key"]) return result @@ -581,9 +580,13 @@ def searchable_doc_for_container( container = lib_api.get_container(container_key) except lib_api.ContentLibraryContainerNotFound: # Container not found, so we can only return the base doc + log.error(f"Container {container_key} not found") return doc - draft_num_children = lib_api.get_container_children_count(container_key, published=False) + draft_children = lib_api.get_container_children( + container_key, + published=False, + ) publish_status = PublishStatus.published if container.last_published is None: publish_status = PublishStatus.never @@ -594,7 +597,13 @@ def searchable_doc_for_container( Fields.display_name: container.display_name, Fields.created: container.created.timestamp(), Fields.modified: container.modified.timestamp(), - Fields.num_children: draft_num_children, + Fields.num_children: len(draft_children), + Fields.content: { + "child_usage_keys": [ + str(child.usage_key) + for child in draft_children + ], + }, Fields.publish_status: publish_status, Fields.last_published: container.last_published.timestamp() if container.last_published else None, }) @@ -603,10 +612,19 @@ def searchable_doc_for_container( doc[Fields.breadcrumbs] = [{"display_name": library.title}] if container.published_version_num is not None: - published_num_children = lib_api.get_container_children_count(container_key, published=True) + published_children = lib_api.get_container_children( + container_key, + published=True, + ) doc[Fields.published] = { - Fields.published_num_children: published_num_children, Fields.published_display_name: container.published_display_name, + Fields.published_num_children: len(published_children), + Fields.published_content: { + "child_usage_keys": [ + str(child.usage_key) + for child in published_children + ], + }, } return doc diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 46354d6eb3cd..492d1b5f7650 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -238,6 +238,7 @@ def setUp(self): "display_name": "Unit 1", # description is not set for containers "num_children": 0, + "content": {"child_usage_keys": []}, "publish_status": "never", "context_key": "lib:org1:lib", "org": "org1", diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index b70594eff0eb..74772c89c017 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -531,6 +531,9 @@ def test_draft_container(self): "display_name": "A Unit in the Search Index", # description is not set for containers "num_children": 0, + "content": { + "child_usage_keys": [], + }, "publish_status": "never", "context_key": "lib:edX:2012_Fall", "access_id": self.library_access_id, @@ -571,6 +574,11 @@ def test_published_container(self): "display_name": "A Unit in the Search Index", # description is not set for containers "num_children": 1, + "content": { + "child_usage_keys": [ + "lb:edX:2012_Fall:html:text2", + ], + }, "publish_status": "published", "context_key": "lib:edX:2012_Fall", "access_id": self.library_access_id, @@ -585,6 +593,11 @@ def test_published_container(self): "published": { "num_children": 1, "display_name": "A Unit in the Search Index", + "content": { + "child_usage_keys": [ + "lb:edX:2012_Fall:html:text2", + ], + }, }, } @@ -627,6 +640,12 @@ def test_published_container_with_changes(self): "display_name": "A Unit in the Search Index", # description is not set for containers "num_children": 2, + "content": { + "child_usage_keys": [ + "lb:edX:2012_Fall:html:text2", + "lb:edX:2012_Fall:html:text3", + ], + }, "publish_status": "modified", "context_key": "lib:edX:2012_Fall", "access_id": self.library_access_id, @@ -641,6 +660,11 @@ def test_published_container_with_changes(self): "published": { "num_children": 1, "display_name": "A Unit in the Search Index", + "content": { + "child_usage_keys": [ + "lb:edX:2012_Fall:html:text2", + ], + }, }, } diff --git a/openedx/core/djangoapps/content_libraries/api/block_metadata.py b/openedx/core/djangoapps/content_libraries/api/block_metadata.py index 032d21431a3a..539b3290631b 100644 --- a/openedx/core/djangoapps/content_libraries/api/block_metadata.py +++ b/openedx/core/djangoapps/content_libraries/api/block_metadata.py @@ -26,8 +26,6 @@ class LibraryXBlockMetadata(PublishableItem): Class that represents the metadata about an XBlock in a content library. """ usage_key: LibraryUsageLocatorV2 - # TODO: move tags_count to LibraryItem as all objects under a library can be tagged. - tags_count: int = 0 @classmethod def from_component(cls, library_key, component, associated_collections=None): diff --git a/openedx/core/djangoapps/content_libraries/api/collections.py b/openedx/core/djangoapps/content_libraries/api/collections.py index 2b0ddc08d89c..da8b47ee5e75 100644 --- a/openedx/core/djangoapps/content_libraries/api/collections.py +++ b/openedx/core/djangoapps/content_libraries/api/collections.py @@ -181,7 +181,7 @@ def update_library_collection_items( def set_library_item_collections( library_key: LibraryLocatorV2, - publishable_entity: PublishableEntity, + entity_key: str, *, collection_keys: list[str], created_by: int | None = None, @@ -207,6 +207,11 @@ def set_library_item_collections( assert content_library.learning_package_id assert content_library.library_key == library_key + publishable_entity = authoring_api.get_publishable_entity_by_key( + content_library.learning_package_id, + key=entity_key, + ) + # Note: Component.key matches its PublishableEntity.key collection_qs = authoring_api.get_collections(content_library.learning_package_id).filter( key__in=collection_keys diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index d7ba0fcac01f..36072caf8215 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -43,7 +43,6 @@ "ContainerMetadata", "ContainerType", # API methods - "get_container_from_key", "get_container", "create_container", "get_container_children", @@ -111,6 +110,7 @@ class ContainerMetadata(PublishableItem): """ container_key: LibraryContainerLocator container_type: ContainerType + container_pk: int published_display_name: str | None @classmethod @@ -139,6 +139,7 @@ def from_container(cls, library_key, container: Container, associated_collection return cls( container_key=container_key, container_type=container_type, + container_pk=container.pk, display_name=draft.title, created=container.created, modified=draft.created, @@ -173,7 +174,7 @@ def library_container_locator( ) -def get_container_from_key(container_key: LibraryContainerLocator, isDeleted=False) -> Container: +def _get_container_from_key(container_key: LibraryContainerLocator, isDeleted=False) -> Container: """ Internal method to fetch the Container object from its LibraryContainerLocator @@ -192,11 +193,15 @@ def get_container_from_key(container_key: LibraryContainerLocator, isDeleted=Fal raise ContentLibraryContainerNotFound -def get_container(container_key: LibraryContainerLocator, include_collections=False) -> ContainerMetadata: +def get_container( + container_key: LibraryContainerLocator, + *, + include_collections=False, +) -> ContainerMetadata: """ Get a container (a Section, Subsection, or Unit). """ - container = get_container_from_key(container_key) + container = _get_container_from_key(container_key) if include_collections: associated_collections = authoring_api.get_entity_collections( container.publishable_entity.learning_package_id, @@ -268,7 +273,7 @@ def update_container( """ Update a container (e.g. a Unit) title. """ - container = get_container_from_key(container_key) + container = _get_container_from_key(container_key) library_key = container_key.lib_key assert container.unit @@ -297,7 +302,7 @@ def delete_container( No-op if container doesn't exist or has already been soft-deleted. """ library_key = container_key.lib_key - container = get_container_from_key(container_key) + container = _get_container_from_key(container_key) affected_collections = authoring_api.get_entity_collections( container.publishable_entity.learning_package_id, @@ -332,7 +337,7 @@ def restore_container(container_key: LibraryContainerLocator) -> None: Restore the specified library container. """ library_key = container_key.lib_key - container = get_container_from_key(container_key, isDeleted=True) + container = _get_container_from_key(container_key, isDeleted=True) affected_collections = authoring_api.get_entity_collections( container.publishable_entity.learning_package_id, @@ -372,12 +377,13 @@ def restore_container(container_key: LibraryContainerLocator) -> None: def get_container_children( container_key: LibraryContainerLocator, + *, published=False, ) -> list[LibraryXBlockMetadata | ContainerMetadata]: """ Get the entities contained in the given container (e.g. the components/xblocks in a unit) """ - container = get_container_from_key(container_key) + container = _get_container_from_key(container_key) if container_key.container_type == ContainerType.Unit.value: child_components = authoring_api.get_components_in_unit(container.unit, published=published) return [LibraryXBlockMetadata.from_component( @@ -399,7 +405,7 @@ def get_container_children_count( """ Get the count of entities contained in the given container (e.g. the components/xblocks in a unit) """ - container = get_container_from_key(container_key) + container = _get_container_from_key(container_key) return authoring_api.get_container_children_count(container, published=published) @@ -414,7 +420,7 @@ def update_container_children( """ library_key = container_key.lib_key container_type = container_key.container_type - container = get_container_from_key(container_key) + container = _get_container_from_key(container_key) match container_type: case ContainerType.Unit.value: components = [get_component_from_usage_key(key) for key in children_ids] # type: ignore[arg-type] @@ -459,7 +465,7 @@ def publish_container_changes(container_key: LibraryContainerLocator, user_id: i Publish all unpublished changes in a container and all its child containers/blocks. """ - container = get_container_from_key(container_key) + container = _get_container_from_key(container_key) library_key = container_key.lib_key content_library = ContentLibrary.objects.get_by_key(library_key) # type: ignore[attr-defined] learning_package = content_library.learning_package diff --git a/openedx/core/djangoapps/content_libraries/api/libraries.py b/openedx/core/djangoapps/content_libraries/api/libraries.py index 8e238f278096..8549060f3e64 100644 --- a/openedx/core/djangoapps/content_libraries/api/libraries.py +++ b/openedx/core/djangoapps/content_libraries/api/libraries.py @@ -184,6 +184,7 @@ class LibraryItem: created: datetime modified: datetime display_name: str + tags_count: int = 0 @dataclass(frozen=True, kw_only=True) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py index 6ab35c47c632..bc314099893c 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/blocks.py @@ -265,14 +265,14 @@ def patch(self, request: RestRequest, usage_key_str) -> Response: request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) - component = api.get_component_from_usage_key(key) serializer = ContentLibraryItemCollectionsUpdateSerializer(data=request.data) serializer.is_valid(raise_exception=True) + component = api.get_component_from_usage_key(key) collection_keys = serializer.validated_data['collection_keys'] api.set_library_item_collections( library_key=key.lib_key, - publishable_entity=component.publishable_entity, + entity_key=component.publishable_entity.key, collection_keys=collection_keys, created_by=request.user.id, content_library=content_library, diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 061f4a0693d1..19f33b7beb2c 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -184,7 +184,7 @@ def get(self, request, container_key: LibraryContainerLocator): request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) - child_entities = api.get_container_children(container_key, published) + child_entities = api.get_container_children(container_key, published=published) if container_key.container_type == api.ContainerType.Unit.value: data = serializers.LibraryXBlockMetadataSerializer(child_entities, many=True).data else: @@ -314,14 +314,13 @@ def patch(self, request: RestRequest, container_key: LibraryContainerLocator) -> request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) - container = api.get_container_from_key(container_key) serializer = serializers.ContentLibraryItemCollectionsUpdateSerializer(data=request.data) serializer.is_valid(raise_exception=True) collection_keys = serializer.validated_data['collection_keys'] api.set_library_item_collections( library_key=container_key.lib_key, - publishable_entity=container.publishable_entity, + entity_key=container_key.container_id, collection_keys=collection_keys, created_by=request.user.id, content_library=content_library, diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index be386e0e9dfe..04c9e6a9901c 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -138,6 +138,7 @@ class PublishableItemSerializer(serializers.Serializer): """ id = serializers.SerializerMethodField() display_name = serializers.CharField() + tags_count = serializers.IntegerField(read_only=True) 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) @@ -149,7 +150,6 @@ class PublishableItemSerializer(serializers.Serializer): # When creating a new XBlock in a library, the slug becomes the ID part of # the definition key and usage key: slug = serializers.CharField(write_only=True) - tags_count = serializers.IntegerField(read_only=True) collections = CollectionMetadataSerializer(many=True, required=False) can_stand_alone = serializers.BooleanField(read_only=True) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 8f79ec7f6339..8dbb5e11946a 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -547,10 +547,9 @@ def test_set_library_component_collections(self): LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver) assert not list(self.col2.entities.all()) component = api.get_component_from_usage_key(UsageKey.from_string(self.lib2_problem_block["id"])) - api.set_library_item_collections( - self.lib2.library_key, - component.publishable_entity, + library_key=self.lib2.library_key, + entity_key=component.publishable_entity.key, collection_keys=[self.col2.key, self.col3.key], )