Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion platform_plugin_aspects/sinks/course_overview_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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)] = (
Expand Down
3 changes: 2 additions & 1 deletion platform_plugin_aspects/sinks/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
24 changes: 21 additions & 3 deletions platform_plugin_aspects/sinks/tests/test_course_overview_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
"""
Expand All @@ -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"}
Expand Down Expand Up @@ -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.
"""
Expand All @@ -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
)
Expand All @@ -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.
Expand All @@ -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)
Expand Down
15 changes: 13 additions & 2 deletions platform_plugin_aspects/sinks/tests/test_serializers.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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

Expand All @@ -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",
Expand All @@ -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):
"""
Expand Down
38 changes: 38 additions & 0 deletions platform_plugin_aspects/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
44 changes: 44 additions & 0 deletions platform_plugin_aspects/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Loading