Merge pull request #1840 from MITx/fix/cdodge/export-draft-modules

Fix/cdodge/export draft modules

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()

common/lib/xmodule/xmodule/modulestore/draft.py

@@ -3,7 +3,6 @@ from datetime import datetime
 from . import ModuleStoreBase, Location, namedtuple_to_son
 from .exceptions import ItemNotFoundError
 from .inheritance import own_metadata
-import logging
 
 DRAFT = 'draft'
 
@@ -107,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
@@ -116,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)
 
@@ -164,7 +167,6 @@ class DraftModuleStore(ModuleStoreBase):
         """
         return super(DraftModuleStore, self).delete_item(as_draft(location))
 
-
     def get_parent_locations(self, location, course_id):
         '''Find all locations that are the parents of this location.  Needed
         for path_to_location().
@@ -178,6 +180,7 @@ class DraftModuleStore(ModuleStoreBase):
         Save a current draft to the underlying modulestore
         """
         draft = self.get_item(location)
+
         draft.cms.published_date = datetime.utcnow()
         draft.cms.published_by = published_by_id
         super(DraftModuleStore, self).update_item(location, draft._model_data._kvs._data)
@@ -221,6 +224,6 @@ class DraftModuleStore(ModuleStoreBase):
 
         # convert the dict - which is used for look ups - back into a list
         for key, value in to_process_dict.iteritems():
-            queried_children.append(value)  
+            queried_children.append(value)
 
         return queried_children

common/lib/xmodule/xmodule/modulestore/xml_exporter.py

@@ -1,12 +1,11 @@
 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):
+def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir, draft_modulestore=None):
 
     course = modulestore.get_item(course_location)
 
@@ -40,6 +39,24 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
         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=''):
     query_loc = Location('i4x', course_location.org, course_location.course, category_type, None)

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

@@ -6,17 +6,17 @@ from path import path
 
 from xblock.core import Scope
 
-from .xml import XMLModuleStore
-from .exceptions import DuplicateItemError
+from .xml import XMLModuleStore, ImportSystem, ParentTracker
 from xmodule.modulestore import Location
-from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX
+from xmodule.contentstore.content import StaticContent
 from .inheritance import own_metadata
+from xmodule.errortracker import make_error_tracker
 
 log = logging.getLogger(__name__)
 
 
 def import_static_content(modules, course_loc, course_data_path, static_content_store, target_location_namespace,
-    subpath='static', verbose=False):
+                          subpath='static', verbose=False):
 
     remap_dict = {}
 
@@ -107,10 +107,10 @@ def import_module_from_xml(modulestore, static_content_store, course_data_path,
         # 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 module.has_children:
@@ -119,7 +119,7 @@ def import_module_from_xml(modulestore, static_content_store, course_data_path,
             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())
 
@@ -139,8 +139,7 @@ def import_module_from_xml(modulestore, static_content_store, course_data_path,
             # 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))
+                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])
@@ -163,9 +162,9 @@ def import_course_from_xml(modulestore, static_content_store, course_data_path,
     # if there is *any* tabs - then there at least needs to be some predefined ones
     if module.tabs is None or len(module.tabs) == 0:
         module.tabs = [{"type": "courseware"},
-            {"type": "course_info", "name": "Course Info"},
-            {"type": "discussion", "name": "Discussion"},
-            {"type": "wiki", "name": "Wiki"}]  # note, add 'progress' when we can support it on Edge
+                       {"type": "course_info", "name": "Course Info"},
+                       {"type": "discussion", "name": "Discussion"},
+                       {"type": "wiki", "name": "Wiki"}]  # note, add 'progress' when we can support it on Edge
 
     # 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
@@ -175,7 +174,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 +190,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 +201,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 +222,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
@@ -235,15 +235,11 @@ def import_from_xml(store, data_dir, course_dirs=None,
                     # if there is *any* tabs - then there at least needs to be some predefined ones
                     if module.tabs is None or len(module.tabs) == 0:
                         module.tabs = [{"type": "courseware"},
-                            {"type": "course_info", "name": "Course Info"}, 
-                            {"type": "discussion", "name": "Discussion"},
-                            {"type": "wiki", "name": "Wiki"}]  # note, add 'progress' when we can support it on Edge
+                                       {"type": "course_info", "name": "Course Info"},
+                                       {"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
@@ -251,17 +247,16 @@ def import_from_xml(store, data_dir, course_dirs=None,
 
                     course_items.append(module)
 
-
             # then import all the static content
             if static_content_store is not 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,
-                    _namespace_rename, subpath='static', verbose=verbose)
+                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,59 +270,149 @@ 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
+                import_module(module, store, course_data_path, static_content_store)
 
-                if 'data' in content:
-                    module_data = content['data']
+            # 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)
 
-                    # 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:
-                        logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location))
-                else:
-                    module_data = content
-
-                store.update_item(module.location, module_data)
-
-                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:
+            if pseudo_course_id in store.ignore_write_events_on_courses:
                 store.ignore_write_events_on_courses.remove(pseudo_course_id)
-                store.refresh_cached_metadata_inheritance_tree(target_location_namespace if 
-                    target_location_namespace is not None else course_location)
+                store.refresh_cached_metadata_inheritance_tree(target_location_namespace if
+                                                               target_location_namespace is not None else course_location)
+
+    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
+
+    module_data = {}
+    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:
+            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 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(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')
+                        # 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)
+
+                    # 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
 
-    return module_store, course_items
 
 def remap_namespace(module, target_location_namespace):
     if target_location_namespace is None:
@@ -337,20 +422,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())
 
@@ -365,7 +450,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):
@@ -380,7 +465,6 @@ def check_module_metadata_editability(module):
 
     allowed = allowed + ['xml_attributes', 'display_name']
     err_cnt = 0
-    my_metadata = dict(own_metadata(module))
     illegal_keys = set(own_metadata(module).keys()) - set(allowed)
 
     if len(illegal_keys) > 0:
@@ -423,7 +507,7 @@ def validate_data_source_path_existence(path, is_err=True, extra_msg=None):
     _cnt = 0
     if not os.path.exists(path):
         print ("{0}: Expected folder at {1}. {2}".format('ERROR' if is_err == True else 'WARNING', path, extra_msg if
-            extra_msg is not None else ''))
+               extra_msg is not None else ''))
         _cnt = 1
     return _cnt
 
@@ -435,13 +519,13 @@ def validate_data_source_paths(data_dir, course_dir):
     warn_cnt = 0
     err_cnt += validate_data_source_path_existence(course_path / 'static')
     warn_cnt += validate_data_source_path_existence(course_path / 'static/subs', is_err=False,
-        extra_msg='Video captions (if they are used) will not work unless they are static/subs.')
+                                                    extra_msg='Video captions (if they are used) will not work unless they are static/subs.')
     return err_cnt, warn_cnt
 
 
 def perform_xlint(data_dir, course_dirs,
-                    default_class='xmodule.raw_module.RawDescriptor',
-                    load_error_modules=True):
+                  default_class='xmodule.raw_module.RawDescriptor',
+                  load_error_modules=True):
     err_cnt = 0
     warn_cnt = 0
 
@@ -497,7 +581,6 @@ def perform_xlint(data_dir, course_dirs,
             print "WARN: Missing course marketing video. It is recommended that every course have a marketing video."
             warn_cnt += 1
 
-
     print "\n\n------------------------------------------\nVALIDATION SUMMARY: {0} Errors   {1} Warnings\n".format(err_cnt, warn_cnt)
 
     if err_cnt > 0:

common/lib/xmodule/xmodule/xml_module.py

@@ -110,8 +110,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',
@@ -135,7 +134,7 @@ class XmlDescriptor(XModuleDescriptor):
         'graded': bool_map,
         'hide_progress_tab': bool_map,
         'allow_anonymous': bool_map,
-        'allow_anonymous_to_peers': bool_map
+        'allow_anonymous_to_peers': bool_map,
     }