implement static tabs. rejigger .tar.gz import to do the re-namespacing as a…

implement static tabs. rejigger .tar.gz import to do the re-namespacing as a post-processing step as we need to retain the original url_name during the import. Also, migrate the presentation tier to use module_render.py::get_module() to better unify HTML'ifying stuff

cms/djangoapps/contentstore/views.py

@@ -1029,18 +1029,8 @@ def import_course(request, org, course, name):
             for fname in os.listdir(r):
                 shutil.move(r/fname, course_dir)
 
-        with open(course_dir / 'course.xml', 'r') as course_file:
-            course_data = etree.parse(course_file, parser=edx_xml_parser)
-            course_data_root = course_data.getroot()
-            course_data_root.set('org', org)
-            course_data_root.set('course', course)
-            course_data_root.set('url_name', name)
-
-        with open(course_dir / 'course.xml', 'w') as course_file:
-            course_data.write(course_file)
-
         module_store, course_items = import_from_xml(modulestore('direct'), settings.GITHUB_REPO_ROOT,
-            [course_dir], load_error_modules=False, static_content_store=contentstore())
+            [course_dir], load_error_modules=False, static_content_store=contentstore(), target_location_namespace = Location(location))
 
         # we can blow this away when we're done importing.
         shutil.rmtree(course_dir)

common/lib/xmodule/setup.py

@@ -35,6 +35,8 @@ setup(
             "videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor",
             "videosequence = xmodule.seq_module:SequenceDescriptor",
             "discussion = xmodule.discussion_module:DiscussionDescriptor",
+            "course_info = xmodule.html_module:HtmlDescriptor",
+            "static_tab = xmodule.html_module:HtmlDescriptor",
         ]
     }
 )

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -4,6 +4,7 @@ import logging
 import os
 import re
 import sys
+import glob
 
 from collections import defaultdict
 from cStringIO import StringIO
@@ -333,7 +334,6 @@ class XMLModuleStore(ModuleStoreBase):
         if not os.path.exists(policy_path):
             return {}
         try:
-            log.debug("Loading policy from {0}".format(policy_path))
             with open(policy_path) as f:
                 return json.load(f)
         except (IOError, ValueError) as err:
@@ -388,6 +388,7 @@ class XMLModuleStore(ModuleStoreBase):
             if url_name:
                 policy_dir = self.data_dir / course_dir / 'policies' / url_name
                 policy_path = policy_dir / 'policy.json'
+
                 policy = self.load_policy(policy_path, tracker)
 
                 # VS[compat]: remove once courses use the policy dirs.
@@ -405,7 +406,6 @@ class XMLModuleStore(ModuleStoreBase):
                     raise ValueError("Can't load a course without a 'url_name' "
                                      "(or 'name') set.  Set url_name.")
 
-
             course_id = CourseDescriptor.make_id(org, course, url_name)
             system = ImportSystem(
                 self,
@@ -438,11 +438,27 @@ class XMLModuleStore(ModuleStoreBase):
                 filepath = info_path / info_filename + '.html'
                 if os.path.exists(filepath):
                     with open(filepath) as info_file:
-                        html = info_file.read()
+                        html = info_file.read().decode('utf-8')
                         loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, 'course_info', info_filename)
-                        html_module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc})
-                        self.modules[course_id][html_module.location] = html_module
+                        info_module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc})
+                        self.modules[course_id][info_module.location] = info_module
 
+            # now import all static tabs which are expected to be stored in
+            # in <content_dir>/tabs or <content_dir>/tabs/<url_name>
+            if url_name:
+                tabs_path = self.data_dir / course_dir / 'tabs' / url_name
+
+            if not os.path.exists(tabs_path):
+                tabs_path = self.data_dir / course_dir / 'tabs'
+
+            for tab_filepath in glob.glob(tabs_path / '*.html'):
+                with open(tab_filepath) as tab_file:
+                    html = tab_file.read().decode('utf-8')
+                    # tabs are referenced in policy.json through a 'slug' which is just the filename without the .html suffix
+                    slug = os.path.splitext(os.path.basename(tab_filepath))[0]
+                    loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, 'static_tab', slug)
+                    tab_module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc})
+                    self.modules[course_id][tab_module.location] = tab_module               
 
             log.debug('========> Done with course import from {0}'.format(course_dir))
             return course_descriptor

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

@@ -67,7 +67,7 @@ def import_static_content(modules, data_dir, static_content_store):
 
 def import_from_xml(store, data_dir, course_dirs=None, 
                     default_class='xmodule.raw_module.RawDescriptor',
-                    load_error_modules=True, static_content_store=None):
+                    load_error_modules=True, static_content_store=None, target_location_namespace = None):
     """
     Import the specified xml data_dir into the "store" modulestore,
     using org and course as the location org and course.
@@ -75,6 +75,11 @@ def import_from_xml(store, data_dir, course_dirs=None,
     course_dirs: If specified, the list of course_dirs to load. Otherwise, load
     all course dirs
 
+    target_location_namespace is the namespace [passed as Location] (i.e. {tag},{org},{course}) that all modules in the should be remapped to
+    after import off disk. We do this remapping as a post-processing step because there's logic in the importing which
+    expects a 'url_name' as an identifier to where things are on disk e.g. ../policies/<url_name>/policy.json as well as metadata keys in
+    the policy.json. so we need to keep the original url_name during import
+
     """
     
     module_store = XMLModuleStore(
@@ -95,6 +100,27 @@ def import_from_xml(store, data_dir, course_dirs=None,
 
         for module in module_store.modules[course_id].itervalues():
 
+            # remap module to the new namespace
+            if target_location_namespace is not None:
+
+                # This looks a bit wonky as we need to also change the 'name' of the imported course to be what
+                # the caller passed in
+                module.location = Location(target_location_namespace.tag, 
+                    target_location_namespace.org, target_location_namespace.course, module.location.category, 
+                    module.location.name if module.location.category != 'course' else target_location_namespace.name)
+
+                # then remap children pointers since they too will be re-namespaced
+                children_locs = module.definition.get('children')
+                if children_locs is not None:
+                    new_locs = []
+                    for child in children_locs:
+                        child_loc = Location(child)
+                        new_locs.append(Location(target_location_namespace.tag, target_location_namespace.org, 
+                            target_location_namespace.course, child_loc.category, child_loc.name).url())
+
+                    module.definition['children'] = new_locs
+
+
             if module.category == 'course':
                 # HACK: for now we don't support progress tabs. There's a special metadata configuration setting for this.
                 module.metadata['hide_progress_tab'] = True

lms/djangoapps/courseware/courses.py

@@ -10,6 +10,7 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.http import Http404
 
+from module_render import get_module
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
@@ -20,6 +21,8 @@ from static_replace import replace_urls, try_staticfiles_lookup
 from courseware.access import has_access
 import branding
 
+
+
 log = logging.getLogger(__name__)
 
 
@@ -140,27 +143,9 @@ def get_course_about_section(course, section_key):
 
     raise KeyError("Invalid about key " + str(section_key))
 
-def get_course_info_section_from_db(course, section_key):
-    loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key)
-    html = ''
-    try:
-        item = modulestore().get_item(loc)
-        # return the raw HTML here which is stored as part of the definition. If we call get_html here, HTMLModule's parent 
-        # descriptors will try to return an 'editing' rendering of the HTML
-        _html = item.definition['data']  
-        try:   
-            # apply link transforms which are defined in XModule, maybe that should actually be a static method in
-            # Content.py
-            html = rewrite_links(_html, XModule.rewrite_content_links)
-        except:
-            logging.error('error rewriting links on the following HTML content: {0}'.format(_html))
-
-    except Exception, e:
-        logging.exception("Could not find course_info section {0} at {1}: {2}".format(section_key, loc, str(e)))
-    return html
 
 
-def get_course_info_section(course, section_key):
+def get_course_info_section(request, cache, course, section_key):
     """
     This returns the snippet of html to be rendered on the course info page,
     given the key for the section.
@@ -172,34 +157,18 @@ def get_course_info_section(course, section_key):
     - guest_updates
     """
 
-    # Many of these are stored as html files instead of some semantic
-    # markup. This can change without effecting this interface when we find a
-    # good format for defining so many snippets of text/html.
+    loc = Location(course.location.tag, course.location.org, course.location.course, 'course_info', section_key)
+    course_module = get_module(request.user, request, loc, cache, course.id)
 
-    if not isinstance(modulestore(), XMLModuleStore):
-        return get_course_info_section_from_db(course, section_key)
+    logging.debug('course_module = {0}'.format(course_module))
 
-    if section_key in ['handouts', 'guest_handouts', 'updates', 'guest_updates']:
-        try:
-            fs = course.system.resources_fs
-            # first look for a run-specific version
-            dirs = [path("info") / course.url_name, path("info")]
-            filepath = find_file(fs, dirs, section_key + ".html")
+    html = ''
 
-            with fs.open(filepath) as htmlFile:
-                # Replace '/static/' urls
-                info_html = replace_urls(htmlFile.read().decode('utf-8'), course.metadata['data_dir'])
+    if course_module is not None:
+        html = course_module.get_html()
 
-                # Replace '/course/' urls
-                course_root = reverse('course_root', args=[course.id])[:-1] # Remove trailing slash
-                info_html = replace_urls(info_html, course_root, '/course/')
-                return info_html
-        except ResourceNotFoundError:
-            log.exception("Missing info section {key} in course {url}".format(
-                key=section_key, url=course.location.url()))
-            return "! Info section missing !"
+    return html
 
-    raise KeyError("Invalid about key " + str(section_key))
 
 
 # TODO: Fix this such that these are pulled in as extra course-specific tabs.

lms/djangoapps/courseware/module_render.py

@@ -256,7 +256,7 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
 
     module.get_html = replace_static_urls(
         wrap_xmodule(module.get_html, module, 'xmodule_display.html'),
-        module.metadata['data_dir'])
+        module.metadata['data_dir'] if 'data_dir' in module.metadata else '')
 
     # Allow URLs of the form '/course/' refer to the root of multicourse directory
     #   hierarchy of this course

lms/djangoapps/courseware/tabs.py

@@ -17,8 +17,17 @@ from django.core.urlresolvers import reverse
 
 from fs.errors import ResourceNotFoundError
 
+from lxml.html import rewrite_links
+
+from module_render import get_module
 from courseware.access import has_access
 from static_replace import replace_urls
+from xmodule.modulestore import Location
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.xml import XMLModuleStore
+from xmodule.x_module import XModule
+
+
 
 log = logging.getLogger(__name__)
 
@@ -248,27 +257,16 @@ def get_static_tab_by_slug(course, tab_slug):
 
     return None
 
+def get_static_tab_contents(request, cache, course, tab):
 
-def get_static_tab_contents(course, tab):
-    """
-    Given a course and a static tab config dict, load the tab contents,
-    returning None if not found.
+    loc = Location(course.location.tag, course.location.org, course.location.course, 'static_tab', tab['url_slug'])
+    course_module = get_module(request.user, request, loc, cache, course.id)
 
-    Looks in tabs/{course_url_name}/{tab_slug}.html first, then tabs/{tab_slug}.html.
-    """
-    slug = tab['url_slug']
-    paths = ['tabs/{0}/{1}.html'.format(course.url_name, slug),
-             'tabs/{0}.html'.format(slug)]
-    fs = course.system.resources_fs
-    for p in paths:
-        if fs.exists(p):
-            try:
-                with fs.open(p) as tabfile:
-                    # TODO: redundant with module_render.py.  Want to be helper methods in static_replace or something.
-                    text = tabfile.read().decode('utf-8')
-                    contents = replace_urls(text, course.metadata['data_dir'])
-                    return replace_urls(contents, staticfiles_prefix='/courses/'+course.id, replace_prefix='/course/')
-            except (ResourceNotFoundError) as err:
-                log.exception("Couldn't load tab contents from '{0}': {1}".format(p, err))
-                return None
-    return None
+    logging.debug('course_module = {0}'.format(course_module))
+
+    html = ''
+
+    if course_module is not None:
+        html = course_module.get_html()
+
+    return html

lms/djangoapps/courseware/views.py

@@ -347,8 +347,13 @@ def course_info(request, course_id):
     course = get_course_with_access(request.user, course_id, 'load')
     staff_access = has_access(request.user, course, 'staff')
 
-    return render_to_response('courseware/info.html', {'course': course,
-                                            'staff_access': staff_access,})
+
+
+    cache = StudentModuleCache.cache_for_descriptor_descendents(
+            course.id, request.user, course, depth=2)
+
+    return render_to_response('courseware/info.html', {'request' : request, 'course_id' : course_id, 'cache' : cache, 
+            'course': course, 'staff_access': staff_access})
 
 @ensure_csrf_cookie
 def static_tab(request, course_id, tab_slug):
@@ -363,7 +368,10 @@ def static_tab(request, course_id, tab_slug):
     if tab is None:
         raise Http404
 
-    contents = tabs.get_static_tab_contents(course, tab)
+    cache = StudentModuleCache.cache_for_descriptor_descendents(
+            course.id, request.user, course, depth=2)
+
+    contents = tabs.get_static_tab_contents(request, cache, course, tab)
     if contents is None:
         raise Http404
 

lms/templates/courseware/info.html

@@ -27,20 +27,20 @@ $(document).ready(function(){
     % if user.is_authenticated():
     <section class="updates">
       <h1>Course Updates &amp; News</h1>
-      ${get_course_info_section(course, 'updates')}
+      ${get_course_info_section(request, cache, course, 'updates')}
     </section>
     <section aria-label="Handout Navigation" class="handouts">
       <h1>${course.info_sidebar_name}</h1>
-      ${get_course_info_section(course, 'handouts')}
+      ${get_course_info_section(request, cache, course, 'handouts')}
     </section>
     % else:
     <section class="updates">
       <h1>Course Updates &amp; News</h1>
-      ${get_course_info_section(course, 'guest_updates')}
+      ${get_course_info_section(request, cache, course, 'guest_updates')}
     </section>
     <section aria-label="Handout Navigation" class="handouts">
       <h1>Course Handouts</h1>
-      ${get_course_info_section(course, 'guest_handouts')}
+      ${get_course_info_section(request, cache, course, 'guest_handouts')}
     </section>
     % endif
   </div>