From 66c91b704a2a50ff7c4ae581de2b451f3b809451 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Fri, 15 Feb 2013 17:16:05 -0500
Subject: [PATCH 01/10] init work. Export the 'draft' store under a distinct
 folder in the export. TBD: do the import from that directory as well.

---
 cms/djangoapps/contentstore/views.py          |  2 +-
 .../lib/xmodule/xmodule/modulestore/draft.py  |  4 ++++
 .../xmodule/modulestore/xml_exporter.py       | 20 ++++++++++++++++++-
 .../xmodule/modulestore/xml_importer.py       |  6 ++++++
 common/lib/xmodule/xmodule/xml_module.py      |  6 +++---
 5 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index 87a2943773..af62276ec4 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -1426,7 +1426,7 @@ def generate_export_course(request, org, course, name):
 
     logging.debug('root = {0}'.format(root_dir))
 
-    export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name)
+    export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name, modulestore())
     #filename = root_dir / name + '.tar.gz'
 
     logging.debug('tar file being generated at {0}'.format(export_file.name))
diff --git a/common/lib/xmodule/xmodule/modulestore/draft.py b/common/lib/xmodule/xmodule/modulestore/draft.py
index 81f4da2780..6124d240a7 100644
--- a/common/lib/xmodule/xmodule/modulestore/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/draft.py
@@ -183,6 +183,10 @@ class DraftModuleStore(ModuleStoreBase):
         metadata.update(draft.metadata)
         metadata['published_date'] = tuple(datetime.utcnow().timetuple())
         metadata['published_by'] = published_by_id
+
+        if 'is_draft' in metadata:
+            del metadata['is_draft']      
+
         super(DraftModuleStore, self).update_item(location, draft.definition.get('data', {}))
         super(DraftModuleStore, self).update_children(location, draft.definition.get('children', []))
         super(DraftModuleStore, self).update_metadata(location, metadata)
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
index 55844116c6..e8d3fb0f82 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -5,7 +5,7 @@ from fs.osfs import OSFS
 from json import dumps
 
 
-def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir):
+def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir, draft_modulestore = None):
 
   course = modulestore.get_item(course_location)
 
@@ -41,6 +41,24 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
     policy = {'course/' + course.location.name: course.metadata}
     course_policy.write(dumps(policy)) 
 
+  # export everything from the draft store, unfortunately this will create lots of duplicates
+  if draft_modulestore is not None:
+      draft_course = draft_modulestore.get_item(course_location)
+      draft_course_dir = export_fs.makeopendir('drafts')
+      xml = draft_course.export_to_xml(draft_course_dir)
+      with draft_course_dir.open('course.xml', 'w') as course_xml:
+        course_xml.write(xml)
+
+  '''
+  draft_items = modulestore.get_items([None, None, None, 'vertical', None, 'draft'])
+  logging.debug('draft_items = {0}'.format(draft_items))
+  if len(draft_items) > 0:
+     
+    for draft_item in draft_items:
+      draft_item.export_to_xml(draft_items_dir)
+    #with draft_items_dir.open(draft_item.location.name + '.xml', 'w'):
+  '''    
+
 
 def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''):
   query_loc = Location('i4x', course_location.org, course_location.course, category_type, None)
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 0b77900ae9..9fcd75d6f4 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -220,9 +220,15 @@ def import_from_xml(store, data_dir, course_dirs=None,
             # inherited metadata everywhere.
             store.update_metadata(module.location, dict(module.own_metadata))
 
+        # now import any 'draft' items
+        import_course_draft(store, course_data_path, target_location_namespace)
+
     return module_store, course_items
 
 
+def import_course_draft(store, course_data_path, target_location_namespace):
+    pass
+
 def remap_namespace(module, target_location_namespace):
     if target_location_namespace is None:
         return module
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 64c3aabbcc..9081206491 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -105,8 +105,7 @@ class XmlDescriptor(XModuleDescriptor):
         'name', 'slug')
 
     metadata_to_strip = ('data_dir',
-            # cdodge: @TODO: We need to figure out a way to export out 'tabs' and 'grading_policy' which is on the course
-            'tabs', 'grading_policy', 'is_draft', 'published_by', 'published_date',
+            'tabs', 'grading_policy', 'published_by', 'published_date',
             'discussion_blackouts', 'testcenter_info',
            # VS[compat] -- remove the below attrs once everything is in the CMS
            'course', 'org', 'url_name', 'filename')
@@ -129,7 +128,8 @@ class XmlDescriptor(XModuleDescriptor):
         'hide_progress_tab': bool_map,
         'allow_anonymous': bool_map,
         'allow_anonymous_to_peers': bool_map,
-        'weight': int_map
+        'weight': int_map,
+        'is_draft': bool_map
     }
 
 

From 1789814ceb59143c85eb1e4c62e8bba78d656cd4 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Tue, 2 Apr 2013 14:19:54 -0400
Subject: [PATCH 02/10] wip

---
 cms/djangoapps/contentstore/views.py          |   3 +-
 common/lib/xmodule/xmodule/html_module.py     |   7 +-
 .../lib/xmodule/xmodule/modulestore/draft.py  |  12 +-
 .../xmodule/modulestore/xml_exporter.py       |  68 +++----
 .../xmodule/modulestore/xml_importer.py       | 171 ++++++++++++------
 5 files changed, 163 insertions(+), 98 deletions(-)

diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index f8960dd65d..4bc9bc512c 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -1580,7 +1580,8 @@ def import_course(request, org, course, name):
                 shutil.move(r / fname, course_dir)
 
         module_store, course_items = import_from_xml(modulestore('direct'), settings.GITHUB_REPO_ROOT,
-            [course_subdir], load_error_modules=False, static_content_store=contentstore(), target_location_namespace=Location(location))
+            [course_subdir], load_error_modules=False, static_content_store=contentstore(), 
+            target_location_namespace=Location(location), draft_store=modulestore())
 
         # we can blow this away when we're done importing.
         shutil.rmtree(course_dir)
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index e9cec32e3e..d901fc5fbe 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -118,8 +118,8 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
                 with system.resources_fs.open(filepath) as file:
                     html = file.read().decode('utf-8')
                     # Log a warning if we can't parse the file, but don't error
-                    if not check_html(html):
-                        msg = "Couldn't parse html in {0}.".format(filepath)
+                    if not check_html(html) and len(html) > 0:
+                        msg = "Couldn't parse html in {0}, content = {1}".format(filepath, html)
                         log.warning(msg)
                         system.error_tracker("Warning: " + msg)
 
@@ -156,7 +156,8 @@ class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):
 
         resource_fs.makedir(os.path.dirname(filepath), recursive=True, allow_recreate=True)
         with resource_fs.open(filepath, 'w') as file:
-            file.write(self.data.encode('utf-8'))
+            html_data = self.data.encode('utf-8')
+            file.write(html_data)
 
         # write out the relative name
         relname = path(pathname).basename()
diff --git a/common/lib/xmodule/xmodule/modulestore/draft.py b/common/lib/xmodule/xmodule/modulestore/draft.py
index ced8e7d42e..9b85fc18aa 100644
--- a/common/lib/xmodule/xmodule/modulestore/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/draft.py
@@ -106,7 +106,7 @@ class DraftModuleStore(ModuleStoreBase):
         """
         return wrap_draft(super(DraftModuleStore, self).clone_item(source, as_draft(location)))
 
-    def update_item(self, location, data):
+    def update_item(self, location, data, allow_not_found=False):
         """
         Set the data in the item specified by the location to
         data
@@ -115,9 +115,13 @@ class DraftModuleStore(ModuleStoreBase):
         data: A nested dictionary of problem data
         """
         draft_loc = as_draft(location)
-        draft_item = self.get_item(location)
-        if not getattr(draft_item, 'is_draft', False):
-            self.clone_item(location, draft_loc)
+        try:
+            draft_item = self.get_item(location)
+            if not getattr(draft_item, 'is_draft', False):
+               self.clone_item(location, draft_loc)
+        except ItemNotFoundError, e:
+            if not allow_not_found:
+                raise e
 
         return super(DraftModuleStore, self).update_item(draft_loc, data)
 
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
index a5a8ee3855..7927b2c68c 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -8,55 +8,59 @@ from json import dumps
 
 def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir, draft_modulestore = None):
 
-    course = modulestore.get_item(course_location)
+  course = modulestore.get_item(course_location)
 
-    fs = OSFS(root_dir)
-    export_fs = fs.makeopendir(course_dir)
+  fs = OSFS(root_dir)
+  export_fs = fs.makeopendir(course_dir)
 
-    xml = course.export_to_xml(export_fs)
-    with export_fs.open('course.xml', 'w') as course_xml:
-        course_xml.write(xml)
+  xml = course.export_to_xml(export_fs)
+  with export_fs.open('course.xml', 'w') as course_xml:
+      course_xml.write(xml)
 
-    # export the static assets
-    contentstore.export_all_for_course(course_location, root_dir + '/' + course_dir + '/static/')
+  # export the static assets
+  contentstore.export_all_for_course(course_location, root_dir + '/' + course_dir + '/static/')
 
-    # export the static tabs
-    export_extra_content(export_fs, modulestore, course_location, 'static_tab', 'tabs', '.html')
+  # export the static tabs
+  export_extra_content(export_fs, modulestore, course_location, 'static_tab', 'tabs', '.html')
 
-    # export the custom tags
-    export_extra_content(export_fs, modulestore, course_location, 'custom_tag_template', 'custom_tags')
+  # export the custom tags
+  export_extra_content(export_fs, modulestore, course_location, 'custom_tag_template', 'custom_tags')
 
-    # export the course updates
-    export_extra_content(export_fs, modulestore, course_location, 'course_info', 'info', '.html')
+  # export the course updates
+  export_extra_content(export_fs, modulestore, course_location, 'course_info', 'info', '.html')
 
-    # export the grading policy
-    policies_dir = export_fs.makeopendir('policies')
-    course_run_policy_dir = policies_dir.makeopendir(course.location.name)
-    with course_run_policy_dir.open('grading_policy.json', 'w') as grading_policy:
-        grading_policy.write(dumps(course.grading_policy))
+  # export the grading policy
+  policies_dir = export_fs.makeopendir('policies')
+  course_run_policy_dir = policies_dir.makeopendir(course.location.name)
+  with course_run_policy_dir.open('grading_policy.json', 'w') as grading_policy:
+      grading_policy.write(dumps(course.grading_policy))
 
-    # export all of the course metadata in policy.json
-    with course_run_policy_dir.open('policy.json', 'w') as course_policy:
-        policy = {'course/' + course.location.name: own_metadata(course)}
-        course_policy.write(dumps(policy))
+  # export all of the course metadata in policy.json
+  with course_run_policy_dir.open('policy.json', 'w') as course_policy:
+      policy = {'course/' + course.location.name: own_metadata(course)}
+      course_policy.write(dumps(policy))
 
   # export everything from the draft store, unfortunately this will create lots of duplicates
+  '''
   if draft_modulestore is not None:
       draft_course = draft_modulestore.get_item(course_location)
-      draft_course_dir = export_fs.makeopendir('drafts')
+      
       xml = draft_course.export_to_xml(draft_course_dir)
       with draft_course_dir.open('course.xml', 'w') as course_xml:
         course_xml.write(xml)
-
   '''
-  draft_items = modulestore.get_items([None, None, None, 'vertical', None, 'draft'])
-  logging.debug('draft_items = {0}'.format(draft_items))
-  if len(draft_items) > 0:
-     
+
+  # export draft content
+  # NOTE: this code assumes that verticals are the top most draftable container
+  # should we change the application, then this assumption will no longer
+  # be valid
+  draft_items = draft_modulestore.get_items([None, course_location.org, course_location.course,
+    'vertical', None, 'draft'])
+  
+  if len(draft_items)>0:
+    draft_course_dir = export_fs.makeopendir('drafts')
     for draft_item in draft_items:
-      draft_item.export_to_xml(draft_items_dir)
-    #with draft_items_dir.open(draft_item.location.name + '.xml', 'w'):
-  '''    
+      draft_item.export_to_xml(draft_course_dir)
 
 
 def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''):
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 1d3de93b38..113389d77a 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -6,11 +6,13 @@ from path import path
 
 from xblock.core import Scope
 
-from .xml import XMLModuleStore
+from .xml import XMLModuleStore, ImportSystem, ParentTracker
 from .exceptions import DuplicateItemError
 from xmodule.modulestore import Location
 from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX
 from .inheritance import own_metadata
+from xmodule.errortracker import make_error_tracker
+from collections import defaultdict
 
 log = logging.getLogger(__name__)
 
@@ -175,7 +177,8 @@ def import_course_from_xml(modulestore, static_content_store, course_data_path,
 
 def import_from_xml(store, data_dir, course_dirs=None,
                     default_class='xmodule.raw_module.RawDescriptor',
-                    load_error_modules=True, static_content_store=None, target_location_namespace=None, verbose=False):
+                    load_error_modules=True, static_content_store=None, target_location_namespace=None, 
+                    verbose=False, draft_store=None):
     """
     Import the specified xml data_dir into the "store" modulestore,
     using org and course as the location org and course.
@@ -190,7 +193,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
 
     """
 
-    module_store = XMLModuleStore(
+    xml_module_store = XMLModuleStore(
         data_dir,
         default_class=default_class,
         course_dirs=course_dirs,
@@ -201,7 +204,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
     # to enumerate the entire collection of course modules. It will be left as a TBD to implement that
     # method on XmlModuleStore.
     course_items = []
-    for course_id in module_store.modules.keys():
+    for course_id in xml_module_store.modules.keys():
 
         if target_location_namespace is not None:
             pseudo_course_id = '/'.join([target_location_namespace.org, target_location_namespace.course])
@@ -222,7 +225,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
 
             # Quick scan to get course module as we need some info from there. Also we need to make sure that the
             # course module is committed first into the store
-            for module in module_store.modules[course_id].itervalues():
+            for module in xml_module_store.modules[course_id].itervalues():
                 if module.category == 'course':
                     course_data_path = path(data_dir) / module.data_dir
                     course_location = module.location
@@ -239,11 +242,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
                             {"type": "discussion", "name": "Discussion"},
                             {"type": "wiki", "name": "Wiki"}]  # note, add 'progress' when we can support it on Edge
 
-
-                    if hasattr(module, 'data'):
-                        store.update_item(module.location, module.data)
-                    store.update_children(module.location, module.children)
-                    store.update_metadata(module.location, dict(own_metadata(module)))
+                    import_module(module, store, course_data_path, static_content_store)
 
                     # a bit of a hack, but typically the "course image" which is shown on marketing pages is hard coded to /images/course_image.jpg
                     # so let's make sure we import in case there are no other references to it in the modules
@@ -257,11 +256,11 @@ def import_from_xml(store, data_dir, course_dirs=None,
                 _namespace_rename = target_location_namespace if target_location_namespace is not None else course_location
 
                 # first pass to find everything in /static/
-                import_static_content(module_store.modules[course_id], course_location, course_data_path, static_content_store,
+                import_static_content(xml_module_store.modules[course_id], course_location, course_data_path, static_content_store,
                     _namespace_rename, subpath='static', verbose=verbose)
 
             # finally loop through all the modules
-            for module in module_store.modules[course_id].itervalues():
+            for module in xml_module_store.modules[course_id].itervalues():
 
                 if module.category == 'course':
                     # we've already saved the course module up at the top of the loop
@@ -275,49 +274,8 @@ def import_from_xml(store, data_dir, course_dirs=None,
                 if verbose:
                     log.debug('importing module location {0}'.format(module.location))
 
-                content = {}
-                for field in module.fields:
-                    if field.scope != Scope.content:
-                        continue
-                    try:
-                        content[field.name] = module._model_data[field.name]
-                    except KeyError:
-                        # Ignore any missing keys in _model_data
-                        pass
-     
-                if 'data' in content:
-                    module_data = content['data']
+                import_module(module, store, course_data_path, static_content_store)
 
-                    # cdodge: now go through any link references to '/static/' and make sure we've imported
-                    # it as a StaticContent asset
-                    try:
-                        remap_dict = {}
-
-                        # use the rewrite_links as a utility means to enumerate through all links
-                        # in the module data. We use that to load that reference into our asset store
-                        # IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to
-                        # do the rewrites natively in that code.
-                        # For example, what I'm seeing is <img src='foo.jpg' />   ->   <img src='bar.jpg'>
-                        # Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's
-                        # no good, so we have to do this kludge
-                        if isinstance(module_data, str) or isinstance(module_data, unicode):   # some module 'data' fields are non strings which blows up the link traversal code
-                            lxml_rewrite_links(module_data, lambda link: verify_content_links(module, course_data_path,
-                                static_content_store, link, remap_dict))                     
-
-                            for key in remap_dict.keys():
-                                module_data = module_data.replace(key, remap_dict[key])
-
-                    except Exception, e:
-                        logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
-
-                store.update_item(module.location, content)
-
-                if hasattr(module, 'children') and module.children != []:
-                    store.update_children(module.location, module.children)
-
-                # NOTE: It's important to use own_metadata here to avoid writing
-                # inherited metadata everywhere.
-                store.update_metadata(module.location, dict(own_metadata(module)))
         finally:
             # turn back on all write signalling
             if pseudo_course_id  in store.ignore_write_events_on_courses:
@@ -326,13 +284,110 @@ def import_from_xml(store, data_dir, course_dirs=None,
                     target_location_namespace is not None else course_location)
 
         # now import any 'draft' items
-        import_course_draft(store, course_data_path, target_location_namespace)
+        if draft_store is not None:
+            import_course_draft(xml_module_store, draft_store, course_data_path, static_content_store, target_location_namespace)
 
-    return module_store, course_items
+    return xml_module_store, course_items
+
+def import_module(module, store, course_data_path, static_content_store, allow_not_found=False):
+    content = {}
+    for field in module.fields:
+        if field.scope != Scope.content:
+            continue
+        try:
+            content[field.name] = module._model_data[field.name]
+        except KeyError:
+            # Ignore any missing keys in _model_data
+            pass
+
+    if 'data' in content:
+        module_data = content['data']
+
+        # cdodge: now go through any link references to '/static/' and make sure we've imported
+        # it as a StaticContent asset
+        try:
+            remap_dict = {}
+
+            # use the rewrite_links as a utility means to enumerate through all links
+            # in the module data. We use that to load that reference into our asset store
+            # IMPORTANT: There appears to be a bug in lxml.rewrite_link which makes us not be able to
+            # do the rewrites natively in that code.
+            # For example, what I'm seeing is <img src='foo.jpg' />   ->   <img src='bar.jpg'>
+            # Note the dropped element closing tag. This causes the LMS to fail when rendering modules - that's
+            # no good, so we have to do this kludge
+            if isinstance(module_data, str) or isinstance(module_data, unicode):   # some module 'data' fields are non strings which blows up the link traversal code
+                lxml_rewrite_links(module_data, lambda link: verify_content_links(module, course_data_path,
+                    static_content_store, link, remap_dict))                     
+
+                for key in remap_dict.keys():
+                    module_data = module_data.replace(key, remap_dict[key])
+
+        except Exception, e:
+            logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
+
+    if allow_not_found:
+        store.update_item(module.location, content, allow_not_found=allow_not_found)
+    else:
+        store.update_item(module.location, content)
+
+    if hasattr(module, 'children') and module.children != []:
+        store.update_children(module.location, module.children)
+
+    # NOTE: It's important to use own_metadata here to avoid writing
+    # inherited metadata everywhere.
+    store.update_metadata(module.location, dict(own_metadata(module)))    
 
 
-def import_course_draft(store, course_data_path, target_location_namespace):
-    pass
+def import_course_draft(xml_module_store, store, course_data_path, static_content_store, target_location_namespace):
+    '''
+    This will import all the content inside of the 'drafts' folder, if it exists
+    NOTE: This is not a full course import, basically in our current application only verticals (and downwards)
+    can be in draft. Therefore, we need to use slightly different call points into the import process_xml
+    as we can't simply call XMLModuleStore() constructor (like we do for importing public content)
+    '''
+    draft_dir = course_data_path + "/drafts"
+    if not os.path.exists(draft_dir):
+        return
+
+    # create a new 'System' object which will manage the importing
+    errorlog = make_error_tracker()
+    system = ImportSystem(
+        xml_module_store,
+        target_location_namespace.course_id,
+        draft_dir,
+        {},
+        errorlog.tracker,
+        ParentTracker(),
+        None,
+    )
+
+    # now walk the /vertical directory where each file in there will be a draft copy of the Vertical
+    for dirname, dirnames, filenames in os.walk(draft_dir + "/vertical"):
+        for filename in filenames:
+            module_path = os.path.join(dirname, filename)
+            with open(module_path) as f:
+                try:
+                    xml = f.read().decode('utf-8')
+                    descriptor = system.process_xml(xml)
+
+                    def _import_module(module):
+                        module.location = module.location._replace(revision='draft')
+                        import_module(module, store, course_data_path, static_content_store, allow_not_found=True)
+                        for child in module.get_children():
+                            _import_module(child)
+
+                    # 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)
+
+                    _import_module(descriptor)
+
+                except Exception, e:
+                    logging.exception('There was an error. {0}'.format(unicode(e)))
+                    pass
 
 
 def remap_namespace(module, target_location_namespace):

From 1a1635d402ea2140b6a6d4307ff4cb23e08cb57a Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Wed, 3 Apr 2013 11:28:25 -0400
Subject: [PATCH 03/10] Fix tests and extend export/import unit test with draft
 testing.

---
 .../contentstore/tests/test_contentstore.py   | 29 ++++++++++++++++---
 .../xmodule/modulestore/xml_exporter.py       | 13 +++++----
 .../xmodule/modulestore/xml_importer.py       | 10 ++++---
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index 49a609a879..b2962d717d 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -11,6 +11,7 @@ import json
 from fs.osfs import OSFS
 import copy
 from json import loads
+import traceback
 
 from django.contrib.auth.models import User
 from contentstore.utils import get_modulestore
@@ -284,17 +285,27 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
     def test_export_course(self):
         module_store = modulestore('direct')
+        draft_store = modulestore('draft')
         content_store = contentstore()
 
         import_from_xml(module_store, 'common/test/data/', ['full'])
         location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
 
+        # get a vertical (and components in it) to put into 'draft'
+        vertical = module_store.get_item(Location(['i4x', 'edX', 'full', 
+            'vertical', 'vertical_66', None]), depth=1)
+
+        draft_store.clone_item(vertical.location, vertical.location)
+
+        for child in vertical.get_children():
+            draft_store.clone_item(child.location, child.location)            
+
         root_dir = path(mkdtemp_clean())
 
         print 'Exporting to tempdir = {0}'.format(root_dir)
 
         # export out to a tempdir
-        export_to_xml(module_store, content_store, location, root_dir, 'test_export')
+        export_to_xml(module_store, content_store, location, root_dir, 'test_export', draft_modulestore=draft_store)
 
         # check for static tabs
         self.verify_content_existence(module_store, root_dir, location, 'tabs', 'static_tab', '.html')
@@ -328,15 +339,24 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         delete_course(module_store, content_store, location)
 
         # reimport
-        import_from_xml(module_store, root_dir, ['test_export'])
+        import_from_xml(module_store, root_dir, ['test_export'], draft_store=draft_store)
 
         items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
         self.assertGreater(len(items), 0)
         for descriptor in items:
             print "Checking {0}....".format(descriptor.location.url())
-            resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
+            non_draft_loc = descriptor.location._replace(revision=None)
+            resp = self.client.get(reverse('edit_unit', kwargs={'location': non_draft_loc.url()}))
             self.assertEqual(resp.status_code, 200)
 
+        # verify that we have the content in the draft store as well
+        vertical = draft_store.get_item(Location(['i4x', 'edX', 'full', 
+            'vertical', 'vertical_66', None]), depth=1)    
+            
+        self.assertTrue(hasattr(vertical, 'is_draft'))
+        for child in vertical.get_children():
+            self.assertTrue(hasattr(child, 'is_draft'))
+
         shutil.rmtree(root_dir)
 
     def test_course_handouts_rewrites(self):
@@ -404,7 +424,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         try:
             export_to_xml(module_store, content_store, location, root_dir, 'test_export')
             exported = True
-        except Exception:
+        except Exception,e:
+            print 'Exception thrown: {0}'.format(traceback.format_exc())
             pass
 
         self.assertTrue(exported)
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
index 7927b2c68c..5930d40cc8 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -54,13 +54,14 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
   # NOTE: this code assumes that verticals are the top most draftable container
   # should we change the application, then this assumption will no longer
   # be valid
-  draft_items = draft_modulestore.get_items([None, course_location.org, course_location.course,
-    'vertical', None, 'draft'])
+  if draft_modulestore is not None:
+    draft_items = draft_modulestore.get_items([None, course_location.org, course_location.course,
+      'vertical', None, 'draft'])
   
-  if len(draft_items)>0:
-    draft_course_dir = export_fs.makeopendir('drafts')
-    for draft_item in draft_items:
-      draft_item.export_to_xml(draft_course_dir)
+    if len(draft_items)>0:
+      draft_course_dir = export_fs.makeopendir('drafts')
+      for draft_item in draft_items:
+        draft_item.export_to_xml(draft_course_dir)
 
 
 def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''):
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index e6c4bc8bc9..204e229fdb 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -276,6 +276,12 @@ def import_from_xml(store, data_dir, course_dirs=None,
 
                 import_module(module, store, course_data_path, static_content_store)
 
+            # now import any 'draft' items
+            if draft_store is not None:
+                import_course_draft(xml_module_store, draft_store, course_data_path, 
+                    static_content_store, target_location_namespace if target_location_namespace is not None 
+                    else course_location)
+
         finally:
             # turn back on all write signalling
             if pseudo_course_id  in store.ignore_write_events_on_courses:
@@ -283,10 +289,6 @@ def import_from_xml(store, data_dir, course_dirs=None,
                 store.refresh_cached_metadata_inheritance_tree(target_location_namespace if 
                     target_location_namespace is not None else course_location)
 
-        # now import any 'draft' items
-        if draft_store is not None:
-            import_course_draft(xml_module_store, draft_store, course_data_path, static_content_store, target_location_namespace)
-
     return xml_module_store, course_items
 
 def import_module(module, store, course_data_path, static_content_store, allow_not_found=False):

From f7a83cdc74507bb4059c82c631851480d8ad45c6 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Wed, 3 Apr 2013 14:03:14 -0400
Subject: [PATCH 04/10] wip: add test case for exporting/importing a private
 module

---
 .../contentstore/tests/test_contentstore.py      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index b2962d717d..dcdba45ac5 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -302,6 +302,16 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         root_dir = path(mkdtemp_clean())
 
+        # now create a private vertical
+        private_vertical = draft_store.clone_item(vertical.location, 
+            Location(['i4x', 'edX', 'full', 'vertical', 'a_private_vertical', None]))
+
+        # add private to list of children
+        sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 
+            'sequential', 'Administrivia_and_Circuit_Elements', None]))
+        module_store.update_children(sequential.location, sequential.children + [private_vertical.location.url()])
+
+
         print 'Exporting to tempdir = {0}'.format(root_dir)
 
         # export out to a tempdir
@@ -357,6 +367,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         for child in vertical.get_children():
             self.assertTrue(hasattr(child, 'is_draft'))
 
+        # verify that we have the private vertical
+        test_private_vertical = draft_store.get_item(Location(['i4x', 'edX', 'full', 
+            'vertical', 'vertical_66', None]))
+
+        self.assertTrue(hasattr(test_private_vertical, 'is_draft'))
+
         shutil.rmtree(root_dir)
 
     def test_course_handouts_rewrites(self):

From 5f9d7db94d12fab47f1b6717e29a772d2c3ad435 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Wed, 3 Apr 2013 14:43:01 -0400
Subject: [PATCH 05/10] actually we need to start the draft content export from
 the sequentials. This is because we need the child pointers to the draft
 verticals. For private items, if we don't do this, then we'll loose those
 pointers

---
 .../contentstore/tests/test_contentstore.py   | 19 ++++++++++++++-----
 .../xmodule/modulestore/xml_exporter.py       | 18 ++++++++++++++----
 .../xmodule/modulestore/xml_importer.py       |  5 +++--
 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index dcdba45ac5..7231d7b150 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -309,8 +309,15 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         # add private to list of children
         sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 
             'sequential', 'Administrivia_and_Circuit_Elements', None]))
-        module_store.update_children(sequential.location, sequential.children + [private_vertical.location.url()])
+        private_location_no_draft = private_vertical.location._replace(revision=None)
+        module_store.update_children(sequential.location, sequential.children + 
+            [private_location_no_draft.url()])
 
+        # read back the sequential, to make sure we have a pointer to 
+        sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 
+            'sequential', 'Administrivia_and_Circuit_Elements', None]))  
+            
+        self.assertIn(private_location_no_draft.url(), sequential.children)
 
         print 'Exporting to tempdir = {0}'.format(root_dir)
 
@@ -354,10 +361,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
         self.assertGreater(len(items), 0)
         for descriptor in items:
-            print "Checking {0}....".format(descriptor.location.url())
-            non_draft_loc = descriptor.location._replace(revision=None)
-            resp = self.client.get(reverse('edit_unit', kwargs={'location': non_draft_loc.url()}))
-            self.assertEqual(resp.status_code, 200)
+            # don't try to look at private verticals. Right now we're running
+            # the service in non-draft aware
+            if hasattr(descriptor, 'is_draft'):
+                print "Checking {0}....".format(descriptor.location.url())
+                resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
+                self.assertEqual(resp.status_code, 200)
 
         # verify that we have the content in the draft store as well
         vertical = draft_store.get_item(Location(['i4x', 'edX', 'full', 
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
index 5930d40cc8..2d92b8010d 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -55,13 +55,23 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
   # should we change the application, then this assumption will no longer
   # be valid
   if draft_modulestore is not None:
-    draft_items = draft_modulestore.get_items([None, course_location.org, course_location.course,
+    draft_verticals = draft_modulestore.get_items([None, course_location.org, course_location.course,
       'vertical', None, 'draft'])
   
-    if len(draft_items)>0:
+    if len(draft_verticals)>0:
+      # now we have to go find every parent for each module and export from that point. We have
+      # to initiate the export from the sequence since we need the child pointers to private
+      # verticals. These will get filtered out from the export of the non-draft store.
+      sequential_locs = []
+      for draft_vertical in draft_verticals:
+        parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
+        if parent_locs[0] not in sequential_locs:
+          sequential_locs.append(parent_locs[0])
+
       draft_course_dir = export_fs.makeopendir('drafts')
-      for draft_item in draft_items:
-        draft_item.export_to_xml(draft_course_dir)
+      for sequential_loc in sequential_locs:
+        sequential = draft_modulestore.get_item(sequential_loc)
+        sequential.export_to_xml(draft_course_dir)
 
 
 def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''):
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 204e229fdb..45e51fc167 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -364,7 +364,7 @@ def import_course_draft(xml_module_store, store, course_data_path, static_conten
     )
 
     # now walk the /vertical directory where each file in there will be a draft copy of the Vertical
-    for dirname, dirnames, filenames in os.walk(draft_dir + "/vertical"):
+    for dirname, dirnames, filenames in os.walk(draft_dir + "/sequential"):
         for filename in filenames:
             module_path = os.path.join(dirname, filename)
             with open(module_path) as f:
@@ -373,7 +373,8 @@ def import_course_draft(xml_module_store, store, course_data_path, static_conten
                     descriptor = system.process_xml(xml)
 
                     def _import_module(module):
-                        module.location = module.location._replace(revision='draft')
+                        if module.location.category != 'sequential':
+                            module.location = module.location._replace(revision='draft')
                         import_module(module, store, course_data_path, static_content_store, allow_not_found=True)
                         for child in module.get_children():
                             _import_module(child)

From 5bbfe870056f83af35dfe84347e9257b11a1f517 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Fri, 5 Apr 2013 13:18:48 -0400
Subject: [PATCH 06/10] fix various pop8/pylint violations. switch hasattr() to
 getattr()

---
 .../contentstore/tests/test_contentstore.py   | 82 +++++++++----------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index 7231d7b150..685867a1cb 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -153,13 +153,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         module_store = modulestore('direct')
         found = False
 
-        item = None
         items = module_store.get_items(['i4x', 'edX', 'full', 'poll_question', None, None])
         found = len(items) > 0
 
         self.assertTrue(found)
         # check that there's actually content in the 'question' field
-        self.assertGreater(len(items[0].question),0)
+        self.assertGreater(len(items[0].question), 0)
 
     def test_xlint_fails(self):
         err_cnt = perform_xlint('common/test/data', ['full'])
@@ -172,14 +171,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None]))
 
-        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter', 'Week_1', None]))
 
         # make sure the parent no longer points to the child object which was deleted
         self.assertTrue(sequential.location.url() in chapter.children)
 
         self.client.post(reverse('delete_item'),
             json.dumps({'id': sequential.location.url(), 'delete_children': 'true', 'delete_all_versions': 'true'}),
-            "application/json")
+                    "application/json")
 
         found = False
         try:
@@ -190,7 +189,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         self.assertFalse(found)
 
-        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter', 'Week_1', None]))
 
         # make sure the parent no longer points to the child object which was deleted
         self.assertFalse(sequential.location.url() in chapter.children)
@@ -213,7 +212,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
         module_store = modulestore('direct')
-        content_store = contentstore()
 
         source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
         course = module_store.get_item(source_location)
@@ -226,7 +224,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
-            }
+        }
 
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
@@ -292,31 +290,31 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
 
         # get a vertical (and components in it) to put into 'draft'
-        vertical = module_store.get_item(Location(['i4x', 'edX', 'full', 
-            'vertical', 'vertical_66', None]), depth=1)
+        vertical = module_store.get_item(Location(['i4x', 'edX', 'full',
+                                         'vertical', 'vertical_66', None]), depth=1)
 
         draft_store.clone_item(vertical.location, vertical.location)
 
         for child in vertical.get_children():
-            draft_store.clone_item(child.location, child.location)            
+            draft_store.clone_item(child.location, child.location)           
 
         root_dir = path(mkdtemp_clean())
 
         # now create a private vertical
-        private_vertical = draft_store.clone_item(vertical.location, 
-            Location(['i4x', 'edX', 'full', 'vertical', 'a_private_vertical', None]))
+        private_vertical = draft_store.clone_item(vertical.location,
+                                                  Location(['i4x', 'edX', 'full', 'vertical', 'a_private_vertical', None]))
 
         # add private to list of children
-        sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 
-            'sequential', 'Administrivia_and_Circuit_Elements', None]))
+        sequential = module_store.get_item(Location(['i4x', 'edX', 'full',
+                                           'sequential', 'Administrivia_and_Circuit_Elements', None]))
         private_location_no_draft = private_vertical.location._replace(revision=None)
-        module_store.update_children(sequential.location, sequential.children + 
-            [private_location_no_draft.url()])
+        module_store.update_children(sequential.location, sequential.children +
+                                     [private_location_no_draft.url()])
 
         # read back the sequential, to make sure we have a pointer to 
-        sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 
-            'sequential', 'Administrivia_and_Circuit_Elements', None]))  
-            
+        sequential = module_store.get_item(Location(['i4x', 'edX', 'full',
+                                                     'sequential', 'Administrivia_and_Circuit_Elements', None]))
+
         self.assertIn(private_location_no_draft.url(), sequential.children)
 
         print 'Exporting to tempdir = {0}'.format(root_dir)
@@ -363,30 +361,29 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         for descriptor in items:
             # don't try to look at private verticals. Right now we're running
             # the service in non-draft aware
-            if hasattr(descriptor, 'is_draft'):
+            if getattr(descriptor, 'is_draft', False):
                 print "Checking {0}....".format(descriptor.location.url())
                 resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
                 self.assertEqual(resp.status_code, 200)
 
         # verify that we have the content in the draft store as well
-        vertical = draft_store.get_item(Location(['i4x', 'edX', 'full', 
-            'vertical', 'vertical_66', None]), depth=1)    
-            
-        self.assertTrue(hasattr(vertical, 'is_draft'))
+        vertical = draft_store.get_item(Location(['i4x', 'edX', 'full',
+                                                  'vertical', 'vertical_66', None]), depth=1)
+
+        self.assertTrue(getattr(vertical, 'is_draft', False))
         for child in vertical.get_children():
-            self.assertTrue(hasattr(child, 'is_draft'))
+            self.assertTrue(getattr(child, 'is_draft', False))
 
         # verify that we have the private vertical
-        test_private_vertical = draft_store.get_item(Location(['i4x', 'edX', 'full', 
-            'vertical', 'vertical_66', None]))
+        test_private_vertical = draft_store.get_item(Location(['i4x', 'edX', 'full',
+                                                               'vertical', 'vertical_66', None]))
 
-        self.assertTrue(hasattr(test_private_vertical, 'is_draft'))
+        self.assertTrue(getattr(test_private_vertical, 'is_draft', False))
 
         shutil.rmtree(root_dir)
 
     def test_course_handouts_rewrites(self):
         module_store = modulestore('direct')
-        content_store = contentstore()
 
         # import a test course
         import_from_xml(module_store, 'common/test/data/', ['full'])
@@ -419,11 +416,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         # make sure we pre-fetched a known sequential which should be at depth=2
         self.assertTrue(Location(['i4x', 'edX', 'full', 'sequential',
-            'Administrivia_and_Circuit_Elements', None]) in course.system.module_data)
+                                  'Administrivia_and_Circuit_Elements', None]) in course.system.module_data)
 
         # make sure we don't have a specific vertical which should be at depth=3
-        self.assertFalse(Location(['i4x', 'edX', 'full', 'vertical', 'vertical_58',
-            None]) in course.system.module_data)
+        self.assertFalse(Location(['i4x', 'edX', 'full', 'vertical', 'vertical_58', None])
+                         in course.system.module_data)
 
     def test_export_course_with_unknown_metadata(self):
         module_store = modulestore('direct')
@@ -449,12 +446,13 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         try:
             export_to_xml(module_store, content_store, location, root_dir, 'test_export')
             exported = True
-        except Exception,e:
+        except Exception:
             print 'Exception thrown: {0}'.format(traceback.format_exc())
             pass
 
         self.assertTrue(exported)
 
+
 class ContentStoreTest(ModuleStoreTestCase):
     """
     Tests for the CMS ContentStore application.
@@ -489,7 +487,7 @@ class ContentStoreTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
-            }
+        }
 
     def test_create_course(self):
         """Test new course creation - happy path"""
@@ -516,7 +514,7 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         self.assertEqual(resp.status_code, 200)
         self.assertEqual(data['ErrMsg'],
-            'There is already a course defined with the same organization and course number.')
+                         'There is already a course defined with the same organization and course number.')
 
     def test_create_course_with_bad_organization(self):
         """Test new course creation - error path for bad organization name"""
@@ -526,7 +524,7 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         self.assertEqual(resp.status_code, 200)
         self.assertEqual(data['ErrMsg'],
-            "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.")
+                         "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.")
 
     def test_course_index_view_with_no_courses(self):
         """Test viewing the index page with no courses"""
@@ -562,10 +560,10 @@ class ContentStoreTest(ModuleStoreTestCase):
         CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
 
         data = {
-                'org': 'MITx',
-                'course': '999',
-                'name': Location.clean('Robot Super Course'),
-                }
+            'org': 'MITx',
+            'course': '999',
+            'name': Location.clean('Robot Super Course'),
+        }
 
         resp = self.client.get(reverse('course_index', kwargs=data))
         self.assertContains(resp,
@@ -581,7 +579,7 @@ class ContentStoreTest(ModuleStoreTestCase):
             'parent_location': 'i4x://MITx/999/course/Robot_Super_Course',
             'template': 'i4x://edx/templates/chapter/Empty',
             'display_name': 'Section One',
-            }
+        }
 
         resp = self.client.post(reverse('clone_item'), section_data)
 
@@ -597,7 +595,7 @@ class ContentStoreTest(ModuleStoreTestCase):
         problem_data = {
             'parent_location': 'i4x://MITx/999/course/Robot_Super_Course',
             'template': 'i4x://edx/templates/problem/Blank_Common_Problem'
-            }
+        }
 
         resp = self.client.post(reverse('clone_item'), problem_data)
 

From 7a334c6789d07b193dfee941397e1d305bd29252 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Fri, 5 Apr 2013 14:34:44 -0400
Subject: [PATCH 07/10] go back to exporting draft/private verticals, but also
 set additional xml attributes so that we can re-tie in the draft stuff into
 the sequence's children

---
 .../xmodule/modulestore/xml_exporter.py       | 18 ++++-----
 .../xmodule/modulestore/xml_importer.py       | 38 +++++++++++++++----
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
index 2d92b8010d..cc147b546f 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -59,19 +59,15 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
       'vertical', None, 'draft'])
   
     if len(draft_verticals)>0:
-      # now we have to go find every parent for each module and export from that point. We have
-      # to initiate the export from the sequence since we need the child pointers to private
-      # verticals. These will get filtered out from the export of the non-draft store.
-      sequential_locs = []
+      draft_course_dir = export_fs.makeopendir('drafts')
       for draft_vertical in draft_verticals:
         parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
-        if parent_locs[0] not in sequential_locs:
-          sequential_locs.append(parent_locs[0])
-
-      draft_course_dir = export_fs.makeopendir('drafts')
-      for sequential_loc in sequential_locs:
-        sequential = draft_modulestore.get_item(sequential_loc)
-        sequential.export_to_xml(draft_course_dir)
+        logging.debug('parent_locs = {0}'.format(parent_locs))
+        draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
+        sequential = modulestore.get_item(Location(parent_locs[0]))
+        index = sequential.children.index(draft_vertical.location.url())
+        draft_vertical.xml_attributes['index_in_children_list'] = str(index)
+        draft_vertical.export_to_xml(draft_course_dir)
 
 
 def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''):
diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index cf9213dd52..3626bc819d 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -364,7 +364,7 @@ def import_course_draft(xml_module_store, store, course_data_path, static_conten
     )
 
     # now walk the /vertical directory where each file in there will be a draft copy of the Vertical
-    for dirname, dirnames, filenames in os.walk(draft_dir + "/sequential"):
+    for dirname, dirnames, filenames in os.walk(draft_dir + "/vertical"):
         for filename in filenames:
             module_path = os.path.join(dirname, filename)
             with open(module_path) as f:
@@ -373,8 +373,30 @@ def import_course_draft(xml_module_store, store, course_data_path, static_conten
                     descriptor = system.process_xml(xml)
 
                     def _import_module(module):
-                        if module.location.category != 'sequential':
-                            module.location = module.location._replace(revision='draft')
+                        module.location = module.location._replace(revision='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':
+                            module.location = module.location._replace(revision=None)
+                            sequential_url = module.xml_attributes['parent_sequential_url']
+                            index = int(module.xml_attributes['index_in_children_list'])
+
+                            seq_location = Location(sequential_url)
+
+                            # IMPORTANT: Be sure to update the sequential in the NEW namespace
+                            seq_location = seq_location._replace(org=target_location_namespace.org,
+                                                                 course=target_location_namespace.course
+                                                                 )
+                            sequential = store.get_item(seq_location)
+
+                            if module.location.url() not in sequential.children:
+                                sequential.children.insert(index, module.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, store, course_data_path, static_content_store, allow_not_found=True)
                         for child in module.get_children():
                             _import_module(child)
@@ -401,20 +423,20 @@ def remap_namespace(module, target_location_namespace):
     # the caller passed in
     if module.location.category != 'course':
         module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
-            course=target_location_namespace.course)
+                                                   course=target_location_namespace.course)
     else:
         module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
-            course=target_location_namespace.course, name=target_location_namespace.name)
+                                                   course=target_location_namespace.course, name=target_location_namespace.name)
 
     # then remap children pointers since they too will be re-namespaced
-    if hasattr(module,'children'):
+    if hasattr(module, 'children'):
         children_locs = module.children
         if children_locs is not None and children_locs != []:
             new_locs = []
             for child in children_locs:
                 child_loc = Location(child)
                 new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
-                    course=target_location_namespace.course)
+                                                   course=target_location_namespace.course)
 
                 new_locs.append(new_child_loc.url())
 
@@ -429,7 +451,7 @@ def allowed_metadata_by_category(category):
         'vertical': [],
         'chapter': ['start'],
         'sequential': ['due', 'format', 'start', 'graded']
-    }.get(category,['*'])
+    }.get(category, ['*'])
 
 
 def check_module_metadata_editability(module):

From c12d265a21ba72d940a57370c5638d62264c6f39 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Mon, 8 Apr 2013 15:00:54 -0400
Subject: [PATCH 08/10] calling update_item implicitly already puts the data in
 definition.data, so we want to make sure we don't end up nesting
 definition.data.data

---
 common/lib/xmodule/xmodule/modulestore/xml_importer.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index 3626bc819d..cd84d2199d 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -327,10 +327,10 @@ def import_module(module, store, course_data_path, static_content_store, allow_n
         except Exception, e:
             logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
 
-    if allow_not_found:
-        store.update_item(module.location, content, allow_not_found=allow_not_found)
-    else:
-        store.update_item(module.location, content)
+        if allow_not_found:
+            store.update_item(module.location, module_data, allow_not_found=allow_not_found)
+        else:
+            store.update_item(module.location, module_data)
 
     if hasattr(module, 'children') and module.children != []:
         store.update_children(module.location, module.children)

From 4cbb92533f8237018875e1f9e2f0e9486d6d4572 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Mon, 8 Apr 2013 15:08:05 -0400
Subject: [PATCH 09/10] fix broken unit test

---
 .../lib/xmodule/xmodule/modulestore/xml_importer.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
index cd84d2199d..a29b56f5b6 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py
@@ -302,6 +302,7 @@ def import_module(module, store, course_data_path, static_content_store, allow_n
             # Ignore any missing keys in _model_data
             pass
 
+    module_data = {}
     if 'data' in content:
         module_data = content['data']
 
@@ -324,13 +325,15 @@ def import_module(module, store, course_data_path, static_content_store, allow_n
                 for key in remap_dict.keys():
                     module_data = module_data.replace(key, remap_dict[key])
 
-        except Exception, e:
+        except Exception:
             logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
+    else:
+        module_data = content
 
-        if allow_not_found:
-            store.update_item(module.location, module_data, allow_not_found=allow_not_found)
-        else:
-            store.update_item(module.location, module_data)
+    if allow_not_found:
+        store.update_item(module.location, module_data, allow_not_found=allow_not_found)
+    else:
+        store.update_item(module.location, module_data)
 
     if hasattr(module, 'children') and module.children != []:
         store.update_children(module.location, module.children)

From 0a2937311d4c319d5233b0a92073a7a3d57d9452 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Wed, 10 Apr 2013 12:34:18 -0400
Subject: [PATCH 10/10] remove commented out code. Also fix up indents and
 other violations

---
 .../xmodule/modulestore/xml_exporter.py       | 104 ++++++++----------
 1 file changed, 46 insertions(+), 58 deletions(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
index cc147b546f..e03c61bb24 100644
--- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
+++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py
@@ -1,73 +1,61 @@
 import logging
 from xmodule.modulestore import Location
-from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.inheritance import own_metadata
 from fs.osfs import OSFS
 from json import dumps
 
 
-def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir, draft_modulestore = None):
+def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir, draft_modulestore=None):
 
-  course = modulestore.get_item(course_location)
+    course = modulestore.get_item(course_location)
 
-  fs = OSFS(root_dir)
-  export_fs = fs.makeopendir(course_dir)
+    fs = OSFS(root_dir)
+    export_fs = fs.makeopendir(course_dir)
 
-  xml = course.export_to_xml(export_fs)
-  with export_fs.open('course.xml', 'w') as course_xml:
-      course_xml.write(xml)
-
-  # export the static assets
-  contentstore.export_all_for_course(course_location, root_dir + '/' + course_dir + '/static/')
-
-  # export the static tabs
-  export_extra_content(export_fs, modulestore, course_location, 'static_tab', 'tabs', '.html')
-
-  # export the custom tags
-  export_extra_content(export_fs, modulestore, course_location, 'custom_tag_template', 'custom_tags')
-
-  # export the course updates
-  export_extra_content(export_fs, modulestore, course_location, 'course_info', 'info', '.html')
-
-  # export the grading policy
-  policies_dir = export_fs.makeopendir('policies')
-  course_run_policy_dir = policies_dir.makeopendir(course.location.name)
-  with course_run_policy_dir.open('grading_policy.json', 'w') as grading_policy:
-      grading_policy.write(dumps(course.grading_policy))
-
-  # export all of the course metadata in policy.json
-  with course_run_policy_dir.open('policy.json', 'w') as course_policy:
-      policy = {'course/' + course.location.name: own_metadata(course)}
-      course_policy.write(dumps(policy))
-
-  # export everything from the draft store, unfortunately this will create lots of duplicates
-  '''
-  if draft_modulestore is not None:
-      draft_course = draft_modulestore.get_item(course_location)
-      
-      xml = draft_course.export_to_xml(draft_course_dir)
-      with draft_course_dir.open('course.xml', 'w') as course_xml:
+    xml = course.export_to_xml(export_fs)
+    with export_fs.open('course.xml', 'w') as course_xml:
         course_xml.write(xml)
-  '''
 
-  # export draft content
-  # NOTE: this code assumes that verticals are the top most draftable container
-  # should we change the application, then this assumption will no longer
-  # be valid
-  if draft_modulestore is not None:
-    draft_verticals = draft_modulestore.get_items([None, course_location.org, course_location.course,
-      'vertical', None, 'draft'])
-  
-    if len(draft_verticals)>0:
-      draft_course_dir = export_fs.makeopendir('drafts')
-      for draft_vertical in draft_verticals:
-        parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
-        logging.debug('parent_locs = {0}'.format(parent_locs))
-        draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
-        sequential = modulestore.get_item(Location(parent_locs[0]))
-        index = sequential.children.index(draft_vertical.location.url())
-        draft_vertical.xml_attributes['index_in_children_list'] = str(index)
-        draft_vertical.export_to_xml(draft_course_dir)
+    # export the static assets
+    contentstore.export_all_for_course(course_location, root_dir + '/' + course_dir + '/static/')
+
+    # export the static tabs
+    export_extra_content(export_fs, modulestore, course_location, 'static_tab', 'tabs', '.html')
+
+    # export the custom tags
+    export_extra_content(export_fs, modulestore, course_location, 'custom_tag_template', 'custom_tags')
+
+    # export the course updates
+    export_extra_content(export_fs, modulestore, course_location, 'course_info', 'info', '.html')
+
+    # export the grading policy
+    policies_dir = export_fs.makeopendir('policies')
+    course_run_policy_dir = policies_dir.makeopendir(course.location.name)
+    with course_run_policy_dir.open('grading_policy.json', 'w') as grading_policy:
+        grading_policy.write(dumps(course.grading_policy))
+
+    # export all of the course metadata in policy.json
+    with course_run_policy_dir.open('policy.json', 'w') as course_policy:
+        policy = {'course/' + course.location.name: own_metadata(course)}
+        course_policy.write(dumps(policy))
+
+    # export draft content
+    # NOTE: this code assumes that verticals are the top most draftable container
+    # should we change the application, then this assumption will no longer
+    # be valid
+    if draft_modulestore is not None:
+        draft_verticals = draft_modulestore.get_items([None, course_location.org, course_location.course,
+                                                       'vertical', None, 'draft'])
+        if len(draft_verticals) > 0:
+            draft_course_dir = export_fs.makeopendir('drafts')
+            for draft_vertical in draft_verticals:
+                parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id)
+                logging.debug('parent_locs = {0}'.format(parent_locs))
+                draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url()
+                sequential = modulestore.get_item(Location(parent_locs[0]))
+                index = sequential.children.index(draft_vertical.location.url())
+                draft_vertical.xml_attributes['index_in_children_list'] = str(index)
+                draft_vertical.export_to_xml(draft_course_dir)
 
 
 def export_extra_content(export_fs, modulestore, course_location, category_type, dirname, file_suffix=''):