diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 9c80c95a04..d18fc5b14b 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 4e54dd589f..fb6b7838a4 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -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): diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index be79787755..269adf6109 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -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 = 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): diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 4e04786974..1fbd185d8a 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -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 diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index e72630df13..2bf2373ec9 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -905,13 +905,15 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase): """ - 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) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 2e9edff020..1cbdb52c28 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -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