From ba157352142ad7d7d904dcad123ba89d0d4b5f25 Mon Sep 17 00:00:00 2001 From: Chris Dodge <cdodge@edx.org> Date: Thu, 25 Oct 2012 14:27:26 -0400 Subject: [PATCH] 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 | 12 +---- common/lib/xmodule/setup.py | 2 + common/lib/xmodule/xmodule/modulestore/xml.py | 26 +++++++-- .../xmodule/modulestore/xml_importer.py | 28 +++++++++- lms/djangoapps/courseware/courses.py | 53 ++++--------------- lms/djangoapps/courseware/module_render.py | 2 +- lms/djangoapps/courseware/tabs.py | 42 +++++++-------- lms/djangoapps/courseware/views.py | 14 +++-- lms/templates/courseware/info.html | 8 +-- 9 files changed, 98 insertions(+), 89 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index eb090d3333d..2cc7cfca0d9 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/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) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index c1237566552..e9e78b57628 100644 --- a/common/lib/xmodule/setup.py +++ b/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", ] } ) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 0206eed242e..ddb437afe28 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 868733a99ce..f27316929bb 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/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 diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 338fcfa42aa..0103e9de004 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/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. diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index b16f1665fee..3c9958b36d9 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/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 diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 980fedb947a..9f826b77f04 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/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 diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index b738b6a0ccb..fc834768e7e 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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 diff --git a/lms/templates/courseware/info.html b/lms/templates/courseware/info.html index a1cab83104e..ff10e645ed7 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -27,20 +27,20 @@ $(document).ready(function(){ % if user.is_authenticated(): <section class="updates"> <h1>Course Updates & 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 & 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> -- GitLab