From c0cdff7071431fb522a05b596bd0aa07232d1584 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Tue, 31 Jul 2012 13:44:04 -0400 Subject: [PATCH] Rename MalformedDescriptor to ErrorDescriptor * change references and tests * add staff/non-staff display * added is_staff to ModuleSystem --- cms/djangoapps/contentstore/views.py | 5 ++- common/lib/xmodule/setup.py | 2 +- common/lib/xmodule/tests/__init__.py | 3 +- common/lib/xmodule/tests/test_import.py | 12 +++--- .../{malformed_module.py => error_module.py} | 41 +++++++++++-------- common/lib/xmodule/xmodule/x_module.py | 26 ++++++++---- lms/djangoapps/courseware/module_render.py | 1 + 7 files changed, 58 insertions(+), 32 deletions(-) rename common/lib/xmodule/xmodule/{malformed_module.py => error_module.py} (58%) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 6d5b2117c4..111c91e03a 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -214,7 +214,10 @@ def preview_module_system(request, preview_id, descriptor): get_module=partial(get_preview_module, request, preview_id), render_template=render_from_lms, debug=True, - replace_urls=replace_urls + replace_urls=replace_urls, + # TODO (vshnayder): CMS users won't see staff view unless they are CMS staff. + # is that what we want? + is_staff=request.user.is_staff ) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 09a6b7e712..8a0a6bb139 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -25,7 +25,7 @@ setup( "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor", "html = xmodule.html_module:HtmlDescriptor", "image = xmodule.backcompat_module:TranslateCustomTagDescriptor", - "malformed = xmodule.malformed_module:MalformedDescriptor", + "error = xmodule.error_module:ErrorDescriptor", "problem = xmodule.capa_module:CapaDescriptor", "problemset = xmodule.vertical_module:VerticalDescriptor", "section = xmodule.backcompat_module:SemanticSectionDescriptor", diff --git a/common/lib/xmodule/tests/__init__.py b/common/lib/xmodule/tests/__init__.py index 94bf1da65e..7f6fcfe00c 100644 --- a/common/lib/xmodule/tests/__init__.py +++ b/common/lib/xmodule/tests/__init__.py @@ -31,7 +31,8 @@ i4xs = ModuleSystem( user=Mock(), filestore=fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))), debug=True, - xqueue_callback_url='/' + xqueue_callback_url='/', + is_staff=False ) diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py index dd55f8ff38..497354a18e 100644 --- a/common/lib/xmodule/tests/test_import.py +++ b/common/lib/xmodule/tests/test_import.py @@ -33,7 +33,7 @@ class ImportTestCase(unittest.TestCase): return system def test_fallback(self): - '''Make sure that malformed xml loads as a MalformedDescriptorb.''' + '''Make sure that malformed xml loads as an ErrorDescriptor.''' bad_xml = '''''' @@ -43,10 +43,10 @@ class ImportTestCase(unittest.TestCase): None) self.assertEqual(descriptor.__class__.__name__, - 'MalformedDescriptor') + 'ErrorDescriptor') def test_reimport(self): - '''Make sure an already-exported malformed xml tag loads properly''' + '''Make sure an already-exported error xml tag loads properly''' bad_xml = '''''' system = self.get_system() @@ -58,7 +58,7 @@ class ImportTestCase(unittest.TestCase): 'org', 'course', None) self.assertEqual(re_import_descriptor.__class__.__name__, - 'MalformedDescriptor') + 'ErrorDescriptor') self.assertEqual(descriptor.definition['data'], re_import_descriptor.definition['data']) @@ -66,8 +66,8 @@ class ImportTestCase(unittest.TestCase): def test_fixed_xml_tag(self): """Make sure a tag that's been fixed exports as the original tag type""" - # create a malformed tag with valid xml contents - root = etree.Element('malformed') + # create a error tag with valid xml contents + root = etree.Element('error') good_xml = '''''' root.text = good_xml diff --git a/common/lib/xmodule/xmodule/malformed_module.py b/common/lib/xmodule/xmodule/error_module.py similarity index 58% rename from common/lib/xmodule/xmodule/malformed_module.py rename to common/lib/xmodule/xmodule/error_module.py index ac419fff26..ee05e9c879 100644 --- a/common/lib/xmodule/xmodule/malformed_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -9,18 +9,26 @@ import logging log = logging.getLogger(__name__) -class MalformedModule(XModule): +class ErrorModule(XModule): def get_html(self): '''Show an error. TODO (vshnayder): proper style, divs, etc. ''' - return "Malformed content--not showing through get_html()" + if not self.system.is_staff: + return self.system.render_template('module-error.html') -class MalformedDescriptor(EditingDescriptor): + # 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') + }) + +class ErrorDescriptor(EditingDescriptor): """ Module that provides a raw editing view of broken xml. """ - module_class = MalformedModule + module_class = ErrorModule @classmethod def from_xml(cls, xml_data, system, org=None, course=None): @@ -29,38 +37,39 @@ class MalformedDescriptor(EditingDescriptor): Does not try to parse the data--just stores it. ''' + definition = {} try: - # If this is already a malformed tag, don't want to re-wrap it. + # If this is already an error tag, don't want to re-wrap it. xml_obj = etree.fromstring(xml_data) - if xml_obj.tag == 'malformed': + if xml_obj.tag == 'error': xml_data = xml_obj.text - # TODO (vshnayder): how does one get back from this to a valid descriptor? - # For now, have to fix manually. - except etree.XMLSyntaxError: - pass + except etree.XMLSyntaxError as err: + # Save the error to display later + definition['error'] = str(err) - definition = { 'data' : xml_data } - # TODO (vshnayder): Do we need a valid slug here? Just pick a random + + definition['data'] = xml_data + # TODO (vshnayder): Do we need a unique slug here? Just pick a random # 64-bit num? - location = ['i4x', org, course, 'malformed', 'slug'] + location = ['i4x', org, course, 'error', 'slug'] metadata = {} # stays in the xml_data return cls(system, definition, location=location, metadata=metadata) def export_to_xml(self, resource_fs): ''' - If the definition data is invalid xml, export it wrapped in a malformed + If the definition data is invalid xml, export it wrapped in an "error" tag. If it is valid, export without the wrapper. NOTE: There may still be problems with the valid xml--it could be missing required attributes, could have the wrong tags, refer to missing - files, etc. + files, etc. That would just get re-wrapped on import. ''' try: xml = etree.fromstring(self.definition['data']) return etree.tostring(xml) except etree.XMLSyntaxError: # still not valid. - root = etree.Element('malformed') + root = etree.Element('error') root.text = self.definition['data'] return etree.tostring(root) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index c4ba3e1e58..2d7447b5f6 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -455,14 +455,14 @@ class XModuleDescriptor(Plugin, HTMLSnippet): descriptor = class_.from_xml(xml_data, system, org, course) except (ResourceNotFoundError, XMLSyntaxError) as err: - # Didn't load properly. Fall back on loading as a malformed + # Didn't load properly. Fall back on loading as an error # descriptor. This should never error due to formatting. # Put import here to avoid circular import errors - from xmodule.malformed_module import MalformedDescriptor + from xmodule.error_module import ErrorDescriptor #system.error_handler("Error loading from xml.") - descriptor = MalformedDescriptor.from_xml(xml_data, system, org, course) + descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course) return descriptor @@ -597,10 +597,18 @@ class ModuleSystem(object): Note that these functions can be closures over e.g. a django request and user, or other environment-specific info. ''' - def __init__(self, ajax_url, track_function, - get_module, render_template, replace_urls, - user=None, filestore=None, debug=False, - xqueue_callback_url=None, xqueue_default_queuename="null"): + def __init__(self, + ajax_url, + track_function, + get_module, + render_template, + replace_urls, + user=None, + filestore=None, + debug=False, + xqueue_callback_url=None, + xqueue_default_queuename="null", + is_staff=False): ''' Create a closure around the system environment. @@ -626,6 +634,9 @@ class ModuleSystem(object): replace_urls - TEMPORARY - A function like static_replace.replace_urls that capa_module can use to fix up the static urls in ajax results. + + is_staff - Is the user making the request a staff user? + TODO (vshnayder): this will need to change once we have real user roles. ''' self.ajax_url = ajax_url self.xqueue_callback_url = xqueue_callback_url @@ -637,6 +648,7 @@ class ModuleSystem(object): self.DEBUG = self.debug = debug self.seed = user.id if user is not None else 0 self.replace_urls = replace_urls + self.is_staff = is_staff def get(self, attr): ''' provide uniform access to attributes (like etree).''' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 80a4ef90fc..1ff39f602e 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -172,6 +172,7 @@ def get_module(user, request, location, student_module_cache, position=None): # a module is coming through get_html and is therefore covered # by the replace_static_urls code below replace_urls=replace_urls, + is_staff=user.is_staff, ) # pass position specified in URL to module through ModuleSystem system.set('position', position)