From 52c0c2d18a3f0103683867d1d57fcaf92b8b6aec Mon Sep 17 00:00:00 2001 From: Sara Burns Date: Thu, 8 Oct 2026 13:32:20 -0400 Subject: [PATCH 1/4] feat: add video clip start and end times to course block data Video blocks can be configured to play only a clip of the source video. The player reports positions against the full source video but the duration of only the clip, so Aspects needs the clip offsets to line them up. Co-Authored-By: Claude Opus 5.5 --- .../sinks/course_overview_sink.py | 24 +++++++++++ .../sinks/tests/test_course_overview_sink.py | 40 ++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/platform_plugin_aspects/sinks/course_overview_sink.py b/src/platform_plugin_aspects/sinks/course_overview_sink.py index 62cf879f..4be60112 100644 --- a/src/platform_plugin_aspects/sinks/course_overview_sink.py +++ b/src/platform_plugin_aspects/sinks/course_overview_sink.py @@ -177,6 +177,18 @@ def serialize_xblock(self, item, detached_xblock_types, dump_id, time_last_dumpe "completion_mode": getattr(item, "completion_mode", ""), } + # Video blocks can be configured to play only a clip of the source + # video. Player events report positions relative to the full source + # video but durations relative to the clip, so reporting needs the + # offsets to line them up. + if block_type == "video": + json_data["video_start_time"] = XBlockSink.get_seconds( + getattr(item, "start_time", None) + ) + json_data["video_end_time"] = XBlockSink.get_seconds( + getattr(item, "end_time", None) + ) + # Core table data, if things change here it's a big deal. serialized_block = { "org": course_key.org, @@ -194,6 +206,18 @@ def serialize_xblock(self, item, detached_xblock_types, dump_id, time_last_dumpe return serialized_block + @staticmethod + def get_seconds(value): + """ + Convert a video RelativeTime field (a timedelta) to seconds. + Args: + value: a timedelta, or None if the field is not set. + Returns: the number of seconds as a float, 0.0 if not set. + """ + if not value: + return 0.0 + return value.total_seconds() + @staticmethod def strip_branch_and_version(location): """ diff --git a/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py b/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py index 2bb58011..0bbf35d3 100644 --- a/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py +++ b/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py @@ -4,7 +4,7 @@ import json import logging -from datetime import datetime +from datetime import datetime, timedelta from unittest.mock import MagicMock, patch import pytest @@ -17,6 +17,7 @@ from platform_plugin_aspects.sinks import CourseOverviewSink, XBlockSink from platform_plugin_aspects.tasks import dump_course_to_clickhouse from test_utils.helpers import ( + FakeXBlock, check_block_csv_matcher, check_overview_csv_matcher, course_factory, @@ -413,3 +414,40 @@ def _check_item_serialized_location( _check_item_serialized_location(results[31], 0, "completable") _check_item_serialized_location(results[32], 0, "aggregator") _check_item_serialized_location(results[33], 0, "excluded") + + +@pytest.mark.parametrize( + "start_time,end_time,expected_start,expected_end", + [ + (timedelta(seconds=60), timedelta(seconds=240), 60.0, 240.0), + (timedelta(seconds=90.5), None, 90.5, 0.0), + (None, timedelta(seconds=30), 0.0, 30.0), + (timedelta(0), timedelta(0), 0.0, 0.0), + ], +) +def test_xblock_video_clip_times(start_time, end_time, expected_start, expected_end): + """ + Test that video start and end times serialize as seconds. + """ + block = FakeXBlock("Video", block_type="video") + block.start_time = start_time + block.end_time = end_time + + sink = XBlockSink(connection_overrides={}, log=MagicMock()) + result = sink.serialize_xblock(block, [], "xyz", "2023-09-05") + + assert result["xblock_data_json"]["video_start_time"] == expected_start + assert result["xblock_data_json"]["video_end_time"] == expected_end + + +def test_xblock_non_video_has_no_clip_times(): + """ + Test that non-video blocks do not get video clip times. + """ + block = FakeXBlock("Problem", block_type="problem") + + sink = XBlockSink(connection_overrides={}, log=MagicMock()) + result = sink.serialize_xblock(block, [], "xyz", "2023-09-05") + + assert "video_start_time" not in result["xblock_data_json"] + assert "video_end_time" not in result["xblock_data_json"] From 5bae68be0840af26549b22833c2ee38f0eb5527a Mon Sep 17 00:00:00 2001 From: Sara Burns Date: Thu, 8 Oct 2026 13:36:37 -0400 Subject: [PATCH 2/4] Update src/platform_plugin_aspects/sinks/course_overview_sink.py --- src/platform_plugin_aspects/sinks/course_overview_sink.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/platform_plugin_aspects/sinks/course_overview_sink.py b/src/platform_plugin_aspects/sinks/course_overview_sink.py index 4be60112..acd11cc3 100644 --- a/src/platform_plugin_aspects/sinks/course_overview_sink.py +++ b/src/platform_plugin_aspects/sinks/course_overview_sink.py @@ -177,10 +177,6 @@ def serialize_xblock(self, item, detached_xblock_types, dump_id, time_last_dumpe "completion_mode": getattr(item, "completion_mode", ""), } - # Video blocks can be configured to play only a clip of the source - # video. Player events report positions relative to the full source - # video but durations relative to the clip, so reporting needs the - # offsets to line them up. if block_type == "video": json_data["video_start_time"] = XBlockSink.get_seconds( getattr(item, "start_time", None) From afbfaf58cf1f87cbd09fc706ad65fa9dc79587df Mon Sep 17 00:00:00 2001 From: Sara Burns Date: Thu, 8 Oct 2026 13:41:25 -0400 Subject: [PATCH 3/4] Delete video clip time tests from test_course_overview_sink Removed tests for video clip times serialization in XBlockSink. --- .../sinks/tests/test_course_overview_sink.py | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py b/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py index 0bbf35d3..86161839 100644 --- a/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py +++ b/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py @@ -17,7 +17,6 @@ from platform_plugin_aspects.sinks import CourseOverviewSink, XBlockSink from platform_plugin_aspects.tasks import dump_course_to_clickhouse from test_utils.helpers import ( - FakeXBlock, check_block_csv_matcher, check_overview_csv_matcher, course_factory, @@ -414,40 +413,3 @@ def _check_item_serialized_location( _check_item_serialized_location(results[31], 0, "completable") _check_item_serialized_location(results[32], 0, "aggregator") _check_item_serialized_location(results[33], 0, "excluded") - - -@pytest.mark.parametrize( - "start_time,end_time,expected_start,expected_end", - [ - (timedelta(seconds=60), timedelta(seconds=240), 60.0, 240.0), - (timedelta(seconds=90.5), None, 90.5, 0.0), - (None, timedelta(seconds=30), 0.0, 30.0), - (timedelta(0), timedelta(0), 0.0, 0.0), - ], -) -def test_xblock_video_clip_times(start_time, end_time, expected_start, expected_end): - """ - Test that video start and end times serialize as seconds. - """ - block = FakeXBlock("Video", block_type="video") - block.start_time = start_time - block.end_time = end_time - - sink = XBlockSink(connection_overrides={}, log=MagicMock()) - result = sink.serialize_xblock(block, [], "xyz", "2023-09-05") - - assert result["xblock_data_json"]["video_start_time"] == expected_start - assert result["xblock_data_json"]["video_end_time"] == expected_end - - -def test_xblock_non_video_has_no_clip_times(): - """ - Test that non-video blocks do not get video clip times. - """ - block = FakeXBlock("Problem", block_type="problem") - - sink = XBlockSink(connection_overrides={}, log=MagicMock()) - result = sink.serialize_xblock(block, [], "xyz", "2023-09-05") - - assert "video_start_time" not in result["xblock_data_json"] - assert "video_end_time" not in result["xblock_data_json"] From a5b24d567b5c2f499e275a9ce74da9d574b81c0c Mon Sep 17 00:00:00 2001 From: Sara Burns Date: Thu, 8 Oct 2026 13:41:47 -0400 Subject: [PATCH 4/4] Remove unused timedelta import from test file --- .../sinks/tests/test_course_overview_sink.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py b/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py index 86161839..2bb58011 100644 --- a/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py +++ b/src/platform_plugin_aspects/sinks/tests/test_course_overview_sink.py @@ -4,7 +4,7 @@ import json import logging -from datetime import datetime, timedelta +from datetime import datetime from unittest.mock import MagicMock, patch import pytest