diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 36698da6a091..29b3673a0172 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -95,6 +95,9 @@ class Fields: published_content = "content" published_num_children = "num_children" + # List of children keys + child_usage_keys = "child_usage_keys" + # Note: new fields or values can be added at any time, but if they need to be indexed for filtering or keyword # search, the index configuration will need to be changed, which is only done as part of the 'reindex_studio' # command (changing those settings on an large active index is not recommended). @@ -656,16 +659,28 @@ def searchable_doc_for_container( elif container.has_unpublished_changes: publish_status = PublishStatus.modified + container_type = lib_api.ContainerType(container_key.container_type) + + def get_child_keys(children): + match container_type: + case lib_api.ContainerType.Unit: + return [ + str(child.usage_key) + for child in children + ] + case lib_api.ContainerType.Subsection | lib_api.ContainerType.Section: + return [ + str(child.container_key) + for child in children + ] + doc.update({ Fields.display_name: container.display_name, Fields.created: container.created.timestamp(), Fields.modified: container.modified.timestamp(), Fields.num_children: len(draft_children), Fields.content: { - "child_usage_keys": [ - str(child.usage_key) - for child in draft_children - ], + Fields.child_usage_keys: get_child_keys(draft_children) }, Fields.publish_status: publish_status, Fields.last_published: container.last_published.timestamp() if container.last_published else None, @@ -683,10 +698,7 @@ def searchable_doc_for_container( 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 - ], + Fields.child_usage_keys: get_child_keys(published_children), }, } diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 2093b3dc8587..2a1aced68601 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -48,6 +48,7 @@ class TestSearchApi(ModuleStoreTestCase): MODULESTORE = TEST_DATA_SPLIT_MODULESTORE def setUp(self): + # pylint: disable=too-many-statements super().setUp() self.user = UserFactory.create() self.user_id = self.user.id @@ -219,7 +220,7 @@ def setUp(self): "breadcrumbs": [{"display_name": "Library"}], } - # Create a unit: + # Create a container: with freeze_time(self.created_date): self.unit = library_api.create_container( library_key=self.library.key, @@ -229,6 +230,23 @@ def setUp(self): user_id=None, ) self.unit_key = "lct:org1:lib:unit:unit-1" + self.subsection = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Subsection, + slug="subsection-1", + title="Subsection 1", + user_id=None, + ) + self.subsection_key = "lct:org1:lib:subsection:subsection-1" + self.section = library_api.create_container( + self.library.key, + container_type=library_api.ContainerType.Section, + slug="section-1", + title="Section 1", + user_id=None, + ) + self.section_key = "lct:org1:lib:section:section-1" + self.unit_dict = { "id": "lctorg1libunitunit-1-e4527f7c", "block_id": "unit-1", @@ -249,6 +267,46 @@ def setUp(self): "breadcrumbs": [{"display_name": "Library"}], # "published" is not set since we haven't published it yet } + self.subsection_dict = { + "id": "lctorg1libsubsectionsubsection-1-cf808309", + "block_id": "subsection-1", + "block_type": "subsection", + "usage_key": self.subsection_key, + "type": "library_container", + "display_name": "Subsection 1", + # description is not set for containers + "num_children": 0, + "content": {"child_usage_keys": []}, + "publish_status": "never", + "context_key": "lib:org1:lib", + "org": "org1", + "created": self.created_date.timestamp(), + "modified": self.created_date.timestamp(), + "last_published": None, + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + # "published" is not set since we haven't published it yet + } + self.section_dict = { + "id": "lctorg1libsectionsection-1-dc4791a4", + "block_id": "section-1", + "block_type": "section", + "usage_key": self.section_key, + "type": "library_container", + "display_name": "Section 1", + # description is not set for containers + "num_children": 0, + "content": {"child_usage_keys": []}, + "publish_status": "never", + "context_key": "lib:org1:lib", + "org": "org1", + "created": self.created_date.timestamp(), + "modified": self.created_date.timestamp(), + "last_published": None, + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + # "published" is not set since we haven't published it yet + } @override_settings(MEILISEARCH_ENABLED=False) def test_reindex_meilisearch_disabled(self, mock_meilisearch): @@ -278,6 +336,12 @@ def test_reindex_meilisearch(self, mock_meilisearch): doc_unit = copy.deepcopy(self.unit_dict) doc_unit["tags"] = {} doc_unit["collections"] = {'display_name': [], 'key': []} + doc_subsection = copy.deepcopy(self.subsection_dict) + doc_subsection["tags"] = {} + doc_subsection["collections"] = {'display_name': [], 'key': []} + doc_section = copy.deepcopy(self.section_dict) + doc_section["tags"] = {} + doc_section["collections"] = {'display_name': [], 'key': []} api.rebuild_index() assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 @@ -286,7 +350,7 @@ def test_reindex_meilisearch(self, mock_meilisearch): call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), call([doc_collection]), - call([doc_unit]), + call([doc_unit, doc_subsection, doc_section]), ], any_order=True, ) @@ -312,6 +376,12 @@ def test_reindex_meilisearch_incremental(self, mock_meilisearch): doc_unit = copy.deepcopy(self.unit_dict) doc_unit["tags"] = {} doc_unit["collections"] = {"display_name": [], "key": []} + doc_subsection = copy.deepcopy(self.subsection_dict) + doc_subsection["tags"] = {} + doc_subsection["collections"] = {'display_name': [], 'key': []} + doc_section = copy.deepcopy(self.section_dict) + doc_section["tags"] = {} + doc_section["collections"] = {'display_name': [], 'key': []} api.rebuild_index(incremental=True) assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 4 @@ -320,7 +390,7 @@ def test_reindex_meilisearch_incremental(self, mock_meilisearch): call([doc_sequential, doc_vertical]), call([doc_problem1, doc_problem2]), call([doc_collection]), - call([doc_unit]), + call([doc_unit, doc_subsection, doc_section]), ], any_order=True, ) @@ -894,15 +964,23 @@ def test_delete_collection(self, mock_meilisearch): any_order=True, ) + @ddt.data( + "unit", + "subsection", + "section", + ) @override_settings(MEILISEARCH_ENABLED=True) - def test_delete_index_container(self, mock_meilisearch): + def test_delete_index_container(self, container_type, mock_meilisearch): """ Test delete a container index. """ - library_api.delete_container(self.unit.container_key) + container = getattr(self, container_type) + container_dict = getattr(self, f"{container_type}_dict") + + library_api.delete_container(container.container_key) mock_meilisearch.return_value.index.return_value.delete_document.assert_called_once_with( - self.unit_dict["id"], + container_dict["id"], ) @override_settings(MEILISEARCH_ENABLED=True) @@ -914,22 +992,30 @@ def test_index_library_container_metadata(self, mock_meilisearch): mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.unit_dict]) + @ddt.data( + ("unit", "lctorg1libunitunit-1-e4527f7c"), + ("subsection", "lctorg1libsubsectionsubsection-1-cf808309"), + ("section", "lctorg1libsectionsection-1-dc4791a4"), + ) + @ddt.unpack @override_settings(MEILISEARCH_ENABLED=True) - def test_index_tags_in_containers(self, mock_meilisearch): - # Tag collection - tagging_api.tag_object(self.unit_key, self.taxonomyA, ["one", "two"]) - tagging_api.tag_object(self.unit_key, self.taxonomyB, ["three", "four"]) + def test_index_tags_in_containers(self, container_type, container_id, mock_meilisearch): + container_key = getattr(self, f"{container_type}_key") + + # Tag container + tagging_api.tag_object(container_key, self.taxonomyA, ["one", "two"]) + tagging_api.tag_object(container_key, self.taxonomyB, ["three", "four"]) # Build expected docs with tags at each stage doc_unit_with_tags1 = { - "id": "lctorg1libunitunit-1-e4527f7c", + "id": container_id, "tags": { 'taxonomy': ['A'], 'level0': ['A > one', 'A > two'] } } doc_unit_with_tags2 = { - "id": "lctorg1libunitunit-1-e4527f7c", + "id": container_id, "tags": { 'taxonomy': ['A', 'B'], 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] @@ -975,3 +1061,51 @@ def test_block_in_units(self, mock_meilisearch): ], any_order=True, ) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_units_in_subsection(self, mock_meilisearch): + with freeze_time(self.created_date): + library_api.update_container_children( + LibraryContainerLocator.from_string(self.subsection_key), + [LibraryContainerLocator.from_string(self.unit_key)], + None, + ) + + # TODO verify subsections in units + + new_subsection_dict = { + **self.subsection_dict, + "num_children": 1, + 'content': {'child_usage_keys': [self.unit_key]} + } + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 1 + mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( + [ + call([new_subsection_dict]), + ], + any_order=True, + ) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_section_in_usbsections(self, mock_meilisearch): + with freeze_time(self.created_date): + library_api.update_container_children( + LibraryContainerLocator.from_string(self.section_key), + [LibraryContainerLocator.from_string(self.subsection_key)], + None, + ) + + # TODO verify section in subsections + + new_section_dict = { + **self.section_dict, + "num_children": 1, + 'content': {'child_usage_keys': [self.subsection_key]} + } + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 1 + mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( + [ + call([new_section_dict]), + ], + any_order=True, + ) diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 74772c89c017..8d019ab31c53 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -1,6 +1,7 @@ """ Tests for the Studio content search documents (what gets stored in the index) """ +import ddt from dataclasses import replace from datetime import datetime, timezone @@ -42,6 +43,7 @@ @skip_unless_cms +@ddt.ddt class StudioDocumentsTest(SharedModuleStoreTestCase): """ Tests for the Studio content search documents (what gets stored in the @@ -100,6 +102,26 @@ def setUpClass(cls): cls.container_key = LibraryContainerLocator.from_string( "lct:edX:2012_Fall:unit:unit1", ) + cls.subsection = library_api.create_container( + cls.library.key, + container_type=library_api.ContainerType.Subsection, + slug="subsection1", + title="A Subsection in the Search Index", + user_id=None, + ) + cls.subsection_key = LibraryContainerLocator.from_string( + "lct:edX:2012_Fall:subsection:subsection1", + ) + cls.section = library_api.create_container( + cls.library.key, + container_type=library_api.ContainerType.Section, + slug="section1", + title="A Section in the Search Index", + user_id=None, + ) + cls.section_key = LibraryContainerLocator.from_string( + "lct:edX:2012_Fall:section:section1", + ) # Add the problem block to the collection library_api.update_library_collection_items( @@ -130,6 +152,8 @@ def setUpClass(cls): tagging_api.tag_object(str(cls.library_block.usage_key), cls.difficulty_tags, tags=["Normal"]) tagging_api.tag_object(str(cls.collection_key), cls.difficulty_tags, tags=["Normal"]) tagging_api.tag_object(str(cls.container_key), cls.difficulty_tags, tags=["Normal"]) + tagging_api.tag_object(str(cls.subsection_key), cls.difficulty_tags, tags=["Normal"]) + tagging_api.tag_object(str(cls.section_key), cls.difficulty_tags, tags=["Normal"]) @property def toy_course_access_id(self): @@ -514,21 +538,28 @@ def test_collection_with_published_library(self): } } - def test_draft_container(self): + @ddt.data( + ("container", "unit1", "unit", "edd13a0c"), + ("subsection", "subsection1", "subsection", "c6c172be"), + ("section", "section1", "section", "79ee8fa2"), + ) + @ddt.unpack + def test_draft_container_1(self, container_name, container_slug, container_type, doc_id): """ Test creating a search document for a draft-only container """ - doc = searchable_doc_for_container(self.container.container_key) - doc.update(searchable_doc_tags(self.container.container_key)) + container = getattr(self, container_name) + doc = searchable_doc_for_container(container.container_key) + doc.update(searchable_doc_tags(container.container_key)) assert doc == { - "id": "lctedx2012_fallunitunit1-edd13a0c", - "block_id": "unit1", - "block_type": "unit", - "usage_key": "lct:edX:2012_Fall:unit:unit1", + "id": f"lctedx2012_fall{container_type}{container_slug}-{doc_id}", + "block_id": container_slug, + "block_type": container_type, + "usage_key": f"lct:edX:2012_Fall:{container_type}:{container_slug}", "type": "library_container", "org": "edX", - "display_name": "A Unit in the Search Index", + "display_name": container.display_name, # description is not set for containers "num_children": 0, "content": { diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index dd67095597a5..7ac9e549ca84 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -58,7 +58,7 @@ from .containers import ( create_container, get_container, - get_containers_contains_component, + get_containers_contains_item, update_container_children, ContainerMetadata, ContainerType, @@ -229,7 +229,7 @@ def set_library_block_olx(usage_key: LibraryUsageLocatorV2, new_olx_str: str) -> # For each container, trigger LIBRARY_CONTAINER_UPDATED signal and set background=True to trigger # container indexing asynchronously. - affected_containers = get_containers_contains_component(usage_key) + affected_containers = get_containers_contains_item(usage_key) for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( @@ -585,7 +585,7 @@ def delete_library_block( component = get_component_from_usage_key(usage_key) library_key = usage_key.context_key affected_collections = authoring_api.get_entity_collections(component.learning_package_id, component.key) - affected_containers = get_containers_contains_component(usage_key) + affected_containers = get_containers_contains_item(usage_key) authoring_api.soft_delete_draft(component.pk, deleted_by=user_id) @@ -673,7 +673,7 @@ def restore_library_block(usage_key: LibraryUsageLocatorV2, user_id: int | None # container indexing asynchronously. # # To update the components count in containers - affected_containers = get_containers_contains_component(usage_key) + affected_containers = get_containers_contains_item(usage_key) for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 01974a441ed2..dcb6ec6a4796 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -25,7 +25,7 @@ LIBRARY_CONTAINER_UPDATED, ) from openedx_learning.api import authoring as authoring_api -from openedx_learning.api.authoring_models import Container +from openedx_learning.api.authoring_models import Container, ContainerVersion, Component from openedx.core.djangoapps.content_libraries.api.collections import library_collection_locator from openedx.core.djangoapps.xblock.api import get_component_from_usage_key @@ -51,7 +51,7 @@ "delete_container", "restore_container", "update_container_children", - "get_containers_contains_component", + "get_containers_contains_item", "publish_container_changes", ] @@ -159,11 +159,15 @@ def library_container_locator( ) -> 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 + if hasattr(container, 'unit'): + container_type = ContainerType.Unit + elif hasattr(container, 'subsection'): + container_type = ContainerType.Subsection + elif hasattr(container, 'section'): + container_type = ContainerType.Section + + assert container_type is not None return LibraryContainerLocator( library_key, @@ -225,7 +229,7 @@ def create_container( created: datetime | None = None, ) -> ContainerMetadata: """ - Create a container (e.g. a Unit) in the specified content library. + Create a container (a Section, Subsection, or Unit) in the specified content library. It will initially be empty. """ @@ -241,6 +245,13 @@ def create_container( container_type=container_type.value, container_id=slug, ) + + if not created: + created = datetime.now(tz=timezone.utc) + + container: Container + _initial_version: ContainerVersion + # Then try creating the actual container: match container_type: case ContainerType.Unit: @@ -248,7 +259,23 @@ def create_container( content_library.learning_package_id, key=slug, title=title, - created=created or datetime.now(tz=timezone.utc), + created=created, + created_by=user_id, + ) + case ContainerType.Subsection: + container, _initial_version = authoring_api.create_subsection_and_version( + content_library.learning_package_id, + key=slug, + title=title, + created=created, + created_by=user_id, + ) + case ContainerType.Section: + container, _initial_version = authoring_api.create_section_and_version( + content_library.learning_package_id, + key=slug, + title=title, + created=created, created_by=user_id, ) case _: @@ -269,18 +296,40 @@ def update_container( user_id: int | None, ) -> ContainerMetadata: """ - Update a container (e.g. a Unit) title. + Update a container (a Section, Subsection, or Unit) title. """ container = _get_container_from_key(container_key) library_key = container_key.lib_key + created = datetime.now(tz=timezone.utc) - assert container.unit - unit_version = authoring_api.create_next_unit_version( - container.unit, - title=display_name, - created=datetime.now(tz=timezone.utc), - created_by=user_id, - ) + container_type = ContainerType(container_key.container_type) + + version: ContainerVersion + + match container_type: + case ContainerType.Unit: + version = authoring_api.create_next_unit_version( + container.unit, + title=display_name, + created=created, + created_by=user_id, + ) + case ContainerType.Subsection: + version = authoring_api.create_next_subsection_version( + container.subsection, + title=display_name, + created=created, + created_by=user_id, + ) + case ContainerType.Section: + version = authoring_api.create_next_section_version( + container.section, + title=display_name, + created=created, + created_by=user_id, + ) + case _: + raise NotImplementedError(f"Library support for {container_type} is in progress") LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( @@ -288,14 +337,14 @@ def update_container( ) ) - return ContainerMetadata.from_container(library_key, unit_version.container) + return ContainerMetadata.from_container(library_key, version.container) def delete_container( container_key: LibraryContainerLocator, ) -> None: """ - Delete a container (e.g. a Unit) (soft delete). + Delete a container (a Section, Subsection, or Unit) (soft delete). No-op if container doesn't exist or has already been soft-deleted. """ @@ -379,21 +428,37 @@ def get_container_children( published=False, ) -> list[LibraryXBlockMetadata | ContainerMetadata]: """ - Get the entities contained in the given container (e.g. the components/xblocks in a unit) + Get the entities contained in the given container + (e.g. the components/xblocks in a unit, units in a subsection, subsections in a section) """ 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( - container_key.lib_key, - entry.component - ) for entry in child_components] - else: - child_entities = authoring_api.get_entities_in_container(container, published=published) - return [ContainerMetadata.from_container( - container_key.lib_key, - entry.entity - ) for entry in child_entities] + container_type = ContainerType(container_key.container_type) + + match container_type: + case ContainerType.Unit: + child_components = authoring_api.get_components_in_unit(container.unit, published=published) + return [LibraryXBlockMetadata.from_component( + container_key.lib_key, + entry.component + ) for entry in child_components] + case ContainerType.Subsection: + child_units = authoring_api.get_units_in_subsection(container.subsection, published=published) + return [ContainerMetadata.from_container( + container_key.lib_key, + entry.unit + ) for entry in child_units] + case ContainerType.Section: + child_subsections = authoring_api.get_subsections_in_section(container.section, published=published) + return [ContainerMetadata.from_container( + container_key.lib_key, + entry.subsection, + ) for entry in child_subsections] + case _: + child_entities = authoring_api.get_entities_in_container(container, published=published) + return [ContainerMetadata.from_container( + container_key.lib_key, + entry.entity + ) for entry in child_entities] def get_container_children_count( @@ -417,15 +482,17 @@ def update_container_children( Adds children components or containers to given container. """ library_key = container_key.lib_key - container_type = container_key.container_type + container_type = ContainerType(container_key.container_type) container = _get_container_from_key(container_key) + created = datetime.now(tz=timezone.utc) + new_version: ContainerVersion match container_type: - case ContainerType.Unit.value: + case ContainerType.Unit: components = [get_component_from_usage_key(key) for key in children_ids] # type: ignore[arg-type] new_version = authoring_api.create_next_unit_version( container.unit, components=components, # type: ignore[arg-type] - created=datetime.now(tz=timezone.utc), + created=created, created_by=user_id, entities_action=entities_action, ) @@ -437,6 +504,50 @@ def update_container_children( changes=["units"], ), ) + case ContainerType.Subsection: + units = [] + for key in children_ids: + # Verify that all children are units + if ( + not isinstance(key, LibraryContainerLocator) + or not key.container_type == ContainerType.Unit.value + ): + raise ValueError( + f"Invalid children type: {key}. All Subsection children must be Units", + ) + units.append(_get_container_from_key(key).unit) + + new_version = authoring_api.create_next_subsection_version( + container.subsection, + units=units, # type: ignore[arg-type] + created=created, + created_by=user_id, + entities_action=entities_action, + ) + + # TODO add CONTENT_OBJECT_ASSOCIATIONS_CHANGED for subsections + case ContainerType.Section: + subsections = [] + for key in children_ids: + # Verify that all children are subsections + if ( + not isinstance(key, LibraryContainerLocator) + or not key.container_type == ContainerType.Subsection.value + ): + raise ValueError( + f"Invalid children type: {key}. All Section children must be Subsections", + ) + subsections.append(_get_container_from_key(key).subsection) + + new_version = authoring_api.create_next_section_version( + container.section, + subsections=subsections, # type: ignore[arg-type] + created=created, + created_by=user_id, + entities_action=entities_action, + ) + + # TODO add CONTENT_OBJECT_ASSOCIATIONS_CHANGED for sections case _: raise ValueError(f"Invalid container type: {container_type}") @@ -449,19 +560,26 @@ def update_container_children( return ContainerMetadata.from_container(library_key, new_version.container) -def get_containers_contains_component( - usage_key: LibraryUsageLocatorV2 +def get_containers_contains_item( + key: LibraryUsageLocatorV2 | LibraryContainerLocator ) -> list[ContainerMetadata]: """ - Get containers that contains the component. + Get containers that contains the item, + that can be a component or another container. """ - assert isinstance(usage_key, LibraryUsageLocatorV2) - component = get_component_from_usage_key(usage_key) + item: Component | Container + + if isinstance(key, LibraryUsageLocatorV2): + item = get_component_from_usage_key(key) + + elif isinstance(key, LibraryContainerLocator): + item = _get_container_from_key(key) + containers = authoring_api.get_containers_with_entity( - component.publishable_entity.pk, + item.publishable_entity.pk, ) return [ - ContainerMetadata.from_container(usage_key.context_key, container) + ContainerMetadata.from_container(key.lib_key, container) for container in containers ] diff --git a/openedx/core/djangoapps/content_libraries/library_context.py b/openedx/core/djangoapps/content_libraries/library_context.py index 34a5a90269fe..4ab66706088d 100644 --- a/openedx/core/djangoapps/content_libraries/library_context.py +++ b/openedx/core/djangoapps/content_libraries/library_context.py @@ -121,7 +121,7 @@ def send_container_updated_events(self, usage_key: UsageKeyV2): with the given usage_key. """ assert isinstance(usage_key, LibraryUsageLocatorV2) - affected_containers = api.get_containers_contains_component(usage_key) + affected_containers = api.get_containers_contains_item(usage_key) for container in affected_containers: LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 06baa6fc4254..36118346daa6 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -64,7 +64,7 @@ def post(self, request, lib_key_str): @view_auth_classes() class LibraryContainerView(GenericAPIView): """ - View to retrieve or update data about a specific container (a section, subsection, or unit) + View to retrieve, delete or update data about a specific container (a section, subsection, or unit) """ serializer_class = serializers.LibraryContainerMetadataSerializer @@ -141,7 +141,7 @@ class LibraryContainerChildrenView(GenericAPIView): ) def get(self, request, container_key: LibraryContainerLocator): """ - Get children components of given container + Get children of given container Example: GET /api/libraries/v2/containers//children/ Result: @@ -205,10 +205,8 @@ def _update_component_children( request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, ) - serializer = serializers.ContentLibraryComponentKeysSerializer(data=request.data) + serializer = serializers.ContentLibraryItemContainerKeysSerializer(data=request.data) serializer.is_valid(raise_exception=True) - # Only components under units are supported for now. - assert container_key.container_type == api.ContainerType.Unit.value container = api.update_container_children( container_key, @@ -220,12 +218,12 @@ def _update_component_children( @convert_exceptions @swagger_auto_schema( - request_body=serializers.ContentLibraryComponentKeysSerializer, + request_body=serializers.ContentLibraryItemContainerKeysSerializer, responses={200: serializers.LibraryContainerMetadataSerializer} ) def post(self, request, container_key: LibraryContainerLocator): """ - Add components to unit + Add items to container Example: POST /api/libraries/v2/containers//children/ Request body: @@ -239,12 +237,12 @@ def post(self, request, container_key: LibraryContainerLocator): @convert_exceptions @swagger_auto_schema( - request_body=serializers.ContentLibraryComponentKeysSerializer, + request_body=serializers.ContentLibraryItemContainerKeysSerializer, responses={200: serializers.LibraryContainerMetadataSerializer} ) def delete(self, request, container_key: LibraryContainerLocator): """ - Remove components from unit + Remove items from container Example: DELETE /api/libraries/v2/containers//children/ Request body: @@ -258,12 +256,12 @@ def delete(self, request, container_key: LibraryContainerLocator): @convert_exceptions @swagger_auto_schema( - request_body=serializers.ContentLibraryComponentKeysSerializer, + request_body=serializers.ContentLibraryItemContainerKeysSerializer, responses={200: serializers.LibraryContainerMetadataSerializer} ) def patch(self, request, container_key: LibraryContainerLocator): """ - Replace components in unit, can be used to reorder components as well. + Replace items in container, can be used to reorder items as well. Example: PATCH /api/libraries/v2/containers//children/ Request body: diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index 38765f0b320f..6734babd00cc 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -339,14 +339,6 @@ def to_internal_value(self, value: str) -> LibraryUsageLocatorV2: raise ValidationError from err -class ContentLibraryComponentKeysSerializer(serializers.Serializer): - """ - Serializer for adding/removing Components to/from a Collection. - """ - - usage_keys = serializers.ListField(child=UsageKeyV2Serializer(), allow_empty=False) - - class OpaqueKeySerializer(serializers.BaseSerializer): """ Serializes a OpaqueKey with the correct class. @@ -372,6 +364,14 @@ def to_internal_value(self, value: str) -> OpaqueKey: raise ValidationError from err +class ContentLibraryItemContainerKeysSerializer(serializers.Serializer): + """ + Serializer for adding/removing items to/from a Container. + """ + + usage_keys = serializers.ListField(child=OpaqueKeySerializer(), allow_empty=False) + + class ContentLibraryItemKeysSerializer(serializers.Serializer): """ Serializer for adding/removing items to/from a Collection. diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index b472126e8ce7..f5c89bb37ddb 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -99,7 +99,7 @@ def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None # Publishing a container will auto-publish its children, but publishing a single component or all changes # in the library will NOT usually include any parent containers. But we do need to notify listeners that the # parent container(s) have changed, e.g. so the search index can update the "has_unpublished_changes" - for parent_container in api.get_containers_contains_component(usage_key): + for parent_container in api.get_containers_contains_item(usage_key): affected_containers.add(parent_container.container_key) # TODO: should this be a CONTAINER_CHILD_PUBLISHED event instead of CONTAINER_PUBLISHED ? elif hasattr(record.entity, "container"): @@ -181,7 +181,8 @@ def send_events_after_revert(draft_change_log_id: int, library_key_str: str) -> # If any containers contain this component, their child list / component count may need to be updated # e.g. if this was a newly created component in the container and is now deleted, or this was deleted and # is now restored. - for parent_container in api.get_containers_contains_component(usage_key): + for parent_container in api.get_containers_contains_item(usage_key): + print(parent_container) updated_container_keys.add(parent_container.container_key) # TODO: do we also need to send CONTENT_OBJECT_ASSOCIATIONS_CHANGED for this component, or is diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index e5a9f5f12ec9..0036c208b0c7 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -38,7 +38,7 @@ 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_CONTAINER_COMPONENTS = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container +URL_LIB_CONTAINER_CHILDREN = URL_LIB_CONTAINER + 'children/' # Get, add or delete a component in this container URL_LIB_CONTAINER_RESTORE = URL_LIB_CONTAINER + 'restore/' # Restore a deleted container URL_LIB_CONTAINER_COLLECTIONS = URL_LIB_CONTAINER + 'collections/' # Handle associated collections URL_LIB_CONTAINER_PUBLISH = URL_LIB_CONTAINER + 'publish/' # Publish changes to the specified container + children @@ -396,25 +396,25 @@ def _restore_container(self, container_key: ContainerKey | str, expect_response= """ Restore a deleted a container (unit etc.) """ return self._api('post', URL_LIB_CONTAINER_RESTORE.format(container_key=container_key), None, expect_response) - def _get_container_components(self, container_key: ContainerKey | str, expect_response=200): - """ Get container components""" + def _get_container_children(self, container_key: ContainerKey | str, expect_response=200): + """ Get container children""" return self._api( 'get', - URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key), None, expect_response ) - def _add_container_components( + def _add_container_children( self, container_key: ContainerKey | str, children_ids: list[str], expect_response=200, ): - """ Add container components""" + """ Add container children""" return self._api( 'post', - URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key), {'usage_keys': children_ids}, expect_response ) @@ -428,7 +428,7 @@ def _remove_container_components( """ Remove container components""" return self._api( 'delete', - URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key), {'usage_keys': children_ids}, expect_response ) @@ -442,7 +442,7 @@ def _patch_container_components( """ Update container components""" return self._api( 'patch', - URL_LIB_CONTAINER_COMPONENTS.format(container_key=container_key), + URL_LIB_CONTAINER_CHILDREN.format(container_key=container_key), {'usage_keys': children_ids}, 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 085f69d0a381..84f9ad218e24 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -743,6 +743,39 @@ def setUp(self): # Create Units self.unit1 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-1', 'Unit 1', None) self.unit2 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-2', 'Unit 2', None) + self.unit3 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-3', 'Unit 3', None) + + # Create Subsections + self.subsection1 = api.create_container( + self.lib1.library_key, + api.ContainerType.Subsection, + 'subsection-1', + 'Subsection 1', + None, + ) + self.subsection2 = api.create_container( + self.lib1.library_key, + api.ContainerType.Subsection, + 'subsection-2', + 'Subsection 2', + None, + ) + + # Create Sections + self.section1 = api.create_container( + self.lib1.library_key, + api.ContainerType.Section, + 'section-1', + 'Section 1', + None, + ) + self.section2 = api.create_container( + self.lib1.library_key, + api.ContainerType.Section, + 'section-2', + 'Section 2', + None, + ) # Create XBlocks # Create some library blocks in lib1 @@ -750,6 +783,9 @@ def setUp(self): self.lib1.library_key, "problem", "problem1", ) self.problem_block_usage_key = UsageKey.from_string(self.problem_block["id"]) + self.problem_block_2 = self._add_block_to_library( + self.lib1.library_key, "problem", "problem2", + ) self.html_block = self._add_block_to_library( self.lib1.library_key, "html", "html1", ) @@ -767,9 +803,37 @@ def setUp(self): None, ) - def test_get_containers_contains_component(self): - problem_block_containers = api.get_containers_contains_component(self.problem_block_usage_key) - html_block_containers = api.get_containers_contains_component(self.html_block_usage_key) + # Add units to subsections + api.update_container_children( + self.subsection1.container_key, + [self.unit1.container_key, self.unit2.container_key], + None, + ) + api.update_container_children( + self.subsection2.container_key, + [self.unit1.container_key], + None, + ) + + # Add subsections to sections + api.update_container_children( + self.section1.container_key, + [self.subsection1.container_key, self.subsection2.container_key], + None, + ) + api.update_container_children( + self.section2.container_key, + [self.subsection1.container_key], + None, + ) + + def test_get_containers_contains_item(self): + problem_block_containers = api.get_containers_contains_item(self.problem_block_usage_key) + html_block_containers = api.get_containers_contains_item(self.html_block_usage_key) + unit_1_containers = api.get_containers_contains_item(self.unit1.container_key) + unit_2_containers = api.get_containers_contains_item(self.unit2.container_key) + subsection_1_containers = api.get_containers_contains_item(self.subsection1.container_key) + subsection_2_containers = api.get_containers_contains_item(self.subsection2.container_key) assert len(problem_block_containers) == 1 assert problem_block_containers[0].container_key == self.unit1.container_key @@ -778,6 +842,20 @@ def test_get_containers_contains_component(self): assert html_block_containers[0].container_key == self.unit1.container_key assert html_block_containers[1].container_key == self.unit2.container_key + assert len(unit_1_containers) == 2 + assert unit_1_containers[0].container_key == self.subsection1.container_key + assert unit_1_containers[1].container_key == self.subsection2.container_key + + assert len(unit_2_containers) == 1 + assert unit_2_containers[0].container_key == self.subsection1.container_key + + assert len(subsection_1_containers) == 2 + assert subsection_1_containers[0].container_key == self.section1.container_key + assert subsection_1_containers[1].container_key == self.section2.container_key + + assert len(subsection_2_containers) == 1 + assert subsection_2_containers[0].container_key == self.section1.container_key + def _validate_calls_of_html_block(self, event_mock): """ Validate that the `event_mock` has been called twice @@ -922,13 +1000,13 @@ def test_delete_component_and_revert(self): will be emitted to update any containing containers. """ # Add components and publish - api.update_container_children(self.unit1.container_key, [ - UsageKey.from_string(self.problem_block["id"]), + api.update_container_children(self.unit3.container_key, [ + UsageKey.from_string(self.problem_block_2["id"]), ], user_id=None) api.publish_changes(self.lib1.library_key) # Delete component and revert - api.delete_library_block(UsageKey.from_string(self.problem_block["id"])) + api.delete_library_block(UsageKey.from_string(self.problem_block_2["id"])) container_event_receiver = mock.Mock() LIBRARY_CONTAINER_UPDATED.connect(container_event_receiver) @@ -939,5 +1017,5 @@ def test_delete_component_and_revert(self): assert { "signal": LIBRARY_CONTAINER_UPDATED, "sender": None, - "library_container": LibraryContainerData(container_key=self.unit1.container_key), + "library_container": LibraryContainerData(container_key=self.unit3.container_key), }.items() <= container_event_receiver.call_args_list[0].kwargs.items() diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 6c59c8c086e4..e0fea5c861d4 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -35,21 +35,125 @@ class ContainersTestCase(ContentLibrariesRestApiTest): break any tests, but backwards-incompatible API changes will. """ - def test_unit_crud(self): + def setUp(self): + super().setUp() + self.create_date = datetime(2024, 9, 8, 7, 6, 5, tzinfo=timezone.utc) + self.lib = self._create_library( + slug="containers", + title="Container Test Library", + description="Units and more", + ) + self.lib_key = LibraryLocatorV2.from_string(self.lib["id"]) + + # Create containers + with freeze_time(self.create_date): + # Unit + self.unit = self._create_container(self.lib["id"], "unit", display_name="Alpha Bravo", slug=None) + self.unit_with_components = self._create_container( + self.lib["id"], + "unit", + display_name="Alpha Charly", + slug=None, + ) + self.unit_2 = self._create_container(self.lib["id"], "unit", display_name="Test Unit 2", slug=None) + self.unit_3 = self._create_container(self.lib["id"], "unit", display_name="Test Unit 3", slug=None) + + # Subsection + self.subsection = self._create_container( + self.lib["id"], + "subsection", + display_name="Subsection Alpha", + slug=None, + ) + self.subsection_with_units = self._create_container( + self.lib["id"], + "subsection", + display_name="Subsection with units", + slug=None, + ) + self.subsection_2 = self._create_container( + self.lib["id"], + "subsection", + display_name="Test Subsection 2", + slug=None, + ) + self.subsection_3 = self._create_container( + self.lib["id"], + "subsection", + display_name="Test Subsection 3", + slug=None, + ) + + # Section + self.section = self._create_container(self.lib["id"], "section", display_name="Section Alpha", slug=None) + self.section_with_subsections = self._create_container( + self.lib["id"], + "section", + display_name="Section with subsections", + slug=None, + ) + + # Create blocks + self.problem_block = self._add_block_to_library(self.lib["id"], "problem", "Problem1", can_stand_alone=False) + self.html_block = self._add_block_to_library(self.lib["id"], "html", "Html1", can_stand_alone=False) + self.problem_block_2 = self._add_block_to_library(self.lib["id"], "problem", "Problem2", can_stand_alone=False) + self.html_block_2 = self._add_block_to_library(self.lib["id"], "html", "Html2") + + # Add components to `unit_with_components` + self._add_container_children( + self.unit_with_components["id"], + children_ids=[ + self.problem_block["id"], + self.html_block["id"], + self.problem_block_2["id"], + self.html_block_2["id"], + ], + ) + # Add units to `subsection_with_units` + self._add_container_children( + self.subsection_with_units["id"], + children_ids=[ + self.unit["id"], + self.unit_with_components["id"], + self.unit_2["id"], + self.unit_3["id"], + ], + ) + # Add subsections to `section_with_subsections` + self._add_container_children( + self.section_with_subsections["id"], + children_ids=[ + self.subsection["id"], + self.subsection_with_units["id"], + self.subsection_2["id"], + self.subsection_3["id"], + ], + ) + + @ddt.data( + ("unit", "u1", "Test Unit"), + ("subsection", "subs1", "Test Subsection"), + ("section", "s1", "Test Section"), + ) + @ddt.unpack + def test_container_crud(self, container_type, slug, display_name): """ - Test Create, Read, Update, and Delete of a Unit + Test Create, Read, Update, and Delete of a Containers """ - lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") - lib_key = LibraryLocatorV2.from_string(lib["id"]) - - # Create a unit: + # Create container: 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") + container_data = self._create_container( + self.lib["id"], + container_type, + slug=slug, + display_name=display_name + ) + container_id = f"lct:CL-TEST:containers:{container_type}:{slug}" expected_data = { - "id": "lct:CL-TEST:containers:unit:u1", - "container_type": "unit", - "display_name": "Test Unit", + "id": container_id, + "container_type": container_type, + "display_name": display_name, "last_published": None, "published_by": "", "last_draft_created": "2024-09-08T07:06:05Z", @@ -62,94 +166,108 @@ def test_unit_crud(self): self.assertDictContainsEntries(container_data, expected_data) - # Fetch the unit: - unit_as_read = self._get_container(container_data["id"]) + # Fetch the container: + container_as_read = self._get_container(container_data["id"]) # make sure it contains the same data when we read it back: - self.assertDictContainsEntries(unit_as_read, expected_data) + self.assertDictContainsEntries(container_as_read, expected_data) - # Update the unit: + # Update the container: 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' + container_data = self._update_container(container_id, display_name=f"New Display Name for {container_type}") + expected_data["last_draft_created"] = expected_data["modified"] = "2024-10-09T08:07:06Z" + expected_data["display_name"] = f"New Display Name for {container_type}" self.assertDictContainsEntries(container_data, expected_data) - # Re-fetch the unit - unit_as_re_read = self._get_container(container_data["id"]) + # Re-fetch the container + container_as_re_read = self._get_container(container_data["id"]) # make sure it contains the same data when we read it back: - self.assertDictContainsEntries(unit_as_re_read, expected_data) + self.assertDictContainsEntries(container_as_re_read, expected_data) - # Delete the unit + # Delete the container self._delete_container(container_data["id"]) self._get_container(container_data["id"], expect_response=404) - def test_unit_permissions(self): + @ddt.data( + ("unit", "u2", "Test Unit"), + ("subsection", "subs2", "Test Subsection"), + ("section", "s2", "Test Section"), + ) + @ddt.unpack + def test_container_permissions(self, container_type, slug, display_name): """ - Test that a regular user with read-only permissions on the library cannot create, update, or delete units. + Test that a regular user with read-only permissions on the library cannot create, update, or delete containers. """ - 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") + container_data = self._create_container(self.lib["id"], container_type, slug=slug, display_name=display_name) 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._create_container( + self.lib["id"], + container_type, + slug="new_slug", + display_name=display_name, + expect_response=403, + ) self._get_container(container_data["id"], expect_response=403) - self._update_container(container_data["id"], display_name="Unit ABC", expect_response=403) + self._update_container(container_data["id"], display_name="New Display Name", expect_response=403) self._delete_container(container_data["id"], 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") + self._add_user_by_email(self.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._create_container( + self.lib["id"], + container_type, + slug=slug, + display_name=display_name, + expect_response=403, + ) self._get_container(container_data["id"], expect_response=200) - self._update_container(container_data["id"], display_name="Unit ABC", expect_response=403) + self._update_container(container_data["id"], display_name="New Display Name", expect_response=403) self._delete_container(container_data["id"], expect_response=403) - def test_unit_gets_auto_slugs(self): + @ddt.data( + ("unit", "Alpha Bravo", "lct:CL-TEST:containers:unit:alpha-bravo-"), + ("subsection", "Subsection Alpha", "lct:CL-TEST:containers:subsection:subsection-alpha-"), + ("section", "Section Alpha", "lct:CL-TEST:containers:section:section-alpha-"), + ) + @ddt.unpack + def test_containers_gets_auto_slugs(self, container_type, display_name, expected_id): """ - Test that we can create units by specifying only a title, and they get + Test that we can create containers 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") + container_1 = getattr(self, container_type) + container_2 = self._create_container(self.lib["id"], container_type, display_name=display_name, slug=None) - # 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["id"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") - assert container2_data["id"].startswith("lct:CL-TEST:containers:unit:alpha-bravo-") - assert container1_data["id"] != container2_data["id"] + assert container_1["id"].startswith(expected_id) + assert container_2["id"].startswith(expected_id) + assert container_1["id"] != container_2["id"] def test_unit_add_children(self): """ Test that we can add and get unit children components """ - lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") - lib_key = LibraryLocatorV2.from_string(lib["id"]) - - # Create container and add some components - container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) - problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) - html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) - self._add_container_components( - container_data["id"], - children_ids=[problem_block["id"], html_block["id"]] + # Add some components + self._add_container_children( + self.unit["id"], + children_ids=[self.problem_block["id"], self.html_block["id"]] ) - data = self._get_container_components(container_data["id"]) + data = self._get_container_children(self.unit["id"]) assert len(data) == 2 - assert data[0]['id'] == problem_block['id'] + assert data[0]['id'] == self.problem_block['id'] assert not data[0]['can_stand_alone'] - assert data[1]['id'] == html_block['id'] + assert data[1]['id'] == self.html_block['id'] assert not data[1]['can_stand_alone'] - problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) - html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") + problem_block_2 = self._add_block_to_library(self.lib["id"], "problem", "Problem_2", can_stand_alone=False) + html_block_2 = self._add_block_to_library(self.lib["id"], "html", "Html_2") # Add two more components - self._add_container_components( - container_data["id"], + self._add_container_children( + self.unit["id"], children_ids=[problem_block_2["id"], html_block_2["id"]] ) - data = self._get_container_components(container_data["id"]) + data = self._get_container_children(self.unit["id"]) # Verify total number of components to be 2 + 2 = 4 assert len(data) == 4 assert data[2]['id'] == problem_block_2['id'] @@ -157,105 +275,262 @@ def test_unit_add_children(self): assert data[3]['id'] == html_block_2['id'] assert data[3]['can_stand_alone'] - def test_unit_remove_children(self): + def test_subsection_add_children(self): + # Create units + child_unit_1 = self._create_container(self.lib["id"], "unit", display_name="Child unit 1", slug=None) + child_unit_2 = self._create_container(self.lib["id"], "unit", display_name="Child unit 2", slug=None) + + # Add the units to subsection + self._add_container_children( + self.subsection["id"], + children_ids=[child_unit_1["id"], child_unit_2["id"]] + ) + data = self._get_container_children(self.subsection["id"]) + assert len(data) == 2 + assert data[0]['id'] == child_unit_1['id'] + assert data[1]['id'] == child_unit_2['id'] + + child_unit_3 = self._create_container(self.lib["id"], "unit", display_name="Child unit 3", slug=None) + child_unit_4 = self._create_container(self.lib["id"], "unit", display_name="Child unit 4", slug=None) + + # Add two more units to subsection + self._add_container_children( + self.subsection["id"], + children_ids=[child_unit_3["id"], child_unit_4["id"]] + ) + data = self._get_container_children(self.subsection["id"]) + # Verify total number of units to be 2 + 2 = 4 + assert len(data) == 4 + assert data[2]['id'] == child_unit_3['id'] + assert data[3]['id'] == child_unit_4['id'] + + def test_section_add_children(self): + # Create Subsections + child_subsection_1 = self._create_container( + self.lib["id"], + "subsection", + display_name="Child Subsection 1", + slug=None, + ) + child_subsection_2 = self._create_container( + self.lib["id"], + "subsection", + display_name="Child Subsection 2", + slug=None, + ) + + # Add the subsections to section + self._add_container_children( + self.section["id"], + children_ids=[child_subsection_1["id"], child_subsection_2["id"]] + ) + data = self._get_container_children(self.section["id"]) + assert len(data) == 2 + assert data[0]['id'] == child_subsection_1['id'] + assert data[1]['id'] == child_subsection_2['id'] + + child_subsection_3 = self._create_container( + self.lib["id"], + "subsection", + display_name="Child Subsection 3", + slug=None, + ) + child_subsection_4 = self._create_container( + self.lib["id"], + "subsection", + display_name="Child Subsection 4", + slug=None, + ) + + # Add two more subsections to section + self._add_container_children( + self.section["id"], + children_ids=[child_subsection_3["id"], child_subsection_4["id"]] + ) + data = self._get_container_children(self.section["id"]) + # Verify total number of subsections to be 2 + 2 = 4 + assert len(data) == 4 + assert data[2]['id'] == child_subsection_3['id'] + assert data[3]['id'] == child_subsection_4['id'] + + @ddt.data( + ("unit_with_components", ["problem_block_2", "problem_block"], ["html_block", "html_block_2"]), + ("subsection_with_units", ["unit", "unit_with_components"], ["unit_2", "unit_3"]), + ("section_with_subsections", ["subsection", "subsection_with_units"], ["subsection_2", "subsection_3"]), + ) + @ddt.unpack + def test_container_remove_children(self, container_name, items_to_remove, expected_items): """ - Test that we can remove unit children components + Test that we can remove container children """ - lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") - lib_key = LibraryLocatorV2.from_string(lib["id"]) - - # Create container and add some components - container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) - problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) - html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) - problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) - html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") - self._add_container_components( - container_data["id"], - children_ids=[problem_block["id"], html_block["id"], problem_block_2["id"], html_block_2["id"]] - ) - data = self._get_container_components(container_data["id"]) + container = getattr(self, container_name) + item_to_remove_1 = getattr(self, items_to_remove[0]) + item_to_remove_2 = getattr(self, items_to_remove[1]) + expected_item_1 = getattr(self, expected_items[0]) + expected_item_2 = getattr(self, expected_items[1]) + data = self._get_container_children(container["id"]) assert len(data) == 4 - # Remove both problem blocks. + # Remove items. self._remove_container_components( - container_data["id"], - children_ids=[problem_block_2["id"], problem_block["id"]] + container["id"], + children_ids=[item_to_remove_1["id"], item_to_remove_2["id"]] ) - data = self._get_container_components(container_data["id"]) + data = self._get_container_children(container["id"]) assert len(data) == 2 - assert data[0]['id'] == html_block['id'] - assert data[1]['id'] == html_block_2['id'] + assert data[0]['id'] == expected_item_1['id'] + assert data[1]['id'] == expected_item_2['id'] def test_unit_replace_children(self): """ Test that we can completely replace/reorder unit children components. """ - lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") - lib_key = LibraryLocatorV2.from_string(lib["id"]) - - # Create container and add some components - container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) - problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) - html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) - problem_block_2 = self._add_block_to_library(lib["id"], "problem", "Problem2", can_stand_alone=False) - html_block_2 = self._add_block_to_library(lib["id"], "html", "Html2") - self._add_container_components( - container_data["id"], - children_ids=[problem_block["id"], html_block["id"], problem_block_2["id"], html_block_2["id"]] - ) - data = self._get_container_components(container_data["id"]) + data = self._get_container_children(self.unit_with_components["id"]) assert len(data) == 4 - assert data[0]['id'] == problem_block['id'] - assert data[1]['id'] == html_block['id'] - assert data[2]['id'] == problem_block_2['id'] - assert data[3]['id'] == html_block_2['id'] + assert data[0]['id'] == self.problem_block['id'] + assert data[1]['id'] == self.html_block['id'] + assert data[2]['id'] == self.problem_block_2['id'] + assert data[3]['id'] == self.html_block_2['id'] # Reorder the components self._patch_container_components( - container_data["id"], - children_ids=[problem_block["id"], problem_block_2["id"], html_block["id"], html_block_2["id"]] + self.unit_with_components["id"], + children_ids=[ + self.problem_block["id"], + self.problem_block_2["id"], + self.html_block["id"], + self.html_block_2["id"], + ] ) - data = self._get_container_components(container_data["id"]) + data = self._get_container_children(self.unit_with_components["id"]) assert len(data) == 4 - assert data[0]['id'] == problem_block['id'] - assert data[1]['id'] == problem_block_2['id'] - assert data[2]['id'] == html_block['id'] - assert data[3]['id'] == html_block_2['id'] + assert data[0]['id'] == self.problem_block['id'] + assert data[1]['id'] == self.problem_block_2['id'] + assert data[2]['id'] == self.html_block['id'] + assert data[3]['id'] == self.html_block_2['id'] # Replace with new components - new_problem_block = self._add_block_to_library(lib["id"], "problem", "New_Problem", can_stand_alone=False) - new_html_block = self._add_block_to_library(lib["id"], "html", "New_Html", can_stand_alone=False) + new_problem_block = self._add_block_to_library(self.lib["id"], "problem", "New_Problem", can_stand_alone=False) + new_html_block = self._add_block_to_library(self.lib["id"], "html", "New_Html", can_stand_alone=False) self._patch_container_components( - container_data["id"], + self.unit_with_components["id"], children_ids=[new_problem_block["id"], new_html_block["id"]], ) - data = self._get_container_components(container_data["id"]) + data = self._get_container_children(self.unit_with_components["id"]) assert len(data) == 2 assert data[0]['id'] == new_problem_block['id'] assert data[1]['id'] == new_html_block['id'] - def test_restore_unit(self): + def test_subsection_replace_children(self): + """ + Test that we can completely replace/reorder subsection children. + """ + data = self._get_container_children(self.subsection_with_units["id"]) + assert len(data) == 4 + assert data[0]['id'] == self.unit['id'] + assert data[1]['id'] == self.unit_with_components['id'] + assert data[2]['id'] == self.unit_2['id'] + assert data[3]['id'] == self.unit_3['id'] + + # Reorder the units + self._patch_container_components( + self.subsection_with_units["id"], + children_ids=[ + self.unit_2["id"], + self.unit["id"], + self.unit_3["id"], + self.unit_with_components["id"], + ] + ) + data = self._get_container_children(self.subsection_with_units["id"]) + assert len(data) == 4 + assert data[0]['id'] == self.unit_2['id'] + assert data[1]['id'] == self.unit['id'] + assert data[2]['id'] == self.unit_3['id'] + assert data[3]['id'] == self.unit_with_components['id'] + + # Replace with new units + new_unit_1 = self._create_container(self.lib["id"], "unit", display_name="New Unit 1", slug=None) + new_unit_2 = self._create_container(self.lib["id"], "unit", display_name="New Unit 2", slug=None) + self._patch_container_components( + self.subsection_with_units["id"], + children_ids=[new_unit_1["id"], new_unit_2["id"]], + ) + data = self._get_container_children(self.subsection_with_units["id"]) + assert len(data) == 2 + assert data[0]['id'] == new_unit_1['id'] + assert data[1]['id'] == new_unit_2['id'] + + def test_section_replace_children(self): """ - Test restore a deleted unit. + Test that we can completely replace/reorder section children. """ - lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") - lib_key = LibraryLocatorV2.from_string(lib["id"]) + data = self._get_container_children(self.section_with_subsections["id"]) + assert len(data) == 4 + assert data[0]['id'] == self.subsection['id'] + assert data[1]['id'] == self.subsection_with_units['id'] + assert data[2]['id'] == self.subsection_2['id'] + assert data[3]['id'] == self.subsection_3['id'] - # 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") + # Reorder the subsections + self._patch_container_components( + self.section_with_subsections["id"], + children_ids=[ + self.subsection_2["id"], + self.subsection["id"], + self.subsection_3["id"], + self.subsection_with_units["id"], + ] + ) + data = self._get_container_children(self.section_with_subsections["id"]) + assert len(data) == 4 + assert data[0]['id'] == self.subsection_2['id'] + assert data[1]['id'] == self.subsection['id'] + assert data[2]['id'] == self.subsection_3['id'] + assert data[3]['id'] == self.subsection_with_units['id'] + + # Replace with new subsections + new_subsection_1 = self._create_container( + self.lib["id"], + "subsection", + display_name="New Subsection 1", + slug=None, + ) + new_subsection_2 = self._create_container( + self.lib["id"], + "subsection", + display_name="New Subsection 2", + slug=None, + ) + self._patch_container_components( + self.section_with_subsections["id"], + children_ids=[new_subsection_1["id"], new_subsection_2["id"]], + ) + data = self._get_container_children(self.section_with_subsections["id"]) + assert len(data) == 2 + assert data[0]['id'] == new_subsection_1['id'] + assert data[1]['id'] == new_subsection_2['id'] + + @ddt.data( + "unit", + "subsection", + "section", + ) + def test_restore_containers(self, container_type): + """ + Test restore a deleted container. + """ + container = getattr(self, container_type) - # Delete the unit - self._delete_container(container_data["id"]) + # Delete container + self._delete_container(container["id"]) # Restore container - self._restore_container(container_data["id"]) - new_container_data = self._get_container(container_data["id"]) + self._restore_container(container["id"]) + new_container_data = self._get_container(container["id"]) expected_data = { - "id": "lct:CL-TEST:containers:unit:u1", - "container_type": "unit", - "display_name": "Test Unit", + "id": container["id"], + "container_type": container_type, + "display_name": container["display_name"], "last_published": None, "published_by": "", "last_draft_created": "2024-09-08T07:06:05Z", @@ -266,17 +541,12 @@ def test_restore_unit(self): 'collections': [], } - def test_container_collections(self): - # Create a library - lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") - lib_key = LibraryLocatorV2.from_string(lib["id"]) - - # Create a unit - container_data = self._create_container(lib["id"], "unit", display_name="Alpha Bravo", slug=None) + self.assertDictContainsEntries(new_container_data, expected_data) + def test_container_collections(self): # Create a collection col1 = api.create_library_collection( - lib_key, + self.lib_key, "COL1", title="Collection 1", created_by=self.user.id, @@ -284,14 +554,14 @@ def test_container_collections(self): ) result = self._patch_container_collections( - container_data["id"], + self.unit["id"], collection_keys=[col1.key], ) assert result['count'] == 1 # Fetch the unit - unit_as_read = self._get_container(container_data["id"]) + unit_as_read = self._get_container(self.unit["id"]) # Verify the collections assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.key}] @@ -300,53 +570,64 @@ def test_publish_container(self): # pylint: disable=too-many-statements """ Test that we can publish the changes to a specific container """ - lib = self._create_library(slug="containers", title="Container Test Library", description="Units and more") - - # Create two containers and add some components - container1 = self._create_container(lib["id"], "unit", display_name="Alpha Unit", slug=None) - container2 = self._create_container(lib["id"], "unit", display_name="Bravo Unit", slug=None) - problem_block = self._add_block_to_library(lib["id"], "problem", "Problem1", can_stand_alone=False) - html_block = self._add_block_to_library(lib["id"], "html", "Html1", can_stand_alone=False) - html_block2 = self._add_block_to_library(lib["id"], "html", "Html2", can_stand_alone=False) - self._add_container_components(container1["id"], children_ids=[problem_block["id"], html_block["id"]]) - self._add_container_components(container2["id"], children_ids=[html_block["id"], html_block2["id"]]) + html_block_3 = self._add_block_to_library(self.lib["id"], "html", "Html3") + self._add_container_children( + self.unit["id"], + children_ids=[ + self.html_block["id"], + html_block_3["id"], + ] + ) + # At first everything is unpublished: - c1_before = self._get_container(container1["id"]) + c1_before = self._get_container(self.unit_with_components["id"]) assert c1_before["has_unpublished_changes"] - c1_components_before = self._get_container_components(container1["id"]) - assert len(c1_components_before) == 2 - assert c1_components_before[0]["id"] == problem_block["id"] + c1_components_before = self._get_container_children(self.unit_with_components["id"]) + assert len(c1_components_before) == 4 + assert c1_components_before[0]["id"] == self.problem_block["id"] assert c1_components_before[0]["has_unpublished_changes"] assert c1_components_before[0]["published_by"] is None - assert c1_components_before[1]["id"] == html_block["id"] + assert c1_components_before[1]["id"] == self.html_block["id"] assert c1_components_before[1]["has_unpublished_changes"] assert c1_components_before[1]["published_by"] is None - c2_before = self._get_container(container2["id"]) + assert c1_components_before[2]["id"] == self.problem_block_2["id"] + assert c1_components_before[2]["has_unpublished_changes"] + assert c1_components_before[2]["published_by"] is None + assert c1_components_before[3]["id"] == self.html_block_2["id"] + assert c1_components_before[3]["has_unpublished_changes"] + assert c1_components_before[3]["published_by"] is None + c2_before = self._get_container(self.unit["id"]) assert c2_before["has_unpublished_changes"] # Now publish only Container 1 - self._publish_container(container1["id"]) + self._publish_container(self.unit_with_components["id"]) # Now it is published: - c1_after = self._get_container(container1["id"]) + c1_after = self._get_container(self.unit_with_components["id"]) assert c1_after["has_unpublished_changes"] is False - c1_components_after = self._get_container_components(container1["id"]) - assert len(c1_components_after) == 2 - assert c1_components_after[0]["id"] == problem_block["id"] + c1_components_after = self._get_container_children(self.unit_with_components["id"]) + assert len(c1_components_after) == 4 + assert c1_components_after[0]["id"] == self.problem_block["id"] assert c1_components_after[0]["has_unpublished_changes"] is False assert c1_components_after[0]["published_by"] == self.user.username - assert c1_components_after[1]["id"] == html_block["id"] + assert c1_components_after[1]["id"] == self.html_block["id"] assert c1_components_after[1]["has_unpublished_changes"] is False assert c1_components_after[1]["published_by"] == self.user.username + assert c1_components_after[2]["id"] == self.problem_block_2["id"] + assert c1_components_after[2]["has_unpublished_changes"] is False + assert c1_components_after[2]["published_by"] == self.user.username + assert c1_components_after[3]["id"] == self.html_block_2["id"] + assert c1_components_after[3]["has_unpublished_changes"] is False + assert c1_components_after[3]["published_by"] == self.user.username # and container 2 is still unpublished, except for the shared HTML block that is also in container 1: - c2_after = self._get_container(container2["id"]) + c2_after = self._get_container(self.unit["id"]) assert c2_after["has_unpublished_changes"] - c2_components_after = self._get_container_components(container2["id"]) + c2_components_after = self._get_container_children(self.unit["id"]) assert len(c2_components_after) == 2 - assert c2_components_after[0]["id"] == html_block["id"] + assert c2_components_after[0]["id"] == self.html_block["id"] assert c2_components_after[0]["has_unpublished_changes"] is False # published since it's also in container 1 assert c2_components_after[0]["published_by"] == self.user.username - assert c2_components_after[1]["id"] == html_block2["id"] + assert c2_components_after[1]["id"] == html_block_3["id"] assert c2_components_after[1]["has_unpublished_changes"] # unaffected assert c2_components_after[1]["published_by"] is None diff --git a/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py b/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py index 8f45fdd8cc9d..5c1bd5774403 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_course_to_library.py @@ -58,7 +58,7 @@ def test_library_paste_unit_from_course(self): "can_stand_alone": True, }) - children = self._get_container_components(paste_data["id"]) + children = self._get_container_children(paste_data["id"]) assert len(children) == 4 self.assertDictContainsEntries(children[0], {"display_name": "default", "block_type": "video"}) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_events.py b/openedx/core/djangoapps/content_libraries/tests/test_events.py index e0e3e7392765..88d426d3ef06 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_events.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_events.py @@ -308,8 +308,8 @@ def test_publish_all_lib_changes(self) -> None: problem_block = self._add_block_to_library(self.lib1_key, "problem", "Problem1", can_stand_alone=False) html_block = self._add_block_to_library(self.lib1_key, "html", "Html1", can_stand_alone=False) html_block2 = self._add_block_to_library(self.lib1_key, "html", "Html2", can_stand_alone=False) - self._add_container_components(container1["id"], children_ids=[problem_block["id"], html_block["id"]]) - self._add_container_components(container2["id"], children_ids=[html_block["id"], html_block2["id"]]) + self._add_container_children(container1["id"], children_ids=[problem_block["id"], html_block["id"]]) + self._add_container_children(container2["id"], children_ids=[html_block["id"], html_block2["id"]]) # Now publish only Container 2 (which will auto-publish both HTML blocks since they're children) self._publish_container(container2["id"]) @@ -351,7 +351,7 @@ def test_publish_child_block(self) -> None: # Create a container and a block container1 = self._create_container(self.lib1_key, "unit", display_name="Alpha Unit", slug=None) problem_block = self._add_block_to_library(self.lib1_key, "problem", "Problem1", can_stand_alone=False) - self._add_container_components(container1["id"], children_ids=[problem_block["id"]]) + self._add_container_children(container1["id"], children_ids=[problem_block["id"]]) # Publish all changes self._commit_library_changes(self.lib1_key) assert self._get_container(container1["id"])["has_unpublished_changes"] is False @@ -396,8 +396,8 @@ def test_publish_container(self) -> None: problem_block = self._add_block_to_library(self.lib1_key, "problem", "Problem1", can_stand_alone=False) html_block = self._add_block_to_library(self.lib1_key, "html", "Html1", can_stand_alone=False) html_block2 = self._add_block_to_library(self.lib1_key, "html", "Html2", can_stand_alone=False) - self._add_container_components(container1["id"], children_ids=[problem_block["id"], html_block["id"]]) - self._add_container_components(container2["id"], children_ids=[html_block["id"], html_block2["id"]]) + self._add_container_children(container1["id"], children_ids=[problem_block["id"], html_block["id"]]) + self._add_container_children(container2["id"], children_ids=[html_block["id"], html_block2["id"]]) # At first everything is unpublished: c1_before = self._get_container(container1["id"]) assert c1_before["has_unpublished_changes"] diff --git a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py index 283f5fbe97e5..ce9b0e880dee 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_views_collections.py @@ -74,6 +74,14 @@ def setUp(self): self.lib2_html_block = self._add_block_to_library( self.lib2.library_key, "html", "html2", ) + self.unit = self._create_container(self.lib1.library_key, "unit", display_name="Unit 1", slug=None) + self.subsection = self._create_container( + self.lib1.library_key, + "subsection", + display_name="Subsection 1", + slug=None, + ) + self.section = self._create_container(self.lib1.library_key, "section", display_name="Section 1", slug=None) def test_get_library_collection(self): """ @@ -407,6 +415,43 @@ def test_update_components(self): assert resp.status_code == 200 assert resp.data == {"count": 1} + def test_update_containers(self): + """ + Test adding and removing containers from a collection. + """ + # Add containers to col1 + resp = self.client.patch( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib1.library_key, + collection_key=self.col1.key, + ), + data={ + "usage_keys": [ + self.unit["id"], + self.subsection["id"], + self.section["id"], + ] + } + ) + assert resp.status_code == 200 + assert resp.data == {"count": 3} + + # Remove one of the added containers from col1 + resp = self.client.delete( + URL_LIB_COLLECTION_COMPONENTS.format( + lib_key=self.lib1.library_key, + collection_key=self.col1.key, + ), + data={ + "usage_keys": [ + self.unit["id"], + self.subsection["id"], + ] + } + ) + assert resp.status_code == 200 + assert resp.data == {"count": 2} + @ddt.data("patch", "delete") def test_update_components_wrong_collection(self, method): """