Merge branch 'master' of github.com:MITx/mitx into discussion2

cms/djangoapps/github_sync/tests/test_views.py

@@ -11,33 +11,33 @@ class PostReceiveTestCase(TestCase):
     def setUp(self):
         self.client = Client()
 
-    @patch('github_sync.views.sync_with_github')
-    def test_non_branch(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_non_branch(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/tags/foo'})
         })
-        self.assertFalse(sync_with_github.called)
+        self.assertFalse(import_from_github.called)
 
-    @patch('github_sync.views.sync_with_github')
-    def test_non_watched_repo(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_non_watched_repo(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/heads/branch',
             'repository': {'name': 'bad_repo'}})
         })
-        self.assertFalse(sync_with_github.called)
+        self.assertFalse(import_from_github.called)
 
-    @patch('github_sync.views.sync_with_github')
-    def test_non_tracked_branch(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_non_tracked_branch(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/heads/non_branch',
             'repository': {'name': 'repo'}})
         })
-        self.assertFalse(sync_with_github.called)
+        self.assertFalse(import_from_github.called)
 
-    @patch('github_sync.views.sync_with_github')
-    def test_tracked_branch(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_tracked_branch(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/heads/branch',
             'repository': {'name': 'repo'}})
         })
-        sync_with_github.assert_called_with(load_repo_settings('repo'))
+        import_from_github.assert_called_with(load_repo_settings('repo'))

cms/djangoapps/github_sync/views.py

@@ -5,7 +5,7 @@ from django.http import HttpResponse
 from django.conf import settings
 from django_future.csrf import csrf_exempt
 
-from . import sync_with_github, load_repo_settings
+from . import import_from_github, load_repo_settings
 
 log = logging.getLogger()
 
@@ -46,6 +46,6 @@ def github_post_receive(request):
         log.info('Ignoring changes to non-tracked branch %s in repo %s' % (branch_name, repo_name))
         return HttpResponse('Ignoring non-tracked branch')
 
-    sync_with_github(repo)
+    import_from_github(repo)
 
     return HttpResponse('Push received')

common/djangoapps/xmodule_modifiers.py

@@ -89,8 +89,8 @@ def add_histogram(get_html, module):
         else:
             edit_link = False
 
-        staff_context = {'definition': dict(module.definition),
-                         'metadata': dict(module.metadata),
+        staff_context = {'definition': json.dumps(module.definition, indent=4),
+                         'metadata': json.dumps(module.metadata, indent=4),
                          'element_id': module.location.html_id(),
                          'edit_link': edit_link,
                          'histogram': json.dumps(histogram),

common/lib/supertrace.py

+"""
+A handy util to print a django-debug-screen-like stack trace with
+values of local variables.
+"""
+
+import sys, traceback
+from django.utils.encoding import smart_unicode
+
+
+def supertrace(max_len=160):
+    """
+    Print the usual traceback information, followed by a listing of all the
+    local variables in each frame.  Should be called from an exception handler.
+
+    if max_len is not None, will print up to max_len chars for each local variable.
+
+    (cite: modified from somewhere on stackoverflow)
+    """
+    tb = sys.exc_info()[2]
+    while True:
+        if not tb.tb_next:
+            break
+        tb = tb.tb_next
+    stack = []
+    frame = tb.tb_frame
+    while frame:
+        stack.append(f)
+        frame = frame.f_back
+    stack.reverse()
+    # First print the regular traceback
+    traceback.print_exc()
+
+    print "Locals by frame, innermost last"
+    for frame in stack:
+        print
+        print "Frame %s in %s at line %s" % (frame.f_code.co_name,
+                                             frame.f_code.co_filename,
+                                             frame.f_lineno)
+        for key, value in frame.f_locals.items():
+            print ("\t%20s = " % smart_unicode(key, errors='ignore')),
+            # We have to be careful not to cause a new error in our error
+            # printer! Calling str() on an unknown object could cause an
+            # error.
+            try:
+                s = smart_unicode(value, errors='ignore')
+                if max_len is not None:
+                    s = s[:max_len]
+                print s
+            except:
+                print "<ERROR WHILE PRINTING VALUE>"
+
+

common/lib/xmodule/lazy_dict.py

-from collections import MutableMapping
-
-class LazyLoadingDict(MutableMapping):
-    """
-    A dictionary object that lazily loads its contents from a provided
-    function on reads (of members that haven't already been set).
-    """
-
-    def __init__(self, loader):
-        '''
-        On the first read from this dictionary, it will call loader() to
-        populate its contents.  loader() must return something dict-like. Any
-        elements set before the first read will be preserved.
-        '''
-        self._contents = {}
-        self._loaded = False
-        self._loader = loader
-        self._deleted = set()
-
-    def __getitem__(self, name):
-        if not (self._loaded or name in self._contents or name in self._deleted):
-            self.load()
-
-        return self._contents[name]
-
-    def __setitem__(self, name, value):
-        self._contents[name] = value
-        self._deleted.discard(name)
-
-    def __delitem__(self, name):
-        del self._contents[name]
-        self._deleted.add(name)
-
-    def __contains__(self, name):
-        self.load()
-        return name in self._contents
-
-    def __len__(self):
-        self.load()
-        return len(self._contents)
-
-    def __iter__(self):
-        self.load()
-        return iter(self._contents)
-
-    def __repr__(self):
-        self.load()
-        return repr(self._contents)
-
-    def load(self):
-        if self._loaded:
-            return
-
-        loaded_contents = self._loader()
-        loaded_contents.update(self._contents)
-        self._contents = loaded_contents
-        self._loaded = True
-

common/lib/xmodule/progress.py

-'''
-Progress class for modules.  Represents where a student is in a module.
-
-Useful things to know:
- - Use Progress.to_js_status_str() to convert a progress into a simple
-   status string to pass to js.
- - Use Progress.to_js_detail_str() to convert a progress into a more detailed
-   string to pass to js.
-
-In particular, these functions have a canonical handing of None.
-
-For most subclassing needs, you should only need to reimplement
-frac() and __str__().
-'''
-
-from collections import namedtuple
-import numbers
-
-
-class Progress(object):
-    '''Represents a progress of a/b (a out of b done)
-
-    a and b must be numeric, but not necessarily integer, with
-    0 <= a <= b and b > 0.
-
-    Progress can only represent Progress for modules where that makes sense.  Other
-    modules (e.g. html) should return None from get_progress().
-
-    TODO: add tag for module type?  Would allow for smarter merging.
-    '''
-
-    def __init__(self, a, b):
-        '''Construct a Progress object.  a and b must be numbers, and must have
-              0 <= a <= b and b > 0
-        '''
-
-        # Want to do all checking at construction time, so explicitly check types
-        if not (isinstance(a, numbers.Number) and
-                isinstance(b, numbers.Number)):
-            raise TypeError('a and b must be numbers.  Passed {0}/{1}'.format(a, b))
-
-        if not (0 <= a <= b and b > 0):
-            raise ValueError(
-                'fraction a/b = {0}/{1} must have 0 <= a <= b and b > 0'.format(a, b))
-
-        self._a = a
-        self._b = b
-
-    def frac(self):
-        ''' Return tuple (a,b) representing progress of a/b'''
-        return (self._a, self._b)
-
-    def percent(self):
-        ''' Returns a percentage progress as a float between 0 and 100.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        (a, b) = self.frac()
-        return 100.0 * a / b
-
-    def started(self):
-        ''' Returns True if fractional progress is greater than 0.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        return self.frac()[0] > 0
-
-    def inprogress(self):
-        ''' Returns True if fractional progress is strictly between 0 and 1.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        (a, b) = self.frac()
-        return a > 0 and a < b
-
-    def done(self):
-        ''' Return True if this represents done.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        (a, b) = self.frac()
-        return a == b
-
-    def ternary_str(self):
-        ''' Return a string version of this progress: either
-           "none", "in_progress", or "done".
-
-        subclassing note: implemented in terms of frac()
-        '''
-        (a, b) = self.frac()
-        if a == 0:
-            return "none"
-        if a < b:
-            return "in_progress"
-        return "done"
-
-    def __eq__(self, other):
-        ''' Two Progress objects are equal if they have identical values.
-        Implemented in terms of frac()'''
-        if not isinstance(other, Progress):
-            return False
-        (a, b) = self.frac()
-        (a2, b2) = other.frac()
-        return a == a2 and b == b2
-
-    def __ne__(self, other):
-        ''' The opposite of equal'''
-        return not self.__eq__(other)
-
-    def __str__(self):
-        ''' Return a string representation of this string.
-
-        subclassing note: implemented in terms of frac().
-        '''
-        (a, b) = self.frac()
-        return "{0}/{1}".format(a, b)
-
-    @staticmethod
-    def add_counts(a, b):
-        '''Add two progress indicators, assuming that each represents items done:
-        (a / b) + (c / d) = (a + c) / (b + d).
-        If either is None, returns the other.
-        '''
-        if a is None:
-            return b
-        if b is None:
-            return a
-        # get numerators + denominators
-        (n, d) = a.frac()
-        (n2, d2) = b.frac()
-        return Progress(n + n2, d + d2)
-
-    @staticmethod
-    def to_js_status_str(progress):
-        '''
-        Return the "status string" version of the passed Progress
-        object that should be passed to js.  Use this function when
-        sending Progress objects to js to limit dependencies.
-        '''
-        if progress is None:
-            return "NA"
-        return progress.ternary_str()
-
-    @staticmethod
-    def to_js_detail_str(progress):
-        '''
-        Return the "detail string" version of the passed Progress
-        object that should be passed to js.  Use this function when
-        passing Progress objects to js to limit dependencies.
-        '''
-        if progress is None:
-            return "NA"
-        return str(progress)

common/lib/xmodule/xmodule/editing_module.py

@@ -20,5 +20,10 @@ class EditingDescriptor(MakoModuleDescriptor):
     def get_context(self):
         return {
             'module': self,
-            'data': self.definition['data'],
+            'data': self.definition.get('data', ''),
+    # TODO (vshnayder): allow children and metadata to be edited.
+    #'children' : self.definition.get('children, ''),
+
+    # TODO: show both own metadata and inherited?
+    #'metadata' : self.own_metadata,
         }

common/lib/xmodule/xmodule/error_module.py

+import sys
+import logging
+
 from pkg_resources import resource_string
 from lxml import etree
 from xmodule.x_module import XModule
 from xmodule.mako_module import MakoModuleDescriptor
 from xmodule.xml_module import XmlDescriptor
 from xmodule.editing_module import EditingDescriptor
+from xmodule.errortracker import exc_info_to_str
 
-import logging
 
 log = logging.getLogger(__name__)
 
@@ -14,14 +17,11 @@ class ErrorModule(XModule):
         '''Show an error.
         TODO (vshnayder): proper style, divs, etc.
         '''
-        if not self.system.is_staff:
-            return self.system.render_template('module-error.html', {})
-
         # staff get to see all the details
-        return self.system.render_template('module-error-staff.html', {
-            'data' : self.definition['data'],
-            # TODO (vshnayder): need to get non-syntax errors in here somehow
-            'error' : self.definition.get('error', 'Error not available')
+        return self.system.render_template('module-error.html', {
+            'data' : self.definition['data']['contents'],
+            'error' : self.definition['data']['error_msg'],
+            'is_staff' : self.system.is_staff,
             })
 
 class ErrorDescriptor(EditingDescriptor):
@@ -31,29 +31,36 @@ class ErrorDescriptor(EditingDescriptor):
     module_class = ErrorModule
 
     @classmethod
-    def from_xml(cls, xml_data, system, org=None, course=None, err=None):
+    def from_xml(cls, xml_data, system, org=None, course=None,
+                 error_msg='Error not available'):
         '''Create an instance of this descriptor from the supplied data.
 
         Does not try to parse the data--just stores it.
 
         Takes an extra, optional, parameter--the error that caused an
-        issue.
+        issue.  (should be a string, or convert usefully into one).
         '''
-
-        definition = {}
-        if err is not None:
-            definition['error'] = err
+        # Use a nested inner dictionary because 'data' is hardcoded
+        inner = {}
+        definition = {'data': inner}
+        inner['error_msg'] = str(error_msg)
 
         try:
             # If this is already an error tag, don't want to re-wrap it.
             xml_obj = etree.fromstring(xml_data)
             if xml_obj.tag == 'error':
                 xml_data = xml_obj.text
-        except etree.XMLSyntaxError as err:
+                error_node = xml_obj.find('error_msg')
+                if error_node is not None:
+                    inner['error_msg'] = error_node.text
+                else:
+                    inner['error_msg'] = 'Error not available'
+
+        except etree.XMLSyntaxError:
             # Save the error to display later--overrides other problems
-            definition['error'] = err
+            inner['error_msg'] = exc_info_to_str(sys.exc_info())
 
-        definition['data'] = xml_data
+        inner['contents'] = xml_data
         # TODO (vshnayder): Do we need a unique slug here?  Just pick a random
         # 64-bit num?
         location = ['i4x', org, course, 'error', 'slug']
@@ -71,10 +78,12 @@ class ErrorDescriptor(EditingDescriptor):
         files, etc.  That would just get re-wrapped on import.
         '''
         try:
-           xml = etree.fromstring(self.definition['data'])
+           xml = etree.fromstring(self.definition['data']['contents'])
            return etree.tostring(xml)
         except etree.XMLSyntaxError:
             # still not valid.
             root = etree.Element('error')
-            root.text = self.definition['data']
+            root.text = self.definition['data']['contents']
+            err_node = etree.SubElement(root, 'error_msg')
+            err_node.text = self.definition['data']['error_msg']
             return etree.tostring(root)

common/lib/xmodule/xmodule/errortracker.py

@@ -8,6 +8,12 @@ log = logging.getLogger(__name__)
 
 ErrorLog = namedtuple('ErrorLog', 'tracker errors')
 
+def exc_info_to_str(exc_info):
+    """Given some exception info, convert it into a string using
+    the traceback.format_exception() function.
+    """
+    return ''.join(traceback.format_exception(*exc_info))
+
 def in_exception_handler():
     '''Is there an active exception?'''
     return sys.exc_info() != (None, None, None)
@@ -27,7 +33,7 @@ def make_error_tracker():
         '''Log errors'''
         exc_str = ''
         if in_exception_handler():
-            exc_str = ''.join(traceback.format_exception(*sys.exc_info()))
+            exc_str = exc_info_to_str(sys.exc_info())
 
         errors.append((msg, exc_str))
 

common/lib/xmodule/xmodule/html_module.py

@@ -5,11 +5,11 @@ import os
 import sys
 from lxml import etree
 
-from xmodule.x_module import XModule
-from xmodule.xml_module import XmlDescriptor
-from xmodule.editing_module import EditingDescriptor
-from stringify import stringify_children
-from html_checker import check_html
+from .x_module import XModule
+from .xml_module import XmlDescriptor
+from .editing_module import EditingDescriptor
+from .stringify import stringify_children
+from .html_checker import check_html
 
 log = logging.getLogger("mitx.courseware")
 

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -3,10 +3,13 @@ This module provides an abstraction for working with XModuleDescriptors
 that are stored in a database an accessible using their Location as an identifier
 """
 
+import logging
 import re
+
 from collections import namedtuple
+
 from .exceptions import InvalidLocationError, InsufficientSpecificationError
-import logging
+from xmodule.errortracker import ErrorLog, make_error_tracker
 
 log = logging.getLogger('mitx.' + 'modulestore')
 
@@ -290,3 +293,38 @@ class ModuleStore(object):
         '''
         raise NotImplementedError
 
+
+class ModuleStoreBase(ModuleStore):
+    '''
+    Implement interface functionality that can be shared.
+    '''
+    def __init__(self):
+        '''
+        Set up the error-tracking logic.
+        '''
+        self._location_errors = {}    # location -> ErrorLog
+
+    def _get_errorlog(self, location):
+        """
+        If we already have an errorlog for this location, return it.  Otherwise,
+        create one.
+        """
+        location = Location(location)
+        if location not in self._location_errors:
+            self._location_errors[location] = make_error_tracker()
+        return self._location_errors[location]
+
+    def get_item_errors(self, location):
+        """
+        Return list of errors for this location, if any.  Raise the same
+        errors as get_item if location isn't present.
+
+        NOTE: For now, the only items that track errors are CourseDescriptors in
+        the xml datastore.  This will return an empty list for all other items
+        and datastores.
+        """
+        # check that item is present and raise the promised exceptions if needed
+        self.get_item(location)
+
+        errorlog = self._get_errorlog(location)
+        return errorlog.errors

common/lib/xmodule/xmodule/modulestore/mongo.py

@@ -11,7 +11,7 @@ from xmodule.x_module import XModuleDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
 from mitxmako.shortcuts import render_to_string
 
-from . import ModuleStore, Location
+from . import ModuleStoreBase, Location
 from .exceptions import (ItemNotFoundError,
                          NoPathToItem, DuplicateItemError)
 
@@ -38,7 +38,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
 
         resources_fs: a filesystem, as per MakoDescriptorSystem
 
-        error_tracker:
+        error_tracker: a function that logs errors for later display to users
 
         render_template: a function for rendering templates, as per
             MakoDescriptorSystem
@@ -73,7 +73,7 @@ def location_to_query(location):
     return query
 
 
-class MongoModuleStore(ModuleStore):
+class MongoModuleStore(ModuleStoreBase):
     """
     A Mongodb backed ModuleStore
     """
@@ -81,6 +81,9 @@ class MongoModuleStore(ModuleStore):
     # TODO (cpennington): Enable non-filesystem filestores
     def __init__(self, host, db, collection, fs_root, port=27017, default_class=None,
                  error_tracker=null_error_tracker):
+
+        ModuleStoreBase.__init__(self)
+
         self.collection = pymongo.connection.Connection(
             host=host,
             port=port

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

@@ -12,7 +12,7 @@ from xmodule.course_module import CourseDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
 from cStringIO import StringIO
 
-from . import ModuleStore, Location
+from . import ModuleStoreBase, Location
 from .exceptions import ItemNotFoundError
 
 etree.set_default_parser(
@@ -98,7 +98,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
                                   error_tracker, process_xml, **kwargs)
 
 
-class XMLModuleStore(ModuleStore):
+class XMLModuleStore(ModuleStoreBase):
     """
     An XML backed ModuleStore
     """
@@ -118,13 +118,12 @@ class XMLModuleStore(ModuleStore):
         course_dirs: If specified, the list of course_dirs to load. Otherwise,
             load all course dirs
         """
+        ModuleStoreBase.__init__(self)
 
         self.eager = eager
         self.data_dir = path(data_dir)
         self.modules = {}  # location -> XModuleDescriptor
         self.courses = {}  # course_dir -> XModuleDescriptor for the course
-        self.location_errors = {}    # location -> ErrorLog
-
 
         if default_class is None:
             self.default_class = None
@@ -148,12 +147,14 @@ class XMLModuleStore(ModuleStore):
 
         for course_dir in course_dirs:
             try:
-                # make a tracker, then stick in the right place once the course loads
-                # and we know its location
+                # Special-case code here, since we don't have a location for the
+                # course before it loads.
+                # So, make a tracker to track load-time errors, then put in the right
+                # place after the course loads and we have its location
                 errorlog = make_error_tracker()
                 course_descriptor = self.load_course(course_dir, errorlog.tracker)
                 self.courses[course_dir] = course_descriptor
-                self.location_errors[course_descriptor.location] = errorlog
+                self._location_errors[course_descriptor.location] = errorlog
             except:
                 msg = "Failed to load course '%s'" % course_dir
                 log.exception(msg)
@@ -221,23 +222,6 @@ class XMLModuleStore(ModuleStore):
             raise ItemNotFoundError(location)
 
 
-    def get_item_errors(self, location):
-        """
-        Return list of errors for this location, if any.  Raise the same
-        errors as get_item if location isn't present.
-
-        NOTE: This only actually works for courses in the xml datastore--
-        will return an empty list for all other modules.
-        """
-        location = Location(location)
-        # check that item is present
-        self.get_item(location)
-
-        # now look up errors
-        if location in self.location_errors:
-            return self.location_errors[location].errors
-        return []
-
     def get_courses(self, depth=0):
         """
         Returns a list of course descriptors.  If there were errors on loading,
@@ -245,9 +229,11 @@ class XMLModuleStore(ModuleStore):
         """
         return self.courses.values()
 
+
     def create_item(self, location):
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_item(self, location, data):
         """
         Set the data in the item specified by the location to
@@ -258,6 +244,7 @@ class XMLModuleStore(ModuleStore):
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_children(self, location, children):
         """
         Set the children for the item specified by the location to
@@ -268,6 +255,7 @@ class XMLModuleStore(ModuleStore):
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_metadata(self, location, metadata):
         """
         Set the metadata for the item specified by the location to

common/lib/xmodule/tests/test_import.py → common/lib/xmodule/xmodule/tests/test_import.py

@@ -73,6 +73,7 @@ class ImportTestCase(unittest.TestCase):
     def test_reimport(self):
         '''Make sure an already-exported error xml tag loads properly'''
 
+        self.maxDiff = None
         bad_xml = '''<sequential display_name="oops"><video url="hi"></sequential>'''
         system = self.get_system()
         descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course',

common/lib/xmodule/tests/test_stringify.py → common/lib/xmodule/xmodule/tests/test_stringify.py

 from nose.tools import assert_equals
 from lxml import etree
-from stringify import stringify_children
+from xmodule.stringify import stringify_children
 
 def test_stringify():
-    html = '''<html a="b" foo="bar">Hi <div x="foo">there <span>Bruce</span><b>!</b></div></html>'''
+    text = 'Hi <div x="foo">there <span>Bruce</span><b>!</b></div>'
+    html = '''<html a="b" foo="bar">{0}</html>'''.format(text)
     xml = etree.fromstring(html)
     out = stringify_children(xml)
-    assert_equals(out, '''Hi <div x="foo">there <span>Bruce</span><b>!</b></div>''')
+    assert_equals(out, text)

common/lib/xmodule/xmodule/x_module.py

-from lxml import etree
-from lxml.etree import XMLSyntaxError
-import pkg_resources
 import logging
+import pkg_resources
+import sys
+
 from fs.errors import ResourceNotFoundError
 from functools import partial
+from lxml import etree
+from lxml.etree import XMLSyntaxError
 
 from xmodule.modulestore import Location
-
+from xmodule.errortracker import exc_info_to_str
 
 log = logging.getLogger('mitx.' + __name__)
 
@@ -471,7 +473,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
             msg = "Error loading from xml."
             log.exception(msg)
             system.error_tracker(msg)
-            descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, err)
+            err_msg = msg + "\n" + exc_info_to_str(sys.exc_info())
+            descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course,
+                                                  err_msg)
 
         return descriptor
 

lms/djangoapps/courseware/views.py

@@ -131,7 +131,7 @@ def profile(request, course_id, student_id=None):
 
     student_module_cache = StudentModuleCache(request.user, course)
     course_module, _, _, _ = get_module(request.user, request, course.location, student_module_cache)
-    
+
     context = {'name': user_info.name,
                'username': student.username,
                'location': user_info.location,
@@ -257,6 +257,7 @@ def index(request, course_id, chapter=None, section=None,
 
     return result
 
+
 @ensure_csrf_cookie
 def jump_to(request, location):
     '''
@@ -283,7 +284,7 @@ def jump_to(request, location):
     except NoPathToItem:
         raise Http404("This location is not in any class: {0}".format(location))
 
-
+    # Rely on index to do all error handling
     return index(request, course_id, chapter, section, position)
 
 @ensure_csrf_cookie

lms/templates/module-error-staff.html

-<section class="outside-app">
-  <h1>There has been an error on the <em>MITx</em> servers</h1>
-  <p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p>
-
-<h1>Staff-only details below:</h1>
-
-<p>Error: ${error}</p>
-
-<p>Raw data: ${data}</p>
-
-</section>

lms/templates/module-error.html

 <section class="outside-app">
   <h1>There has been an error on the <em>MITx</em> servers</h1>
   <p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p>
+
+% if is_staff:
+<h1>Staff-only details below:</h1>
+
+<p>Error: ${error | h}</p>
+
+<p>Raw data: ${data | h}</p>
+% endif
+
 </section>