diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py index 1541a0e5e0ba..b7890f821dcc 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py @@ -44,7 +44,7 @@ def setUp(self): 'This is the HTML.' ) self.upstream_unit = self._create_container(lib_id, "unit", slug="u1", display_name="Unit 1 Title") - self._add_container_components(self.upstream_unit["id"], [ + self._add_container_children(self.upstream_unit["id"], [ self.upstream_html1["id"], self.upstream_problem1["id"], self.upstream_problem2["id"], @@ -362,7 +362,7 @@ def test_unit_sync(self): upstream_problem3["id"], 'single select...' ) - self._add_container_components(self.upstream_unit["id"], [upstream_problem3["id"]]) + self._add_container_children(self.upstream_unit["id"], [upstream_problem3["id"]]) self._remove_container_components(self.upstream_unit["id"], [self.upstream_problem2["id"]]) self._commit_library_changes(self.library["id"]) # publish everything diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 36698da6a091..34a3c86b375e 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) -> list[str]: + 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..3b22ce389d0f 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -47,7 +47,8 @@ class TestSearchApi(ModuleStoreTestCase): MODULESTORE = TEST_DATA_SPLIT_MODULESTORE - def setUp(self): + def setUp(self) -> None: + # 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,16 +267,56 @@ 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): + def test_reindex_meilisearch_disabled(self, mock_meilisearch) -> None: with self.assertRaises(RuntimeError): api.rebuild_index() mock_meilisearch.return_value.swap_indexes.assert_not_called() @override_settings(MEILISEARCH_ENABLED=True) - def test_reindex_meilisearch(self, mock_meilisearch): + def test_reindex_meilisearch(self, mock_meilisearch) -> None: # Add tags field to doc, since reindex calls includes tags doc_sequential = copy.deepcopy(self.doc_sequential) @@ -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,13 +350,13 @@ 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, ) @override_settings(MEILISEARCH_ENABLED=True) - def test_reindex_meilisearch_incremental(self, mock_meilisearch): + def test_reindex_meilisearch_incremental(self, mock_meilisearch) -> None: # Add tags field to doc, since reindex calls includes tags doc_sequential = copy.deepcopy(self.doc_sequential) @@ -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, ) @@ -343,7 +413,7 @@ def simulated_interruption(message): assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 8 @override_settings(MEILISEARCH_ENABLED=True) - def test_reset_meilisearch_index(self, mock_meilisearch): + def test_reset_meilisearch_index(self, mock_meilisearch) -> None: api.reset_index() mock_meilisearch.return_value.swap_indexes.assert_called_once() mock_meilisearch.return_value.create_index.assert_called_once() @@ -352,7 +422,7 @@ def test_reset_meilisearch_index(self, mock_meilisearch): mock_meilisearch.return_value.delete_index.call_count = 4 @override_settings(MEILISEARCH_ENABLED=True) - def test_init_meilisearch_index(self, mock_meilisearch): + def test_init_meilisearch_index(self, mock_meilisearch) -> None: # Test index already exists api.init_index() mock_meilisearch.return_value.swap_indexes.assert_not_called() @@ -383,7 +453,7 @@ def test_init_meilisearch_index(self, mock_meilisearch): "openedx.core.djangoapps.content.search.api.searchable_doc_for_collection", Mock(side_effect=Exception("Failed to generate document")), ) - def test_reindex_meilisearch_collection_error(self, mock_meilisearch): + def test_reindex_meilisearch_collection_error(self, mock_meilisearch) -> None: mock_logger = Mock() api.rebuild_index(mock_logger) @@ -399,7 +469,7 @@ def test_reindex_meilisearch_collection_error(self, mock_meilisearch): "openedx.core.djangoapps.content.search.api.searchable_doc_for_container", Mock(side_effect=Exception("Failed to generate document")), ) - def test_reindex_meilisearch_container_error(self, mock_meilisearch): + def test_reindex_meilisearch_container_error(self, mock_meilisearch) -> None: mock_logger = Mock() api.rebuild_index(mock_logger) @@ -411,7 +481,7 @@ def test_reindex_meilisearch_container_error(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_reindex_meilisearch_library_block_error(self, mock_meilisearch): + def test_reindex_meilisearch_library_block_error(self, mock_meilisearch) -> None: # Add tags field to doc, since reindex calls includes tags doc_sequential = copy.deepcopy(self.doc_sequential) @@ -469,7 +539,7 @@ def mocked_from_component(lib_key, component): False ) @override_settings(MEILISEARCH_ENABLED=True) - def test_index_xblock_metadata(self, recursive, mock_meilisearch): + def test_index_xblock_metadata(self, recursive, mock_meilisearch) -> None: """ Test indexing an XBlock. """ @@ -483,13 +553,13 @@ def test_index_xblock_metadata(self, recursive, mock_meilisearch): mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with(expected_docs) @override_settings(MEILISEARCH_ENABLED=True) - def test_no_index_excluded_xblocks(self, mock_meilisearch): + def test_no_index_excluded_xblocks(self, mock_meilisearch) -> None: api.upsert_xblock_index_doc(UsageKey.from_string(self.course_block_key)) mock_meilisearch.return_value.index.return_value.update_document.assert_not_called() @override_settings(MEILISEARCH_ENABLED=True) - def test_index_xblock_tags(self, mock_meilisearch): + def test_index_xblock_tags(self, mock_meilisearch) -> None: """ Test indexing an XBlock with tags. """ @@ -523,7 +593,7 @@ def test_index_xblock_tags(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_delete_index_xblock(self, mock_meilisearch): + def test_delete_index_xblock(self, mock_meilisearch) -> None: """ Test deleting an XBlock doc from the index. """ @@ -534,7 +604,7 @@ def test_delete_index_xblock(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_index_library_block_metadata(self, mock_meilisearch): + def test_index_library_block_metadata(self, mock_meilisearch) -> None: """ Test indexing a Library Block. """ @@ -543,7 +613,7 @@ def test_index_library_block_metadata(self, mock_meilisearch): mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.doc_problem1]) @override_settings(MEILISEARCH_ENABLED=True) - def test_index_library_block_tags(self, mock_meilisearch): + def test_index_library_block_tags(self, mock_meilisearch) -> None: """ Test indexing an Library Block with tags. """ @@ -578,7 +648,7 @@ def test_index_library_block_tags(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_index_library_block_and_collections(self, mock_meilisearch): + def test_index_library_block_and_collections(self, mock_meilisearch) -> None: """ Test indexing an Library Block and the Collections it's in. """ @@ -718,7 +788,7 @@ def test_index_library_block_and_collections(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_delete_index_library_block(self, mock_meilisearch): + def test_delete_index_library_block(self, mock_meilisearch) -> None: """ Test deleting a Library Block doc from the index. """ @@ -729,7 +799,7 @@ def test_delete_index_library_block(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_index_content_library_metadata(self, mock_meilisearch): + def test_index_content_library_metadata(self, mock_meilisearch) -> None: """ Test indexing a whole content library. """ @@ -740,7 +810,7 @@ def test_index_content_library_metadata(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_index_tags_in_collections(self, mock_meilisearch): + def test_index_tags_in_collections(self, mock_meilisearch) -> None: # Tag collection tagging_api.tag_object(str(self.collection_key), self.taxonomyA, ["one", "two"]) tagging_api.tag_object(str(self.collection_key), self.taxonomyB, ["three", "four"]) @@ -771,7 +841,7 @@ def test_index_tags_in_collections(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_delete_collection(self, mock_meilisearch): + def test_delete_collection(self, mock_meilisearch) -> None: """ Test soft-deleting, restoring, and hard-deleting a collection. """ @@ -894,19 +964,27 @@ 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) -> None: """ 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) - def test_index_library_container_metadata(self, mock_meilisearch): + def test_index_library_container_metadata(self, mock_meilisearch) -> None: """ Test indexing a Library Container. """ @@ -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) -> None: + 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'] @@ -946,7 +1032,7 @@ def test_index_tags_in_containers(self, mock_meilisearch): ) @override_settings(MEILISEARCH_ENABLED=True) - def test_block_in_units(self, mock_meilisearch): + def test_block_in_units(self, mock_meilisearch) -> None: with freeze_time(self.created_date): library_api.update_container_children( LibraryContainerLocator.from_string(self.unit_key), @@ -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) -> None: + 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) -> None: + 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..0ff45fa53e9b 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(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..3f6867e72daa 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -493,7 +493,7 @@ def _import_staged_block_as_container( title=title, user_id=user.id, ) - new_child_keys: list[UsageKeyV2] = [] + new_child_keys: list[LibraryUsageLocatorV2] = [] for child_node in olx_node: try: child_metadata = _import_staged_block( diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 01974a441ed2..8e2e486377da 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -10,7 +10,6 @@ from uuid import uuid4 from django.utils.text import slugify -from opaque_keys.edx.keys import UsageKeyV2 from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_events.content_authoring.data import ( ContentObjectChangedData, @@ -25,7 +24,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 from openedx.core.djangoapps.content_libraries.api.collections import library_collection_locator from openedx.core.djangoapps.xblock.api import get_component_from_usage_key @@ -159,11 +158,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 +228,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 +244,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,11 +258,27 @@ 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 _: - raise NotImplementedError(f"Library support for {container_type} is in progress") + raise NotImplementedError(f"Library does not support {container_type} yet") LIBRARY_CONTAINER_CREATED.send_event( library_container=LibraryContainerData( @@ -269,18 +295,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 does not support {container_type} yet") LIBRARY_CONTAINER_UPDATED.send_event( library_container=LibraryContainerData( @@ -288,14 +336,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 +427,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( @@ -409,7 +473,7 @@ def get_container_children_count( def update_container_children( container_key: LibraryContainerLocator, - children_ids: list[UsageKeyV2] | list[LibraryContainerLocator], + children_ids: list[LibraryUsageLocatorV2] | list[LibraryContainerLocator], user_id: int | None, entities_action: authoring_api.ChildrenEntitiesAction = authoring_api.ChildrenEntitiesAction.REPLACE, ): @@ -417,15 +481,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 +503,28 @@ def update_container_children( changes=["units"], ), ) + case ContainerType.Subsection: + units = [_get_container_from_key(key).unit for key in children_ids] # type: ignore[arg-type] + 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 = [_get_container_from_key(key).subsection for key in children_ids] # type: ignore[arg-type] + 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}") 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/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..9517e5d0d2f6 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -11,8 +11,9 @@ from opaque_keys.edx.keys import ( CourseKey, UsageKey, + UsageKeyV2, ) -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocatorV2, LibraryUsageLocatorV2 from openedx_events.content_authoring.data import ( ContentObjectChangedData, LibraryCollectionData, @@ -265,7 +266,7 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest): Same guidelines as ContentLibrariesTestCase. """ - def setUp(self): + def setUp(self) -> None: super().setUp() # Create Content Libraries @@ -317,7 +318,7 @@ def setUp(self): self.lib2.library_key, "problem", "problem2", ) - def test_create_library_collection(self): + def test_create_library_collection(self) -> None: event_receiver = mock.Mock() LIBRARY_COLLECTION_CREATED.connect(event_receiver) @@ -348,7 +349,7 @@ def test_create_library_collection(self): event_receiver.call_args_list[0].kwargs, ) - def test_create_library_collection_invalid_library(self): + def test_create_library_collection_invalid_library(self) -> None: library_key = LibraryLocatorV2.from_string("lib:INVALID:test-lib-does-not-exist") with self.assertRaises(api.ContentLibraryNotFound) as exc: api.create_library_collection( @@ -357,7 +358,7 @@ def test_create_library_collection_invalid_library(self): title="Collection 3", ) - def test_update_library_collection(self): + def test_update_library_collection(self) -> None: event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(event_receiver) @@ -386,17 +387,18 @@ def test_update_library_collection(self): event_receiver.call_args_list[0].kwargs, ) - def test_update_library_collection_wrong_library(self): + def test_update_library_collection_wrong_library(self) -> None: with self.assertRaises(api.ContentLibraryCollectionNotFound) as exc: api.update_library_collection( self.lib1.library_key, self.col2.key, ) - def test_delete_library_collection(self): + def test_delete_library_collection(self) -> None: event_receiver = mock.Mock() LIBRARY_COLLECTION_DELETED.connect(event_receiver) + assert self.lib1.learning_package_id is not None authoring_api.delete_collection( self.lib1.learning_package_id, self.col1.key, @@ -418,15 +420,15 @@ def test_delete_library_collection(self): event_receiver.call_args_list[0].kwargs, ) - def test_update_library_collection_items(self): + def test_update_library_collection_items(self) -> None: assert not list(self.col1.entities.all()) self.col1 = api.update_library_collection_items( self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), LibraryContainerLocator.from_string(self.unit1["id"]), ], ) @@ -436,13 +438,13 @@ def test_update_library_collection_items(self): self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), ], remove=True, ) assert len(self.col1.entities.all()) == 2 - def test_update_library_collection_components_event(self): + def test_update_library_collection_components_event(self) -> None: """ Check that a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event is raised for each added/removed component. """ @@ -454,8 +456,8 @@ def test_update_library_collection_components_event(self): self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), LibraryContainerLocator.from_string(self.unit1["id"]), ], ) @@ -508,32 +510,33 @@ def test_update_library_collection_components_event(self): event_receiver.call_args_list[3].kwargs, ) - def test_update_collection_components_from_wrong_library(self): + def test_update_collection_components_from_wrong_library(self) -> None: with self.assertRaises(api.ContentLibraryBlockNotFound) as exc: api.update_library_collection_items( self.lib2.library_key, self.col2.key, opaque_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), LibraryContainerLocator.from_string(self.unit1["id"]), ], ) assert self.lib1_problem_block["id"] in str(exc.exception) - def test_set_library_component_collections(self): + def test_set_library_component_collections(self) -> None: event_receiver = mock.Mock() CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_receiver) collection_update_event_receiver = mock.Mock() 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"])) + component = api.get_component_from_usage_key(UsageKeyV2.from_string(self.lib2_problem_block["id"])) api.set_library_item_collections( library_key=self.lib2.library_key, entity_key=component.publishable_entity.key, collection_keys=[self.col2.key, self.col3.key], ) + assert self.lib2.learning_package_id is not None assert len(authoring_api.get_collection(self.lib2.learning_package_id, self.col2.key).entities.all()) == 1 assert len(authoring_api.get_collection(self.lib2.learning_package_id, self.col3.key).entities.all()) == 1 assert { @@ -559,20 +562,20 @@ def test_set_library_component_collections(self): ) } - def test_delete_library_block(self): + def test_delete_library_block(self) -> None: api.update_library_collection_items( self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), ], ) event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(event_receiver) - api.delete_library_block(UsageKey.from_string(self.lib1_problem_block["id"])) + api.delete_library_block(LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"])) assert event_receiver.call_count == 1 self.assertDictContainsSubset( @@ -590,13 +593,13 @@ def test_delete_library_block(self): event_receiver.call_args_list[0].kwargs, ) - def test_delete_library_container(self): + def test_delete_library_container(self) -> None: api.update_library_collection_items( self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), LibraryContainerLocator.from_string(self.unit1["id"]), ], ) @@ -622,20 +625,20 @@ def test_delete_library_container(self): event_receiver.call_args_list[0].kwargs, ) - def test_restore_library_block(self): + def test_restore_library_block(self) -> None: api.update_library_collection_items( self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), ], ) event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(event_receiver) - api.restore_library_block(UsageKey.from_string(self.lib1_problem_block["id"])) + api.restore_library_block(LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"])) assert event_receiver.call_count == 1 self.assertDictContainsSubset( @@ -653,7 +656,7 @@ def test_restore_library_block(self): event_receiver.call_args_list[0].kwargs, ) - def test_add_component_and_revert(self): + def test_add_component_and_revert(self) -> None: # Publish changes api.publish_changes(self.lib1.library_key) @@ -667,8 +670,8 @@ def test_add_component_and_revert(self): self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_html_block["id"]), - UsageKey.from_string(new_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), + LibraryUsageLocatorV2.from_string(new_problem_block["id"]), ], ) @@ -689,7 +692,7 @@ def test_add_component_and_revert(self): ), }.items() <= collection_update_event_receiver.call_args_list[0].kwargs.items() - def test_delete_component_and_revert(self): + def test_delete_component_and_revert(self) -> None: """ When a component is deleted and then the delete is reverted, signals will be emitted to update any containing collections. @@ -699,14 +702,14 @@ def test_delete_component_and_revert(self): self.lib1.library_key, self.col1.key, opaque_keys=[ - UsageKey.from_string(self.lib1_problem_block["id"]), - UsageKey.from_string(self.lib1_html_block["id"]) + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]) ], ) api.publish_changes(self.lib1.library_key) # Delete component and revert - api.delete_library_block(UsageKey.from_string(self.lib1_problem_block["id"])) + api.delete_library_block(LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"])) collection_update_event_receiver = mock.Mock() LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver) @@ -731,7 +734,7 @@ class ContentLibraryContainersTest(ContentLibrariesRestApiTest): Tests for Content Library API containers methods. """ - def setUp(self): + def setUp(self) -> None: super().setUp() # Create Content Libraries @@ -749,11 +752,11 @@ def setUp(self): self.problem_block = self._add_block_to_library( self.lib1.library_key, "problem", "problem1", ) - self.problem_block_usage_key = UsageKey.from_string(self.problem_block["id"]) + self.problem_block_usage_key = LibraryUsageLocatorV2.from_string(self.problem_block["id"]) self.html_block = self._add_block_to_library( self.lib1.library_key, "html", "html1", ) - self.html_block_usage_key = UsageKey.from_string(self.html_block["id"]) + self.html_block_usage_key = LibraryUsageLocatorV2.from_string(self.html_block["id"]) # Add content to units api.update_container_children( @@ -767,7 +770,7 @@ def setUp(self): None, ) - def test_get_containers_contains_component(self): + def test_get_containers_contains_component(self) -> None: 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) @@ -778,7 +781,7 @@ 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 - def _validate_calls_of_html_block(self, event_mock): + def _validate_calls_of_html_block(self, event_mock) -> None: """ Validate that the `event_mock` has been called twice using the `LIBRARY_CONTAINER_UPDATED` signal. @@ -807,14 +810,14 @@ def _validate_calls_of_html_block(self, event_mock): event_mock.call_args_list[1].kwargs, ) - def test_call_container_update_signal_when_delete_component(self): + def test_call_container_update_signal_when_delete_component(self) -> None: container_update_event_receiver = mock.Mock() LIBRARY_CONTAINER_UPDATED.connect(container_update_event_receiver) api.delete_library_block(self.html_block_usage_key) self._validate_calls_of_html_block(container_update_event_receiver) - def test_call_container_update_signal_when_restore_component(self): + def test_call_container_update_signal_when_restore_component(self) -> None: api.delete_library_block(self.html_block_usage_key) container_update_event_receiver = mock.Mock() @@ -823,7 +826,7 @@ def test_call_container_update_signal_when_restore_component(self): self._validate_calls_of_html_block(container_update_event_receiver) - def test_call_container_update_signal_when_update_olx(self): + def test_call_container_update_signal_when_update_olx(self) -> None: block_olx = "Hello world!" container_update_event_receiver = mock.Mock() LIBRARY_CONTAINER_UPDATED.connect(container_update_event_receiver) @@ -831,7 +834,7 @@ def test_call_container_update_signal_when_update_olx(self): self._set_library_block_olx(self.html_block_usage_key, block_olx) self._validate_calls_of_html_block(container_update_event_receiver) - def test_call_container_update_signal_when_update_component(self): + def test_call_container_update_signal_when_update_component(self) -> None: block_olx = "Hello world!" container_update_event_receiver = mock.Mock() LIBRARY_CONTAINER_UPDATED.connect(container_update_event_receiver) @@ -839,13 +842,13 @@ def test_call_container_update_signal_when_update_component(self): self._set_library_block_fields(self.html_block_usage_key, {"data": block_olx, "metadata": {}}) self._validate_calls_of_html_block(container_update_event_receiver) - def test_call_object_changed_signal_when_remove_component(self): + def test_call_object_changed_signal_when_remove_component(self) -> None: html_block_1 = self._add_block_to_library( self.lib1.library_key, "html", "html3", ) api.update_container_children( self.unit2.container_key, - [UsageKey.from_string(html_block_1["id"])], + [LibraryUsageLocatorV2.from_string(html_block_1["id"])], None, entities_action=authoring_api.ChildrenEntitiesAction.APPEND, ) @@ -854,7 +857,7 @@ def test_call_object_changed_signal_when_remove_component(self): CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_reciver) api.update_container_children( self.unit2.container_key, - [UsageKey.from_string(html_block_1["id"])], + [LibraryUsageLocatorV2.from_string(html_block_1["id"])], None, entities_action=authoring_api.ChildrenEntitiesAction.REMOVE, ) @@ -872,7 +875,7 @@ def test_call_object_changed_signal_when_remove_component(self): event_reciver.call_args_list[0].kwargs, ) - def test_call_object_changed_signal_when_add_component(self): + def test_call_object_changed_signal_when_add_component(self) -> None: event_reciver = mock.Mock() CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_reciver) html_block_1 = self._add_block_to_library( @@ -885,8 +888,8 @@ def test_call_object_changed_signal_when_add_component(self): api.update_container_children( self.unit2.container_key, [ - UsageKey.from_string(html_block_1["id"]), - UsageKey.from_string(html_block_2["id"]) + LibraryUsageLocatorV2.from_string(html_block_1["id"]), + LibraryUsageLocatorV2.from_string(html_block_2["id"]) ], None, entities_action=authoring_api.ChildrenEntitiesAction.APPEND, @@ -916,19 +919,19 @@ def test_call_object_changed_signal_when_add_component(self): event_reciver.call_args_list[1].kwargs, ) - def test_delete_component_and_revert(self): + def test_delete_component_and_revert(self) -> None: """ When a component is deleted and then the delete is reverted, signals 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"]), + LibraryUsageLocatorV2.from_string(self.problem_block["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(LibraryUsageLocatorV2.from_string(self.problem_block["id"])) container_event_receiver = mock.Mock() LIBRARY_CONTAINER_UPDATED.connect(container_event_receiver) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 6c59c8c086e4..26196d8394d8 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) -> None: + 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) -> None: """ - 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) -> None: """ - 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) -> None: """ - 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): + def test_unit_add_children(self) -> None: """ 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) -> None: + # 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) -> None: + # 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) -> None: """ - 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): + def test_unit_replace_children(self) -> None: """ 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) -> None: """ - Test restore a deleted unit. + Test that we can completely replace/reorder subsection 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.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'] - # 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 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'] - # Delete the unit - self._delete_container(container_data["id"]) + def test_section_replace_children(self) -> None: + """ + Test that we can completely replace/reorder section children. + """ + 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'] + + # 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) -> None: + """ + Test restore a deleted container. + """ + container = getattr(self, container_type) + + # 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) -> None: # Create a collection col1 = api.create_library_collection( - lib_key, + self.lib_key, "COL1", title="Collection 1", created_by=self.user.id, @@ -284,69 +554,80 @@ 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}] - def test_publish_container(self): # pylint: disable=too-many-statements + def test_publish_container(self) -> None: # 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): """