Skip to content

feat!: remove multi-speed YouTube fields from VideoBlock [DEPR] #254

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.

Sign up for GitHub

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

Draft
farhan wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feat!: remove multi-speed YouTube fields from VideoBlock [DEPR] #254

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
444893d
feat!: remove multi-speed YouTube fields from VideoBlock [DEPR]
farhan May 25, 2026
cb6c9ab
refactor: simplify _parse_youtube to only track 1.00 speed
farhan May 25, 2026
7ee3b93
fix: preserve YouTube ID from legacy OLX with no 1.00 speed entry
farhan May 25, 2026
fefd3ed
refactor: remove dead multi-speed loop from parse_video_xml
farhan May 25, 2026
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
60 changes: 5 additions & 55 deletions xblocks_contrib/video/tests/test_video.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,21 +197,16 @@ def test_create_youtube_string(self):
"""
Test that Youtube ID strings are correctly created when writing back out to XML.
"""
self.block.youtube_id_0_75 = 'izygArpw-Qo'
self.block.youtube_id_1_0 = 'p2Q6BrNhdh8'
self.block.youtube_id_1_25 = '1EeWXzPdhSA'
self.block.youtube_id_1_5 = 'rABDYkeK0x8'
expected = "0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8"
expected = "1.00:p2Q6BrNhdh8"
assert create_youtube_string(self.block) == expected

def test_create_youtube_string_missing(self):
"""
Test that Youtube IDs which aren't explicitly set aren't included in the output string.
"""
self.block.youtube_id_0_75 = 'izygArpw-Qo'
self.block.youtube_id_1_0 = 'p2Q6BrNhdh8'
self.block.youtube_id_1_25 = '1EeWXzPdhSA'
expected = "0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA"
expected = "1.00:p2Q6BrNhdh8"
assert create_youtube_string(self.block) == expected
Comment thread
farhan marked this conversation as resolved.
Outdated


Expand Down Expand Up @@ -260,10 +255,7 @@ def test_constructor(self):
'''
block = instantiate_block(data=sample_xml)
self.assert_attributes_equal(block, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'download_video': True,
'show_captions': False,
'start_time': datetime.timedelta(seconds=1),
Expand Down Expand Up @@ -296,10 +288,7 @@ def test_parse_xml(self):
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': datetime.timedelta(seconds=1),
'end_time': datetime.timedelta(seconds=60),
Expand Down Expand Up @@ -374,10 +363,7 @@ def test_parse_xml_when_handout_is_course_asset(self, course_id_string, expected

output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': datetime.timedelta(seconds=1),
'end_time': datetime.timedelta(seconds=60),
Expand Down Expand Up @@ -406,10 +392,7 @@ def test_parse_xml_missing_attributes(self):
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': '',
'show_captions': True,
'start_time': datetime.timedelta(seconds=0.0),
'end_time': datetime.timedelta(seconds=0.0),
Expand Down Expand Up @@ -438,10 +421,7 @@ def test_parse_xml_missing_download_track(self):
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': '',
'show_captions': True,
'start_time': datetime.timedelta(seconds=0.0),
'end_time': datetime.timedelta(seconds=0.0),
Expand All @@ -462,10 +442,7 @@ def test_parse_xml_no_attributes(self):
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': '3_yD_cEKoCk',
'youtube_id_1_25': '',
'youtube_id_1_5': '',
'show_captions': True,
'start_time': datetime.timedelta(seconds=0.0),
'end_time': datetime.timedelta(seconds=0.0),
Expand Down Expand Up @@ -493,19 +470,13 @@ def test_parse_xml_double_quotes(self):
track=""http://www.example.com/track""
handout=""http://www.example.com/handout""
download_track="true"
youtube_id_0_75=""OEoXaMPEzf65""
youtube_id_1_25=""OEoXaMPEzf125""
youtube_id_1_5=""OEoXaMPEzf15""
youtube_id_1_0=""OEoXaMPEzf10""
/>
'''
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'OEoXaMPEzf65',
'youtube_id_1_0': 'OEoXaMPEzf10',
'youtube_id_1_25': 'OEoXaMPEzf125',
'youtube_id_1_5': 'OEoXaMPEzf15',
'show_captions': False,
'start_time': datetime.timedelta(seconds=0.0),
'end_time': datetime.timedelta(seconds=0.0),
Expand All @@ -527,10 +498,7 @@ def test_parse_xml_double_quote_concatenated_youtube(self):
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': '',
'show_captions': True,
'start_time': datetime.timedelta(seconds=0.0),
'end_time': datetime.timedelta(seconds=0.0),
Expand Down Expand Up @@ -561,10 +529,7 @@ def test_old_video_format(self):
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': datetime.timedelta(seconds=1),
'end_time': datetime.timedelta(seconds=60),
Expand Down Expand Up @@ -592,10 +557,7 @@ def test_old_video_data(self):
xml_object = etree.fromstring(xml_data)
video = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': datetime.timedelta(seconds=1),
'end_time': datetime.timedelta(seconds=60),
Expand Down Expand Up @@ -623,10 +585,7 @@ def test_import_with_float_times(self):
xml_object = etree.fromstring(xml_data)
video = VideoBlock.parse_xml(xml_object, module_system, None)
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': datetime.timedelta(seconds=1),
'end_time': datetime.timedelta(seconds=60),
Expand Down Expand Up @@ -722,10 +681,7 @@ def test_export_to_xml(self, mock_get_edxval_api):
mock_val_api.export_to_xml = Mock(
return_value={"xml": etree.Element('video_asset'), "transcripts": {}}
)
self.block.youtube_id_0_75 = 'izygArpw-Qo'
self.block.youtube_id_1_0 = 'p2Q6BrNhdh8'
self.block.youtube_id_1_25 = '1EeWXzPdhSA'
self.block.youtube_id_1_5 = 'rABDYkeK0x8'
self.block.show_captions = False
self.block.start_time = datetime.timedelta(seconds=1.0)
self.block.end_time = datetime.timedelta(seconds=60)
Expand All @@ -747,7 +703,7 @@ def test_export_to_xml(self, mock_get_edxval_api):
end_time="0:01:00"
download_video="true"
download_track="true"
youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8"
youtube="1.00:p2Q6BrNhdh8"
transcripts='{"ge": "german_translation.srt", "ua": "ukrainian_translation.srt"}'
>
<source src="http://www.example.com/source.mp4"/>
Expand All @@ -772,10 +728,7 @@ def test_export_to_xml_without_video_id(self):
"""
Test that we write the correct XML on export of a video without edx_video_id.
"""
self.block.youtube_id_0_75 = 'izygArpw-Qo'
self.block.youtube_id_1_0 = 'p2Q6BrNhdh8'
self.block.youtube_id_1_25 = '1EeWXzPdhSA'
self.block.youtube_id_1_5 = 'rABDYkeK0x8'
self.block.show_captions = False
self.block.start_time = datetime.timedelta(seconds=1.0)
self.block.end_time = datetime.timedelta(seconds=60)
Expand All @@ -796,7 +749,7 @@ def test_export_to_xml_without_video_id(self):
end_time="0:01:00"
download_video="true"
download_track="true"
youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8"
youtube="1.00:p2Q6BrNhdh8"
transcripts='{"ge": "german_translation.srt", "ua": "ukrainian_translation.srt"}'
>
<source src="http://www.example.com/source.mp4"/>
Expand Down Expand Up @@ -829,10 +782,7 @@ def test_export_to_xml_empty_end_time(self, _mock_get_edxval_api):
"""
Test that we write the correct XML on export.
"""
self.block.youtube_id_0_75 = 'izygArpw-Qo'
self.block.youtube_id_1_0 = 'p2Q6BrNhdh8'
self.block.youtube_id_1_25 = '1EeWXzPdhSA'
self.block.youtube_id_1_5 = 'rABDYkeK0x8'
self.block.show_captions = False
self.block.start_time = datetime.timedelta(seconds=5.0)
self.block.end_time = datetime.timedelta(seconds=0.0)
Expand All @@ -845,7 +795,7 @@ def test_export_to_xml_empty_end_time(self, _mock_get_edxval_api):
parser = etree.XMLParser(remove_blank_text=True)
xml_string = '''\
<video url_name="SampleProblem" start_time="0:00:05"
youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8"
youtube="1.00:p2Q6BrNhdh8"
show_captions="false" download_video="true" download_track="true">
<source src="http://www.example.com/source.mp4"/>
<source src="http://www.example.com/source.ogg"/>
Expand Down
5 changes: 1 addition & 4 deletions xblocks_contrib/video/video_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,9 @@ def create_youtube_string(block):
courses.
"""
youtube_ids = [
block.youtube_id_0_75,
block.youtube_id_1_0,
block.youtube_id_1_25,
block.youtube_id_1_5
]
youtube_speeds = ['0.75', '1.00', '1.25', '1.50']
youtube_speeds = ['1.00']
return ','.join([
':'.join(pair)
for pair
Expand Down
18 changes: 0 additions & 18 deletions xblocks_contrib/video/video_xfields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,6 @@ class VideoFields:
scope=Scope.settings,
default="3_yD_cEKoCk"
)
Comment thread
farhan marked this conversation as resolved.
youtube_id_0_75 = String(
help=_("Optional, for older browsers: the YouTube ID for the .75x speed video."),
display_name=_("YouTube ID for .75x speed"),
scope=Scope.settings,
default=""
)
youtube_id_1_25 = String(
help=_("Optional, for older browsers: the YouTube ID for the 1.25x speed video."),
display_name=_("YouTube ID for 1.25x speed"),
scope=Scope.settings,
default=""
)
youtube_id_1_5 = String(
help=_("Optional, for older browsers: the YouTube ID for the 1.5x speed video."),
display_name=_("YouTube ID for 1.5x speed"),
scope=Scope.settings,
default=""
)
start_time = RelativeTime( # datetime.timedelta object
help=_(
"Time you want the video to start if you don't want the entire video to play. "
Expand Down