From 797438e91b6e58452fa1efe102bafd6d7d2da765 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Tue, 8 Apr 2025 13:12:41 +0500 Subject: [PATCH] fix: save video asides during XML course import --- xmodule/tests/test_video.py | 32 ++++++++++++++++++++++++++++++ xmodule/video_block/video_block.py | 6 +++++- xmodule/xml_block.py | 17 +++++++++++----- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/xmodule/tests/test_video.py b/xmodule/tests/test_video.py index e98c78244d..1179feebf3 100644 --- a/xmodule/tests/test_video.py +++ b/xmodule/tests/test_video.py @@ -37,6 +37,8 @@ from xmodule.tests import get_test_descriptor_system from xmodule.validation import StudioValidationMessage from xmodule.video_block import EXPORT_IMPORT_STATIC_DIR, VideoBlock, create_youtube_string from xmodule.video_block.transcripts_utils import save_to_store +from xblock.core import XBlockAside +from xmodule.modulestore.tests.test_asides import AsideTestType from .test_import import DummySystem @@ -316,6 +318,36 @@ class VideoBlockImportTestCase(TestCase): 'transcripts': {'uk': 'ukrainian_translation.srt', 'de': 'german_translation.srt'}, }) + @XBlockAside.register_temp_plugin(AsideTestType, "test_aside") + @patch('xmodule.video_block.video_block.VideoBlock.load_file') + @patch('xmodule.video_block.video_block.is_pointer_tag') + @ddt.data(True, False) + def test_parse_xml_with_asides(self, video_xml_has_aside, mock_is_pointer_tag, mock_load_file): + """Test that `parse_xml` parses asides from the video xml""" + runtime = DummySystem(load_error_blocks=True) + if video_xml_has_aside: + xml_data = ''' + + ''' + else: + xml_data = ''' + + ''' + mock_is_pointer_tag.return_value = True + xml_object = etree.fromstring(xml_data) + mock_load_file.return_value = xml_object + output = VideoBlock.parse_xml(xml_object, runtime, None) + aside = runtime.get_aside_of_type(output, "test_aside") + if video_xml_has_aside: + assert aside.content == "default_content" + assert aside.data_field == "aside parsed" + else: + assert aside.content == "default_content" + assert aside.data_field == "default_data" + @ddt.data( ('course-v1:test_org+test_course+test_run', '/asset-v1:test_org+test_course+test_run+type@asset+block@test.png'), diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index b0e01068fb..89718bf224 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -752,10 +752,11 @@ class _BuiltInVideoBlock( block_type = 'video' definition_id = runtime.id_generator.create_definition(block_type, url_name) usage_id = runtime.id_generator.create_usage(definition_id) + aside_children = [] if is_pointer_tag(node): filepath = cls._format_filepath(node.tag, name_to_pathname(url_name)) node = cls.load_file(filepath, runtime.resources_fs, usage_id) - runtime.parse_asides(node, definition_id, usage_id, runtime.id_generator) + aside_children = runtime.parse_asides(node, definition_id, usage_id, runtime.id_generator) field_data = cls.parse_video_xml(node, runtime.id_generator) kvs = InheritanceKeyValueStore(initial_values=field_data) field_data = KvsFieldData(kvs) @@ -775,6 +776,9 @@ class _BuiltInVideoBlock( getattr(runtime.id_generator, 'target_course_id', None) ) + if aside_children: + cls.add_applicable_asides_to_block(video, runtime, aside_children) + return video def definition_to_xml(self, resource_fs): # lint-amnesty, pylint: disable=too-many-statements diff --git a/xmodule/xml_block.py b/xmodule/xml_block.py index 63dbedb17e..4093d982fa 100644 --- a/xmodule/xml_block.py +++ b/xmodule/xml_block.py @@ -381,14 +381,21 @@ class XmlMixin: ) if aside_children: - asides_tags = [x.tag for x in aside_children] - asides = runtime.get_asides(xblock) - for asd in asides: - if asd.scope_ids.block_type in asides_tags: - xblock.add_aside(asd) + cls.add_applicable_asides_to_block(xblock, runtime, aside_children) return xblock + @classmethod + def add_applicable_asides_to_block(cls, block, runtime, aside_children): + """ + Add asides to the block. Moved this out of the parse_xml method to use it in the VideoBlock.parse_xml + """ + asides_tags = [aside_child.tag for aside_child in aside_children] + asides = runtime.get_asides(block) + for aside in asides: + if aside.scope_ids.block_type in asides_tags: + block.add_aside(aside) + @classmethod def parse_xml_new_runtime(cls, node, runtime, keys): """