diff --git a/platform_plugin_aspects/sinks/course_overview_sink.py b/platform_plugin_aspects/sinks/course_overview_sink.py index d7b0fb9..ad4e78e 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_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)] = ( diff --git a/platform_plugin_aspects/sinks/serializers.py b/platform_plugin_aspects/sinks/serializers.py index 1057d80..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 +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) 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..1811938 100644 --- a/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py +++ b/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) diff --git a/platform_plugin_aspects/sinks/tests/test_serializers.py b/platform_plugin_aspects/sinks/tests/test_serializers.py index 4d41211..127451e 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 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): """ diff --git a/platform_plugin_aspects/tests/test_utils.py b/platform_plugin_aspects/tests/test_utils.py index b483b8b..f192b3c 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_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) diff --git a/platform_plugin_aspects/utils.py b/platform_plugin_aspects/utils.py index 2537495..d120bcf 100644 --- a/platform_plugin_aspects/utils.py +++ b/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)