Rename MalformedDescriptor to ErrorDescriptor

* change references and tests
* add staff/non-staff display
* added is_staff to ModuleSystem

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
     )
 
 

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",

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
 )
 
 

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 = '''<sequential display_name="oops"><video url="hi"></sequential>'''
 
@@ -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 = '''<sequential display_name="oops"><video url="hi"></sequential>'''
         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 = '''<sequential display_name="fixed"><video url="hi"/></sequential>'''
         root.text = good_xml
 

common/lib/xmodule/xmodule/malformed_module.py → 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)

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).'''

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)