From fec6c42adc4ddbf8261ec01591e3c9557ae41a53 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 24 Oct 2012 20:55:04 -0400 Subject: [PATCH 01/12] implement importing of course info sections as modules in the course --- common/lib/xmodule/xmodule/modulestore/xml.py | 21 +++++++++++++++ common/lib/xmodule/xmodule/x_module.py | 9 ++++--- lms/djangoapps/courseware/courses.py | 26 +++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 7990bafe7d..0206eed242 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -17,6 +17,8 @@ from xmodule.course_module import CourseDescriptor from xmodule.mako_module import MakoDescriptorSystem from xmodule.x_module import XModuleDescriptor, XMLParsingSystem +from xmodule.html_module import HtmlDescriptor + from . import ModuleStoreBase, Location from .exceptions import ItemNotFoundError @@ -423,6 +425,25 @@ class XMLModuleStore(ModuleStoreBase): # after we have the course descriptor. XModuleDescriptor.compute_inherited_metadata(course_descriptor) + # now import all pieces of course_info which is expected to be stored + # in /info or /info/ + if url_name: + info_path = self.data_dir / course_dir / 'info' / url_name + + if not os.path.exists(info_path): + info_path = self.data_dir / course_dir / 'info' + + # we have a fixed number of .html info files that we expect there + for info_filename in ['handouts', 'guest_handouts', 'updates', 'guest_updates']: + filepath = info_path / info_filename + '.html' + if os.path.exists(filepath): + with open(filepath) as info_file: + html = info_file.read() + 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 + + log.debug('========> Done with course import from {0}'.format(course_dir)) return course_descriptor diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index a53f2db7da..99468946d7 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -323,11 +323,15 @@ class XModule(HTMLSnippet): # cdodge: added to support dynamic substitutions of # links for courseware assets (e.g. images). is passed through from lxml.html parser def rewrite_content_links(self, link): - # see if we start with our format, e.g. 'xasset:' + loc = Location(self.location) + return XModule._rewrite_content_links(loc, link) + + + @staticmethod + def _rewrite_content_links(loc, link): if link.startswith(XASSET_SRCREF_PREFIX): # yes, then parse out the name name = link[len(XASSET_SRCREF_PREFIX):] - loc = Location(self.location) # resolve the reference to our internal 'filepath' which content_loc = StaticContent.compute_location(loc.org, loc.course, name) link = StaticContent.get_url_path_from_location(content_loc) @@ -335,7 +339,6 @@ class XModule(HTMLSnippet): return link - def policy_key(location): """ Get the key for a location in a policy file. (Since the policy file is diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 65a1eee25b..338fcfa42a 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -3,6 +3,8 @@ from fs.errors import ResourceNotFoundError from functools import wraps import logging +from lxml.html import rewrite_links + from path import path from django.conf import settings from django.core.urlresolvers import reverse @@ -11,7 +13,9 @@ from django.http import Http404 from xmodule.course_module import CourseDescriptor from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore +from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.x_module import XModule from static_replace import replace_urls, try_staticfiles_lookup from courseware.access import has_access import branding @@ -136,6 +140,25 @@ 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): """ @@ -153,6 +176,9 @@ def get_course_info_section(course, section_key): # markup. This can change without effecting this interface when we find a # good format for defining so many snippets of text/html. + if not isinstance(modulestore(), XMLModuleStore): + return get_course_info_section_from_db(course, section_key) + if section_key in ['handouts', 'guest_handouts', 'updates', 'guest_updates']: try: fs = course.system.resources_fs From ba157352142ad7d7d904dcad123ba89d0d4b5f25 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Oct 2012 14:27:26 -0400 Subject: [PATCH 02/12] 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 eb090d3333..2cc7cfca0d 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 c123756655..e9e78b5762 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 0206eed242..ddb437afe2 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 /tabs or /tabs/ + 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 868733a99c..f27316929b 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//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 338fcfa42a..0103e9de00 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 b16f1665fe..3c9958b36d 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 980fedb947..9f826b77f0 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 b738b6a0cc..fc834768e7 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 a1cab83104..ff10e645ed 100644 --- a/lms/templates/courseware/info.html +++ b/lms/templates/courseware/info.html @@ -27,20 +27,20 @@ $(document).ready(function(){ % if user.is_authenticated():

Course Updates & News

- ${get_course_info_section(course, 'updates')} + ${get_course_info_section(request, cache, course, 'updates')}

${course.info_sidebar_name}

- ${get_course_info_section(course, 'handouts')} + ${get_course_info_section(request, cache, course, 'handouts')}
% else:

Course Updates & News

- ${get_course_info_section(course, 'guest_updates')} + ${get_course_info_section(request, cache, course, 'guest_updates')}

Course Handouts

- ${get_course_info_section(course, 'guest_handouts')} + ${get_course_info_section(request, cache, course, 'guest_handouts')}
% endif From b7c830a861d6c835c13868ff3e610880fafd6234 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Oct 2012 16:37:31 -0400 Subject: [PATCH 03/12] use existing _replace() method on Location to do the re-namespacing on import --- .../xmodule/xmodule/modulestore/xml_importer.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index f27316929b..bced371001 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -105,9 +105,12 @@ def import_from_xml(store, data_dir, course_dirs=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) + 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) + 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) # then remap children pointers since they too will be re-namespaced children_locs = module.definition.get('children') @@ -115,8 +118,10 @@ def import_from_xml(store, data_dir, course_dirs=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()) + new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, + course=target_location_namespace.course) + + new_locs.append(new_child_loc) module.definition['children'] = new_locs From f4822c23dee665b676b2219bd01f6fef4afabef0 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 30 Oct 2012 11:52:31 -0400 Subject: [PATCH 04/12] lots of tweeks to better support importing of existing courseware --- cms/djangoapps/contentstore/views.py | 9 +- common/djangoapps/mitxmako/shortcuts.py | 2 +- common/djangoapps/static_replace.py | 24 +++- common/djangoapps/xmodule_modifiers.py | 4 +- common/lib/xmodule/setup.py | 2 + .../xmodule/xmodule/contentstore/content.py | 7 ++ common/lib/xmodule/xmodule/course_module.py | 3 +- .../lib/xmodule/xmodule/modulestore/mongo.py | 2 + common/lib/xmodule/xmodule/modulestore/xml.py | 52 ++++----- .../xmodule/modulestore/xml_importer.py | 103 ++++++++++++++---- common/lib/xmodule/xmodule/template_module.py | 20 +++- lms/djangoapps/courseware/courses.py | 26 +++-- lms/djangoapps/courseware/module_render.py | 3 +- lms/djangoapps/courseware/tests/tests.py | 11 +- lms/djangoapps/courseware/views.py | 2 +- 15 files changed, 186 insertions(+), 84 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index be4a157de4..4d1cfed553 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -287,9 +287,13 @@ def edit_unit(request, location): # TODO (cpennington): If we share units between courses, # this will need to change to check permissions correctly so as # to pick the correct parent subsection + + logging.debug('looking for parent of {0}'.format(location)) + containing_subsection_locs = modulestore().get_parent_locations(location) containing_subsection = modulestore().get_item(containing_subsection_locs[0]) + logging.debug('looking for parent of {0}'.format(containing_subsection.location)) containing_section_locs = modulestore().get_parent_locations(containing_subsection.location) containing_section = modulestore().get_item(containing_section_locs[0]) @@ -997,7 +1001,8 @@ def import_course(request, org, course, name): data_root = path(settings.GITHUB_REPO_ROOT) - course_dir = data_root / "{0}-{1}-{2}".format(org, course, name) + course_subdir = "{0}-{1}-{2}".format(org, course, name) + course_dir = data_root / course_subdir if not course_dir.isdir(): os.mkdir(course_dir) @@ -1033,7 +1038,7 @@ 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_dir], 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)) # we can blow this away when we're done importing. shutil.rmtree(course_dir) diff --git a/common/djangoapps/mitxmako/shortcuts.py b/common/djangoapps/mitxmako/shortcuts.py index ba22f2db20..181d3befd5 100644 --- a/common/djangoapps/mitxmako/shortcuts.py +++ b/common/djangoapps/mitxmako/shortcuts.py @@ -42,7 +42,7 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'): context_dictionary.update(context) # fetch and render template template = middleware.lookup[namespace].get_template(template_name) - return template.render(**context_dictionary) + return template.render_unicode(**context_dictionary) def render_to_response(template_name, dictionary, context_instance=None, namespace='main', **kwargs): diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py index 58e2c8da15..ee63e3cf93 100644 --- a/common/djangoapps/static_replace.py +++ b/common/djangoapps/static_replace.py @@ -5,6 +5,10 @@ from staticfiles.storage import staticfiles_storage from staticfiles import finders from django.conf import settings +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.xml import XMLModuleStore +from xmodule.contentstore.content import StaticContent + log = logging.getLogger(__name__) def try_staticfiles_lookup(path): @@ -22,7 +26,7 @@ def try_staticfiles_lookup(path): return url -def replace(static_url, prefix=None): +def replace(static_url, prefix=None, course_namespace=None): if prefix is None: prefix = '' else: @@ -41,13 +45,23 @@ def replace(static_url, prefix=None): return static_url.group(0) else: # don't error if file can't be found - url = try_staticfiles_lookup(prefix + static_url.group('rest')) - return "".join([quote, url, quote]) + # cdodge: to support the change over to Mongo backed content stores, lets + # use the utility functions in StaticContent.py + if static_url.group('prefix') == '/static/' and not isinstance(modulestore(), XMLModuleStore): + if course_namespace is None: + raise BaseException('You must pass in course_namespace when remapping static content urls with MongoDB stores') + url = StaticContent.convert_legacy_static_url(static_url.group('rest'), course_namespace) + else: + url = try_staticfiles_lookup(prefix + static_url.group('rest')) + + new_link = "".join([quote, url, quote]) + return new_link -def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/'): + +def replace_urls(text, staticfiles_prefix=None, replace_prefix='/static/', course_namespace=None): def replace_url(static_url): - return replace(static_url, staticfiles_prefix) + return replace(static_url, staticfiles_prefix, course_namespace = course_namespace) return re.sub(r""" (?x) # flags=re.VERBOSE diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 9473dfe26b..cda3d013cd 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -50,7 +50,7 @@ def replace_course_urls(get_html, course_id): return replace_urls(get_html(), staticfiles_prefix='/courses/'+course_id, replace_prefix='/course/') return _get_html -def replace_static_urls(get_html, prefix): +def replace_static_urls(get_html, prefix, course_namespace=None): """ Updates the supplied module with a new get_html function that wraps the old get_html function and substitutes urls of the form /static/... @@ -59,7 +59,7 @@ def replace_static_urls(get_html, prefix): @wraps(get_html) def _get_html(): - return replace_urls(get_html(), staticfiles_prefix=prefix) + return replace_urls(get_html(), staticfiles_prefix=prefix, course_namespace = course_namespace) return _get_html diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index e9e78b5762..ff5be4d6df 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -37,6 +37,8 @@ setup( "discussion = xmodule.discussion_module:DiscussionDescriptor", "course_info = xmodule.html_module:HtmlDescriptor", "static_tab = xmodule.html_module:HtmlDescriptor", + "custom_tag_template = xmodule.raw_module:RawDescriptor", + "about = xmodule.html_module:HtmlDescriptor" ] } ) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 6dcf70fbe1..a7a76fa242 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -62,6 +62,13 @@ class StaticContent(object): @staticmethod def get_id_from_path(path): return get_id_from_location(get_location_from_path(path)) + + @staticmethod + def convert_legacy_static_url(path, course_namespace): + loc = StaticContent.compute_location(course_namespace.org, course_namespace.course, path) + return StaticContent.get_url_path_from_location(loc) + + class ContentStore(object): diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index ae3c01f639..94ab02cf39 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -202,7 +202,7 @@ class CourseDescriptor(SequenceDescriptor): # Try to load grading policy paths = ['grading_policy.json'] if policy_dir: - paths = [policy_dir + 'grading_policy.json'] + paths + paths = [policy_dir + '/grading_policy.json'] + paths policy = json.loads(cls.read_grading_policy(paths, system)) @@ -394,3 +394,4 @@ class CourseDescriptor(SequenceDescriptor): return self.location.org + diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 164bfd3590..deef3af3bf 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -1,5 +1,6 @@ import pymongo import sys +import logging from bson.son import SON from fs.osfs import OSFS @@ -49,6 +50,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): self.load_item, resources_fs, error_tracker, render_template) self.modulestore = modulestore self.module_data = module_data + self.default_class = default_class def load_item(self, location): diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index ddb437afe2..6794703998 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -427,42 +427,38 @@ class XMLModuleStore(ModuleStoreBase): # now import all pieces of course_info which is expected to be stored # in /info or /info/ - if url_name: - info_path = self.data_dir / course_dir / 'info' / url_name - - if not os.path.exists(info_path): - info_path = self.data_dir / course_dir / 'info' - - # we have a fixed number of .html info files that we expect there - for info_filename in ['handouts', 'guest_handouts', 'updates', 'guest_updates']: - filepath = info_path / info_filename + '.html' - if os.path.exists(filepath): - with open(filepath) as info_file: - html = info_file.read().decode('utf-8') - loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, 'course_info', info_filename) - info_module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc}) - self.modules[course_id][info_module.location] = info_module + self.load_extra_content(system, course_descriptor, 'course_info', self.data_dir / course_dir / 'info', course_dir, url_name) # now import all static tabs which are expected to be stored in - # in /tabs or /tabs/ - if url_name: - tabs_path = self.data_dir / course_dir / 'tabs' / url_name + # in /tabs or /tabs/ + self.load_extra_content(system, course_descriptor, 'static_tab', self.data_dir / course_dir / 'tabs', course_dir, url_name) - if not os.path.exists(tabs_path): - tabs_path = self.data_dir / course_dir / 'tabs' + self.load_extra_content(system, course_descriptor, 'custom_tag_template', self.data_dir / course_dir / 'custom_tags', course_dir, url_name) - 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 + self.load_extra_content(system, course_descriptor, 'about', self.data_dir / course_dir / 'about', course_dir, url_name) log.debug('========> Done with course import from {0}'.format(course_dir)) return course_descriptor + def load_extra_content(self, system, course_descriptor, category, base_dir, course_dir, url_name): + if url_name: + path = base_dir / url_name + + if not os.path.exists(path): + path = base_dir + + for filepath in glob.glob(path/ '*'): + with open(filepath) as f: + try: + html = f.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(filepath))[0] + loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug) + module = HtmlDescriptor(system, definition={'data' : html}, **{'location' : loc}) + module.metadata['data_dir'] = course_dir + self.modules[course_descriptor.id][module.location] = module + except Exception, e: + logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) def get_instance(self, course_id, location, depth=0): """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index bced371001..6a7d44489b 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -1,6 +1,7 @@ import logging import os import mimetypes +from lxml.html import rewrite_links as lxml_rewrite_links from .xml import XMLModuleStore from .exceptions import DuplicateItemError @@ -9,7 +10,7 @@ from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX log = logging.getLogger(__name__) -def import_static_content(modules, data_dir, static_content_store): +def import_static_content(modules, data_dir, static_content_store, target_location_namespace): remap_dict = {} @@ -31,15 +32,13 @@ def import_static_content(modules, data_dir, static_content_store): # now import all static assets static_dir = '{0}/static/'.format(course_data_dir) - logging.debug("Importing static assets in {0}".format(static_dir)) - for dirname, dirnames, filenames in os.walk(static_dir): for filename in filenames: try: content_path = os.path.join(dirname, filename) fullname_with_subpath = content_path.replace(static_dir, '') # strip away leading path from the name - content_loc = StaticContent.compute_location(course_loc.org, course_loc.course, fullname_with_subpath) + content_loc = StaticContent.compute_location(target_location_namespace.org, target_location_namespace.course, fullname_with_subpath) mime_type = mimetypes.guess_type(filename)[0] f = open(content_path, 'rb') @@ -59,12 +58,50 @@ def import_static_content(modules, data_dir, static_content_store): #store the remapping information which will be needed to subsitute in the module data remap_dict[fullname_with_subpath] = content_loc.name - except: - raise + raise return remap_dict +def verify_content_links(module, base_dir, static_content_store, link, remap_dict = None): + if link.startswith('/static/'): + # yes, then parse out the name + path = link[len('/static/'):] + + static_pathname = base_dir / path + + if os.path.exists(static_pathname): + try: + content_loc = StaticContent.compute_location(module.location.org, module.location.course, path) + filename = os.path.basename(path) + mime_type = mimetypes.guess_type(filename)[0] + + f = open(static_pathname, 'rb') + data = f.read() + f.close() + + content = StaticContent(content_loc, filename, mime_type, data) + + # first let's save a thumbnail so we can get back a thumbnail location + thumbnail_content = static_content_store.generate_thumbnail(content) + + if thumbnail_content is not None: + content.thumbnail_location = thumbnail_content.location + + #then commit the content + static_content_store.save(content) + + new_link = StaticContent.get_url_path_from_location(content_loc) + + if remap_dict is not None: + remap_dict[link] = new_link + + return new_link + except Exception, e: + logging.exception('Skipping failed content load from {0}. Exception: {1}'.format(path, e)) + + return link + 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): @@ -94,15 +131,20 @@ def import_from_xml(store, data_dir, course_dirs=None, # method on XmlModuleStore. course_items = [] for course_id in module_store.modules.keys(): - remap_dict = {} + + course_data_dir = None + for module in module_store.modules[course_id].itervalues(): + if module.category == 'course': + course_data_dir = module.metadata['data_dir'] + if static_content_store is not None: - remap_dict = import_static_content(module_store.modules[course_id], data_dir, static_content_store) + import_static_content(module_store.modules[course_id], data_dir, static_content_store, + target_location_namespace if target_location_namespace is not None else module_store.modules[course_id].location) 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 if module.location.category != 'course': @@ -120,8 +162,8 @@ def import_from_xml(store, data_dir, course_dirs=None, 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) - - new_locs.append(new_child_loc) + + new_locs.append(new_child_loc.url()) module.definition['children'] = new_locs @@ -129,22 +171,37 @@ def import_from_xml(store, data_dir, course_dirs=None, 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 + + # 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 + verify_content_links(module, data_dir / course_data_dir, static_content_store, '/static/images/course_image.jpg') + course_items.append(module) if 'data' in module.definition: module_data = module.definition['data'] - # cdodge: update any references to the static content paths - # This is a bit brute force - simple search/replace - but it's unlikely that such references to '/static/....' - # would occur naturally (in the wild) - # @TODO, sorry a bit of technical debt here. There are some helper methods in xmodule_modifiers.py and static_replace.py which could - # better do the url replace on the html rendering side rather than on the ingest side - try: - if '/static/' in module_data: - for subkey in remap_dict.keys(): - module_data = module_data.replace('/static/' + subkey, 'xasset:' + remap_dict[subkey]) - except: - pass # part of the techincal debt is that module_data might not be a string (e.g. ABTest) + # 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 -> + # 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, data_dir / course_data_dir, + 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, module_data) @@ -156,4 +213,6 @@ def import_from_xml(store, data_dir, course_dirs=None, # inherited metadata everywhere. store.update_metadata(module.location, dict(module.own_metadata)) + + return module_store, course_items diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 4df1c4ee3a..cca2cb0ca8 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -2,6 +2,7 @@ from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from lxml import etree from mako.template import Template +from xmodule.modulestore.django import modulestore class CustomTagModule(XModule): @@ -40,8 +41,7 @@ class CustomTagDescriptor(RawDescriptor): module_class = CustomTagModule template_dir_name = 'customtag' - @staticmethod - def render_template(system, xml_data): + def render_template(self, system, xml_data): '''Render the template, given the definition xml_data''' xmltree = etree.fromstring(xml_data) if 'impl' in xmltree.attrib: @@ -57,15 +57,23 @@ class CustomTagDescriptor(RawDescriptor): .format(location)) params = dict(xmltree.items()) - with system.resources_fs.open('custom_tags/{name}' - .format(name=template_name)) as template: - return Template(template.read()).render(**params) + + # cdodge: look up the template as a module + template_loc = self.location._replace(category='custom_tag_template', name=template_name) + + template_module = modulestore().get_item(template_loc) + template_module_data = template_module.definition['data'] + template = Template(template_module_data) + return template.render(**params) def __init__(self, system, definition, **kwargs): '''Render and save the template for this descriptor instance''' super(CustomTagDescriptor, self).__init__(system, definition, **kwargs) - self.rendered_html = self.render_template(system, definition['data']) + + @property + def rendered_html(self): + return self.render_template(self.system, self.definition['data']) def export_to_file(self): """ diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 0103e9de00..c4dc6fa77b 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -14,6 +14,7 @@ from module_render import get_module from xmodule.course_module import CourseDescriptor from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore +from xmodule.contentstore.content import StaticContent from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.x_module import XModule @@ -68,8 +69,13 @@ def get_opt_course_with_access(user, course_id, action): def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" - path = course.metadata['data_dir'] + "/images/course_image.jpg" - return try_staticfiles_lookup(path) + if isinstance(modulestore(), XMLModuleStore): + path = course.metadata['data_dir'] + "/images/course_image.jpg" + return try_staticfiles_lookup(path) + else: + loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg') + path = StaticContent.get_url_path_from_location(loc) + return path def find_file(fs, dirs, filename): """ @@ -123,14 +129,12 @@ def get_course_about_section(course, section_key): 'effort', 'end_date', 'prerequisites', 'ocw_links']: try: - fs = course.system.resources_fs - # first look for a run-specific version - dirs = [path("about") / course.url_name, path("about")] - filepath = find_file(fs, dirs, section_key + ".html") - with fs.open(filepath) as htmlFile: - return replace_urls(htmlFile.read().decode('utf-8'), - course.metadata['data_dir']) - except ResourceNotFoundError: + loc = course.location._replace(category='about', name=section_key) + item = modulestore().get_instance(course.id, loc) + + return item.definition['data'] + + except ItemNotFoundError: log.warning("Missing about section {key} in course {url}".format( key=section_key, url=course.location.url())) return None @@ -160,8 +164,6 @@ def get_course_info_section(request, cache, course, section_key): 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) - logging.debug('course_module = {0}'.format(course_module)) - html = '' if course_module is not None: diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 3c9958b36d..d9f87d77b6 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -256,7 +256,8 @@ 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'] if 'data_dir' in module.metadata else '') + module.metadata['data_dir'] if 'data_dir' in module.metadata else '', + course_namespace = module.location._replace(category=None, name=None)) # Allow URLs of the form '/course/' refer to the root of multicourse directory # hierarchy of this course diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 3de344b156..8bdcf59815 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -247,6 +247,7 @@ class PageLoader(ActivateLoginTestCase): all_ok = True for descriptor in modstore.get_items( Location(None, None, None, None, None)): + n += 1 print "Checking ", descriptor.location.url() #print descriptor.__class__, descriptor.location @@ -256,9 +257,13 @@ class PageLoader(ActivateLoginTestCase): msg = str(resp.status_code) if resp.status_code != 302: - msg = "ERROR " + msg - all_ok = False - num_bad += 1 + # cdodge: we're adding new module category as part of the Studio work + # such as 'course_info', etc, which are not supposed to be jump_to'able + # so a successful return value here should be a 404 + if descriptor.location.category not in ['about', 'static_tab', 'course_info', 'custom_tag_template'] or resp.status_code != 404: + msg = "ERROR " + msg + all_ok = False + num_bad += 1 print msg self.assertTrue(all_ok) # fail fast diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index fc834768e7..bacc41fdf4 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -421,7 +421,7 @@ def course_about(request, course_id): settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION')) return render_to_response('portal/course_about.html', - {'course': course, + { 'course': course, 'registered': registered, 'course_target': course_target, 'show_courseware_link' : show_courseware_link}) From 743f2b56dd1d3c1c80ce239be8539338ed1967d7 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 30 Oct 2012 15:12:13 -0400 Subject: [PATCH 05/12] make course about view methods render the about content as a module, so we get all the url rewriting goodness. Also, since we're now handling the url re-writing via the module get_html pipelines, we can remove the link rewriting inside the xmodules itself - which is good because there's a wierd bug in lxml.html rewriting --- common/djangoapps/xmodule_modifiers.py | 1 + common/lib/xmodule/xmodule/capa_module.py | 12 ------- common/lib/xmodule/xmodule/html_module.py | 10 +----- .../xmodule/modulestore/xml_importer.py | 4 +-- common/lib/xmodule/xmodule/x_module.py | 18 ---------- lms/djangoapps/courseware/courses.py | 34 +++++++++++++++++-- lms/djangoapps/courseware/module_render.py | 2 ++ 7 files changed, 37 insertions(+), 44 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index cda3d013cd..431181bfac 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -59,6 +59,7 @@ def replace_static_urls(get_html, prefix, course_namespace=None): @wraps(get_html) def _get_html(): + logging.debug('in replace_static_urls') return replace_urls(get_html(), staticfiles_prefix=prefix, course_namespace = course_namespace) return _get_html diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index d75e0ff860..587fc09eed 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -10,7 +10,6 @@ import sys from datetime import timedelta from lxml import etree -from lxml.html import rewrite_links from pkg_resources import resource_string from capa.capa_problem import LoncapaProblem @@ -342,17 +341,6 @@ class CapaModule(XModule): html = '
'.format( id=self.location.html_id(), ajax_url=self.system.ajax_url) + html + "
" - # cdodge: OK, we have to do two rounds of url reference subsitutions - # one which uses the 'asset library' that is served by the contentstore and the - # more global /static/ filesystem based static content. - # NOTE: rewrite_content_links is defined in XModule - # This is a bit unfortunate and I'm sure we'll try to considate this into - # a one step process. - try: - html = rewrite_links(html, self.rewrite_content_links) - except: - logging.error('error rewriting links in {0}'.format(html)) - # now do the substitutions which are filesystem based, e.g. '/static/' prefixes return self.system.replace_urls(html, self.metadata['data_dir']) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 00577912c8..f6dddfdd4c 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -4,7 +4,6 @@ import logging import os import sys from lxml import etree -from lxml.html import rewrite_links from path import path from .x_module import XModule @@ -29,14 +28,7 @@ class HtmlModule(XModule): js_module_name = "HTMLModule" def get_html(self): - # cdodge: perform link substitutions for any references to course static content (e.g. images) - _html = self.html - try: - _html = rewrite_links(_html, self.rewrite_content_links) - except: - logging.error('error rewriting links on the following HTML content: {0}'.format(_html)) - - return _html + return self.html def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 6a7d44489b..3c94e25aa2 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -30,7 +30,7 @@ def import_static_content(modules, data_dir, static_content_store, target_locati # now import all static assets - static_dir = '{0}/static/'.format(course_data_dir) + static_dir = '{0}/static/'.format(data_dir / course_data_dir) for dirname, dirnames, filenames in os.walk(static_dir): for filename in filenames: @@ -185,7 +185,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # 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 diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 99468946d7..2174d28112 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -320,24 +320,6 @@ class XModule(HTMLSnippet): get is a dictionary-like object ''' return "" - # cdodge: added to support dynamic substitutions of - # links for courseware assets (e.g. images). is passed through from lxml.html parser - def rewrite_content_links(self, link): - loc = Location(self.location) - return XModule._rewrite_content_links(loc, link) - - - @staticmethod - def _rewrite_content_links(loc, link): - if link.startswith(XASSET_SRCREF_PREFIX): - # yes, then parse out the name - name = link[len(XASSET_SRCREF_PREFIX):] - # resolve the reference to our internal 'filepath' which - content_loc = StaticContent.compute_location(loc.org, loc.course, name) - link = StaticContent.get_url_path_from_location(content_loc) - - return link - def policy_key(location): """ diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index c4dc6fa77b..4b020f3ead 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -2,6 +2,7 @@ from collections import defaultdict from fs.errors import ResourceNotFoundError from functools import wraps import logging +import inspect from lxml.html import rewrite_links @@ -21,11 +22,24 @@ from xmodule.x_module import XModule from static_replace import replace_urls, try_staticfiles_lookup from courseware.access import has_access import branding - - +from courseware.models import StudentModuleCache log = logging.getLogger(__name__) +def get_request_for_thread(): + """Walk up the stack, return the nearest first argument named "request".""" + frame = None + try: + for f in inspect.stack()[1:]: + frame = f[0] + code = frame.f_code + if code.co_varnames[:1] == ("request",): + return frame.f_locals["request"] + elif code.co_varnames[:2] == ("self", "request",): + return frame.f_locals["request"] + finally: + del frame + def get_course_by_id(course_id): """ @@ -129,10 +143,23 @@ def get_course_about_section(course, section_key): 'effort', 'end_date', 'prerequisites', 'ocw_links']: try: + + request = get_request_for_thread() + + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + course.id, request.user, course, depth=2) + loc = course.location._replace(category='about', name=section_key) + course_module = get_module(request.user, request, loc, student_module_cache, course.id) + + html = '' + + if course_module is not None: + html = course_module.get_html() + item = modulestore().get_instance(course.id, loc) - return item.definition['data'] + return html except ItemNotFoundError: log.warning("Missing about section {key} in course {url}".format( @@ -161,6 +188,7 @@ def get_course_info_section(request, cache, course, section_key): - guest_updates """ + 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) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index d9f87d77b6..0e3b3a326c 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -259,6 +259,8 @@ def _get_module(user, request, location, student_module_cache, course_id, positi module.metadata['data_dir'] if 'data_dir' in module.metadata else '', course_namespace = module.location._replace(category=None, name=None)) + logging.debug('in get_module') + # Allow URLs of the form '/course/' refer to the root of multicourse directory # hierarchy of this course module.get_html = replace_course_urls(module.get_html, course_id) From ee1098c03247aa6b7ba32283b95d81e4714320e3 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 30 Oct 2012 15:37:40 -0400 Subject: [PATCH 06/12] remove unused line --- lms/djangoapps/courseware/courses.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 4b020f3ead..ca84fb3c65 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -157,8 +157,6 @@ def get_course_about_section(course, section_key): if course_module is not None: html = course_module.get_html() - item = modulestore().get_instance(course.id, loc) - return html except ItemNotFoundError: From ba9a03410b39017005c0e31c57641139f2f81546 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 30 Oct 2012 15:56:02 -0400 Subject: [PATCH 07/12] remove unneeded debugging traces --- cms/djangoapps/contentstore/views.py | 3 --- common/lib/xmodule/xmodule/modulestore/mongo.py | 1 - lms/djangoapps/courseware/module_render.py | 2 -- 3 files changed, 6 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 4d1cfed553..b3af6f7e7b 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -288,12 +288,9 @@ def edit_unit(request, location): # this will need to change to check permissions correctly so as # to pick the correct parent subsection - logging.debug('looking for parent of {0}'.format(location)) - containing_subsection_locs = modulestore().get_parent_locations(location) containing_subsection = modulestore().get_item(containing_subsection_locs[0]) - logging.debug('looking for parent of {0}'.format(containing_subsection.location)) containing_section_locs = modulestore().get_parent_locations(containing_subsection.location) containing_section = modulestore().get_item(containing_section_locs[0]) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index deef3af3bf..550e6570ac 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -50,7 +50,6 @@ class CachingDescriptorSystem(MakoDescriptorSystem): self.load_item, resources_fs, error_tracker, render_template) self.modulestore = modulestore self.module_data = module_data - self.default_class = default_class def load_item(self, location): diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 0e3b3a326c..d9f87d77b6 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -259,8 +259,6 @@ def _get_module(user, request, location, student_module_cache, course_id, positi module.metadata['data_dir'] if 'data_dir' in module.metadata else '', course_namespace = module.location._replace(category=None, name=None)) - logging.debug('in get_module') - # Allow URLs of the form '/course/' refer to the root of multicourse directory # hierarchy of this course module.get_html = replace_course_urls(module.get_html, course_id) From f0b4371c90d812ad62f686fa0dc788f4a2b1f28e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 30 Oct 2012 16:01:28 -0400 Subject: [PATCH 08/12] addressed Cale's feedback --- lms/djangoapps/courseware/courses.py | 2 +- lms/djangoapps/courseware/tabs.py | 8 ++++---- lms/djangoapps/courseware/views.py | 7 +------ 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index ca84fb3c65..1d88c4e5a3 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -150,7 +150,7 @@ def get_course_about_section(course, section_key): course.id, request.user, course, depth=2) loc = course.location._replace(category='about', name=section_key) - course_module = get_module(request.user, request, loc, student_module_cache, course.id) + course_module = get_module(request.user, request, loc, None, course.id) html = '' diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 9f826b77f0..e926a0b114 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -260,13 +260,13 @@ def get_static_tab_by_slug(course, tab_slug): def get_static_tab_contents(request, cache, course, tab): 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) + tab_module = get_module(request.user, request, loc, cache, course.id) - logging.debug('course_module = {0}'.format(course_module)) + logging.debug('course_module = {0}'.format(tab_module)) html = '' - if course_module is not None: - html = course_module.get_html() + if tab_module is not None: + html = tab_module.get_html() return html diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index bacc41fdf4..14bec50af3 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -347,12 +347,7 @@ def course_info(request, course_id): course = get_course_with_access(request.user, course_id, 'load') staff_access = has_access(request.user, course, 'staff') - - - 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, + return render_to_response('courseware/info.html', {'request' : request, 'course_id' : course_id, 'cache' : None, 'course': course, 'staff_access': staff_access}) @ensure_csrf_cookie From 34c442a1dcd40e4947b775d1de39d2f098bd50ce Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 30 Oct 2012 16:04:51 -0400 Subject: [PATCH 09/12] removed unneeded trace log. Also don't need StudentModuleCache in 'about' section rendering --- common/djangoapps/xmodule_modifiers.py | 1 - lms/djangoapps/courseware/courses.py | 3 --- 2 files changed, 4 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 431181bfac..cda3d013cd 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -59,7 +59,6 @@ def replace_static_urls(get_html, prefix, course_namespace=None): @wraps(get_html) def _get_html(): - logging.debug('in replace_static_urls') return replace_urls(get_html(), staticfiles_prefix=prefix, course_namespace = course_namespace) return _get_html diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 1d88c4e5a3..aa3fbf12bb 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -146,9 +146,6 @@ def get_course_about_section(course, section_key): request = get_request_for_thread() - student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( - course.id, request.user, course, depth=2) - loc = course.location._replace(category='about', name=section_key) course_module = get_module(request.user, request, loc, None, course.id) From 87dadd40c400066dcc8973a8f733e1bb6afc217b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 31 Oct 2012 14:11:24 -0400 Subject: [PATCH 10/12] address some of Cale's feedback --- common/lib/xmodule/xmodule/modulestore/__init__.py | 1 + common/lib/xmodule/xmodule/modulestore/xml_importer.py | 10 ++++------ lms/djangoapps/courseware/views.py | 5 +---- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index fcbc2d0fbd..55236b4f8e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -377,6 +377,7 @@ class ModuleStore(object): return courses + class ModuleStoreBase(ModuleStore): ''' Implement interface functionality that can be shared. diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 3c94e25aa2..2677efdb98 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -41,9 +41,8 @@ def import_static_content(modules, data_dir, static_content_store, target_locati content_loc = StaticContent.compute_location(target_location_namespace.org, target_location_namespace.course, fullname_with_subpath) mime_type = mimetypes.guess_type(filename)[0] - f = open(content_path, 'rb') - data = f.read() - f.close() + with open(content_path, 'rb') as f: + data = f.read() content = StaticContent(content_loc, filename, mime_type, data) @@ -76,9 +75,8 @@ def verify_content_links(module, base_dir, static_content_store, link, remap_dic filename = os.path.basename(path) mime_type = mimetypes.guess_type(filename)[0] - f = open(static_pathname, 'rb') - data = f.read() - f.close() + with open(static_pathname, 'rb') as f: + data = f.read() content = StaticContent(content_loc, filename, mime_type, data) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 14bec50af3..c8ac714084 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -363,10 +363,7 @@ def static_tab(request, course_id, tab_slug): if tab is None: raise Http404 - cache = StudentModuleCache.cache_for_descriptor_descendents( - course.id, request.user, course, depth=2) - - contents = tabs.get_static_tab_contents(request, cache, course, tab) + contents = tabs.get_static_tab_contents(request, None, course, tab) if contents is None: raise Http404 From 17528906abd84e0bdb8aac10f0da3f7e55d92b2a Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 31 Oct 2012 22:16:11 -0400 Subject: [PATCH 11/12] make sure paths are path() objects rather than strings so that the '/' operator works as expected. Also some path cleanup and remove duplicate computation --- .../xmodule/modulestore/xml_importer.py | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 2677efdb98..00ddb6a948 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -2,6 +2,7 @@ import logging import os import mimetypes from lxml.html import rewrite_links as lxml_rewrite_links +from path import path from .xml import XMLModuleStore from .exceptions import DuplicateItemError @@ -10,27 +11,12 @@ from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX log = logging.getLogger(__name__) -def import_static_content(modules, data_dir, static_content_store, target_location_namespace): +def import_static_content(modules, course_loc, course_data_path, static_content_store, target_location_namespace): remap_dict = {} - - course_data_dir = None - course_loc = None - - # quick scan to find the course module and pull out the data_dir and location - # maybe there an easier way to look this up?!? - - for module in modules.itervalues(): - if module.category == 'course': - course_loc = module.location - course_data_dir = module.metadata['data_dir'] - - if course_data_dir is None or course_loc is None: - return remap_dict - # now import all static assets - static_dir = '{0}/static/'.format(data_dir / course_data_dir) + static_dir = course_data_path / 'static' for dirname, dirnames, filenames in os.walk(static_dir): for filename in filenames: @@ -130,13 +116,16 @@ def import_from_xml(store, data_dir, course_dirs=None, course_items = [] for course_id in module_store.modules.keys(): - course_data_dir = None + course_data_path = None + course_location = None + # Quick scan to get course Location as well as the course_data_path for module in module_store.modules[course_id].itervalues(): if module.category == 'course': - course_data_dir = module.metadata['data_dir'] + course_data_path = path(data_dir) / module.metadata['data_dir'] + course_location = module.location if static_content_store is not None: - import_static_content(module_store.modules[course_id], data_dir, static_content_store, + import_static_content(module_store.modules[course_id], course_location, course_data_path, static_content_store, target_location_namespace if target_location_namespace is not None else module_store.modules[course_id].location) for module in module_store.modules[course_id].itervalues(): @@ -172,7 +161,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # 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 - verify_content_links(module, data_dir / course_data_dir, static_content_store, '/static/images/course_image.jpg') + verify_content_links(module, course_data_path, static_content_store, '/static/images/course_image.jpg') course_items.append(module) @@ -192,7 +181,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # 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, data_dir / course_data_dir, + 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(): From b788b9d6591653f5f9f455dad7d946f2739fc18a Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 1 Nov 2012 15:08:26 -0400 Subject: [PATCH 12/12] add to existing test cases to exercise the 'course extras as modules' work in the CMS import. Also add to the existing 'full' test data collection to include policy, tabs, etc. --- common/lib/xmodule/xmodule/xml_module.py | 7 ++- common/test/data/full/grading_policy.js | 22 +++++++++ .../data/full/policies/6.002_Spring_2012.json | 16 +++++++ common/test/data/full/tabs/syllabus.html | 1 + lms/djangoapps/courseware/courses.py | 3 +- lms/djangoapps/courseware/module_render.py | 8 +++- lms/djangoapps/courseware/tests/tests.py | 47 +++++++++++++++---- 7 files changed, 90 insertions(+), 14 deletions(-) create mode 100644 common/test/data/full/grading_policy.js create mode 100644 common/test/data/full/policies/6.002_Spring_2012.json create mode 100644 common/test/data/full/tabs/syllabus.html diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index a5b3460f17..24bbe40c13 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -93,7 +93,9 @@ class XmlDescriptor(XModuleDescriptor): # VS[compat] Remove once unused. 'name', 'slug') - metadata_to_strip = ('data_dir', + 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', # VS[compat] -- remove the below attrs once everything is in the CMS 'course', 'org', 'url_name', 'filename') @@ -355,7 +357,8 @@ class XmlDescriptor(XModuleDescriptor): for attr in sorted(self.own_metadata): # don't want e.g. data_dir if attr not in self.metadata_to_strip: - xml_object.set(attr, val_for_xml(attr)) + val = val_for_xml(attr) + xml_object.set(attr, val) if self.export_to_file(): # Write the definition to a file diff --git a/common/test/data/full/grading_policy.js b/common/test/data/full/grading_policy.js new file mode 100644 index 0000000000..e48627e711 --- /dev/null +++ b/common/test/data/full/grading_policy.js @@ -0,0 +1,22 @@ +{ + "GRADER" : [ + { + "type" : "Homework", + "min_count" : 3, + "drop_count" : 1, + "short_label" : "HW", + "weight" : 0.5 + }, + { + "type" : "Final", + "name" : "Final Question", + "short_label" : "Final", + "weight" : 0.5 + } + ], + "GRADE_CUTOFFS" : { + "A" : 0.8, + "B" : 0.7, + "C" : 0.44 + } +} diff --git a/common/test/data/full/policies/6.002_Spring_2012.json b/common/test/data/full/policies/6.002_Spring_2012.json new file mode 100644 index 0000000000..345309ff5c --- /dev/null +++ b/common/test/data/full/policies/6.002_Spring_2012.json @@ -0,0 +1,16 @@ +{ + "course/6.002_Spring_2012": { + "graceperiod": "1 day 12 hours 59 minutes 59 seconds", + "start": "2012-09-21T12:00", + "display_name": "Testing", + "xqa_key": "5HapHs6tHhu1iN1ZX5JGNYKRkXrXh7NC", + "tabs": [ + {"type": "courseware"}, + {"type": "course_info", "name": "Course Info"}, + {"type": "static_tab", "url_slug": "syllabus", "name": "Syllabus"}, + {"type": "discussion", "name": "Discussion"}, + {"type": "wiki", "name": "Wiki"}, + {"type": "progress", "name": "Progress"} + ] + } +} \ No newline at end of file diff --git a/common/test/data/full/tabs/syllabus.html b/common/test/data/full/tabs/syllabus.html new file mode 100644 index 0000000000..72241b39b3 --- /dev/null +++ b/common/test/data/full/tabs/syllabus.html @@ -0,0 +1 @@ +

This is a sample tab

\ No newline at end of file diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index aa3fbf12bb..6300b2c491 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -23,6 +23,7 @@ from static_replace import replace_urls, try_staticfiles_lookup from courseware.access import has_access import branding from courseware.models import StudentModuleCache +from xmodule.modulestore.exceptions import ItemNotFoundError log = logging.getLogger(__name__) @@ -147,7 +148,7 @@ def get_course_about_section(course, section_key): request = get_request_for_thread() loc = course.location._replace(category='about', name=section_key) - course_module = get_module(request.user, request, loc, None, course.id) + course_module = get_module(request.user, request, loc, None, course.id, not_found_ok = True) html = '' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index d9f87d77b6..5e79226a21 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -28,6 +28,8 @@ from xmodule.x_module import ModuleSystem from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule +from xmodule.modulestore.exceptions import ItemNotFoundError + log = logging.getLogger("mitx.courseware") @@ -112,7 +114,7 @@ def toc_for_course(user, request, course, active_chapter, active_section): return chapters -def get_module(user, request, location, student_module_cache, course_id, position=None): +def get_module(user, request, location, student_module_cache, course_id, position=None, not_found_ok = False): """ Get an instance of the xmodule class identified by location, setting the state based on an existing StudentModule, or creating one if none @@ -134,6 +136,10 @@ def get_module(user, request, location, student_module_cache, course_id, positio """ try: return _get_module(user, request, location, student_module_cache, course_id, position) + except ItemNotFoundError: + if not not_found_ok: + log.exception("Error in get_module") + return None except: # Something has gone terribly wrong, but still not letting it turn into a 500. log.exception("Error in get_module") diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 8bdcf59815..5af79d4983 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -250,20 +250,47 @@ class PageLoader(ActivateLoginTestCase): n += 1 print "Checking ", descriptor.location.url() - #print descriptor.__class__, descriptor.location - resp = self.client.get(reverse('jump_to', - kwargs={'course_id': course_id, - 'location': descriptor.location.url()})) - msg = str(resp.status_code) - if resp.status_code != 302: - # cdodge: we're adding new module category as part of the Studio work - # such as 'course_info', etc, which are not supposed to be jump_to'able - # so a successful return value here should be a 404 - if descriptor.location.category not in ['about', 'static_tab', 'course_info', 'custom_tag_template'] or resp.status_code != 404: + # We have ancillary course information now as modules and we can't simply use 'jump_to' to view them + if descriptor.location.category == 'about': + resp = self.client.get(reverse('about_course', kwargs={'course_id': course_id})) + msg = str(resp.status_code) + + if resp.status_code != 200: msg = "ERROR " + msg all_ok = False num_bad += 1 + elif descriptor.location.category == 'static_tab': + resp = self.client.get(reverse('static_tab', kwargs={'course_id': course_id, 'tab_slug' : descriptor.location.name})) + msg = str(resp.status_code) + + if resp.status_code != 200: + msg = "ERROR " + msg + all_ok = False + num_bad += 1 + elif descriptor.location.category == 'course_info': + resp = self.client.get(reverse('info', kwargs={'course_id': course_id})) + msg = str(resp.status_code) + + if resp.status_code != 200: + msg = "ERROR " + msg + all_ok = False + num_bad += 1 + else: + #print descriptor.__class__, descriptor.location + resp = self.client.get(reverse('jump_to', + kwargs={'course_id': course_id, + 'location': descriptor.location.url()})) + msg = str(resp.status_code) + + if resp.status_code != 302: + # cdodge: we're adding 'custom_tag_template' which is the Mako template used to render + # the custom tag. We can't 'jump-to' this module. Unfortunately, we also can't test render + # it easily + if descriptor.location.category not in ['custom_tag_template'] or resp.status_code != 404: + msg = "ERROR " + msg + all_ok = False + num_bad += 1 print msg self.assertTrue(all_ok) # fail fast