Merge pull request #7541 from edx/mobile/import_fix_course_id
MA-410 Update video_module import to use target course_id.
This commit is contained in:
@@ -64,7 +64,7 @@ def clean_out_mako_templating(xml_string):
|
||||
class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
|
||||
def __init__(self, xmlstore, course_id, course_dir,
|
||||
error_tracker,
|
||||
load_error_modules=True, **kwargs):
|
||||
load_error_modules=True, target_course_id=None, **kwargs):
|
||||
"""
|
||||
A class that handles loading from xml. Does some munging to ensure that
|
||||
all elements have unique slugs.
|
||||
@@ -255,7 +255,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
|
||||
|
||||
resources_fs = OSFS(xmlstore.data_dir / course_dir)
|
||||
|
||||
id_manager = CourseLocationManager(course_id)
|
||||
id_manager = CourseImportLocationManager(course_id, target_course_id)
|
||||
|
||||
super(ImportSystem, self).__init__(
|
||||
load_item=load_item,
|
||||
@@ -306,6 +306,25 @@ class CourseLocationManager(OpaqueKeyReader, AsideKeyGenerator):
|
||||
return usage_id
|
||||
|
||||
|
||||
class CourseImportLocationManager(CourseLocationManager):
|
||||
"""
|
||||
IdGenerator for Location-based definition ids and usage ids
|
||||
based within a course, for use during course import.
|
||||
|
||||
In addition to the functionality provided by CourseLocationManager,
|
||||
this class also contains the target_course_id for the course import
|
||||
process.
|
||||
|
||||
Note: This is a temporary solution to workaround the fact that
|
||||
the from_xml method is passed the source course_id instead of the
|
||||
target course_id in the import process. For a more ideal solution,
|
||||
see https://openedx.atlassian.net/browse/MA-417 as a pending TODO.
|
||||
"""
|
||||
def __init__(self, course_id, target_course_id):
|
||||
super(CourseImportLocationManager, self).__init__(course_id=course_id)
|
||||
self.target_course_id = target_course_id
|
||||
|
||||
|
||||
class XMLModuleStore(ModuleStoreReadBase):
|
||||
"""
|
||||
An XML backed ModuleStore
|
||||
@@ -315,7 +334,7 @@ class XMLModuleStore(ModuleStoreReadBase):
|
||||
def __init__(
|
||||
self, data_dir, default_class=None, source_dirs=None, course_ids=None,
|
||||
load_error_modules=True, i18n_service=None, fs_service=None, user_service=None,
|
||||
signal_handler=None, **kwargs # pylint: disable=unused-argument
|
||||
signal_handler=None, target_course_id=None, **kwargs # pylint: disable=unused-argument
|
||||
):
|
||||
"""
|
||||
Initialize an XMLModuleStore from data_dir
|
||||
@@ -365,9 +384,9 @@ class XMLModuleStore(ModuleStoreReadBase):
|
||||
source_dirs = sorted([d for d in os.listdir(self.data_dir) if
|
||||
os.path.exists(self.data_dir / d / self.parent_xml)])
|
||||
for course_dir in source_dirs:
|
||||
self.try_load_course(course_dir, course_ids)
|
||||
self.try_load_course(course_dir, course_ids, target_course_id)
|
||||
|
||||
def try_load_course(self, course_dir, course_ids=None):
|
||||
def try_load_course(self, course_dir, course_ids=None, target_course_id=None):
|
||||
'''
|
||||
Load a course, keeping track of errors as we go along. If course_ids is not None,
|
||||
then reject the course unless its id is in course_ids.
|
||||
@@ -379,7 +398,7 @@ class XMLModuleStore(ModuleStoreReadBase):
|
||||
errorlog = make_error_tracker()
|
||||
course_descriptor = None
|
||||
try:
|
||||
course_descriptor = self.load_course(course_dir, course_ids, errorlog.tracker)
|
||||
course_descriptor = self.load_course(course_dir, course_ids, errorlog.tracker, target_course_id)
|
||||
except Exception as exc: # pylint: disable=broad-except
|
||||
msg = "ERROR: Failed to load courselike '{0}': {1}".format(
|
||||
course_dir.encode("utf-8"), unicode(exc)
|
||||
@@ -432,7 +451,7 @@ class XMLModuleStore(ModuleStoreReadBase):
|
||||
log.warning(msg + " " + str(err))
|
||||
return {}
|
||||
|
||||
def load_course(self, course_dir, course_ids, tracker):
|
||||
def load_course(self, course_dir, course_ids, tracker, target_course_id=None):
|
||||
"""
|
||||
Load a course into this module store
|
||||
course_path: Course directory name
|
||||
@@ -551,6 +570,7 @@ class XMLModuleStore(ModuleStoreReadBase):
|
||||
select=self.xblock_select,
|
||||
field_data=self.field_data,
|
||||
services=services,
|
||||
target_course_id=target_course_id,
|
||||
)
|
||||
course_descriptor = system.process_xml(etree.tostring(course_data, encoding='unicode'))
|
||||
# If we fail to load the course, then skip the rest of the loading steps
|
||||
|
||||
@@ -200,6 +200,7 @@ class ImportManager(object):
|
||||
load_error_modules=load_error_modules,
|
||||
xblock_mixins=store.xblock_mixins,
|
||||
xblock_select=store.xblock_select,
|
||||
target_course_id=target_id,
|
||||
)
|
||||
self.logger, self.errors = make_error_tracker()
|
||||
|
||||
@@ -728,6 +729,7 @@ def _import_course_draft(
|
||||
load_error_modules=False,
|
||||
mixins=xml_module_store.xblock_mixins,
|
||||
field_data=KvsFieldData(kvs=DictKeyValueStore()),
|
||||
target_course_id=target_id,
|
||||
)
|
||||
|
||||
def _import_module(module):
|
||||
|
||||
@@ -553,11 +553,12 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
|
||||
|
||||
@patch('xmodule.video_module.video_module.edxval_api')
|
||||
def test_import_val_data(self, mock_val_api):
|
||||
def mock_val_import(xml, edx_video_id):
|
||||
def mock_val_import(xml, edx_video_id, course_id):
|
||||
"""Mock edxval.api.import_from_xml"""
|
||||
self.assertEqual(xml.tag, 'video_asset')
|
||||
self.assertEqual(dict(xml.items()), {'mock_attr': ''})
|
||||
self.assertEqual(edx_video_id, 'test_edx_video_id')
|
||||
self.assertEqual(course_id, 'test_course_id')
|
||||
|
||||
mock_val_api.import_from_xml = Mock(wraps=mock_val_import)
|
||||
module_system = DummySystem(load_error_modules=True)
|
||||
@@ -568,10 +569,12 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
|
||||
<video_asset mock_attr=""/>
|
||||
</video>
|
||||
"""
|
||||
video = VideoDescriptor.from_xml(xml_data, module_system, id_generator=Mock())
|
||||
id_generator = Mock()
|
||||
id_generator.target_course_id = 'test_course_id'
|
||||
video = VideoDescriptor.from_xml(xml_data, module_system, id_generator)
|
||||
|
||||
self.assert_attributes_equal(video, {'edx_video_id': 'test_edx_video_id'})
|
||||
mock_val_api.import_from_xml.assert_called_once_with(ANY, 'test_edx_video_id')
|
||||
mock_val_api.import_from_xml.assert_called_once_with(ANY, 'test_edx_video_id', course_id='test_course_id')
|
||||
|
||||
@patch('xmodule.video_module.video_module.edxval_api')
|
||||
def test_import_val_data_invalid(self, mock_val_api):
|
||||
|
||||
@@ -415,7 +415,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
|
||||
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)
|
||||
field_data = cls._parse_video_xml(xml_object, id_generator)
|
||||
kvs = InheritanceKeyValueStore(initial_values=field_data)
|
||||
field_data = KvsFieldData(kvs)
|
||||
video = system.construct_xblock_from_class(
|
||||
@@ -557,10 +557,13 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
|
||||
return ret
|
||||
|
||||
@classmethod
|
||||
def _parse_video_xml(cls, xml):
|
||||
def _parse_video_xml(cls, xml, id_generator=None):
|
||||
"""
|
||||
Parse video fields out of xml_data. The fields are set if they are
|
||||
present in the XML.
|
||||
|
||||
Arguments:
|
||||
id_generator is used to generate course-specific urls and identifiers
|
||||
"""
|
||||
field_data = {}
|
||||
|
||||
@@ -633,7 +636,11 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
|
||||
'edx_video_id' in field_data
|
||||
):
|
||||
# Allow ValCannotCreateError to escape
|
||||
edxval_api.import_from_xml(video_asset_elem, field_data['edx_video_id'])
|
||||
edxval_api.import_from_xml(
|
||||
video_asset_elem,
|
||||
field_data['edx_video_id'],
|
||||
course_id=getattr(id_generator, 'target_course_id', None)
|
||||
)
|
||||
|
||||
return field_data
|
||||
|
||||
|
||||
@@ -905,13 +905,15 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
|
||||
</video_asset>
|
||||
</video>
|
||||
"""
|
||||
video = VideoDescriptor.from_xml(xml_data, module_system, id_generator=Mock())
|
||||
id_generator = Mock()
|
||||
id_generator.target_course_id = "test_course_id"
|
||||
video = VideoDescriptor.from_xml(xml_data, module_system, id_generator)
|
||||
self.assertEqual(video.edx_video_id, 'test_edx_video_id')
|
||||
video_data = get_video_info(video.edx_video_id)
|
||||
self.assertEqual(video_data['client_video_id'], 'test_client_video_id')
|
||||
self.assertEqual(video_data['duration'], 111)
|
||||
self.assertEqual(video_data['status'], 'imported')
|
||||
self.assertEqual(video_data['courses'], [])
|
||||
self.assertEqual(video_data['courses'], [id_generator.target_course_id])
|
||||
self.assertEqual(video_data['encoded_videos'][0]['profile'], 'mobile')
|
||||
self.assertEqual(video_data['encoded_videos'][0]['url'], 'http://example.com/video')
|
||||
self.assertEqual(video_data['encoded_videos'][0]['file_size'], 222)
|
||||
|
||||
@@ -35,7 +35,7 @@ git+https://github.com/mitocw/django-cas.git@60a5b8e5a62e63e0d5d224a87f0b489201a
|
||||
-e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease
|
||||
-e git+https://github.com/edx/i18n-tools.git@193cebd9aa784f8899ef496f2aa050b08eff402b#egg=i18n-tools
|
||||
-e git+https://github.com/edx/edx-oauth2-provider.git@0.5.1#egg=oauth2-provider
|
||||
-e git+https://github.com/edx/edx-val.git@cb9cf1a37124ad8e589734b36d4e8199e0082a02#egg=edx-val
|
||||
-e git+https://github.com/edx/edx-val.git@d6087908aa3dd05ceaa7f56a21284f86c53cb3f0#egg=edx-val
|
||||
-e git+https://github.com/pmitros/RecommenderXBlock.git@9b07e807c89ba5761827d0387177f71aa57ef056#egg=recommender-xblock
|
||||
-e git+https://github.com/edx/edx-milestones.git@547f2250ee49e73ce8d7ff4e78ecf1b049892510#egg=edx-milestones
|
||||
-e git+https://github.com/edx/edx-search.git@21ac6b06b3bfe789dcaeaf4e2ab5b00a688324d4#egg=edx-search
|
||||
|
||||
Reference in New Issue
Block a user