diff --git a/docs/hooks/events.rst b/docs/hooks/events.rst index 461f484a58d6..7f9584c9e8e1 100644 --- a/docs/hooks/events.rst +++ b/docs/hooks/events.rst @@ -244,3 +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 diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 382e8f8d759a..24d69dfd9fcf 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -117,11 +117,9 @@ def _get_meili_api_key_uid(): Helper method to get the UID of the API key we're using for Meilisearch """ global _MEILI_API_KEY_UID # pylint: disable=global-statement - - if _MEILI_API_KEY_UID is not None: - return _MEILI_API_KEY_UID - - _MEILI_API_KEY_UID = _get_meilisearch_client().get_key(settings.MEILISEARCH_API_KEY).uid + if _MEILI_API_KEY_UID is None: + _MEILI_API_KEY_UID = _get_meilisearch_client().get_key(settings.MEILISEARCH_API_KEY).uid + return _MEILI_API_KEY_UID def _wait_for_meili_task(info: TaskInfo) -> None: @@ -274,6 +272,7 @@ def is_meilisearch_enabled() -> bool: return False +# pylint: disable=too-many-statements def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: """ Rebuild the Meilisearch index from scratch @@ -369,6 +368,7 @@ def rebuild_index(status_cb: Callable[[str], None] | None = None) -> None: def add_with_children(block): """ Recursively index the given XBlock/component """ doc = searchable_doc_for_course_block(block) + doc.update(searchable_doc_tags(block.usage_key)) docs.append(doc) # pylint: disable=cell-var-from-loop _recurse_children(block, add_with_children) # pylint: disable=cell-var-from-loop @@ -456,6 +456,15 @@ def upsert_content_library_index_docs(library_key: LibraryLocatorV2) -> None: _update_index_docs(docs) +def upsert_block_tags_index_docs(usage_key: UsageKey): + """ + Updates the tags data in documents for the given Course/Library block + """ + doc = {Fields.id: meili_id_from_opaque_key(usage_key)} + doc.update(searchable_doc_tags(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 e74bfaac0381..062c5c44a827 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -172,7 +172,9 @@ def _tags_for_content_object(object_id: UsageKey | LearningContextKey) -> dict: # tags for each component separately. all_tags = tagging_api.get_object_tags(str(object_id)).all() if not all_tags: - return {} + # 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: [], diff --git a/openedx/core/djangoapps/content/search/handlers.py b/openedx/core/djangoapps/content/search/handlers.py index d782d4a07040..1a80b2215781 100644 --- a/openedx/core/djangoapps/content/search/handlers.py +++ b/openedx/core/djangoapps/content/search/handlers.py @@ -6,7 +6,7 @@ from django.db.models.signals import post_delete from django.dispatch import receiver -from openedx_events.content_authoring.data import ContentLibraryData, LibraryBlockData, XBlockData +from openedx_events.content_authoring.data import ContentLibraryData, ContentObjectData, LibraryBlockData, XBlockData from openedx_events.content_authoring.signals import ( CONTENT_LIBRARY_DELETED, CONTENT_LIBRARY_UPDATED, @@ -14,13 +14,15 @@ LIBRARY_BLOCK_DELETED, XBLOCK_CREATED, XBLOCK_DELETED, - XBLOCK_UPDATED + XBLOCK_UPDATED, + CONTENT_OBJECT_TAGS_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 +from .api import only_if_meilisearch_enabled, upsert_block_tags_index_docs from .tasks import ( delete_library_block_index_doc, delete_xblock_index_doc, @@ -133,3 +135,24 @@ def content_library_updated_handler(**kwargs) -> None: return update_content_library_index_docs.delay(str(content_library_data.library_key)) + + +@receiver(CONTENT_OBJECT_TAGS_CHANGED) +@only_if_meilisearch_enabled +def content_object_tags_changed_handler(**kwargs) -> None: + """ + Update the 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): + 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: + log.error("Received invalid content object id") + return + + upsert_block_tags_index_docs(content_object_tags.object_id) diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 43e1eb20e68e..89913dee41dc 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -3,6 +3,8 @@ """ from __future__ import annotations +import copy + from unittest.mock import MagicMock, call, patch import ddt @@ -11,6 +13,7 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content_libraries import api as library_api +from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase @@ -117,6 +120,16 @@ def setUp(self): "access_id": lib_access.id, } + # Create a couple of taxonomies with tags + self.taxonomyA = tagging_api.create_taxonomy(name="A", export_id="A") + self.taxonomyB = tagging_api.create_taxonomy(name="B", export_id="B") + tagging_api.set_taxonomy_orgs(self.taxonomyA, all_orgs=True) + tagging_api.set_taxonomy_orgs(self.taxonomyB, all_orgs=True) + tagging_api.add_tag_to_taxonomy(self.taxonomyA, "one") + tagging_api.add_tag_to_taxonomy(self.taxonomyA, "two") + tagging_api.add_tag_to_taxonomy(self.taxonomyB, "three") + tagging_api.add_tag_to_taxonomy(self.taxonomyB, "four") + @override_settings(MEILISEARCH_ENABLED=False) def test_reindex_meilisearch_disabled(self, mock_meilisearch): with self.assertRaises(RuntimeError): @@ -127,11 +140,19 @@ def test_reindex_meilisearch_disabled(self, mock_meilisearch): @override_settings(MEILISEARCH_ENABLED=True) def test_reindex_meilisearch(self, mock_meilisearch): + # Add tags field to doc, since reindex calls includes tags + doc_sequential = copy.deepcopy(self.doc_sequential) + doc_sequential["tags"] = {} + doc_vertical = copy.deepcopy(self.doc_vertical) + doc_vertical["tags"] = {} + doc_problem = copy.deepcopy(self.doc_problem) + doc_problem["tags"] = {} + api.rebuild_index() mock_meilisearch.return_value.index.return_value.add_documents.assert_has_calls( [ - call([self.doc_sequential, self.doc_vertical]), - call([self.doc_problem]), + call([doc_sequential, doc_vertical]), + call([doc_problem]), ], any_order=True, ) @@ -154,6 +175,40 @@ 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_index_xblock_tags(self, mock_meilisearch): + """ + Test indexing an XBlock with tags. + """ + + # Tag XBlock (these internally call `upsert_block_tags_index_docs`) + tagging_api.tag_object(str(self.sequential.usage_key), self.taxonomyA, ["one", "two"]) + tagging_api.tag_object(str(self.sequential.usage_key), self.taxonomyB, ["three", "four"]) + + # Build expected docs with tags at each stage + doc_sequential_with_tags1 = { + "id": self.doc_sequential["id"], + "tags": { + 'taxonomy': ['A'], + 'level0': ['A > one', 'A > two'] + } + } + doc_sequential_with_tags2 = { + "id": self.doc_sequential["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_sequential_with_tags1]), + call([doc_sequential_with_tags2]), + ], + any_order=True, + ) + @override_settings(MEILISEARCH_ENABLED=True) def test_delete_index_xblock(self, mock_meilisearch): """ @@ -174,6 +229,40 @@ def test_index_library_block_metadata(self, mock_meilisearch): mock_meilisearch.return_value.index.return_value.update_documents.assert_called_once_with([self.doc_problem]) + @override_settings(MEILISEARCH_ENABLED=True) + 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.problem.usage_key), self.taxonomyA, ["one", "two"]) + tagging_api.tag_object(str(self.problem.usage_key), self.taxonomyB, ["three", "four"]) + + # Build expected docs with tags at each stage + doc_problem_with_tags1 = { + "id": self.doc_problem["id"], + "tags": { + 'taxonomy': ['A'], + 'level0': ['A > one', 'A > two'] + } + } + doc_problem_with_tags2 = { + "id": self.doc_problem["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_delete_index_library_block(self, mock_meilisearch): """ diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 1a6662c52911..47b157c1a34e 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -11,12 +11,15 @@ import openedx_tagging.core.tagging.api as oel_tagging from django.db.models import Exists, OuterRef, Q, QuerySet +from django.utils.timezone import now from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy 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 .models import TaxonomyOrg from .types import ContentKey, TagValuesByObjectIdDict, TagValuesByTaxonomyIdDict, TaxonomyDict @@ -298,6 +301,10 @@ def set_exported_object_tags( create_invalid=True, taxonomy_export_id=str(taxonomy_export_id), ) + 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: @@ -371,6 +378,9 @@ 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 + when tagging an object. + tags: A list of the values of the tags from this taxonomy to apply. object_tag_class: Optional. Use a proxy subclass of ObjectTag for additional @@ -389,7 +399,10 @@ def tag_object( taxonomy=taxonomy, tags=tags, ) - + CONTENT_OBJECT_TAGS_CHANGED.send_event( + time=now(), + content_object=ContentObjectData(object_id=object_id) + ) # 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 fc14037ef16f..71b210b9e561 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -5,16 +5,18 @@ from django.db.models import Count from django.http import StreamingHttpResponse -from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView from openedx_tagging.core.tagging import rules as oel_tagging_rules +from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView from rest_framework import status from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView -from ...auth import has_view_object_tags_access +from openedx_events.content_authoring.data import ContentObjectData +from openedx_events.content_authoring.signals import CONTENT_OBJECT_TAGS_CHANGED +from ...auth import has_view_object_tags_access from ...api import ( create_taxonomy, generate_csv_rows, @@ -138,12 +140,25 @@ def orgs(self, request, **_kwargs) -> Response: class ObjectTagOrgView(ObjectTagView): """ View to create and retrieve ObjectTags for a provided Object ID (object_id). - This view extends the ObjectTagView to add Organization filters for the results. + This view extends the ObjectTagView to add Organization filters for the results, + and fires events when the tags are updated. Refer to ObjectTagView docstring for usage details. """ filter_backends = [ObjectTagTaxonomyOrgFilterBackend] + def update(self, request, *args, **kwargs) -> Response: + """ + Extend the update method to fire CONTENT_OBJECT_TAGS_CHANGED event + """ + response = super().update(request, *args, **kwargs) + if response.status_code == 200: + object_id = kwargs.get('object_id') + CONTENT_OBJECT_TAGS_CHANGED.send_event( + content_object=ContentObjectData(object_id=object_id) + ) + return response + class ObjectTagExportView(APIView): """" diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3826782cc2d9..53e65a470605 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -782,7 +782,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==9.5.2 +openedx-events==9.9.1 # via # -r requirements/edx/kernel.in # edx-event-bus-kafka diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index f17103ccdfa3..3fb59f9c44d4 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1303,7 +1303,7 @@ openedx-django-wiki==2.1.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -openedx-events==9.5.2 +openedx-events==9.9.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 2ecfda3de161..9085355b559b 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -920,7 +920,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==9.5.2 +openedx-events==9.9.1 # via # -r requirements/edx/base.txt # edx-event-bus-kafka diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index d377211ffe76..e1edcfa1fd2c 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -975,7 +975,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==9.5.2 +openedx-events==9.9.1 # via # -r requirements/edx/base.txt # edx-event-bus-kafka