From 0e2febbce2b6d5e4920b6b37a05b3430aea1a0d7 Mon Sep 17 00:00:00 2001 From: Amir Qayyum Khan Date: Mon, 27 Aug 2018 16:54:45 +0500 Subject: [PATCH] OLX export: Reverted url_name from a constant to course run - New exported courses include course run information in: - `url_name` of root course node - file name of root node in course folder - root key name in policy.json - directory name inside policies folder - when imported via management command, the OLX will overwrite an available existing course with the same course key (i.e. same org, course number and course run) - if there is no matching course, one will be created - when imported via the studio web ui (or import API), the OLX will replace the current course (no change in behavior) - courses exported with this commit have been tested to import via management command and studio web UI in hawthorn and ginkgo releases. They should also work in prior releases, but have not been tested. --- .../tests/test_cross_modulestore_import_export.py | 2 +- .../lib/xmodule/xmodule/modulestore/xml_exporter.py | 7 ++----- common/lib/xmodule/xmodule/tests/test_import.py | 11 ++++++----- common/lib/xmodule/xmodule/x_module.py | 4 ++++ common/lib/xmodule/xmodule/xml_module.py | 6 +++++- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py index e8e49f0b53..a438554586 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py @@ -188,7 +188,7 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase): self.assertEqual(source_course.url_name, 'course') export_dir_path = path(self.export_dir) - policy_dir = export_dir_path / 'exported_source_course' / 'policies' / source_course.url_name + policy_dir = export_dir_path / 'exported_source_course' / 'policies' / source_course_key.run policy_path = policy_dir / 'policy.json' self.assertTrue(os.path.exists(policy_path)) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 1a16533670..42e9096f0e 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -166,6 +166,7 @@ class ExportManager(object): # change all of the references inside the course to use the xml expected key type w/o version & branch xml_centric_courselike_key = self.get_key() adapt_references(courselike, xml_centric_courselike_key, export_fs) + root.set('url_name', self.courselike_key.run) courselike.add_xml_to_node(root) # Make any needed adjustments to the root node. @@ -267,10 +268,6 @@ class CourseExportManager(ExportManager): ) course_policy_dir_name = courselike.location.run - if courselike.url_name != courselike.location.run and courselike.url_name == 'course': - # Use url_name for split mongo because course_run is not used when loading policies. - course_policy_dir_name = courselike.url_name - course_run_policy_dir = policies_dir.makedir(course_policy_dir_name, recreate=True) # export the grading policy @@ -280,7 +277,7 @@ class CourseExportManager(ExportManager): # export all of the course metadata in policy.json with course_run_policy_dir.open(u'policy.json', 'wb') as course_policy: - policy = {'course/' + courselike.location.block_id: own_metadata(courselike)} + policy = {'course/' + courselike.location.run: own_metadata(courselike)} course_policy.write(dumps(policy, cls=EdxJSONEncoder, sort_keys=True, indent=4).encode('utf-8')) _export_drafts(self.modulestore, self.courselike_key, export_fs, xml_centric_courselike_key) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 079e76281e..dc5a4ea36e 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -32,6 +32,7 @@ from xblock.runtime import KvsFieldData, DictKeyValueStore ORG = 'test_org' COURSE = 'test_course' +RUN = 'test_run' class DummySystem(ImportSystem): @@ -43,7 +44,7 @@ class DummySystem(ImportSystem): xmlstore = LibraryXMLModuleStore("data_dir", source_dirs=[], load_error_modules=load_error_modules) else: xmlstore = XMLModuleStore("data_dir", source_dirs=[], load_error_modules=load_error_modules) - course_id = CourseKey.from_string('/'.join([ORG, COURSE, 'test_run'])) + course_id = CourseKey.from_string('/'.join([ORG, COURSE, RUN])) course_dir = "test_dir" error_tracker = Mock() @@ -197,7 +198,7 @@ class ImportTestCase(BaseCourseTestCase): # Now make sure the exported xml is a sequential self.assertEqual(node.tag, 'sequential') - def course_descriptor_inheritance_check(self, descriptor, from_date_string, unicorn_color, url_name): + def course_descriptor_inheritance_check(self, descriptor, from_date_string, unicorn_color, course_run=RUN): """ Checks to make sure that metadata inheritance on a course descriptor is respected. """ @@ -228,7 +229,7 @@ class ImportTestCase(BaseCourseTestCase): self.assertEqual(node.attrib['org'], ORG) # Does the course still have unicorns? - with descriptor.runtime.export_fs.open(u'course/{url_name}.xml'.format(url_name=url_name)) as f: + with descriptor.runtime.export_fs.open(u'course/{course_run}.xml'.format(course_run=course_run)) as f: course_xml = etree.fromstring(f.read()) self.assertEqual(course_xml.attrib['unicorn'], unicorn_color) @@ -267,7 +268,7 @@ class ImportTestCase(BaseCourseTestCase): ) descriptor = system.process_xml(start_xml) compute_inherited_metadata(descriptor) - self.course_descriptor_inheritance_check(descriptor, from_date_string, unicorn_color, url_name) + self.course_descriptor_inheritance_check(descriptor, from_date_string, unicorn_color) def test_library_metadata_import_export(self): """Two checks: @@ -300,7 +301,7 @@ class ImportTestCase(BaseCourseTestCase): compute_inherited_metadata(descriptor) # Check the course module, since it has inheritance descriptor = descriptor.get_children()[0] - self.course_descriptor_inheritance_check(descriptor, from_date_string, unicorn_color, url_name) + self.course_descriptor_inheritance_check(descriptor, from_date_string, unicorn_color) def test_metadata_no_inheritance(self): """ diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 88848c17a5..3c3b39ce50 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1116,7 +1116,11 @@ class XModuleDescriptor(HTMLSnippet, ResourceTemplates, XModuleMixin): node.tag = exported_node.tag node.text = exported_node.text node.tail = exported_node.tail + for key, value in exported_node.items(): + if key == 'url_name' and value == 'course' and key in node.attrib: + # if url_name is set in ExportManager then do not override it here. + continue node.set(key, value) node.extend(list(exported_node)) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 1c6e16fe01..1f3c0000d0 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -487,7 +487,11 @@ class XmlParserMixin(object): if self.export_to_file(): # Write the definition to a file url_path = name_to_pathname(self.url_name) - filepath = self._format_filepath(self.category, url_path) + # if folder is course then create file with name {course_run}.xml + filepath = self._format_filepath( + self.category, + self.location.run if self.category == 'course' else url_path, + ) self.runtime.export_fs.makedirs(os.path.dirname(filepath), recreate=True) with self.runtime.export_fs.open(filepath, 'wb') as fileobj: ElementTree(xml_object).write(fileobj, pretty_print=True, encoding='utf-8')