diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 05cde6043d..b3af6f7e7b 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -287,6 +287,7 @@ 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 + containing_subsection_locs = modulestore().get_parent_locations(location) containing_subsection = modulestore().get_item(containing_subsection_locs[0]) @@ -997,7 +998,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) @@ -1032,18 +1034,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_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 0441357427..ba5bcd872f 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -35,6 +35,10 @@ 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", + "custom_tag_template = xmodule.raw_module:RawDescriptor", + "about = xmodule.html_module:HtmlDescriptor" ] } ) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index f7e93a3262..28ae472e3e 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 @@ -345,17 +344,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/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 e8b7d5c1ab..6e3f450324 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -421,3 +421,4 @@ class CourseDescriptor(SequenceDescriptor): return self.location.org + 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/__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/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 164bfd3590..550e6570ac 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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 7990bafe7d..6794703998 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 @@ -17,6 +18,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 @@ -331,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: @@ -386,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. @@ -403,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, @@ -423,9 +425,40 @@ 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/ + 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/ + self.load_extra_content(system, course_descriptor, 'static_tab', self.data_dir / course_dir / 'tabs', course_dir, url_name) + + self.load_extra_content(system, course_descriptor, 'custom_tag_template', self.data_dir / course_dir / 'custom_tags', course_dir, url_name) + + 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 868733a99c..00ddb6a948 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -1,6 +1,8 @@ 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 @@ -9,29 +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): +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(course_data_dir) - - logging.debug("Importing static assets in {0}".format(static_dir)) + static_dir = course_data_path / 'static' for dirname, dirnames, filenames in os.walk(static_dir): for filename in filenames: @@ -39,12 +24,11 @@ def import_static_content(modules, data_dir, static_content_store): 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') - data = f.read() - f.close() + with open(content_path, 'rb') as f: + data = f.read() content = StaticContent(content_loc, filename, mime_type, data) @@ -59,15 +43,52 @@ 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] + + with open(static_pathname, 'rb') as f: + data = f.read() + + 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): + 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 +96,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( @@ -89,31 +115,80 @@ 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_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_path = path(data_dir) / module.metadata['data_dir'] + course_location = module.location + 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], 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(): + # 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': + 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') + if children_locs is not None: + new_locs = [] + for child in children_locs: + child_loc = Location(child) + new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org, + course=target_location_namespace.course) + + new_locs.append(new_child_loc.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 + + # 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, course_data_path, 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, course_data_path, + static_content_store, link, remap_dict)) + + for key in remap_dict.keys(): + module_data = module_data.replace(key, remap_dict[key]) + + except Exception, e: + logging.exception("failed to rewrite links on {0}. Continuing...".format(module.location)) store.update_item(module.location, module_data) @@ -125,4 +200,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/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index a53f2db7da..2174d28112 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -320,21 +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): - # see if we start with our format, e.g. 'xasset:' - 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) - - return link - - def policy_key(location): """ diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index ec755af4ef..c56803f3c4 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -96,7 +96,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') @@ -358,7 +360,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 65a1eee25b..6300b2c491 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -2,22 +2,45 @@ from collections import defaultdict from fs.errors import ResourceNotFoundError from functools import wraps import logging +import inspect + +from lxml.html import rewrite_links from path import path 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 +from xmodule.contentstore.content import StaticContent +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 +from courseware.models import StudentModuleCache +from xmodule.modulestore.exceptions import ItemNotFoundError 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): """ @@ -61,8 +84,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): """ @@ -116,14 +144,20 @@ 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: + + 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, not_found_ok = True) + + html = '' + + if course_module is not None: + html = course_module.get_html() + + return html + + except ItemNotFoundError: log.warning("Missing about section {key} in course {url}".format( key=section_key, url=course.location.url())) return None @@ -137,7 +171,8 @@ def get_course_about_section(course, section_key): raise KeyError("Invalid about key " + str(section_key)) -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. @@ -149,31 +184,17 @@ 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. - 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") + 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) - with fs.open(filepath) as htmlFile: - # Replace '/static/' urls - info_html = replace_urls(htmlFile.read().decode('utf-8'), course.metadata['data_dir']) + 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 !" + if course_module is not None: + html = course_module.get_html() + + 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 98c46640a4..3880170169 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -28,6 +28,7 @@ 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 from statsd import statsd log = logging.getLogger("mitx.courseware") @@ -114,7 +115,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 @@ -136,6 +137,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") @@ -258,7 +263,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']) + 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/tabs.py b/lms/djangoapps/courseware/tabs.py index 980fedb947..e926a0b114 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']) + tab_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(tab_module)) + + html = '' + + if tab_module is not None: + html = tab_module.get_html() + + return html diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 3de344b156..5af79d4983 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -247,18 +247,50 @@ 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 - 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: - msg = "ERROR " + msg - all_ok = False - num_bad += 1 + # 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 diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 32fb44ff15..fffbd2fc18 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -348,8 +348,8 @@ 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,}) + return render_to_response('courseware/info.html', {'request' : request, 'course_id' : course_id, 'cache' : None, + 'course': course, 'staff_access': staff_access}) @ensure_csrf_cookie def static_tab(request, course_id, tab_slug): @@ -364,7 +364,7 @@ def static_tab(request, course_id, tab_slug): if tab is None: raise Http404 - contents = tabs.get_static_tab_contents(course, tab) + contents = tabs.get_static_tab_contents(request, None, course, tab) if contents is None: raise Http404 @@ -414,7 +414,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}) 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