From 21f4b058133934465e1d5d983a622eac23d67b25 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 21 Aug 2013 14:46:38 -0400 Subject: [PATCH] fix some gaps which would allow the temporary xml attributes 'parent_sequential_url' and 'index_in_children_list' to get persisted in the database, whereas they are meant to be only scoped during export/import --- .../contentstore/tests/test_contentstore.py | 15 ++++++++++++++ .../xmodule/modulestore/xml_importer.py | 20 ++++++++++++------- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 96b0b84e36..1cadcd69bf 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -945,8 +945,23 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): 'vertical', 'vertical_test', None]), depth=1) self.assertTrue(getattr(vertical, 'is_draft', False)) + self.assertNotIn('index_in_children_list', child.xml_attributes) + if hasattr(vertical, 'data'): + self.assertNotIn('index_in_children_list', vertical.data) + self.assertNotIn('parent_sequential_url', vertical.xml_attributes) + if hasattr(vertical, 'data'): + self.assertNotIn('parent_sequential_url', vertical.data) + for child in vertical.get_children(): self.assertTrue(getattr(child, 'is_draft', False)) + self.assertNotIn('index_in_children_list', child.xml_attributes) + if hasattr(child, 'data'): + self.assertNotIn('index_in_children_list', child.data) + self.assertNotIn('parent_sequential_url', child.xml_attributes) + if hasattr(child, 'data'): + self.assertNotIn('parent_sequential_url', child.data) + + # make sure that we don't have a sequential that is in draft mode sequential = draft_store.get_item(Location(['i4x', 'edX', 'toy', diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 7bea0fdcac..109d759693 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -212,6 +212,15 @@ def import_module(module, store, course_data_path, static_content_store, # NOTE: It's important to use own_metadata here to avoid writing # inherited metadata everywhere. + + # remove any export/import only xml_attributes which are used to wire together draft imports + if 'parent_sequential_url' in module.xml_attributes: + del module.xml_attributes['parent_sequential_url'] + + if 'index_in_children_list' in module.xml_attributes: + del module.xml_attributes['index_in_children_list'] + module.save() + store.update_metadata(module.location, dict(own_metadata(module))) @@ -281,7 +290,7 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path, # 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': - module.location = module.location._replace(revision=None) + non_draft_location = module.location._replace(revision=None) sequential_url = module.xml_attributes['parent_sequential_url'] index = int(module.xml_attributes['index_in_children_list']) @@ -291,15 +300,12 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path, seq_location = seq_location._replace(org=target_location_namespace.org, course=target_location_namespace.course ) - sequential = store.get_item(seq_location) + sequential = store.get_item(seq_location, depth=0) - if module.location.url() not in sequential.children: - sequential.children.insert(index, module.location.url()) + if non_draft_location.url() not in sequential.children: + sequential.children.insert(index, non_draft_location.url()) store.update_children(sequential.location, sequential.children) - del module.xml_attributes['parent_sequential_url'] - del module.xml_attributes['index_in_children_list'] - import_module(module, draft_store, course_data_path, static_content_store, source_location_namespace, target_location_namespace, allow_not_found=True) for child in module.get_children():