diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index b942d6726b..4d412000ec 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/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), diff --git a/common/lib/supertrace.py b/common/lib/supertrace.py new file mode 100644 index 0000000000..e17cd7a8ba --- /dev/null +++ b/common/lib/supertrace.py @@ -0,0 +1,52 @@ +""" +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 "" + + diff --git a/common/lib/xmodule/lazy_dict.py b/common/lib/xmodule/lazy_dict.py deleted file mode 100644 index fa614843ec..0000000000 --- a/common/lib/xmodule/lazy_dict.py +++ /dev/null @@ -1,58 +0,0 @@ -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 - diff --git a/common/lib/xmodule/progress.py b/common/lib/xmodule/progress.py deleted file mode 100644 index 70c8ec9da1..0000000000 --- a/common/lib/xmodule/progress.py +++ /dev/null @@ -1,157 +0,0 @@ -''' -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) diff --git a/common/lib/xmodule/tests/test_stringify.py b/common/lib/xmodule/tests/test_stringify.py deleted file mode 100644 index 62d7683886..0000000000 --- a/common/lib/xmodule/tests/test_stringify.py +++ /dev/null @@ -1,9 +0,0 @@ -from nose.tools import assert_equals -from lxml import etree -from stringify import stringify_children - -def test_stringify(): - html = '''Hi
there Bruce!
''' - xml = etree.fromstring(html) - out = stringify_children(xml) - assert_equals(out, '''Hi
there Bruce!
''') diff --git a/common/lib/xmodule/xmodule/editing_module.py b/common/lib/xmodule/xmodule/editing_module.py index 4188165a24..67a4d66dad 100644 --- a/common/lib/xmodule/xmodule/editing_module.py +++ b/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, } diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 62b3b82a39..ecc90873b9 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -1,11 +1,14 @@ +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: - # Save the error to display later--overrides other problems - definition['error'] = 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' - definition['data'] = xml_data + except etree.XMLSyntaxError: + # Save the error to display later--overrides other problems + inner['error_msg'] = exc_info_to_str(sys.exc_info()) + + 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) diff --git a/common/lib/xmodule/xmodule/errortracker.py b/common/lib/xmodule/xmodule/errortracker.py index b8d42a6983..8ac2903149 100644 --- a/common/lib/xmodule/xmodule/errortracker.py +++ b/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)) diff --git a/common/lib/xmodule/html_checker.py b/common/lib/xmodule/xmodule/html_checker.py similarity index 100% rename from common/lib/xmodule/html_checker.py rename to common/lib/xmodule/xmodule/html_checker.py diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index b2a5df9803..7c3456e5ad 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/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") diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 7241179d8e..e59e4bd68e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/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 re -from collections import namedtuple -from .exceptions import InvalidLocationError, InsufficientSpecificationError import logging +import re + +from collections import namedtuple + +from .exceptions import InvalidLocationError, InsufficientSpecificationError +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 diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 76769b25b0..1cec6c7f87 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index d6540023e8..0567e4e7a7 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/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 diff --git a/common/lib/xmodule/stringify.py b/common/lib/xmodule/xmodule/stringify.py similarity index 100% rename from common/lib/xmodule/stringify.py rename to common/lib/xmodule/xmodule/stringify.py diff --git a/common/lib/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py similarity index 100% rename from common/lib/xmodule/tests/__init__.py rename to common/lib/xmodule/xmodule/tests/__init__.py diff --git a/common/lib/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py similarity index 100% rename from common/lib/xmodule/tests/test_export.py rename to common/lib/xmodule/xmodule/tests/test_export.py diff --git a/common/lib/xmodule/tests/test_files/choiceresponse_checkbox.xml b/common/lib/xmodule/xmodule/tests/test_files/choiceresponse_checkbox.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/choiceresponse_checkbox.xml rename to common/lib/xmodule/xmodule/tests/test_files/choiceresponse_checkbox.xml diff --git a/common/lib/xmodule/tests/test_files/choiceresponse_radio.xml b/common/lib/xmodule/xmodule/tests/test_files/choiceresponse_radio.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/choiceresponse_radio.xml rename to common/lib/xmodule/xmodule/tests/test_files/choiceresponse_radio.xml diff --git a/common/lib/xmodule/tests/test_files/coderesponse.xml b/common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/coderesponse.xml rename to common/lib/xmodule/xmodule/tests/test_files/coderesponse.xml diff --git a/common/lib/xmodule/tests/test_files/formularesponse_with_hint.xml b/common/lib/xmodule/xmodule/tests/test_files/formularesponse_with_hint.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/formularesponse_with_hint.xml rename to common/lib/xmodule/xmodule/tests/test_files/formularesponse_with_hint.xml diff --git a/common/lib/xmodule/tests/test_files/imageresponse.xml b/common/lib/xmodule/xmodule/tests/test_files/imageresponse.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/imageresponse.xml rename to common/lib/xmodule/xmodule/tests/test_files/imageresponse.xml diff --git a/common/lib/xmodule/tests/test_files/multi_bare.xml b/common/lib/xmodule/xmodule/tests/test_files/multi_bare.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/multi_bare.xml rename to common/lib/xmodule/xmodule/tests/test_files/multi_bare.xml diff --git a/common/lib/xmodule/tests/test_files/multichoice.xml b/common/lib/xmodule/xmodule/tests/test_files/multichoice.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/multichoice.xml rename to common/lib/xmodule/xmodule/tests/test_files/multichoice.xml diff --git a/common/lib/xmodule/tests/test_files/optionresponse.xml b/common/lib/xmodule/xmodule/tests/test_files/optionresponse.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/optionresponse.xml rename to common/lib/xmodule/xmodule/tests/test_files/optionresponse.xml diff --git a/common/lib/xmodule/tests/test_files/stringresponse_with_hint.xml b/common/lib/xmodule/xmodule/tests/test_files/stringresponse_with_hint.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/stringresponse_with_hint.xml rename to common/lib/xmodule/xmodule/tests/test_files/stringresponse_with_hint.xml diff --git a/common/lib/xmodule/tests/test_files/symbolicresponse.xml b/common/lib/xmodule/xmodule/tests/test_files/symbolicresponse.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/symbolicresponse.xml rename to common/lib/xmodule/xmodule/tests/test_files/symbolicresponse.xml diff --git a/common/lib/xmodule/tests/test_files/truefalse.xml b/common/lib/xmodule/xmodule/tests/test_files/truefalse.xml similarity index 100% rename from common/lib/xmodule/tests/test_files/truefalse.xml rename to common/lib/xmodule/xmodule/tests/test_files/truefalse.xml diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py similarity index 99% rename from common/lib/xmodule/tests/test_import.py rename to common/lib/xmodule/xmodule/tests/test_import.py index 6a407fe189..0d3e2260fb 100644 --- a/common/lib/xmodule/tests/test_import.py +++ b/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 = '''''' system = self.get_system() descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course', diff --git a/common/lib/xmodule/xmodule/tests/test_stringify.py b/common/lib/xmodule/xmodule/tests/test_stringify.py new file mode 100644 index 0000000000..1c6ee855f3 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_stringify.py @@ -0,0 +1,10 @@ +from nose.tools import assert_equals +from lxml import etree +from xmodule.stringify import stringify_children + +def test_stringify(): + text = 'Hi
there Bruce!
' + html = '''{0}'''.format(text) + xml = etree.fromstring(html) + out = stringify_children(xml) + assert_equals(out, text) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 360f1b07d0..ac6b5db5a4 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1,12 +1,14 @@ -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 diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index a049638d1b..2b55b48caf 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -117,7 +117,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, @@ -243,6 +243,7 @@ def index(request, course_id, chapter=None, section=None, return result + @ensure_csrf_cookie def jump_to(request, location): ''' @@ -269,7 +270,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 diff --git a/lms/templates/module-error-staff.html b/lms/templates/module-error-staff.html deleted file mode 100644 index 955f413db3..0000000000 --- a/lms/templates/module-error-staff.html +++ /dev/null @@ -1,11 +0,0 @@ -
-

There has been an error on the MITx servers

-

We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at technical@mitx.mit.edu to report any problems or downtime.

- -

Staff-only details below:

- -

Error: ${error}

- -

Raw data: ${data}

- -
diff --git a/lms/templates/module-error.html b/lms/templates/module-error.html index 28597fa13c..7c731db17a 100644 --- a/lms/templates/module-error.html +++ b/lms/templates/module-error.html @@ -1,4 +1,13 @@

There has been an error on the MITx servers

We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at technical@mitx.mit.edu to report any problems or downtime.

+ +% if is_staff: +

Staff-only details below:

+ +

Error: ${error | h}

+ +

Raw data: ${data | h}

+% endif +