Getting draft split_test_module instances to import/export.

STUD-327

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)

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -424,7 +424,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(),
@@ -478,21 +478,27 @@ def _import_course_draft(
                         # Not a 'hidden file', then re-raise exception
                         raise err
 
-                    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)
-
-                    index = int(descriptor.xml_attributes['index_in_children_list'])
-                    if index in drafts:
-                        drafts[index].append(descriptor)
-                    else:
-                        drafts[index] = [descriptor]
+                    # 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)
+
+                        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.')
@@ -503,24 +509,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)
 

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

common/test/data/split_test_module_draft/chapter/7ba42782322541d29a4d1be1eae4e4e8.xml

+<chapter display_name="New Section Name">
+  <sequential url_name="75f1ab8297514713b8d8935f6745a0f7"/>
+</chapter>

common/test/data/split_test_module_draft/course.xml

+<course url_name="3111" org="foo" course="1111"/>

common/test/data/split_test_module_draft/course/3111.xml

+<course advanced_modules="[&quot;split_test&quot;]" display_name="split export test" user_partitions="[{&quot;name&quot;: &quot;Experiment 0,1&quot;, &quot;version&quot;: 1, &quot;description&quot;: &quot;Experiment 1&quot;, &quot;groups&quot;: [{&quot;version&quot;: 1, &quot;id&quot;: 0, &quot;name&quot;: &quot;group 0&quot;}, {&quot;version&quot;: 1, &quot;id&quot;: 1, &quot;name&quot;: &quot;group 1&quot;}], &quot;id&quot;: 0}, {&quot;name&quot;: &quot;Experiment A,B,C&quot;, &quot;version&quot;: 1, &quot;description&quot;: &quot;Experiment 2&quot;, &quot;groups&quot;: [{&quot;version&quot;: 1, &quot;id&quot;: 0, &quot;name&quot;: &quot;group A&quot;}, {&quot;version&quot;: 1, &quot;id&quot;: 1, &quot;name&quot;: &quot;group B&quot;}, {&quot;version&quot;: 1, &quot;id&quot;: 2, &quot;name&quot;: &quot;group C&quot;}], &quot;id&quot;: 1}]">
+  <chapter url_name="7ba42782322541d29a4d1be1eae4e4e8"/>
+  <wiki slug="foo.1111.3111"/>
+</course>

common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.html

+<ol>
+<li>
+<h2> September 21 </h2>
+<section class='update-description'>
+	       <section class='primary'>
+	       		<p> Words of encouragement! This is a short note that most students will read. </p>
+	       		<p class='author'>Anant  Agarwal (6.002x Principal Instructor)</p>
+	       </section>
+	       <h3>Primary versus Secondary Updates:</h3><p>Unfortunately, the internet throws a lot of text at students, and they
+	       do not read everything that they are given. However, many students <em>do</em> 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.</p>
+	       <h3>Star Forum Poster</h3><p>
+	       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.
+	       </p>
+</section>
+</li>
+</ol>

common/test/data/split_test_module_draft/drafts/html/5f2b89070ad64ea7846800c11d44ed72.xml

+<html filename="5f2b89070ad64ea7846800c11d44ed72" display_name="Announcement"/>

common/test/data/split_test_module_draft/drafts/problem/50cef3605ec6445b9f11360640d9a192.xml

+<problem display_name="Blank Common Problem" markdown=""/>

common/test/data/split_test_module_draft/drafts/split_test/fb34c21fe64941999eaead421a8711b8.xml

+<split_test group_id_to_child="{&quot;1&quot;: &quot;i4x://foo/1111/vertical/0faf29473cf1497baa33fcc828b179cd&quot;, &quot;0&quot;: &quot;i4x://foo/1111/vertical/9f0941d021414798836ef140fb5f6841&quot;}" user_partition_id="0">
+  <vertical url_name="9f0941d021414798836ef140fb5f6841"/>
+  <vertical url_name="0faf29473cf1497baa33fcc828b179cd"/>
+</split_test>

common/test/data/split_test_module_draft/drafts/vertical/0faf29473cf1497baa33fcc828b179cd.xml

+<vertical display_name="group 1">
+  <problem url_name="50cef3605ec6445b9f11360640d9a192"/>
+</vertical>

common/test/data/split_test_module_draft/drafts/vertical/5276f65d0e184dcca0841044576a540b.xml

+<vertical display_name="New Unit" parent_sequential_url="i4x://foo/1111/sequential/75f1ab8297514713b8d8935f6745a0f7" index_in_children_list="0">
+  <split_test url_name="fb34c21fe64941999eaead421a8711b8"/>
+</vertical>

common/test/data/split_test_module_draft/drafts/vertical/9f0941d021414798836ef140fb5f6841.xml

+<vertical display_name="group 0">
+  <html url_name="5f2b89070ad64ea7846800c11d44ed72"/>
+</vertical>

common/test/data/split_test_module_draft/policies/3111/grading_policy.json

+{"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}}

common/test/data/split_test_module_draft/policies/3111/policy.json

+{"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"}}}}

common/test/data/split_test_module_draft/policies/assets.json

+{}

common/test/data/split_test_module_draft/sequential/75f1ab8297514713b8d8935f6745a0f7.xml

+<sequential display_name="New Subsection"/>