Merge pull request #41 from openedx/jill/sync-tags-on-course-sink

feat: send course/block tags with the CourseOverview and XBlock sinks

platform_plugin_aspects/sinks/course_overview_sink.py

@@ -17,7 +17,11 @@
 
 from platform_plugin_aspects.sinks.base_sink import ModelBaseSink
 from platform_plugin_aspects.sinks.serializers import CourseOverviewSerializer
-from platform_plugin_aspects.utils import get_detached_xblock_types, get_modulestore
+from platform_plugin_aspects.utils import (
+    get_detached_xblock_types,
+    get_modulestore,
+    get_tags_for_block,
+)
 
 # Defaults we want to ensure we fail early on bulk inserts
 CLICKHOUSE_BULK_INSERT_PARAMS = {
@@ -86,6 +90,9 @@ def serialize_item(self, item, many=False, initial=None):
             fields["xblock_data_json"]["section"] = section_idx
             fields["xblock_data_json"]["subsection"] = subsection_idx
             fields["xblock_data_json"]["unit"] = unit_idx
+            fields["xblock_data_json"]["tags"] = get_tags_for_block(
+                fields["location"],
+            )
 
             fields["xblock_data_json"] = json.dumps(fields["xblock_data_json"])
             location_to_node[XBlockSink.strip_branch_and_version(block.location)] = (

platform_plugin_aspects/sinks/serializers.py

@@ -6,7 +6,7 @@
 from django.utils import timezone
 from rest_framework import serializers
 
-from platform_plugin_aspects.utils import get_model
+from platform_plugin_aspects.utils import get_model, get_tags_for_block
 
 
 class BaseSinkSerializer(serializers.Serializer):  # pylint: disable=abstract-method
@@ -146,6 +146,7 @@ def get_course_data_json(self, overview):
             "entrance_exam_enabled": getattr(overview, "entrance_exam_enabled", ""),
             "external_id": getattr(overview, "external_id", ""),
             "language": getattr(overview, "language", ""),
+            "tags": get_tags_for_block(overview.id),
         }
         return json.dumps(json_fields)
 

platform_plugin_aspects/sinks/tests/test_course_overview_sink.py

@@ -32,6 +32,7 @@
     registry=OrderedRegistry
 )
 @override_settings(EVENT_SINK_CLICKHOUSE_COURSE_OVERVIEW_ENABLED=True)
+@patch("platform_plugin_aspects.sinks.course_overview_sink.get_tags_for_block")
 @patch("platform_plugin_aspects.sinks.CourseOverviewSink.serialize_item")
 @patch("platform_plugin_aspects.sinks.CourseOverviewSink.get_model")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_detached_xblock_types")
@@ -43,6 +44,7 @@ def test_course_publish_success(
     mock_detached,
     mock_overview,
     mock_serialize_item,
+    mock_get_tags,
 ):
     """
     Test of a successful end-to-end run.
@@ -62,6 +64,9 @@ def test_course_publish_success(
     mock_overview.return_value.get_from_id.return_value = course_overview
     mock_get_ccx_courses.return_value = []
 
+    # Fake the "get_tags_for_block" api since we can't import it here
+    mock_get_tags.return_value = {}
+
     # Use the responses library to catch the POSTs to ClickHouse
     # and match them against the expected values, including CSV
     # content
@@ -89,6 +94,7 @@ def test_course_publish_success(
     assert mock_modulestore.call_count == 1
     assert mock_detached.call_count == 1
     mock_get_ccx_courses.assert_called_once_with(course_overview.id)
+    mock_get_tags.call_count == len(course)
 
 
 @responses.activate(  # pylint: disable=unexpected-keyword-arg,no-value-for-parameter
@@ -271,10 +277,11 @@ def test_get_last_dump_time():
     assert dt
 
 
+@patch("platform_plugin_aspects.sinks.course_overview_sink.get_tags_for_block")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_detached_xblock_types")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_modulestore")
 # pytest:disable=unused-argument
-def test_xblock_tree_structure(mock_modulestore, mock_detached):
+def test_xblock_tree_structure(mock_modulestore, mock_detached, mock_get_tags):
     """
     Test that our calculations of section/subsection/unit are correct.
     """
@@ -289,6 +296,10 @@ def test_xblock_tree_structure(mock_modulestore, mock_detached):
     fake_serialized_course_overview = fake_serialize_fake_course_overview(
         course_overview
     )
+
+    # Fake the "get_tags_for_course" api since we can't import it here
+    mock_get_tags.return_value = {}
+
     sink = XBlockSink(connection_overrides={}, log=MagicMock())
 
     initial_data = {"dump_id": "xyz", "time_last_dumped": "2023-09-05"}
@@ -331,9 +342,10 @@ def _check_tree_location(
     _check_tree_location(results[27], 3, 3, 3)
 
 
+@patch("platform_plugin_aspects.sinks.course_overview_sink.get_tags_for_block")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_detached_xblock_types")
 @patch("platform_plugin_aspects.sinks.course_overview_sink.get_modulestore")
-def test_xblock_graded_completable_mode(mock_modulestore, mock_detached):
+def test_xblock_graded_completable_mode(mock_modulestore, mock_detached, mock_get_tags):
     """
     Test that our grading and completion fields serialize.
     """
@@ -345,6 +357,9 @@ def test_xblock_graded_completable_mode(mock_modulestore, mock_detached):
     # Fake the "detached types" list since we can't import it here
     mock_detached.return_value = mock_detached_xblock_types()
 
+    expected_tags = ["TAX1=tag1", "TAX1=tag2", "TAX1=tag3"]
+    mock_get_tags.return_value = expected_tags
+
     fake_serialized_course_overview = fake_serialize_fake_course_overview(
         course_overview
     )
@@ -354,7 +369,9 @@ def test_xblock_graded_completable_mode(mock_modulestore, mock_detached):
     results = sink.serialize_item(fake_serialized_course_overview, initial=initial_data)
 
     def _check_item_serialized_location(
-        block, expected_graded=0, expected_completion_mode="unknown"
+        block,
+        expected_graded=0,
+        expected_completion_mode="unknown",
     ):
         """
         Assert the expected values in certain returned blocks or print useful debug information.
@@ -363,6 +380,7 @@ def _check_item_serialized_location(
             j = json.loads(block["xblock_data_json"])
             assert j["graded"] == expected_graded
             assert j["completion_mode"] == expected_completion_mode
+            assert j["tags"] == expected_tags
         except AssertionError as e:
             print(e)
             print(block)

platform_plugin_aspects/sinks/tests/test_serializers.py

@@ -1,12 +1,13 @@
 import json
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
 from django.test import TestCase
 
 from platform_plugin_aspects.sinks.serializers import (
     BaseSinkSerializer,
     CourseOverviewSerializer,
 )
+from test_utils.helpers import course_key_factory
 
 
 class TestBaseSinkSerializer(TestCase):
@@ -35,7 +36,8 @@ class TestCourseOverviewSerializer(TestCase):
     def setUp(self):
         self.serializer = CourseOverviewSerializer()
 
-    def test_get_course_data_json(self):
+    @patch("platform_plugin_aspects.sinks.serializers.get_tags_for_block")
+    def test_get_course_data_json(self, mock_get_tags):
         """
         Test to_representation
 
@@ -56,6 +58,7 @@ def test_get_course_data_json(self):
             "language": getattr(overview, "language", ""),
         }
         """
+        expected_tags = ["TAX1=tag1", "TAX2=tag2"]
         json_fields = {
             "advertised_start": "2018-01-01T00:00:00Z",
             "announcement": "announcement",
@@ -67,11 +70,19 @@ def test_get_course_data_json(self):
             "entrance_exam_enabled": "entrance_exam_enabled",
             "external_id": "external_id",
             "language": "language",
+            "tags": expected_tags,
         }
         mock_overview = Mock(**json_fields)
+        mock_overview.id = course_key_factory()
+
+        # Fake the "get_tags_for_course" api since we can't import it here
+        mock_course_block = Mock(location=mock_overview.id)
+        mock_get_tags.return_value = expected_tags
+
         self.assertEqual(
             self.serializer.get_course_data_json(mock_overview), json.dumps(json_fields)
         )
+        mock_get_tags.assert_called_once_with(mock_overview.id)
 
     def test_get_course_key(self):
         """

platform_plugin_aspects/tests/test_utils.py

@@ -14,7 +14,9 @@
     generate_superset_context,
     get_ccx_courses,
     get_model,
+    get_tags_for_block,
 )
+from test_utils.helpers import course_factory
 
 COURSE_ID = "course-v1:org+course+run"
 
@@ -230,3 +232,39 @@ def test_generate_guest_token_loc(self, mock_superset_client):
         # We should have one resource for en_US, one for es_419, and one untranslated
         calls = mock_superset_client.return_value.session.post.call_args
         self.assertEqual(len(calls[1]["json"]["resources"]), 3)
+
+    @patch("platform_plugin_aspects.utils._get_object_tags")
+    def test_get_tags_for_block(self, mock_get_object_tags):
+        """
+        Tests that get_tags_for_block works when mocking the openedx dependency.
+        """
+        course = course_factory()
+        mock_taxonomy1 = Mock()
+        mock_taxonomy1.name = "Taxonomy One"
+        mock_taxonomy2 = Mock()
+        mock_taxonomy2.name = "Taxonomy Two"
+
+        def mock_tag(taxonomy, value, parent=None):
+            """
+            Returns a mock ObjectTag.
+            """
+            mock_tag = Mock()
+            mock_tag.taxonomy = taxonomy
+            mock_tag.value = value
+            mock_tag.tag = mock_tag
+            mock_tag.tag.parent = parent
+            return mock_tag
+
+        mock_tag11 = mock_tag(mock_taxonomy1, "tag1.1")
+        mock_tag12 = mock_tag(mock_taxonomy1, "tag1.2", mock_tag11.tag)
+        mock_tag13 = mock_tag(mock_taxonomy1, "tag1.3", mock_tag12.tag)
+        mock_tag21 = mock_tag(mock_taxonomy2, "tag2.1")
+        mock_tag22 = mock_tag(mock_taxonomy2, "tag2.2")
+        mock_get_object_tags.return_value = [mock_tag13, mock_tag21, mock_tag22]
+
+        course_tags = get_tags_for_block(course[0].location)
+        assert course_tags == {
+            "Taxonomy One": ["tag1.3", "tag1.2", "tag1.1"],
+            "Taxonomy Two": ["tag2.1", "tag2.2"],
+        }
+        mock_get_object_tags.assert_called_once_with(course[0].location)

platform_plugin_aspects/utils.py

@@ -278,3 +278,47 @@ def get_localized_uuid(base_uuid, language):
     base_namespace = uuid.uuid5(base_uuid, "superset")
     normalized_language = language.lower().replace("-", "_")
     return str(uuid.uuid5(base_namespace, normalized_language))
+
+
+def _get_object_tags(usage_key):  # pragma: no cover
+    """
+    Wrap the Open edX tagging API method get_object_tags.
+    """
+    try:
+        # pylint: disable=import-outside-toplevel
+        from openedx.core.djangoapps.content_tagging.api import get_object_tags
+
+        return get_object_tags(object_id=str(usage_key))
+    # Pre-Redwood versions of Open edX don't have this API
+    except ImportError:
+        return {}
+
+
+def get_tags_for_block(usage_key) -> dict:
+    """
+    Return all the tags (and their parent tags) applied to the given block.
+
+    Returns a dict of [taxonomy]: [tag, tag, tag]
+    """
+    tags = _get_object_tags(usage_key)
+    serialized_tags = {}
+
+    for explicit_tag in tags:
+        _add_tag(explicit_tag, serialized_tags)
+        implicit_tag = explicit_tag.tag.parent
+
+        while implicit_tag:
+            _add_tag(implicit_tag, serialized_tags)
+            implicit_tag = implicit_tag.parent
+
+    return serialized_tags
+
+
+def _add_tag(tag, serialized_tags):
+    """
+    Add a tag to our serialized list of tags.
+    """
+    if tag.taxonomy.name not in serialized_tags:
+        serialized_tags[tag.taxonomy.name] = [tag.value]
+    else:
+        serialized_tags[tag.taxonomy.name].append(tag.value)