From c41fe8991832de7e987368a533d4b0e9b91fefce Mon Sep 17 00:00:00 2001 From: Jillian Date: Fri, 13 Sep 2024 22:50:54 +0930 Subject: [PATCH] Store content object collections in search index [FC-0062] (#35469) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Add Library Collections REST endpoints * test: Add tests for Collections REST APIs * chore: Add missing __init__ files * docs: Add warning about unstable REST APIs * feat: emit CONTENT_OBJECT_ASSOCIATIONS_CHANGED whenever a content object's tags or collections have changed, and handle that event in content/search. The deprecated CONTENT_OBJECT_TAGS_CHANGED event is still emitted when tags change; to be removed after Sumac. * docs: replaces CONTENT_OBJECT_TAGS_CHANGED with CONTENT_OBJECT_ASSOCIATIONS_CHANGED * chore: updates openedx-events==9.14.0 * chore: updates openedx-learning==0.11.4 Co-authored-by: Yusuf Musleh Co-authored-by: Chris Chávez Co-authored-by: Rômulo Penido --- docs/hooks/events.rst | 8 +- openedx/core/djangoapps/content/search/api.py | 35 +++- .../djangoapps/content/search/documents.py | 78 ++++++++- .../djangoapps/content/search/handlers.py | 62 ++++++-- .../core/djangoapps/content/search/tasks.py | 13 ++ .../content/search/tests/test_api.py | 150 ++++++++++++++++-- .../content/search/tests/test_documents.py | 114 ++++++++++++- .../core/djangoapps/content_libraries/api.py | 13 +- .../content_libraries/tests/test_api.py | 24 +-- .../core/djangoapps/content_tagging/api.py | 28 +++- .../content_tagging/rest_api/v1/views.py | 19 ++- 11 files changed, 491 insertions(+), 53 deletions(-) diff --git a/docs/hooks/events.rst b/docs/hooks/events.rst index 2a7561df24e1..bccb98e56a42 100644 --- a/docs/hooks/events.rst +++ b/docs/hooks/events.rst @@ -244,10 +244,6 @@ Content Authoring Events - org.openedx.content_authoring.library_block.deleted.v1 - 2023-07-20 - * - `CONTENT_OBJECT_TAGS_CHANGED `_ - - org.openedx.content_authoring.content.object.tags.changed.v1 - - 2024-03-31 - * - `LIBRARY_COLLECTION_CREATED `_ - org.openedx.content_authoring.content_library.collection.created.v1 - 2024-08-23 @@ -259,3 +255,7 @@ Content Authoring Events * - `LIBRARY_COLLECTION_DELETED `_ - org.openedx.content_authoring.content_library.collection.deleted.v1 - 2024-08-23 + + * - `CONTENT_OBJECT_ASSOCIATIONS_CHANGED `_ + - org.openedx.content_authoring.content.object.associations.changed.v1 + - 2024-09-06 diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 76bb5eb3f4a4..4a775a710da6 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -34,6 +34,7 @@ searchable_doc_for_course_block, searchable_doc_for_collection, searchable_doc_for_library_block, + searchable_doc_collections, searchable_doc_tags, ) @@ -322,6 +323,9 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.tags + "." + Fields.tags_level1, Fields.tags + "." + Fields.tags_level2, Fields.tags + "." + Fields.tags_level3, + Fields.collections, + Fields.collections + "." + Fields.collections_display_name, + Fields.collections + "." + Fields.collections_key, Fields.type, Fields.access_id, Fields.last_published, @@ -333,8 +337,9 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.display_name, Fields.block_id, Fields.content, - Fields.tags, Fields.description, + Fields.tags, + Fields.collections, # If we don't list the following sub-fields _explicitly_, they're only sometimes searchable - that is, they # are searchable only if at least one document in the index has a value. If we didn't list them here and, # say, there were no tags.level3 tags in the index, the client would get an error if trying to search for @@ -344,6 +349,8 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.tags + "." + Fields.tags_level1, Fields.tags + "." + Fields.tags_level2, Fields.tags + "." + Fields.tags_level3, + Fields.collections + "." + Fields.collections_display_name, + Fields.collections + "." + Fields.collections_key, ]) # Mark which attributes can be used for sorting search results: client.index(temp_index_name).update_sortable_attributes([ @@ -375,6 +382,7 @@ def index_library(lib_key: str) -> list: doc = {} doc.update(searchable_doc_for_library_block(metadata)) doc.update(searchable_doc_tags(metadata.usage_key)) + doc.update(searchable_doc_collections(metadata.usage_key)) docs.append(doc) except Exception as err: # pylint: disable=broad-except status_cb(f"Error indexing library component {component}: {err}") @@ -549,6 +557,22 @@ def upsert_library_block_index_doc(usage_key: UsageKey) -> None: _update_index_docs(docs) +def upsert_library_collection_index_doc(library_key: LibraryLocatorV2, collection_key: str) -> None: + """ + Creates or updates the document for the given Library Collection in the search index + """ + content_library = lib_api.ContentLibrary.objects.get_by_key(library_key) + collection = authoring_api.get_collection( + learning_package_id=content_library.learning_package_id, + collection_key=collection_key, + ) + docs = [ + searchable_doc_for_collection(collection) + ] + + _update_index_docs(docs) + + def upsert_content_library_index_docs(library_key: LibraryLocatorV2) -> None: """ Creates or updates the documents for the given Content Library in the search index @@ -571,6 +595,15 @@ def upsert_block_tags_index_docs(usage_key: UsageKey): _update_index_docs([doc]) +def upsert_block_collections_index_docs(usage_key: UsageKey): + """ + Updates the collections data in documents for the given Course/Library block + """ + doc = {Fields.id: meili_id_from_opaque_key(usage_key)} + doc.update(searchable_doc_collections(usage_key)) + _update_index_docs([doc]) + + def _get_user_orgs(request: Request) -> list[str]: """ Get the org.short_names for the organizations that the requesting user has OrgStaffRole or OrgInstructorRole. diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index a45b37ab2ad2..6f19b610fe86 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -7,7 +7,9 @@ from hashlib import blake2b from django.utils.text import slugify +from django.core.exceptions import ObjectDoesNotExist from opaque_keys.edx.keys import LearningContextKey, UsageKey +from openedx_learning.api import authoring as authoring_api from openedx.core.djangoapps.content.search.models import SearchAccess from openedx.core.djangoapps.content_libraries import api as lib_api @@ -26,7 +28,11 @@ class Fields: id = "id" usage_key = "usage_key" type = "type" # DocType.course_block or DocType.library_block (see below) - block_id = "block_id" # The block_id part of the usage key. Sometimes human-readable, sometimes a random hex ID + # The block_id part of the usage key for course or library blocks. + # If it's a collection, the collection.key is stored here. + # Sometimes human-readable, sometimes a random hex ID + # Is only unique within the given context_key. + block_id = "block_id" display_name = "display_name" description = "description" modified = "modified" @@ -52,11 +58,21 @@ class Fields: tags_level1 = "level1" tags_level2 = "level2" tags_level3 = "level3" + # Collections (dictionary) that this object belongs to. + # Similarly to tags above, we collect the collection.titles and collection.keys into hierarchical facets. + collections = "collections" + collections_display_name = "display_name" + collections_key = "key" + # The "content" field is a dictionary of arbitrary data, depending on the block_type. # It comes from each XBlock's index_dictionary() method (if present) plus some processing. # Text (html) blocks have an "html_content" key in here, capa has "capa_content" and "problem_types", and so on. content = "content" + # Collections use this field to communicate how many entities/components they contain. + # Structural XBlocks may use this one day to indicate how many child blocks they ocntain. + num_children = "num_children" + # Note: new fields or values can be added at any time, but if they need to be indexed for filtering or keyword # 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). @@ -223,6 +239,51 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: return {Fields.tags: result} +def _collections_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: + """ + Given an XBlock, course, library, etc., get the collections for its index doc. + + e.g. for something in Collections "COL_A" and "COL_B", this would return: + { + "collections": { + "display_name": ["Collection A", "Collection B"], + "key": ["COL_A", "COL_B"], + } + } + + If the object is in no collections, returns: + { + "collections": {}, + } + + """ + # Gather the collections associated with this object + collections = None + try: + component = lib_api.get_component_from_usage_key(object_id) + collections = authoring_api.get_entity_collections( + component.learning_package_id, + component.key, + ) + except ObjectDoesNotExist: + log.warning(f"No component found for {object_id}") + + if not collections: + return {Fields.collections: {}} + + result = { + Fields.collections: { + Fields.collections_display_name: [], + Fields.collections_key: [], + } + } + for collection in collections: + result[Fields.collections][Fields.collections_display_name].append(collection.title) + result[Fields.collections][Fields.collections_key].append(collection.key) + + return result + + def searchable_doc_for_library_block(xblock_metadata: lib_api.LibraryXBlockMetadata) -> dict: """ Generate a dictionary document suitable for ingestion into a search engine @@ -265,6 +326,19 @@ def searchable_doc_tags(usage_key: UsageKey) -> dict: return doc +def searchable_doc_collections(usage_key: UsageKey) -> dict: + """ + Generate a dictionary document suitable for ingestion into a search engine + like Meilisearch or Elasticsearch, with the collections data for the given content object. + """ + doc = { + Fields.id: meili_id_from_opaque_key(usage_key), + } + doc.update(_collections_for_content_object(usage_key)) + + return doc + + def searchable_doc_for_course_block(block) -> dict: """ Generate a dictionary document suitable for ingestion into a search engine @@ -289,6 +363,7 @@ def searchable_doc_for_collection(collection) -> dict: """ doc = { Fields.id: collection.id, + Fields.block_id: collection.key, Fields.type: DocType.collection, Fields.display_name: collection.title, Fields.description: collection.description, @@ -298,6 +373,7 @@ def searchable_doc_for_collection(collection) -> dict: # If related contentlibrary is found, it will override this value below. # Mostly contentlibrary.library_key == learning_package.key Fields.context_key: collection.learning_package.key, + Fields.num_children: collection.entities.count(), } # Just in case learning_package is not related to a library try: diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 94ac02ea1606..6a341c92ed2b 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -6,30 +6,40 @@ from django.db.models.signals import post_delete from django.dispatch import receiver -from openedx_events.content_authoring.data import ContentLibraryData, ContentObjectData, LibraryBlockData, XBlockData +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey +from openedx_events.content_authoring.data import ( + ContentLibraryData, + ContentObjectChangedData, + LibraryBlockData, + LibraryCollectionData, + XBlockData, +) from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_DELETED, CONTENT_LIBRARY_UPDATED, LIBRARY_BLOCK_CREATED, LIBRARY_BLOCK_DELETED, LIBRARY_BLOCK_UPDATED, + LIBRARY_COLLECTION_CREATED, + LIBRARY_COLLECTION_UPDATED, XBLOCK_CREATED, XBLOCK_DELETED, XBLOCK_UPDATED, - CONTENT_OBJECT_TAGS_CHANGED, + CONTENT_OBJECT_ASSOCIATIONS_CHANGED, ) -from openedx.core.djangoapps.content_tagging.utils import get_content_key_from_string from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.search.models import SearchAccess -from .api import only_if_meilisearch_enabled, upsert_block_tags_index_docs +from .api import only_if_meilisearch_enabled, upsert_block_collections_index_docs, upsert_block_tags_index_docs from .tasks import ( delete_library_block_index_doc, delete_xblock_index_doc, update_content_library_index_docs, + update_library_collection_index_doc, upsert_library_block_index_doc, - upsert_xblock_index_doc + upsert_xblock_index_doc, ) log = logging.getLogger(__name__) @@ -145,22 +155,48 @@ def content_library_updated_handler(**kwargs) -> None: update_content_library_index_docs.apply(args=[str(content_library_data.library_key)]) -@receiver(CONTENT_OBJECT_TAGS_CHANGED) +@receiver(LIBRARY_COLLECTION_CREATED) +@receiver(LIBRARY_COLLECTION_UPDATED) +@only_if_meilisearch_enabled +def library_collection_updated_handler(**kwargs) -> None: + """ + Create or update the index for the content library collection + """ + library_collection = kwargs.get("library_collection", None) + if not library_collection or not isinstance(library_collection, LibraryCollectionData): # pragma: no cover + log.error("Received null or incorrect data for event") + return + + # Update collection index synchronously to make sure that search index is updated before + # the frontend invalidates/refetches index. + # See content_library_updated_handler for more details. + update_library_collection_index_doc.apply(args=[ + str(library_collection.library_key), + library_collection.collection_key, + ]) + + +@receiver(CONTENT_OBJECT_ASSOCIATIONS_CHANGED) @only_if_meilisearch_enabled -def content_object_tags_changed_handler(**kwargs) -> None: +def content_object_associations_changed_handler(**kwargs) -> None: """ - Update the tags data in the index for the Content Object + Update the collections/tags data in the index for the Content Object """ - content_object_tags = kwargs.get("content_object", None) - if not content_object_tags or not isinstance(content_object_tags, ContentObjectData): + content_object = kwargs.get("content_object", None) + if not content_object or not isinstance(content_object, ContentObjectChangedData): log.error("Received null or incorrect data for event") return try: # Check if valid if course or library block - get_content_key_from_string(content_object_tags.object_id) - except ValueError: + usage_key = UsageKey.from_string(str(content_object.object_id)) + except InvalidKeyError: log.error("Received invalid content object id") return - upsert_block_tags_index_docs(content_object_tags.object_id) + # This event's changes may contain both "tags" and "collections", but this will happen rarely, if ever. + # So we allow a potential double "upsert" here. + if not content_object.changes or "tags" in content_object.changes: + upsert_block_tags_index_docs(usage_key) + if not content_object.changes or "collections" in content_object.changes: + upsert_block_collections_index_docs(usage_key) diff --git a/openedx/core/djangoapps/content/search/tasks.py b/openedx/core/djangoapps/content/search/tasks.py index dfd603776981..d9dad834db29 100644 --- a/openedx/core/djangoapps/content/search/tasks.py +++ b/openedx/core/djangoapps/content/search/tasks.py @@ -84,3 +84,16 @@ def update_content_library_index_docs(library_key_str: str) -> None: # Delete all documents in this library that were not published by above function # as this task is also triggered on discard event. api.delete_all_draft_docs_for_library(library_key) + + +@shared_task(base=LoggedTask, autoretry_for=(MeilisearchError, ConnectionError)) +@set_code_owner_attribute +def update_library_collection_index_doc(library_key_str: str, collection_key: str) -> None: + """ + Celery task to update the content index documents for a library collection + """ + library_key = LibraryLocatorV2.from_string(library_key_str) + + log.info("Updating content index documents for collection %s in library%s", collection_key, library_key) + + api.upsert_library_collection_index_doc(library_key, collection_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index d6b58674f5ec..023265f4d0f5 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -186,16 +186,18 @@ def setUp(self): description="my collection description" ) self.collection_dict = { - 'id': self.collection.id, - 'type': 'collection', - 'display_name': 'my_collection', - 'description': 'my collection description', - 'context_key': 'lib:org1:lib', - 'org': 'org1', - 'created': created_date.timestamp(), - 'modified': created_date.timestamp(), + "id": self.collection.id, + "block_id": self.collection.key, + "type": "collection", + "display_name": "my_collection", + "description": "my collection description", + "num_children": 0, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), "access_id": lib_access.id, - 'breadcrumbs': [{'display_name': 'Library'}] + "breadcrumbs": [{"display_name": "Library"}], } @override_settings(MEILISEARCH_ENABLED=False) @@ -215,10 +217,13 @@ def test_reindex_meilisearch(self, mock_meilisearch): doc_vertical["tags"] = {} doc_problem1 = copy.deepcopy(self.doc_problem1) doc_problem1["tags"] = {} + doc_problem1["collections"] = {} doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} + doc_problem2["collections"] = {} api.rebuild_index() + assert mock_meilisearch.return_value.index.return_value.add_documents.call_count == 3 mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ call([doc_sequential, doc_vertical]), @@ -254,6 +259,7 @@ def test_reindex_meilisearch_library_block_error(self, mock_meilisearch): doc_vertical["tags"] = {} doc_problem2 = copy.deepcopy(self.doc_problem2) doc_problem2["tags"] = {} + doc_problem2["collections"] = {} orig_from_component = library_api.LibraryXBlockMetadata.from_component @@ -346,6 +352,7 @@ def test_index_xblock_tags(self, mock_meilisearch): } } + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2 mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( [ call([doc_sequential_with_tags1]), @@ -400,6 +407,7 @@ def test_index_library_block_tags(self, mock_meilisearch): } } + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 2 mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( [ call([doc_problem_with_tags1]), @@ -408,6 +416,130 @@ def test_index_library_block_tags(self, mock_meilisearch): any_order=True, ) + @override_settings(MEILISEARCH_ENABLED=True) + def test_index_library_block_and_collections(self, mock_meilisearch): + """ + Test indexing an Library Block and the Collections it's in. + """ + # Create collections (these internally call `upsert_library_collection_index_doc`) + created_date = datetime(2023, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(created_date): + collection1 = library_api.create_library_collection( + self.library.key, + collection_key="COL1", + title="Collection 1", + created_by=None, + description="First Collection", + ) + + collection2 = library_api.create_library_collection( + self.library.key, + collection_key="COL2", + title="Collection 2", + created_by=None, + description="Second Collection", + ) + + # Add Problem1 to both Collections (these internally call `upsert_block_collections_index_docs` and + # `upsert_library_collection_index_doc`) + # (adding in reverse order to test sorting of collection tag) + updated_date = datetime(2023, 6, 7, 8, 9, 10, tzinfo=timezone.utc) + with freeze_time(updated_date): + for collection in (collection2, collection1): + library_api.update_library_collection_components( + self.library.key, + collection_key=collection.key, + usage_keys=[ + self.problem1.usage_key, + ], + ) + + # Build expected docs at each stage + lib_access, _ = SearchAccess.objects.get_or_create(context_key=self.library.key) + doc_collection1_created = { + "id": collection1.id, + "block_id": collection1.key, + "type": "collection", + "display_name": "Collection 1", + "description": "First Collection", + "num_children": 0, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } + doc_collection2_created = { + "id": collection2.id, + "block_id": collection2.key, + "type": "collection", + "display_name": "Collection 2", + "description": "Second Collection", + "num_children": 0, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": created_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } + doc_collection2_updated = { + "id": collection2.id, + "block_id": collection2.key, + "type": "collection", + "display_name": "Collection 2", + "description": "Second Collection", + "num_children": 1, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": updated_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } + doc_collection1_updated = { + "id": collection1.id, + "block_id": collection1.key, + "type": "collection", + "display_name": "Collection 1", + "description": "First Collection", + "num_children": 1, + "context_key": "lib:org1:lib", + "org": "org1", + "created": created_date.timestamp(), + "modified": updated_date.timestamp(), + "access_id": lib_access.id, + "breadcrumbs": [{"display_name": "Library"}], + } + doc_problem_with_collection1 = { + "id": self.doc_problem1["id"], + "collections": { + "display_name": ["Collection 2"], + "key": ["COL2"], + }, + } + doc_problem_with_collection2 = { + "id": self.doc_problem1["id"], + "collections": { + "display_name": ["Collection 1", "Collection 2"], + "key": ["COL1", "COL2"], + }, + } + + assert mock_meilisearch.return_value.index.return_value.update_documents.call_count == 6 + mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( + [ + call([doc_collection1_created]), + call([doc_collection2_created]), + call([doc_collection2_updated]), + call([doc_collection1_updated]), + call([doc_problem_with_collection1]), + call([doc_problem_with_collection2]), + ], + any_order=True, + ) + @override_settings(MEILISEARCH_ENABLED=True) def test_delete_index_library_block(self, mock_meilisearch): """ diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 1f10fc3c988f..7ff330c0b491 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -8,6 +8,7 @@ from openedx_learning.api import authoring as authoring_api from openedx.core.djangoapps.content_tagging import api as tagging_api +from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -15,12 +16,19 @@ try: # This import errors in the lms because content.search is not an installed app there. - from ..documents import searchable_doc_for_course_block, searchable_doc_tags, searchable_doc_for_collection + from ..documents import ( + searchable_doc_for_course_block, + searchable_doc_tags, + searchable_doc_collections, + searchable_doc_for_collection, + searchable_doc_for_library_block, + ) from ..models import SearchAccess except RuntimeError: searchable_doc_for_course_block = lambda x: x searchable_doc_tags = lambda x: x searchable_doc_for_collection = lambda x: x + searchable_doc_for_library_block = lambda x: x SearchAccess = {} @@ -38,12 +46,13 @@ class StudioDocumentsTest(SharedModuleStoreTestCase): def setUpClass(cls): super().setUpClass() cls.store = modulestore() + cls.org = Organization.objects.create(name="edX", short_name="edX") cls.toy_course = ToyCourseFactory.create() # See xmodule/modulestore/tests/sample_courses.py cls.toy_course_key = cls.toy_course.id # Get references to some blocks in the toy course cls.html_block_key = cls.toy_course_key.make_usage_key("html", "toyjumpto") - # Create a problem in library + # Create a problem in course cls.problem_block = BlockFactory.create( category="problem", parent_location=cls.toy_course_key.make_usage_key("vertical", "vertical_test"), @@ -51,8 +60,38 @@ def setUpClass(cls): data="What is a test?", ) + # Create a library and collection with a block + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + cls.library = library_api.create_library( + org=cls.org, + slug="2012_Fall", + title="some content_library", + description="some description", + ) + cls.collection = library_api.create_library_collection( + cls.library.key, + collection_key="TOY_COLLECTION", + title="Toy Collection", + created_by=None, + description="my toy collection description" + ) + cls.library_block = library_api.create_library_block( + cls.library.key, + "html", + "text2", + ) + + # Add the problem block to the collection + library_api.update_library_collection_components( + cls.library.key, + collection_key="TOY_COLLECTION", + usage_keys=[ + cls.library_block.usage_key, + ] + ) + # Create a couple taxonomies and some tags - cls.org = Organization.objects.create(name="edX", short_name="edX") cls.difficulty_tags = tagging_api.create_taxonomy(name="Difficulty", orgs=[cls.org], allow_multiple=False) tagging_api.add_tag_to_taxonomy(cls.difficulty_tags, tag="Easy") tagging_api.add_tag_to_taxonomy(cls.difficulty_tags, tag="Normal") @@ -69,6 +108,7 @@ def setUpClass(cls): tagging_api.tag_object(str(cls.problem_block.usage_key), cls.difficulty_tags, tags=["Easy"]) tagging_api.tag_object(str(cls.html_block_key), cls.subject_tags, tags=["Chinese", "Jump Links"]) tagging_api.tag_object(str(cls.html_block_key), cls.difficulty_tags, tags=["Normal"]) + tagging_api.tag_object(str(cls.library_block.usage_key), cls.difficulty_tags, tags=["Normal"]) @property def toy_course_access_id(self): @@ -80,6 +120,16 @@ def toy_course_access_id(self): """ return SearchAccess.objects.get(context_key=self.toy_course_key).id + @property + def library_access_id(self): + """ + Returns the SearchAccess.id created for the library. + + This SearchAccess object is created when documents are added to the search index, so this method must be called + after this step, or risk a DoesNotExist error. + """ + return SearchAccess.objects.get(context_key=self.library.key).id + def test_problem_block(self): """ Test how a problem block gets represented in the search index @@ -205,6 +255,62 @@ def test_video_block_untagged(self): # This video has no tags. } + def test_html_library_block(self): + """ + Test how a library block gets represented in the search index + """ + doc = {} + doc.update(searchable_doc_for_library_block(self.library_block)) + doc.update(searchable_doc_tags(self.library_block.usage_key)) + doc.update(searchable_doc_collections(self.library_block.usage_key)) + assert doc == { + "id": "lbedx2012_fallhtmltext2-4bb47d67", + "type": "library_block", + "block_type": "html", + "usage_key": "lb:edX:2012_Fall:html:text2", + "block_id": "text2", + "context_key": "lib:edX:2012_Fall", + "org": "edX", + "access_id": self.library_access_id, + "display_name": "Text", + "breadcrumbs": [ + { + "display_name": "some content_library", + }, + ], + "last_published": None, + "created": 1680674828.0, + "modified": 1680674828.0, + "content": { + "html_content": "", + }, + "collections": { + "key": ["TOY_COLLECTION"], + "display_name": ["Toy Collection"], + }, + "tags": { + "taxonomy": ["Difficulty"], + "level0": ["Difficulty > Normal"], + }, + } + + def test_collection_with_library(self): + doc = searchable_doc_for_collection(self.collection) + assert doc == { + "id": self.collection.id, + "block_id": self.collection.key, + "type": "collection", + "org": "edX", + "display_name": "Toy Collection", + "description": "my toy collection description", + "num_children": 1, + "context_key": "lib:edX:2012_Fall", + "access_id": self.library_access_id, + "breadcrumbs": [{"display_name": "some content_library"}], + "created": 1680674828.0, + "modified": 1680674828.0, + } + def test_collection_with_no_library(self): created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) with freeze_time(created_date): @@ -223,9 +329,11 @@ def test_collection_with_no_library(self): doc = searchable_doc_for_collection(collection) assert doc == { "id": collection.id, + "block_id": collection.key, "type": "collection", "display_name": "my_collection", "description": "my collection description", + "num_children": 0, "context_key": learning_package.key, "access_id": self.toy_course_access_id, "breadcrumbs": [{"display_name": "some learning_package"}], diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 00110143456a..c19c9bf880d0 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -78,12 +78,12 @@ from opaque_keys import InvalidKeyError from openedx_events.content_authoring.data import ( ContentLibraryData, - ContentObjectData, + ContentObjectChangedData, LibraryBlockData, LibraryCollectionData, ) from openedx_events.content_authoring.signals import ( - CONTENT_OBJECT_TAGS_CHANGED, + CONTENT_OBJECT_ASSOCIATIONS_CHANGED, CONTENT_LIBRARY_CREATED, CONTENT_LIBRARY_DELETED, CONTENT_LIBRARY_UPDATED, @@ -1235,10 +1235,13 @@ def update_library_collection_components( ) ) - # Emit a CONTENT_OBJECT_TAGS_CHANGED event for each of the objects added/removed + # Emit a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event for each of the objects added/removed for usage_key in usage_keys: - CONTENT_OBJECT_TAGS_CHANGED.send_event( - content_object=ContentObjectData(object_id=usage_key), + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData( + object_id=str(usage_key), + changes=["collections"], + ), ) return collection diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index b7b646acac56..b02e71b002a3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -14,11 +14,11 @@ ) from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_events.content_authoring.data import ( - ContentObjectData, + ContentObjectChangedData, LibraryCollectionData, ) from openedx_events.content_authoring.signals import ( - CONTENT_OBJECT_TAGS_CHANGED, + CONTENT_OBJECT_ASSOCIATIONS_CHANGED, LIBRARY_COLLECTION_CREATED, LIBRARY_COLLECTION_UPDATED, ) @@ -262,7 +262,7 @@ class ContentLibraryCollectionsTest(ContentLibrariesRestApiTest, OpenEdxEventsTe Same guidelines as ContentLibrariesTestCase. """ ENABLED_OPENEDX_EVENTS = [ - CONTENT_OBJECT_TAGS_CHANGED.event_type, + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.event_type, LIBRARY_COLLECTION_CREATED.event_type, LIBRARY_COLLECTION_UPDATED.event_type, ] @@ -411,10 +411,10 @@ def test_update_library_collection_components(self): def test_update_library_collection_components_event(self): """ - Check that a CONTENT_OBJECT_TAGS_CHANGED event is raised for each added/removed component. + Check that a CONTENT_OBJECT_ASSOCIATIONS_CHANGED event is raised for each added/removed component. """ event_receiver = mock.Mock() - CONTENT_OBJECT_TAGS_CHANGED.connect(event_receiver) + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_receiver) LIBRARY_COLLECTION_UPDATED.connect(event_receiver) api.update_library_collection_components( @@ -440,20 +440,22 @@ def test_update_library_collection_components_event(self): ) self.assertDictContainsSubset( { - "signal": CONTENT_OBJECT_TAGS_CHANGED, + "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "sender": None, - "content_object": ContentObjectData( - object_id=UsageKey.from_string(self.lib1_problem_block["id"]), + "content_object": ContentObjectChangedData( + object_id=self.lib1_problem_block["id"], + changes=["collections"], ), }, event_receiver.call_args_list[1].kwargs, ) self.assertDictContainsSubset( { - "signal": CONTENT_OBJECT_TAGS_CHANGED, + "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "sender": None, - "content_object": ContentObjectData( - object_id=UsageKey.from_string(self.lib1_html_block["id"]), + "content_object": ContentObjectChangedData( + object_id=self.lib1_html_block["id"], + changes=["collections"], ), }, event_receiver.call_args_list[2].kwargs, diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 47b157c1a34e..b63c70c5f0b8 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -18,8 +18,11 @@ from openedx_tagging.core.tagging.models.utils import TAGS_CSV_SEPARATOR from organizations.models import Organization from .helpers.objecttag_export_helpers import build_object_tree_with_objecttags, iterate_with_level -from openedx_events.content_authoring.data import ContentObjectData -from openedx_events.content_authoring.signals import CONTENT_OBJECT_TAGS_CHANGED +from openedx_events.content_authoring.data import ContentObjectData, ContentObjectChangedData +from openedx_events.content_authoring.signals import ( + CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + CONTENT_OBJECT_TAGS_CHANGED, +) from .models import TaxonomyOrg from .types import ContentKey, TagValuesByObjectIdDict, TagValuesByTaxonomyIdDict, TaxonomyDict @@ -301,6 +304,16 @@ def set_exported_object_tags( create_invalid=True, taxonomy_export_id=str(taxonomy_export_id), ) + + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + time=now(), + content_object=ContentObjectChangedData( + object_id=content_key_str, + changes=["tags"], + ) + ) + + # Emit a (deprecated) CONTENT_OBJECT_TAGS_CHANGED event too CONTENT_OBJECT_TAGS_CHANGED.send_event( time=now(), content_object=ContentObjectData(object_id=content_key_str) @@ -378,7 +391,7 @@ def tag_object( Replaces the existing ObjectTag entries for the given taxonomy + object_id with the given list of tags, if the taxonomy can be used by the given object_id. - This is a wrapper around oel_tagging.tag_object that adds emitting the `CONTENT_OBJECT_TAGS_CHANGED` event + This is a wrapper around oel_tagging.tag_object that adds emitting the `CONTENT_OBJECT_ASSOCIATIONS_CHANGED` event when tagging an object. tags: A list of the values of the tags from this taxonomy to apply. @@ -399,6 +412,15 @@ def tag_object( taxonomy=taxonomy, tags=tags, ) + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + time=now(), + content_object=ContentObjectChangedData( + object_id=object_id, + changes=["tags"], + ) + ) + + # Emit a (deprecated) CONTENT_OBJECT_TAGS_CHANGED event too CONTENT_OBJECT_TAGS_CHANGED.send_event( time=now(), content_object=ContentObjectData(object_id=object_id) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 71b210b9e561..3fc99736bae9 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -13,8 +13,11 @@ from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView -from openedx_events.content_authoring.data import ContentObjectData -from openedx_events.content_authoring.signals import CONTENT_OBJECT_TAGS_CHANGED +from openedx_events.content_authoring.data import ContentObjectData, ContentObjectChangedData +from openedx_events.content_authoring.signals import ( + CONTENT_OBJECT_ASSOCIATIONS_CHANGED, + CONTENT_OBJECT_TAGS_CHANGED, +) from ...auth import has_view_object_tags_access from ...api import ( @@ -149,14 +152,24 @@ class ObjectTagOrgView(ObjectTagView): def update(self, request, *args, **kwargs) -> Response: """ - Extend the update method to fire CONTENT_OBJECT_TAGS_CHANGED event + Extend the update method to fire CONTENT_OBJECT_ASSOCIATIONS_CHANGED event """ response = super().update(request, *args, **kwargs) if response.status_code == 200: object_id = kwargs.get('object_id') + + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData( + object_id=object_id, + changes=["tags"], + ) + ) + + # Emit a (deprecated) CONTENT_OBJECT_TAGS_CHANGED event too CONTENT_OBJECT_TAGS_CHANGED.send_event( content_object=ContentObjectData(object_id=object_id) ) + return response