From 9583bc381668863abe2b171169dd7e1c5be857e2 Mon Sep 17 00:00:00 2001
From: Jillian Vogel <jill@opencraft.com>
Date: Wed, 1 May 2024 14:21:48 +0930
Subject: [PATCH 1/5] feat: send course/block tags with the CourseOverview and
 XBlock sinks

Tags are stored on the course_block_json/block_data_json, serialized as
a list of "tag name=tag value" strings.

If the course/block has no tags, its json "tags" value will be null.
---
 .../sinks/course_overview_sink.py             | 11 +++++-
 platform_plugin_aspects/sinks/serializers.py  |  3 +-
 .../sinks/tests/test_course_overview_sink.py  | 36 +++++++++++++++--
 .../sinks/tests/test_serializers.py           | 17 +++++++-
 platform_plugin_aspects/tests/test_utils.py   | 38 ++++++++++++++++++
 platform_plugin_aspects/utils.py              | 39 +++++++++++++++++++
 test_utils/helpers.py                         |  7 ++++
 7 files changed, 144 insertions(+), 7 deletions(-)

diff --git a/platform_plugin_aspects/sinks/course_overview_sink.py b/platform_plugin_aspects/sinks/course_overview_sink.py
index d7b0fb9..9bc481c 100644
--- a/platform_plugin_aspects/sinks/course_overview_sink.py
+++ b/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_course,
+)
 
 # Defaults we want to ensure we fail early on bulk inserts
 CLICKHOUSE_BULK_INSERT_PARAMS = {
@@ -56,6 +60,8 @@ 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
@@ -86,6 +92,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"] = tags_by_object_id.get(
+                fields["location"],
+            )
 
             fields["xblock_data_json"] = json.dumps(fields["xblock_data_json"])
             location_to_node[XBlockSink.strip_branch_and_version(block.location)] = (
diff --git a/platform_plugin_aspects/sinks/serializers.py b/platform_plugin_aspects/sinks/serializers.py
index 1057d80..de6ffc7 100644
--- a/platform_plugin_aspects/sinks/serializers.py
+++ b/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_course
 
 
 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_course(overview.id).get(str(overview.id)),
         }
         return json.dumps(json_fields)
 
diff --git a/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py b/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
index 47c04a8..f81dab9 100644
--- a/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
+++ b/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
@@ -25,6 +25,7 @@
     fake_serialize_fake_course_overview,
     get_clickhouse_http_params,
     mock_detached_xblock_types,
+    mock_get_tags_for_course,
 )
 
 
@@ -32,6 +33,7 @@
     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.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 +45,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 +65,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_course" api since we can't import it here
+    mock_get_tags.return_value = mock_get_tags_for_course(course, [])
+
     # Use the responses library to catch the POSTs to ClickHouse
     # and match them against the expected values, including CSV
     # content
@@ -89,6 +95,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)
 
 
 @responses.activate(  # pylint: disable=unexpected-keyword-arg,no-value-for-parameter
@@ -271,10 +278,11 @@ 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_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 +297,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 = mock_get_tags_for_course(course, [])
+
     sink = XBlockSink(connection_overrides={}, log=MagicMock())
 
     initial_data = {"dump_id": "xyz", "time_last_dumped": "2023-09-05"}
@@ -331,9 +343,10 @@ 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_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 +358,14 @@ 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()
 
+    # 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,
+    )
+
     fake_serialized_course_overview = fake_serialize_fake_course_overview(
         course_overview
     )
@@ -354,7 +375,10 @@ 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",
+        expected_tags=None,
     ):
         """
         Assert the expected values in certain returned blocks or print useful debug information.
@@ -363,6 +387,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)
@@ -377,3 +402,8 @@ 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)
diff --git a/platform_plugin_aspects/sinks/tests/test_serializers.py b/platform_plugin_aspects/sinks/tests/test_serializers.py
index 4d41211..ded3722 100644
--- a/platform_plugin_aspects/sinks/tests/test_serializers.py
+++ b/platform_plugin_aspects/sinks/tests/test_serializers.py
@@ -1,5 +1,5 @@
 import json
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
 from django.test import TestCase
 
@@ -7,6 +7,7 @@
     BaseSinkSerializer,
     CourseOverviewSerializer,
 )
+from test_utils.helpers import course_key_factory, mock_get_tags_for_course
 
 
 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_course")
+    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,21 @@ 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 = mock_get_tags_for_course(
+            [mock_course_block], 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):
         """
diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py
index b483b8b..5551383 100644
--- a/platform_plugin_aspects/tests/test_utils.py
+++ b/platform_plugin_aspects/tests/test_utils.py
@@ -14,7 +14,9 @@
     generate_superset_context,
     get_ccx_courses,
     get_model,
+    get_tags_for_course,
 )
+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_all_object_tags")
+    def test_get_tags_for_course(self, mock_get_all_object_tags):
+        """
+        Tests that get_tags_for_course 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)
diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py
index 2537495..e2d5cf3 100644
--- a/platform_plugin_aspects/utils.py
+++ b/platform_plugin_aspects/utils.py
@@ -278,3 +278,42 @@ 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_all_object_tags(course_key):  # pragma: no cover
+    """
+    Wrap the Open edX content_tagging API method get_all_object_tags.
+    """
+    # pylint: disable=import-outside-toplevel,import-error
+    from openedx.core.djangoapps.content_tagging.api import get_all_object_tags
+
+    return get_all_object_tags(course_key)
+
+
+def get_tags_for_course(course_key) -> dict:
+    """
+    Return all the tags applied to the given course and its blocks.
+
+    Returned dict is a mapping between the usage key string and a list of string tags, of the form:
+
+        "taxonomy_name=tag_value"
+
+    """
+    tags_by_object_id, taxonomies = _get_all_object_tags(course_key)
+
+    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 tag in tag_values:
+                tags_and_taxonomy_by_object_id[object_id].append(
+                    f"{taxonomy.name}={tag}"
+                )
+
+    return tags_and_taxonomy_by_object_id
diff --git a/test_utils/helpers.py b/test_utils/helpers.py
index 8e09fa1..11cc573 100644
--- a/test_utils/helpers.py
+++ b/test_utils/helpers.py
@@ -184,6 +184,13 @@ 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.

From 6055922e75b5582f83f63adaf8fd244338503b92 Mon Sep 17 00:00:00 2001
From: Jillian Vogel <jill@opencraft.com>
Date: Thu, 2 May 2024 09:11:05 +0930
Subject: [PATCH 2/5] 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.
---
 .../sinks/course_overview_sink.py             |  6 +-
 platform_plugin_aspects/sinks/serializers.py  |  4 +-
 .../sinks/tests/test_course_overview_sink.py  | 28 +++-----
 .../sinks/tests/test_serializers.py           |  8 +--
 platform_plugin_aspects/tests/test_utils.py   | 64 ++++++++++---------
 platform_plugin_aspects/utils.py              | 39 +++++------
 test_utils/helpers.py                         |  7 --
 7 files changed, 67 insertions(+), 89 deletions(-)

diff --git a/platform_plugin_aspects/sinks/course_overview_sink.py b/platform_plugin_aspects/sinks/course_overview_sink.py
index 9bc481c..ad4e78e 100644
--- a/platform_plugin_aspects/sinks/course_overview_sink.py
+++ b/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"],
             )
 
diff --git a/platform_plugin_aspects/sinks/serializers.py b/platform_plugin_aspects/sinks/serializers.py
index de6ffc7..a477780 100644
--- a/platform_plugin_aspects/sinks/serializers.py
+++ b/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)
 
diff --git a/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py b/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
index f81dab9..ee6a0bd 100644
--- a/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
+++ b/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
@@ -25,7 +25,6 @@
     fake_serialize_fake_course_overview,
     get_clickhouse_http_params,
     mock_detached_xblock_types,
-    mock_get_tags_for_course,
 )
 
 
@@ -33,7 +32,7 @@
     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)
diff --git a/platform_plugin_aspects/sinks/tests/test_serializers.py b/platform_plugin_aspects/sinks/tests/test_serializers.py
index ded3722..127451e 100644
--- a/platform_plugin_aspects/sinks/tests/test_serializers.py
+++ b/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)
diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py
index 5551383..bbb8202 100644
--- a/platform_plugin_aspects/tests/test_utils.py
+++ b/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)
diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py
index e2d5cf3..ebfaf80 100644
--- a/platform_plugin_aspects/utils.py
+++ b/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
diff --git a/test_utils/helpers.py b/test_utils/helpers.py
index 11cc573..8e09fa1 100644
--- a/test_utils/helpers.py
+++ b/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.

From d6e70139f547ac13078159b13056cafd3902bf6f Mon Sep 17 00:00:00 2001
From: Jillian Vogel <jill@opencraft.com>
Date: Mon, 6 May 2024 09:02:15 +0930
Subject: [PATCH 3/5] fix: object tag "parent" tags must be accessed through
 their "tag"

---
 platform_plugin_aspects/tests/test_utils.py | 37 ++++++++++-----------
 platform_plugin_aspects/utils.py            |  4 +--
 2 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py
index bbb8202..a01f210 100644
--- a/platform_plugin_aspects/tests/test_utils.py
+++ b/platform_plugin_aspects/tests/test_utils.py
@@ -243,26 +243,23 @@ def test_get_tags_for_block(self, mock_get_object_tags):
         mock_taxonomy1.name = "Taxonomy One"
         mock_taxonomy2 = Mock()
         mock_taxonomy2.name = "Taxonomy Two"
-        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
+
+        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)
diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py
index ebfaf80..27b3a1b 100644
--- a/platform_plugin_aspects/utils.py
+++ b/platform_plugin_aspects/utils.py
@@ -304,9 +304,7 @@ def get_tags_for_block(usage_key) -> dict:
 
     for explicit_tag in tags:
         serialized_tags.append(f"{explicit_tag.taxonomy.name}={explicit_tag.value}")
-        logging.critical(serialized_tags)
-
-        implicit_tag = explicit_tag.parent
+        implicit_tag = explicit_tag.tag.parent
         while implicit_tag:
             serialized_tags.append(f"{implicit_tag.taxonomy.name}={implicit_tag.value}")
             implicit_tag = implicit_tag.parent

From 30103e61626c2af6dee94de52e6fa2d3d902982e Mon Sep 17 00:00:00 2001
From: Brian Mesick <bmesick@tcril.org>
Date: Mon, 6 May 2024 10:36:19 -0400
Subject: [PATCH 4/5] refactor: Make tag output a dict

---
 .../sinks/tests/test_course_overview_sink.py  |  4 +--
 platform_plugin_aspects/tests/test_utils.py   | 11 +++----
 platform_plugin_aspects/utils.py              | 32 +++++++++++++------
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py b/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
index ee6a0bd..1811938 100644
--- a/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
+++ b/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
@@ -65,7 +65,7 @@ def test_course_publish_success(
     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 = []
+    mock_get_tags.return_value = {}
 
     # Use the responses library to catch the POSTs to ClickHouse
     # and match them against the expected values, including CSV
@@ -298,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.return_value = {}
 
     sink = XBlockSink(connection_overrides={}, log=MagicMock())
 
diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py
index a01f210..f192b3c 100644
--- a/platform_plugin_aspects/tests/test_utils.py
+++ b/platform_plugin_aspects/tests/test_utils.py
@@ -263,11 +263,8 @@ def mock_tag(taxonomy, value, 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",
-        ]
+        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)
diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py
index 27b3a1b..e19bf06 100644
--- a/platform_plugin_aspects/utils.py
+++ b/platform_plugin_aspects/utils.py
@@ -284,29 +284,41 @@ def _get_object_tags(usage_key):  # pragma: no cover
     """
     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_object_tags
+    try:
+        # pylint: disable=import-outside-toplevel,import-error
+        from openedx.core.djangoapps.content_tagging.api import get_object_tags
 
-    return get_object_tags(object_id=str(usage_key))
+        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 list of string tags, of the form:
-
-        "taxonomy_name=tag_value"
-
+    Returns a dict of [taxonomy]: [tag, tag, tag]
     """
     tags = _get_object_tags(usage_key)
-    serialized_tags = []
+    serialized_tags = {}
 
     for explicit_tag in tags:
-        serialized_tags.append(f"{explicit_tag.taxonomy.name}={explicit_tag.value}")
+        _add_tag(explicit_tag, serialized_tags)
         implicit_tag = explicit_tag.tag.parent
+
         while implicit_tag:
-            serialized_tags.append(f"{implicit_tag.taxonomy.name}={implicit_tag.value}")
+            _add_tag(implicit_tag, serialized_tags)
             implicit_tag = implicit_tag.parent
 
     return serialized_tags
+
+
+def _add_tag(tag, serialized_tags):
+    """
+    Adds 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)

From 3249924454a57df002a5ae56dbf9a262fb8ed70d Mon Sep 17 00:00:00 2001
From: Brian Mesick <bmesick@tcril.org>
Date: Mon, 6 May 2024 10:42:53 -0400
Subject: [PATCH 5/5] style: Linting fixes

---
 platform_plugin_aspects/utils.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py
index e19bf06..d120bcf 100644
--- a/platform_plugin_aspects/utils.py
+++ b/platform_plugin_aspects/utils.py
@@ -285,7 +285,7 @@ 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,import-error
+        # 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))
@@ -316,7 +316,7 @@ def get_tags_for_block(usage_key) -> dict:
 
 def _add_tag(tag, serialized_tags):
     """
-    Adds a tag to our serialized list of 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]