From bcc5cbbff0996e67ee02be6a3c89719550978a53 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 28 Aug 2024 20:11:22 +0930 Subject: [PATCH 01/12] fix: pass a ContentKey to the search doc handlers They expect a UsageKey object, not the string object_id. --- openedx/core/djangoapps/content/search/handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index ba0e8c1a1680..835810879780 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -152,9 +152,9 @@ def content_object_tags_changed_handler(**kwargs) -> None: try: # Check if valid if course or library block - get_content_key_from_string(content_object_tags.object_id) + content_key = get_content_key_from_string(str(content_object_tags.object_id)) except ValueError: log.error("Received invalid content object id") return - upsert_block_tags_index_docs(content_object_tags.object_id) + upsert_block_tags_index_docs(content_key) From 0e32451b35ff2128676045188b68975bc48d9602 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 28 Aug 2024 20:15:01 +0930 Subject: [PATCH 02/12] feat: add tags.collections to content object search index and ensures this new field is searchable and filterable. Serializes the object's collections to a list of collection.key values. --- openedx/core/djangoapps/content/search/api.py | 2 + .../djangoapps/content/search/documents.py | 35 ++++-- .../content/search/tests/test_api.py | 46 +++++++- .../content/search/tests/test_documents.py | 105 +++++++++++++++++- 4 files changed, 173 insertions(+), 15 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 76bb5eb3f4a4..00b908e26cc4 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -317,6 +317,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.context_key, Fields.org, Fields.tags, + Fields.tags + "." + Fields.tags_collections, Fields.tags + "." + Fields.tags_taxonomy, Fields.tags + "." + Fields.tags_level0, Fields.tags + "." + Fields.tags_level1, @@ -339,6 +340,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: # 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 # these sub-fields: "Attribute `tags.level3` is not searchable." + Fields.tags + "." + Fields.tags_collections, Fields.tags + "." + Fields.tags_taxonomy, Fields.tags + "." + Fields.tags_level0, Fields.tags + "." + Fields.tags_level1, diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index be5428a0a9c2..c92776e12a4b 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 @@ -52,6 +54,7 @@ class Fields: tags_level1 = "level1" tags_level2 = "level2" tags_level3 = "level3" + tags_collections = "collections" # subfield of tags, i.e. tags.collections # 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. @@ -164,10 +167,11 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: See the comments above on "Field.tags" for an explanation of the format. - e.g. for something tagged "Difficulty: Hard" and "Location: Vancouver" this - would return: + e.g. for something tagged "Difficulty: Hard" and "Location: Vancouver", + and in Collections 3 and 4, this would return: { "tags": { + "collections": [3, 4], "taxonomy": ["Location", "Difficulty"], "level0": ["Location > North America", "Difficulty > Hard"], "level1": ["Location > North America > Canada"], @@ -182,21 +186,16 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: strings in a particular format that the frontend knows how to render to support hierarchical refinement by tag. """ + result = {} + # Note that we could improve performance for indexing many components from the same library/course, # if we used get_all_object_tags() to load all the tags for the library in a single query rather than loading the # tags for each component separately. all_tags = tagging_api.get_object_tags(str(object_id)).all() - if not all_tags: - # Clear out tags in the index when unselecting all tags for the block, otherwise - # it would remain the last value if a cleared Fields.tags field is not included - return {Fields.tags: {}} - result = { - Fields.tags_taxonomy: [], - Fields.tags_level0: [], - # ... other levels added as needed - } for obj_tag in all_tags: # Add the taxonomy name: + if Fields.tags_taxonomy not in result: + result[Fields.tags_taxonomy] = [] if obj_tag.taxonomy.name not in result[Fields.tags_taxonomy]: result[Fields.tags_taxonomy].append(obj_tag.taxonomy.name) # Taxonomy name plus each level of tags, in a list: # e.g. ["Location", "North America", "Canada", "Vancouver"] @@ -220,6 +219,20 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: if len(parts) == level + 2: break # We have all the levels for this tag now (e.g. parts=["Difficulty", "Hard"] -> need level0 only) + # Gather the collections associated with this object + collections = [] + try: + component = lib_api.get_component_from_usage_key(object_id) + collections = authoring_api.get_entity_collections( + component.learning_package_id, + component.key, + ).values_list("key", flat=True) + except ObjectDoesNotExist: + log.warning(f"No component found for {object_id}") + + if collections: + result[Fields.tags_collections] = list(collections) + return {Fields.tags: result} diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 1b4f9b6abe7f..68a734c9e8aa 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -175,7 +175,7 @@ def setUp(self): tagging_api.add_tag_to_taxonomy(self.taxonomyB, "three") tagging_api.add_tag_to_taxonomy(self.taxonomyB, "four") - # Create a collection: + # Create collections: self.learning_package = authoring_api.get_learning_package_by_key(self.library.key) with freeze_time(created_date): self.collection = authoring_api.create_collection( @@ -379,11 +379,37 @@ def test_index_library_block_tags(self, mock_meilisearch): """ Test indexing an Library Block with tags. """ + collection1 = authoring_api.create_collection( + learning_package_id=self.library.learning_package.id, + key="COL1", + title="Collection 1", + created_by=None, + description="First Collection", + ) + + collection2 = authoring_api.create_collection( + learning_package_id=self.library.learning_package.id, + key="COL2", + title="Collection 2", + created_by=None, + description="Second Collection", + ) # Tag XBlock (these internally call `upsert_block_tags_index_docs`) tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyA, ["one", "two"]) tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyB, ["three", "four"]) + # Add Problem1 to both Collections (these internally call `upsert_block_tags_index_docs`) + # (adding in reverse order to test sorting of collection tag)) + 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 with tags at each stage doc_problem_with_tags1 = { "id": self.doc_problem1["id"], @@ -399,11 +425,29 @@ def test_index_library_block_tags(self, mock_meilisearch): 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] } } + doc_problem_with_collection2 = { + "id": self.doc_problem1["id"], + "tags": { + 'collections': [collection2.key], + 'taxonomy': ['A', 'B'], + 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] + } + } + doc_problem_with_collection1 = { + "id": self.doc_problem1["id"], + "tags": { + 'collections': [collection1.key, collection2.key], + 'taxonomy': ['A', 'B'], + 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] + } + } mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( [ call([doc_problem_with_tags1]), call([doc_problem_with_tags2]), + call([doc_problem_with_collection2]), + call([doc_problem_with_collection1]), ], 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 998be96d0023..c8442f406460 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,18 @@ 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_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 +45,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 +59,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 +107,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 +119,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 +254,56 @@ 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)) + 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": "", + }, + "tags": { + "collections": ["TOY_COLLECTION"], + "taxonomy": ["Difficulty"], + "level0": ["Difficulty > Normal"], + }, + } + + def test_collection_with_library(self): + doc = searchable_doc_for_collection(self.collection) + assert doc == { + "id": "TOY_COLLECTION", + "type": "collection", + "org": "edX", + "display_name": "Toy Collection", + "description": "my toy collection description", + "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): From 3c6617e3d39a0c3f809302c6fdbe9075ab3e74ef Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 5 Sep 2024 12:20:47 +0930 Subject: [PATCH 03/12] temp: use openedx-events temporary branch which adds CONTENT_OBJECT_ASSOCIATIONS_CHANGED --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 9097af24cc38..4830f3522cb3 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -811,7 +811,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/kernel.in openedx-django-wiki==2.1.0 # via -r requirements/edx/kernel.in -openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals +openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations # via # -r requirements/edx/kernel.in # edx-event-bus-kafka diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 4e48379cfa5e..e2ec6f46b81d 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1358,7 +1358,7 @@ openedx-django-wiki==2.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals +openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 30f35ebaba8a..e1de91706bdc 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -970,7 +970,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals +openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations # via # -r requirements/edx/base.txt # edx-event-bus-kafka diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 5f9417fd0326..782bb0fe73e1 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -119,7 +119,7 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require # openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) # TODO: Remove this once new version released -openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals +openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) openedx-learning # Open edX Learning core (experimental) openedx-mongodbproxy diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index cb0a41683736..654d2fa2361d 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1021,7 +1021,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals +openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations # via # -r requirements/edx/base.txt # edx-event-bus-kafka From 0c29552bb3b591181f8d2e3f986ad9e0df1899be Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Thu, 5 Sep 2024 12:21:13 +0930 Subject: [PATCH 04/12] 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. --- .../djangoapps/content/search/handlers.py | 28 +++++++++++-------- .../core/djangoapps/content_libraries/api.py | 13 +++++---- .../content_libraries/tests/test_api.py | 20 +++++++------ .../core/djangoapps/content_tagging/api.py | 20 ++++++++----- .../content_tagging/rest_api/v1/views.py | 19 +++++++++++-- 5 files changed, 65 insertions(+), 35 deletions(-) diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 835810879780..7b008b7a454e 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -6,7 +6,14 @@ 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 openedx_events.content_authoring.data import ( + ContentLibraryData, + ContentObjectChangedData, + LibraryBlockData, + XBlockData, +) +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_DELETED, CONTENT_LIBRARY_UPDATED, @@ -16,9 +23,8 @@ 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 @@ -139,22 +145,22 @@ def content_library_updated_handler(**kwargs) -> None: update_content_library_index_docs.delay(str(content_library_data.library_key)) -@receiver(CONTENT_OBJECT_TAGS_CHANGED) +@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 - content_key = get_content_key_from_string(str(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_key) + upsert_block_tags_index_docs(usage_key) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 2253514ee0f9..649aaa5b3287 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, @@ -1228,10 +1228,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..46015a50d750 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( + "content_object": ContentObjectChangedData( object_id=UsageKey.from_string(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( + "content_object": ContentObjectChangedData( object_id=UsageKey.from_string(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..80de81958abc 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -18,8 +18,8 @@ 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 ContentObjectChangedData +from openedx_events.content_authoring.signals import CONTENT_OBJECT_ASSOCIATIONS_CHANGED from .models import TaxonomyOrg from .types import ContentKey, TagValuesByObjectIdDict, TagValuesByTaxonomyIdDict, TaxonomyDict @@ -301,9 +301,12 @@ def set_exported_object_tags( create_invalid=True, taxonomy_export_id=str(taxonomy_export_id), ) - CONTENT_OBJECT_TAGS_CHANGED.send_event( + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( time=now(), - content_object=ContentObjectData(object_id=content_key_str) + content_object=ContentObjectChangedData( + object_id=content_key_str, + changes=["tags"], + ) ) @@ -378,7 +381,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,9 +402,12 @@ def tag_object( taxonomy=taxonomy, tags=tags, ) - CONTENT_OBJECT_TAGS_CHANGED.send_event( + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( time=now(), - content_object=ContentObjectData(object_id=object_id) + content_object=ContentObjectChangedData( + object_id=object_id, + changes=["tags"], + ) ) # Expose the oel_tagging APIs 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 From e20ee245915e0afbc7ae7fecd955cd06eec732da Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 6 Sep 2024 13:56:29 +0930 Subject: [PATCH 05/12] temp: use WIP openedx-events branch --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/kernel.in | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 4830f3522cb3..9097af24cc38 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -811,7 +811,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/kernel.in openedx-django-wiki==2.1.0 # via -r requirements/edx/kernel.in -openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations +openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals # via # -r requirements/edx/kernel.in # edx-event-bus-kafka diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index e2ec6f46b81d..4e48379cfa5e 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1358,7 +1358,7 @@ openedx-django-wiki==2.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations +openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index e1de91706bdc..30f35ebaba8a 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -970,7 +970,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations +openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals # via # -r requirements/edx/base.txt # edx-event-bus-kafka diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 782bb0fe73e1..5f9417fd0326 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -119,7 +119,7 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require # openedx-events # Open edX Events from Hooks Extension Framework (OEP-50) # TODO: Remove this once new version released -openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations +openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) openedx-learning # Open edX Learning core (experimental) openedx-mongodbproxy diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 654d2fa2361d..cb0a41683736 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1021,7 +1021,7 @@ openedx-django-require==2.1.0 # via -r requirements/edx/base.txt openedx-django-wiki==2.1.0 # via -r requirements/edx/base.txt -openedx-events @ git+https://github.com/open-craft/openedx-events.git@jill/content-object-associations +openedx-events @ git+https://github.com/open-craft/openedx-events.git@yusuf-musleh/content-library-collections-signals # via # -r requirements/edx/base.txt # edx-event-bus-kafka From 9619341dbcdef79de7f8bbbd83c106d062df7b58 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 6 Sep 2024 13:57:03 +0930 Subject: [PATCH 06/12] test: fix test failing after changes to openedx-learning --- openedx/core/djangoapps/content_libraries/tests/test_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 46015a50d750..b02e71b002a3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -443,7 +443,7 @@ def test_update_library_collection_components_event(self): "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "sender": None, "content_object": ContentObjectChangedData( - object_id=UsageKey.from_string(self.lib1_problem_block["id"]), + object_id=self.lib1_problem_block["id"], changes=["collections"], ), }, @@ -454,7 +454,7 @@ def test_update_library_collection_components_event(self): "signal": CONTENT_OBJECT_ASSOCIATIONS_CHANGED, "sender": None, "content_object": ContentObjectChangedData( - object_id=UsageKey.from_string(self.lib1_html_block["id"]), + object_id=self.lib1_html_block["id"], changes=["collections"], ), }, From 1764e87b5a37010c1962e02c2e5d026bd47523d0 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 6 Sep 2024 14:04:20 +0930 Subject: [PATCH 07/12] fix: emit both CONTENT_OBJECT_ASSOCIATIONS_CHANGED and CONTENT_OBJECT_TAGS_CHANGED in content_tagging app, while CONTENT_OBJECT_TAGS_CHANGED is being deprecated. --- .../core/djangoapps/content_tagging/api.py | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 80de81958abc..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 ContentObjectChangedData -from openedx_events.content_authoring.signals import CONTENT_OBJECT_ASSOCIATIONS_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,7 @@ 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( @@ -309,6 +313,12 @@ def set_exported_object_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) + ) + def import_course_tags_from_csv(csv_path, course_id) -> None: """ @@ -410,6 +420,12 @@ def tag_object( ) ) + # Emit a (deprecated) CONTENT_OBJECT_TAGS_CHANGED event too + CONTENT_OBJECT_TAGS_CHANGED.send_event( + time=now(), + content_object=ContentObjectData(object_id=object_id) + ) + # Expose the oel_tagging APIs add_tag_to_taxonomy = oel_tagging.add_tag_to_taxonomy From ae7926f0a66f9b01ae15361eaf7017dd79f607b1 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 6 Sep 2024 14:08:18 +0930 Subject: [PATCH 08/12] docs: minor comment update Collection.key is stored here, not ID --- openedx/core/djangoapps/content/search/documents.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index c92776e12a4b..27383260115e 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -171,7 +171,7 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: and in Collections 3 and 4, this would return: { "tags": { - "collections": [3, 4], + "collections": ["Col_key1", "Col_key2"], "taxonomy": ["Location", "Difficulty"], "level0": ["Location > North America", "Difficulty > Hard"], "level1": ["Location > North America > Canada"], From 436822e4fd05118b5de1c85e90d02b809c7d3f1c Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 10 Sep 2024 13:06:18 +0930 Subject: [PATCH 09/12] docs: replaces CONTENT_OBJECT_TAGS_CHANGED with CONTENT_OBJECT_ASSOCIATIONS_CHANGED --- docs/hooks/events.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/hooks/events.rst b/docs/hooks/events.rst index 2bceee5050ea..2ee070b8a276 100644 --- a/docs/hooks/events.rst +++ b/docs/hooks/events.rst @@ -244,10 +244,6 @@ Content Authoring Events - org.openedx.content_authoring.content_library.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 From d517d801067409de9f0d35b596ce0d208c40ba55 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 10 Sep 2024 13:06:58 +0930 Subject: [PATCH 10/12] refactor: store collections outside of tags index --- openedx/core/djangoapps/content/search/api.py | 15 +++- .../djangoapps/content/search/documents.py | 53 +++++++++++--- .../djangoapps/content/search/handlers.py | 7 +- .../content/search/tests/test_api.py | 72 ++++++++++--------- .../content/search/tests/test_documents.py | 2 +- 5 files changed, 102 insertions(+), 47 deletions(-) diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 00b908e26cc4..d601abde8e6d 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, ) @@ -317,12 +318,12 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.context_key, Fields.org, Fields.tags, - Fields.tags + "." + Fields.tags_collections, Fields.tags + "." + Fields.tags_taxonomy, Fields.tags + "." + Fields.tags_level0, Fields.tags + "." + Fields.tags_level1, Fields.tags + "." + Fields.tags_level2, Fields.tags + "." + Fields.tags_level3, + Fields.collections, Fields.type, Fields.access_id, Fields.last_published, @@ -336,11 +337,11 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: Fields.content, Fields.tags, Fields.description, + 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 # these sub-fields: "Attribute `tags.level3` is not searchable." - Fields.tags + "." + Fields.tags_collections, Fields.tags + "." + Fields.tags_taxonomy, Fields.tags + "." + Fields.tags_level0, Fields.tags + "." + Fields.tags_level1, @@ -377,6 +378,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}") @@ -573,6 +575,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 27383260115e..51eca8816430 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -54,7 +54,8 @@ class Fields: tags_level1 = "level1" tags_level2 = "level2" tags_level3 = "level3" - tags_collections = "collections" # subfield of tags, i.e. tags.collections + # List of collection.key strings this object belongs to. + collections = "collections" # 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. @@ -167,11 +168,10 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: See the comments above on "Field.tags" for an explanation of the format. - e.g. for something tagged "Difficulty: Hard" and "Location: Vancouver", - and in Collections 3 and 4, this would return: + e.g. for something tagged "Difficulty: Hard" and "Location: Vancouver" this + would return: { "tags": { - "collections": ["Col_key1", "Col_key2"], "taxonomy": ["Location", "Difficulty"], "level0": ["Location > North America", "Difficulty > Hard"], "level1": ["Location > North America > Canada"], @@ -186,16 +186,21 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: strings in a particular format that the frontend knows how to render to support hierarchical refinement by tag. """ - result = {} - # Note that we could improve performance for indexing many components from the same library/course, # if we used get_all_object_tags() to load all the tags for the library in a single query rather than loading the # tags for each component separately. all_tags = tagging_api.get_object_tags(str(object_id)).all() + if not all_tags: + # Clear out tags in the index when unselecting all tags for the block, otherwise + # it would remain the last value if a cleared Fields.tags field is not included + return {Fields.tags: {}} + result = { + Fields.tags_taxonomy: [], + Fields.tags_level0: [], + # ... other levels added as needed + } for obj_tag in all_tags: # Add the taxonomy name: - if Fields.tags_taxonomy not in result: - result[Fields.tags_taxonomy] = [] if obj_tag.taxonomy.name not in result[Fields.tags_taxonomy]: result[Fields.tags_taxonomy].append(obj_tag.taxonomy.name) # Taxonomy name plus each level of tags, in a list: # e.g. ["Location", "North America", "Canada", "Vancouver"] @@ -219,7 +224,22 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: if len(parts) == level + 2: break # We have all the levels for this tag now (e.g. parts=["Difficulty", "Hard"] -> need level0 only) + 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": ["COL_A", "COL_B"], + } + + Returns an empty dict if the object is not in any collections. + """ # Gather the collections associated with this object + result = {} collections = [] try: component = lib_api.get_component_from_usage_key(object_id) @@ -231,9 +251,9 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: log.warning(f"No component found for {object_id}") if collections: - result[Fields.tags_collections] = list(collections) + result[Fields.collections] = list(collections) - return {Fields.tags: result} + return result def searchable_doc_for_library_block(xblock_metadata: lib_api.LibraryXBlockMetadata) -> dict: @@ -278,6 +298,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 diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index 7b008b7a454e..bcd7120d79bc 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -163,4 +163,9 @@ def content_object_associations_changed_handler(**kwargs) -> None: log.error("Received invalid content object id") return - upsert_block_tags_index_docs(usage_key) + # 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 "tags" in content_object.changes: + upsert_block_tags_index_docs(usage_key) + elif "collections" in content_object.changes: + upsert_block_collections_index_docs(usage_key) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 68a734c9e8aa..f8f13d8b9a6f 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -175,7 +175,7 @@ def setUp(self): tagging_api.add_tag_to_taxonomy(self.taxonomyB, "three") tagging_api.add_tag_to_taxonomy(self.taxonomyB, "four") - # Create collections: + # Create a collection: self.learning_package = authoring_api.get_learning_package_by_key(self.library.key) with freeze_time(created_date): self.collection = authoring_api.create_collection( @@ -379,6 +379,40 @@ def test_index_library_block_tags(self, mock_meilisearch): """ Test indexing an Library Block with tags. """ + + # Tag XBlock (these internally call `upsert_block_tags_index_docs`) + tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyA, ["one", "two"]) + tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyB, ["three", "four"]) + + # Build expected docs with tags at each stage + doc_problem_with_tags1 = { + "id": self.doc_problem1["id"], + "tags": { + 'taxonomy': ['A'], + 'level0': ['A > one', 'A > two'] + } + } + doc_problem_with_tags2 = { + "id": self.doc_problem1["id"], + "tags": { + 'taxonomy': ['A', 'B'], + 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] + } + } + + mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( + [ + call([doc_problem_with_tags1]), + call([doc_problem_with_tags2]), + ], + any_order=True, + ) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_index_library_block_collections(self, mock_meilisearch): + """ + Test indexing an Library Block that is in two collections. + """ collection1 = authoring_api.create_collection( learning_package_id=self.library.learning_package.id, key="COL1", @@ -395,11 +429,7 @@ def test_index_library_block_tags(self, mock_meilisearch): description="Second Collection", ) - # Tag XBlock (these internally call `upsert_block_tags_index_docs`) - tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyA, ["one", "two"]) - tagging_api.tag_object(str(self.problem1.usage_key), self.taxonomyB, ["three", "four"]) - - # Add Problem1 to both Collections (these internally call `upsert_block_tags_index_docs`) + # Add Problem1 to both Collections (these internally call `upsert_block_collections_index_docs`) # (adding in reverse order to test sorting of collection tag)) for collection in (collection2, collection1): library_api.update_library_collection_components( @@ -410,42 +440,18 @@ def test_index_library_block_tags(self, mock_meilisearch): ], ) - # Build expected docs with tags at each stage - doc_problem_with_tags1 = { - "id": self.doc_problem1["id"], - "tags": { - 'taxonomy': ['A'], - 'level0': ['A > one', 'A > two'] - } - } - doc_problem_with_tags2 = { - "id": self.doc_problem1["id"], - "tags": { - 'taxonomy': ['A', 'B'], - 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] - } - } + # Build expected docs with collections at each stage doc_problem_with_collection2 = { "id": self.doc_problem1["id"], - "tags": { - 'collections': [collection2.key], - 'taxonomy': ['A', 'B'], - 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] - } + "collections": [collection2.key], } doc_problem_with_collection1 = { "id": self.doc_problem1["id"], - "tags": { - 'collections': [collection1.key, collection2.key], - 'taxonomy': ['A', 'B'], - 'level0': ['A > one', 'A > two', 'B > four', 'B > three'] - } + "collections": [collection1.key, collection2.key], } mock_meilisearch.return_value.index.return_value.update_documents.assert_has_calls( [ - call([doc_problem_with_tags1]), - call([doc_problem_with_tags2]), call([doc_problem_with_collection2]), call([doc_problem_with_collection1]), ], diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index c8442f406460..62ae6d66593a 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -282,8 +282,8 @@ def test_html_library_block(self): "content": { "html_content": "", }, + "collections": ["TOY_COLLECTION"], "tags": { - "collections": ["TOY_COLLECTION"], "taxonomy": ["Difficulty"], "level0": ["Difficulty > Normal"], }, From 66b2aa89478b59040c207345fb37cf77815d779d Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Tue, 10 Sep 2024 13:34:02 +0930 Subject: [PATCH 11/12] fix: missing import --- openedx/core/djangoapps/content/search/handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index a6126501ef2c..0c2054ad39a8 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -29,13 +29,13 @@ 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, upsert_library_block_index_doc, - upsert_xblock_index_doc + upsert_xblock_index_doc, ) log = logging.getLogger(__name__) From bd448743c974447520650949c3f973a05836bf5b Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 11 Sep 2024 12:16:34 +0930 Subject: [PATCH 12/12] test: fixed failing test after refactor --- openedx/core/djangoapps/content/search/tests/test_documents.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index 62ae6d66593a..d972c9ac4956 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -19,6 +19,7 @@ from ..documents import ( searchable_doc_for_course_block, searchable_doc_tags, + searchable_doc_collections, searchable_doc_for_collection, searchable_doc_for_library_block, ) @@ -261,6 +262,7 @@ def test_html_library_block(self): 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",