diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index c373be611c..478227d386 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/cms/djangoapps/contentstore/tests/test_import.py @@ -198,24 +198,45 @@ class ContentStoreImportTest(ModuleStoreTestCase): peergrading_module.link_to_location ) - def test_rewrite_reference_value_dict(self): + def test_rewrite_reference_value_dict_published(self): + """ + Test rewriting references in ReferenceValueDict, specifically with published content. + """ + self._verify_split_test_import( + 'split_test_copy', + 'split_test_module', + 'split1', + {"0": 'sample_0', "2": 'sample_2'}, + ) + + def test_rewrite_reference_value_dict_draft(self): + """ + Test rewriting references in ReferenceValueDict, specifically with draft content. + """ + self._verify_split_test_import( + 'split_test_copy_with_draft', + 'split_test_module_draft', + 'fb34c21fe64941999eaead421a8711b8', + {"0": '9f0941d021414798836ef140fb5f6841', "1": '0faf29473cf1497baa33fcc828b179cd'}, + ) + + def _verify_split_test_import(self, target_course_name, source_course_name, split_test_name, groups_to_verticals): module_store = modulestore() - target_course_id = SlashSeparatedCourseKey('testX', 'split_test_copy', 'copy_run') + target_course_id = SlashSeparatedCourseKey('testX', target_course_name, 'copy_run') import_from_xml( module_store, self.user.id, 'common/test/data/', - ['split_test_module'], + [source_course_name], target_course_id=target_course_id ) split_test_module = module_store.get_item( - target_course_id.make_usage_key('split_test', 'split1') + target_course_id.make_usage_key('split_test', split_test_name) ) self.assertIsNotNone(split_test_module) - self.assertEqual( - { - "0": target_course_id.make_usage_key('vertical', 'sample_0'), - "2": target_course_id.make_usage_key('vertical', 'sample_2'), - }, - split_test_module.group_id_to_child, - ) + + remapped_verticals = { + key: target_course_id.make_usage_key('vertical', value) for key, value in groups_to_verticals.iteritems() + } + + self.assertEqual(remapped_verticals, split_test_module.group_id_to_child) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 21be802a9d..d3f729ee2e 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -426,7 +426,7 @@ def _import_course_draft( draft_course_dir = draft_dir.replace(data_dir, '', 1) system = ImportSystem( xmlstore=xml_module_store, - course_id=target_course_id, + course_id=source_course_id, course_dir=draft_course_dir, error_tracker=errorlog.tracker, parent_tracker=ParentTracker(), @@ -480,21 +480,27 @@ def _import_course_draft( # Not a 'hidden file', then re-raise exception raise err - descriptor = system.process_xml(xml) + # process_xml call below recursively processes all descendants. If + # we call this on all verticals in a course with verticals nested below + # the unit level, we try to import the same content twice, causing naming conflicts. + # Therefore only process verticals at the unit level, assuming that any other + # verticals must be descendants. + if 'index_in_children_list' in xml: + descriptor = system.process_xml(xml) - # HACK: since we are doing partial imports of drafts - # the vertical doesn't have the 'url-name' set in the - # attributes (they are normally in the parent object, - # aka sequential), so we have to replace the location.name - # with the XML filename that is part of the pack - fn, fileExtension = os.path.splitext(filename) - descriptor.location = descriptor.location.replace(name=fn) + # HACK: since we are doing partial imports of drafts + # the vertical doesn't have the 'url-name' set in the + # attributes (they are normally in the parent object, + # aka sequential), so we have to replace the location.name + # with the XML filename that is part of the pack + fn, fileExtension = os.path.splitext(filename) + descriptor.location = descriptor.location.replace(name=fn) - index = int(descriptor.xml_attributes['index_in_children_list']) - if index in drafts: - drafts[index].append(descriptor) - else: - drafts[index] = [descriptor] + index = int(descriptor.xml_attributes['index_in_children_list']) + if index in drafts: + drafts[index].append(descriptor) + else: + drafts[index] = [descriptor] except Exception: logging.exception('Error while parsing course xml.') @@ -505,24 +511,27 @@ def _import_course_draft( course_key = descriptor.location.course_key try: def _import_module(module): + # IMPORTANT: Be sure to update the module location in the NEW namespace + module_location = module.location.map_into_course(target_course_id) # Update the module's location to DRAFT revision # We need to call this method (instead of updating the location directly) # to ensure that pure XBlock field data is updated correctly. - _update_module_location(module, module.location.replace(revision=MongoRevisionKey.draft)) + _update_module_location(module, module_location.replace(revision=MongoRevisionKey.draft)) # make sure our parent has us in its list of children # this is to make sure private only verticals show up # in the list of children since they would have been - # filtered out from the non-draft store export - if module.location.category == 'vertical': + # filtered out from the non-draft store export. + # Note though that verticals nested below the unit level will not have + # a parent_sequential_url and do not need special handling. + if module.location.category == 'vertical' and 'parent_sequential_url' in module.xml_attributes: non_draft_location = module.location.replace(revision=MongoRevisionKey.published) sequential_url = module.xml_attributes['parent_sequential_url'] index = int(module.xml_attributes['index_in_children_list']) seq_location = course_key.make_usage_key_from_deprecated_string(sequential_url) - # IMPORTANT: Be sure to update the sequential - # in the NEW namespace + # IMPORTANT: Be sure to update the sequential in the NEW namespace seq_location = seq_location.map_into_course(target_course_id) sequential = store.get_item(seq_location, depth=0) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 4676b14159..dd710947c4 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -381,7 +381,7 @@ class XmlDescriptor(XModuleDescriptor): for key, value in self.xml_attributes.items(): if key not in self.metadata_to_strip: - xml_object.set(key, value) + xml_object.set(key, serialize_field(value)) if self.export_to_file(): # Write the definition to a file diff --git a/common/test/data/split_test_module_draft/chapter/7ba42782322541d29a4d1be1eae4e4e8.xml b/common/test/data/split_test_module_draft/chapter/7ba42782322541d29a4d1be1eae4e4e8.xml new file mode 100644 index 0000000000..29148503a9 --- /dev/null +++ b/common/test/data/split_test_module_draft/chapter/7ba42782322541d29a4d1be1eae4e4e8.xml @@ -0,0 +1,3 @@ + + + diff --git a/common/test/data/split_test_module_draft/course.xml b/common/test/data/split_test_module_draft/course.xml new file mode 100644 index 0000000000..551834dcfb --- /dev/null +++ b/common/test/data/split_test_module_draft/course.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/split_test_module_draft/course/3111.xml b/common/test/data/split_test_module_draft/course/3111.xml new file mode 100644 index 0000000000..f340b1e8cc --- /dev/null +++ b/common/test/data/split_test_module_draft/course/3111.xml @@ -0,0 +1,4 @@ + + + + diff --git a/common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.html b/common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.html new file mode 100644 index 0000000000..117df64707 --- /dev/null +++ b/common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.html @@ -0,0 +1,19 @@ +
    +
  1. +

    September 21

    +
    +
    +

    Words of encouragement! This is a short note that most students will read.

    +

    Anant Agarwal (6.002x Principal Instructor)

    +
    +

    Primary versus Secondary Updates:

    Unfortunately, the internet throws a lot of text at students, and they + do not read everything that they are given. However, many students do read all that they are + given, and so detailed explainations in this section will benefit the most concientious. + Any essential information should be extremely quickly summarized in the primary section for skimmers.

    +

    Star Forum Poster

    + Students appreciate knowing that the course staff is reading what they post, and one of several ways + that you can do this is by acknowledging the star posters in your announcements. +

    +
    +
    2. +
diff --git a/common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.xml b/common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.xml new file mode 100644 index 0000000000..d438daa6b9 --- /dev/null +++ b/common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/split_test_module_draft/drafts/problem/50cef3605ec6445b9f11360640d9a192.xml b/common/test/data/split_test_module_draft/drafts/problem/50cef3605ec6445b9f11360640d9a192.xml new file mode 100644 index 0000000000..3fca10c48a --- /dev/null +++ b/common/test/data/split_test_module_draft/drafts/problem/50cef3605ec6445b9f11360640d9a192.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/split_test_module_draft/drafts/split_test/fb34c21fe64941999eaead421a8711b8.xml b/common/test/data/split_test_module_draft/drafts/split_test/fb34c21fe64941999eaead421a8711b8.xml new file mode 100644 index 0000000000..9497dece19 --- /dev/null +++ b/common/test/data/split_test_module_draft/drafts/split_test/fb34c21fe64941999eaead421a8711b8.xml @@ -0,0 +1,4 @@ + + + + diff --git a/common/test/data/split_test_module_draft/drafts/vertical/0faf29473cf1497baa33fcc828b179cd.xml b/common/test/data/split_test_module_draft/drafts/vertical/0faf29473cf1497baa33fcc828b179cd.xml new file mode 100644 index 0000000000..dfb150b91c --- /dev/null +++ b/common/test/data/split_test_module_draft/drafts/vertical/0faf29473cf1497baa33fcc828b179cd.xml @@ -0,0 +1,3 @@ + + + diff --git a/common/test/data/split_test_module_draft/drafts/vertical/5276f65d0e184dcca0841044576a540b.xml b/common/test/data/split_test_module_draft/drafts/vertical/5276f65d0e184dcca0841044576a540b.xml new file mode 100644 index 0000000000..f70874a8d6 --- /dev/null +++ b/common/test/data/split_test_module_draft/drafts/vertical/5276f65d0e184dcca0841044576a540b.xml @@ -0,0 +1,3 @@ + + + diff --git a/common/test/data/split_test_module_draft/drafts/vertical/9f0941d021414798836ef140fb5f6841.xml b/common/test/data/split_test_module_draft/drafts/vertical/9f0941d021414798836ef140fb5f6841.xml new file mode 100644 index 0000000000..83c4fd81da --- /dev/null +++ b/common/test/data/split_test_module_draft/drafts/vertical/9f0941d021414798836ef140fb5f6841.xml @@ -0,0 +1,3 @@ + + + diff --git a/common/test/data/split_test_module_draft/policies/3111/grading_policy.json b/common/test/data/split_test_module_draft/policies/3111/grading_policy.json new file mode 100644 index 0000000000..2095bb70f7 --- /dev/null +++ b/common/test/data/split_test_module_draft/policies/3111/grading_policy.json @@ -0,0 +1 @@ +{"GRADER": [{"short_label": "HW", "min_count": 12, "type": "Homework", "drop_count": 2, "weight": 0.15}, {"min_count": 12, "type": "Lab", "drop_count": 2, "weight": 0.15}, {"short_label": "Midterm", "min_count": 1, "type": "Midterm Exam", "drop_count": 0, "weight": 0.3}, {"short_label": "Final", "min_count": 1, "type": "Final Exam", "drop_count": 0, "weight": 0.4}], "GRADE_CUTOFFS": {"Pass": 0.5}} diff --git a/common/test/data/split_test_module_draft/policies/3111/policy.json b/common/test/data/split_test_module_draft/policies/3111/policy.json new file mode 100644 index 0000000000..c9f658dab4 --- /dev/null +++ b/common/test/data/split_test_module_draft/policies/3111/policy.json @@ -0,0 +1 @@ +{"course/3111": {"tabs": [{"type": "courseware", "name": "Courseware"}, {"type": "course_info", "name": "Course Info"}, {"type": "textbooks", "name": "Textbooks"}, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}, {"type": "progress", "name": "Progress"}], "advanced_modules": ["split_test"], "display_name": "split export test", "user_partitions": [{"description": "Experiment 1", "version": 1, "id": 0, "groups": [{"version": 1, "id": 0, "name": "group 0"}, {"version": 1, "id": 1, "name": "group 1"}], "name": "Experiment 0,1"}, {"description": "Experiment 2", "version": 1, "id": 1, "groups": [{"version": 1, "id": 0, "name": "group A"}, {"version": 1, "id": 1, "name": "group B"}, {"version": 1, "id": 2, "name": "group C"}], "name": "Experiment A,B,C"}], "discussion_topics": {"General": {"id": "i4x-foo-1111-course-3111"}}}} diff --git a/common/test/data/split_test_module_draft/policies/assets.json b/common/test/data/split_test_module_draft/policies/assets.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/common/test/data/split_test_module_draft/policies/assets.json @@ -0,0 +1 @@ +{} diff --git a/common/test/data/split_test_module_draft/sequential/75f1ab8297514713b8d8935f6745a0f7.xml b/common/test/data/split_test_module_draft/sequential/75f1ab8297514713b8d8935f6745a0f7.xml new file mode 100644 index 0000000000..44bd5aff27 --- /dev/null +++ b/common/test/data/split_test_module_draft/sequential/75f1ab8297514713b8d8935f6745a0f7.xml @@ -0,0 +1 @@ +