feat: include implicit tags in course and block json

Unfortunately the tagging API doesn't have a single-query mechanism for
returning all explicit and implicit tags for all blocks in a course, so
we need to query once per block.

platform_plugin_aspects/sinks/course_overview_sink.py

@@ -20,7 +20,7 @@
 from platform_plugin_aspects.utils import (
     get_detached_xblock_types,
     get_modulestore,
-    get_tags_for_course,
+    get_tags_for_block,
 )
 
 # Defaults we want to ensure we fail early on bulk inserts
@@ -60,8 +60,6 @@ def serialize_item(self, item, many=False, initial=None):
         location_to_node = {}
         items = modulestore.get_items(course_key)
 
-        tags_by_object_id = get_tags_for_course(course_key)
-
         # Serialize the XBlocks to dicts and map them with their location as keys the
         # whole map needs to be completed before we can define relationships
         index = 0
@@ -92,7 +90,7 @@ 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"] = tags_by_object_id.get(
+            fields["xblock_data_json"]["tags"] = get_tags_for_block(
                 fields["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, get_tags_for_course
+from platform_plugin_aspects.utils import get_model, get_tags_for_block
 
 
 class BaseSinkSerializer(serializers.Serializer):  # pylint: disable=abstract-method
@@ -146,7 +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_course(overview.id).get(str(overview.id)),
+            "tags": get_tags_for_block(overview.id),
         }
         return json.dumps(json_fields)
 

platform_plugin_aspects/sinks/tests/test_course_overview_sink.py

@@ -25,15 +25,14 @@
     fake_serialize_fake_course_overview,
     get_clickhouse_http_params,
     mock_detached_xblock_types,
-    mock_get_tags_for_course,
 )
 
 
 @responses.activate(  # pylint: disable=unexpected-keyword-arg,no-value-for-parameter
     registry=OrderedRegistry
 )
 @override_settings(EVENT_SINK_CLICKHOUSE_COURSE_OVERVIEW_ENABLED=True)
-@patch("platform_plugin_aspects.sinks.course_overview_sink.get_tags_for_course")
+@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")
@@ -65,8 +64,8 @@ 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_course" api since we can't import it here
-    mock_get_tags.return_value = mock_get_tags_for_course(course, [])
+    # 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
@@ -95,7 +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.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
@@ -278,7 +277,7 @@ def test_get_last_dump_time():
     assert dt
 
 
-@patch("platform_plugin_aspects.sinks.course_overview_sink.get_tags_for_course")
+@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
@@ -299,7 +298,7 @@ def test_xblock_tree_structure(mock_modulestore, mock_detached, mock_get_tags):
     )
 
     # Fake the "get_tags_for_course" api since we can't import it here
-    mock_get_tags.return_value = mock_get_tags_for_course(course, [])
+    mock_get_tags.return_value = []
 
     sink = XBlockSink(connection_overrides={}, log=MagicMock())
 
@@ -343,7 +342,7 @@ def _check_tree_location(
     _check_tree_location(results[27], 3, 3, 3)
 
 
-@patch("platform_plugin_aspects.sinks.course_overview_sink.get_tags_for_course")
+@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, mock_get_tags):
@@ -358,13 +357,8 @@ def test_xblock_graded_completable_mode(mock_modulestore, mock_detached, mock_ge
     # Fake the "detached types" list since we can't import it here
     mock_detached.return_value = mock_detached_xblock_types()
 
-    # Fake the "get_tags_for_course" api since we can't import it here,
-    # to show the course block + 2 other blocks as "tagged".
     expected_tags = ["TAX1=tag1", "TAX1=tag2", "TAX1=tag3"]
-    mock_get_tags.return_value = mock_get_tags_for_course(
-        [course[0], course[21], course[22]],
-        expected_tags,
-    )
+    mock_get_tags.return_value = expected_tags
 
     fake_serialized_course_overview = fake_serialize_fake_course_overview(
         course_overview
@@ -378,7 +372,6 @@ def _check_item_serialized_location(
         block,
         expected_graded=0,
         expected_completion_mode="unknown",
-        expected_tags=None,
     ):
         """
         Assert the expected values in certain returned blocks or print useful debug information.
@@ -402,8 +395,3 @@ def _check_item_serialized_location(
     _check_item_serialized_location(results[34], 0, "completable")
     _check_item_serialized_location(results[35], 0, "aggregator")
     _check_item_serialized_location(results[36], 0, "excluded")
-
-    # These blocks should be tagged
-    _check_item_serialized_location(results[0], expected_tags=expected_tags)
-    _check_item_serialized_location(results[21], expected_tags=expected_tags)
-    _check_item_serialized_location(results[22], expected_tags=expected_tags)

platform_plugin_aspects/sinks/tests/test_serializers.py

@@ -7,7 +7,7 @@
     BaseSinkSerializer,
     CourseOverviewSerializer,
 )
-from test_utils.helpers import course_key_factory, mock_get_tags_for_course
+from test_utils.helpers import course_key_factory
 
 
 class TestBaseSinkSerializer(TestCase):
@@ -36,7 +36,7 @@ class TestCourseOverviewSerializer(TestCase):
     def setUp(self):
         self.serializer = CourseOverviewSerializer()
 
-    @patch("platform_plugin_aspects.sinks.serializers.get_tags_for_course")
+    @patch("platform_plugin_aspects.sinks.serializers.get_tags_for_block")
     def test_get_course_data_json(self, mock_get_tags):
         """
         Test to_representation
@@ -77,9 +77,7 @@ def test_get_course_data_json(self, mock_get_tags):
 
         # 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 = mock_get_tags_for_course(
-            [mock_course_block], expected_tags
-        )
+        mock_get_tags.return_value = expected_tags
 
         self.assertEqual(
             self.serializer.get_course_data_json(mock_overview), json.dumps(json_fields)

platform_plugin_aspects/tests/test_utils.py

@@ -14,7 +14,7 @@
     generate_superset_context,
     get_ccx_courses,
     get_model,
-    get_tags_for_course,
+    get_tags_for_block,
 )
 from test_utils.helpers import course_factory
 
@@ -233,38 +233,44 @@ def test_generate_guest_token_loc(self, mock_superset_client):
         calls = mock_superset_client.return_value.session.post.call_args
         self.assertEqual(len(calls[1]["json"]["resources"]), 3)
 
-    @patch("platform_plugin_aspects.utils._get_all_object_tags")
-    def test_get_tags_for_course(self, mock_get_all_object_tags):
+    @patch("platform_plugin_aspects.utils._get_object_tags")
+    def test_get_tags_for_block(self, mock_get_object_tags):
         """
-        Tests that get_tags_for_course works when mocking the openedx dependency.
+        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"
-        mock_taxonomies = {1: mock_taxonomy1, 2: mock_taxonomy2}
-        mock_get_all_object_tags.return_value = (
-            {
-                str(block.location): {
-                    1: ["tag1.1", "tag1.2", "tag1.3"],
-                    2: ["tag2.1", "tag2.2"],
-                    3: ["tag3.1"],
-                }
-                for block in course
-            },
-            mock_taxonomies,
-        )
-
-        course_tags = get_tags_for_course(course[0].location)
-        assert course_tags == {
-            str(block.location): [
-                "Taxonomy One=tag1.1",
-                "Taxonomy One=tag1.2",
-                "Taxonomy One=tag1.3",
-                "Taxonomy Two=tag2.1",
-                "Taxonomy Two=tag2.2",
-            ]
-            for block in course
-        }
-        mock_get_all_object_tags.assert_called_once_with(course[0].location)
+        mock_tag11 = Mock()
+        mock_tag11.taxonomy = mock_taxonomy1
+        mock_tag11.value = "tag1.1"
+        mock_tag11.parent = None
+        mock_tag12 = Mock()
+        mock_tag12.taxonomy = mock_taxonomy1
+        mock_tag12.value = "tag1.2"
+        mock_tag12.parent = mock_tag11
+        mock_tag13 = Mock()
+        mock_tag13.taxonomy = mock_taxonomy1
+        mock_tag13.value = "tag1.3"
+        mock_tag13.parent = mock_tag12
+        mock_tag21 = Mock()
+        mock_tag21.taxonomy = mock_taxonomy2
+        mock_tag21.value = "tag2.1"
+        mock_tag21.parent = None
+        mock_tag22 = Mock()
+        mock_tag22.taxonomy = mock_taxonomy2
+        mock_tag22.value = "tag2.2"
+        mock_tag22.parent = None
+        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",
+            "Taxonomy One=tag1.2",
+            "Taxonomy One=tag1.1",
+            "Taxonomy Two=tag2.1",
+            "Taxonomy Two=tag2.2",
+        ]
+        mock_get_object_tags.assert_called_once_with(course[0].location)

platform_plugin_aspects/utils.py

@@ -280,40 +280,35 @@ def get_localized_uuid(base_uuid, language):
     return str(uuid.uuid5(base_namespace, normalized_language))
 
 
-def _get_all_object_tags(course_key):  # pragma: no cover
+def _get_object_tags(usage_key):  # pragma: no cover
     """
-    Wrap the Open edX content_tagging API method get_all_object_tags.
+    Wrap the Open edX tagging API method get_object_tags.
     """
     # pylint: disable=import-outside-toplevel,import-error
-    from openedx.core.djangoapps.content_tagging.api import get_all_object_tags
+    from openedx.core.djangoapps.content_tagging.api import get_object_tags
 
-    return get_all_object_tags(course_key)
+    return get_object_tags(object_id=str(usage_key))
 
 
-def get_tags_for_course(course_key) -> dict:
+def get_tags_for_block(usage_key) -> dict:
     """
-    Return all the tags applied to the given course and its blocks.
+    Return all the tags (and their parent tags) applied to the given block.
 
-    Returned dict is a mapping between the usage key string and a list of string tags, of the form:
+    Returns a list of string tags, of the form:
 
         "taxonomy_name=tag_value"
 
     """
-    tags_by_object_id, taxonomies = _get_all_object_tags(course_key)
+    tags = _get_object_tags(usage_key)
+    serialized_tags = []
 
-    tags_and_taxonomy_by_object_id = {}
-    for object_id, tags in tags_by_object_id.items():
-        for taxonomy_id, tag_values in tags.items():
-            taxonomy = taxonomies.get(taxonomy_id)
-            if not taxonomy:
-                continue
-
-            if object_id not in tags_and_taxonomy_by_object_id:
-                tags_and_taxonomy_by_object_id[object_id] = []
+    for explicit_tag in tags:
+        serialized_tags.append(f"{explicit_tag.taxonomy.name}={explicit_tag.value}")
+        logging.critical(serialized_tags)
 
-            for tag in tag_values:
-                tags_and_taxonomy_by_object_id[object_id].append(
-                    f"{taxonomy.name}={tag}"
-                )
+        implicit_tag = explicit_tag.parent
+        while implicit_tag:
+            serialized_tags.append(f"{implicit_tag.taxonomy.name}={implicit_tag.value}")
+            implicit_tag = implicit_tag.parent
 
-    return tags_and_taxonomy_by_object_id
+    return serialized_tags

test_utils/helpers.py

@@ -184,13 +184,6 @@ def mock_detached_xblock_types():
     return {"static_tab", "about", "course_info"}
 
 
-def mock_get_tags_for_course(blocks_list, expected_tags):
-    """
-    Mock the return results of utils.get_tags_for_course for all blocks in the given list.
-    """
-    return {str(block.location): expected_tags for block in blocks_list}
-
-
 def get_clickhouse_http_params():
     """
     Get the params used in ClickHouse queries.