Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: convert from_xml to parse_xml for VideoBlock

Browse Source
This commit is contained in:
Agrendalath
2022-03-16 02:58:25 +01:00
committed by Piotr Surowiec
parent 1bfd3842c7
commit 20f6650f53
3 changed files with 58 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
lms/djangoapps/courseware/tests/test_video_mongo.py
View File

@@ -1839,9 +1839,10 @@ class VideoBlockTest(TestCase, VideoBlockTestBase):
val_transcript_language_code=val_transcript_language_code,
val_transcript_provider=val_transcript_provider
)
xml_object = etree.fromstring(xml_data)
id_generator = Mock()
id_generator.target_course_id = "test_course_id"
video = self.descriptor.from_xml(xml_data, module_system, id_generator)
video = self.descriptor.parse_xml(xml_object, module_system, None, id_generator)
assert video.edx_video_id == 'test_edx_video_id'
video_data = get_video_info(video.edx_video_id)
@@ -1887,13 +1888,14 @@ class VideoBlockTest(TestCase, VideoBlockTestBase):
Test that importing a video with no video id, creates a new external video.
"""
xml_data = """<video><video_asset></video_asset></video>"""
xml_object = etree.fromstring(xml_data)
module_system = DummySystem(load_error_modules=True)
id_generator = Mock()
# Verify edx_video_id is empty before.
assert self.descriptor.edx_video_id == ''
video = self.descriptor.from_xml(xml_data, module_system, id_generator)
video = self.descriptor.parse_xml(xml_object, module_system, None, id_generator)
# Verify edx_video_id is populated after the import.
assert video.edx_video_id != ''
@@ -1923,6 +1925,7 @@ class VideoBlockTest(TestCase, VideoBlockTestBase):
val_transcript_language_code=val_transcript_language_code,
val_transcript_provider=val_transcript_provider
)
xml_object = etree.fromstring(xml_data)
module_system = DummySystem(load_error_modules=True)
id_generator = Mock()
@@ -1940,7 +1943,7 @@ class VideoBlockTest(TestCase, VideoBlockTestBase):
# Verify edx_video_id is empty before.
assert self.descriptor.edx_video_id == ''
video = self.descriptor.from_xml(xml_data, module_system, id_generator)
video = self.descriptor.parse_xml(xml_object, module_system, None, id_generator)
# Verify edx_video_id is populated after the import.
assert video.edx_video_id != ''
@@ -2077,11 +2080,12 @@ class VideoBlockTest(TestCase, VideoBlockTestBase):
xml_data += val_transcripts
xml_data += '</video_asset></video>'
xml_object = etree.fromstring(xml_data)
# Verify edx_video_id is empty before import.
assert self.descriptor.edx_video_id == ''
video = self.descriptor.from_xml(xml_data, module_system, id_generator)
video = self.descriptor.parse_xml(xml_object, module_system, None, id_generator)
# Verify edx_video_id is not empty after import.
assert video.edx_video_id != ''
@@ -2107,8 +2111,9 @@ class VideoBlockTest(TestCase, VideoBlockTestBase):
</video_asset>
</video>
"""
xml_object = etree.fromstring(xml_data)
with pytest.raises(ValCannotCreateError):
VideoBlock.from_xml(xml_data, module_system, id_generator=Mock())
VideoBlock.parse_xml(xml_object, module_system, None, id_generator=Mock())
with pytest.raises(ValVideoNotFoundError):
get_video_info("test_edx_video_id")

54
xmodule/tests/test_video.py
View File

@@ -282,7 +282,7 @@ class VideoBlockImportTestCase(TestCase):
'transcripts': {'ua': 'ukrainian_translation.srt', 'ge': 'german_translation.srt'}
})
def test_from_xml(self):
def test_parse_xml(self):
module_system = DummySystem(load_error_modules=True)
xml_data = '''
<video display_name="Test Video"
@@ -299,7 +299,8 @@ class VideoBlockImportTestCase(TestCase):
<transcript language="de" src="german_translation.srt" />
</video>
'''
output = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -323,7 +324,7 @@ class VideoBlockImportTestCase(TestCase):
('test_org/test_course/test_run', '/c4x/test_org/test_course/asset/test.png')
)
@ddt.unpack
def test_from_xml_when_handout_is_course_asset(self, course_id_string, expected_handout_link):
def test_parse_xml_when_handout_is_course_asset(self, course_id_string, expected_handout_link):
"""
Test that if handout link is course_asset then it will contain targeted course_id in handout link.
"""
@@ -344,10 +345,11 @@ class VideoBlockImportTestCase(TestCase):
<transcript language="de" src="german_translation.srt" />
</video>
'''
xml_object = etree.fromstring(xml_data)
id_generator = Mock()
id_generator.target_course_id = course_id
output = VideoBlock.from_xml(xml_data, module_system, id_generator)
output = VideoBlock.parse_xml(xml_object, module_system, None, id_generator)
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -365,7 +367,7 @@ class VideoBlockImportTestCase(TestCase):
'transcripts': {'uk': 'ukrainian_translation.srt', 'de': 'german_translation.srt'},
})
def test_from_xml_missing_attributes(self):
def test_parse_xml_missing_attributes(self):
"""
Ensure that attributes have the right values if they aren't
explicitly set in XML.
@@ -378,7 +380,8 @@ class VideoBlockImportTestCase(TestCase):
<source src="http://www.example.com/source.mp4"/>
</video>
'''
output = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -395,7 +398,7 @@ class VideoBlockImportTestCase(TestCase):
'data': ''
})
def test_from_xml_missing_download_track(self):
def test_parse_xml_missing_download_track(self):
"""
Ensure that attributes have the right values if they aren't
explicitly set in XML.
@@ -409,7 +412,8 @@ class VideoBlockImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
'''
output = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -426,13 +430,14 @@ class VideoBlockImportTestCase(TestCase):
'transcripts': {},
})
def test_from_xml_no_attributes(self):
def test_parse_xml_no_attributes(self):
"""
Make sure settings are correct if none are explicitly set in XML.
"""
module_system = DummySystem(load_error_modules=True)
xml_data = '<video></video>'
output = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': '3_yD_cEKoCk',
@@ -450,7 +455,7 @@ class VideoBlockImportTestCase(TestCase):
'transcripts': {},
})
def test_from_xml_double_quotes(self):
def test_parse_xml_double_quotes(self):
"""
Make sure we can handle the double-quoted string format (which was used for exporting for
a few weeks).
@@ -471,7 +476,8 @@ class VideoBlockImportTestCase(TestCase):
youtube_id_1_0="&quot;OEoXaMPEzf10&quot;"
/>
'''
output = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'OEoXaMPEzf65',
'youtube_id_1_0': 'OEoXaMPEzf10',
@@ -488,14 +494,15 @@ class VideoBlockImportTestCase(TestCase):
'data': ''
})
def test_from_xml_double_quote_concatenated_youtube(self):
def test_parse_xml_double_quote_concatenated_youtube(self):
module_system = DummySystem(load_error_modules=True)
xml_data = '''
<video display_name="Test Video"
youtube="1.0:&quot;p2Q6BrNhdh8&quot;,1.25:&quot;1EeWXzPdhSA&quot;">
</video>
'''
output = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -528,7 +535,8 @@ class VideoBlockImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
"""
output = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
output = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -558,7 +566,8 @@ class VideoBlockImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
"""
video = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
video = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -588,7 +597,8 @@ class VideoBlockImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
"""
video = VideoBlock.from_xml(xml_data, module_system, Mock())
xml_object = etree.fromstring(xml_data)
video = VideoBlock.parse_xml(xml_object, module_system, None, Mock())
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -606,10 +616,10 @@ class VideoBlockImportTestCase(TestCase):
@patch('xmodule.video_module.video_module.edxval_api')
def test_import_val_data(self, mock_val_api):
"""
Test that `from_xml` works method works as expected.
Test that `parse_xml` works method works as expected.
"""
def mock_val_import(xml, edx_video_id, resource_fs, static_dir, external_transcripts, course_id):
"""Mock edxval.api.import_from_xml"""
"""Mock edxval.api.import_parse_xml"""
assert xml.tag == 'video_asset'
assert dict(list(xml.items())) == {'mock_attr': ''}
assert edx_video_id == 'test_edx_video_id'
@@ -634,9 +644,10 @@ class VideoBlockImportTestCase(TestCase):
""".format(
edx_video_id=edx_video_id
)
xml_object = etree.fromstring(xml_data)
id_generator = Mock()
id_generator.target_course_id = 'test_course_id'
video = VideoBlock.from_xml(xml_data, module_system, id_generator)
video = VideoBlock.parse_xml(xml_object, module_system, None, id_generator)
self.assert_attributes_equal(video, {'edx_video_id': edx_video_id})
mock_val_api.import_from_xml.assert_called_once_with(
@@ -660,8 +671,9 @@ class VideoBlockImportTestCase(TestCase):
<video_asset client_video_id="test_client_video_id" duration="-1"/>
</video>
"""
xml_object = etree.fromstring(xml_data)
with pytest.raises(mock_val_api.ValCannotCreateError):
VideoBlock.from_xml(xml_data, module_system, id_generator=Mock())
VideoBlock.parse_xml(xml_object, module_system, None, Mock())
class VideoExportTestCase(VideoBlockTestBase):

35
xmodule/video_module/video_module.py
View File

@@ -624,8 +624,7 @@ class VideoBlock(
def parse_xml_new_runtime(cls, node, runtime, keys):
"""
Implement the video block's special XML parsing requirements for the
new runtime only. For all other runtimes, use the existing XModule-style
methods like .from_xml().
new runtime only. For all other runtimes, use .parse_xml().
"""
video_block = runtime.construct_xblock_from_class(cls, keys)
field_data = cls.parse_video_xml(node)
@@ -638,28 +637,24 @@ class VideoBlock(
return video_block
@classmethod
def from_xml(cls, xml_data, system, id_generator):
def parse_xml(cls, node, runtime, _keys, id_generator):
"""
Creates an instance of this descriptor from the supplied xml_data.
This may be overridden by subclasses
xml_data: A string of xml that will be translated into data and children for
this module
system: A DescriptorSystem for interacting with external resources
id_generator is used to generate course-specific urls and identifiers
Use `node` to construct a new block.
See XmlMixin.parse_xml for the detailed description.
"""
xml_object = etree.fromstring(xml_data)
url_name = xml_object.get('url_name', xml_object.get('slug'))
url_name = node.get('url_name')
block_type = 'video'
definition_id = id_generator.create_definition(block_type, url_name)
usage_id = id_generator.create_usage(definition_id)
if is_pointer_tag(xml_object):
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
xml_object = cls.load_file(filepath, system.resources_fs, usage_id)
system.parse_asides(xml_object, definition_id, usage_id, id_generator)
field_data = cls.parse_video_xml(xml_object, id_generator)
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, id_generator)
field_data = cls.parse_video_xml(node, id_generator)
kvs = InheritanceKeyValueStore(initial_values=field_data)
field_data = KvsFieldData(kvs)
video = system.construct_xblock_from_class(
video = runtime.construct_xblock_from_class(
cls,
# We're loading a descriptor, so student_id is meaningless
# We also don't have separate notions of definition and usage ids yet,
@@ -668,10 +663,10 @@ class VideoBlock(
field_data,
)
# Update VAL with info extracted from `xml_object`
# Update VAL with info extracted from `node`
video.edx_video_id = video.import_video_info_into_val(
xml_object,
system.resources_fs,
node,
runtime.resources_fs,
getattr(id_generator, 'target_course_id', None)
)