[FC-0040] feat: update search index when object tags change

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 <https://github.com/openedx/openedx-events/blob/c0eb4ba1a3d7d066d58e5c87920b8ccb0645f769/openedx_events/content_authoring/signals.py#L207>`_
+     - org.openedx.content_authoring.content.object.tags.changed.v1
+     - 2024-03-31

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.

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: [],

openedx/core/djangoapps/content/search/handlers.py

@@ -6,21 +6,23 @@
 
 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,
     LIBRARY_BLOCK_CREATED,
     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)

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):
         """

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
 

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):
     """"

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

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

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

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