Merge pull request #2147 from edx/diana/fix-error-descriptor

Fix XML Error Module so that it no longer shows the data to non-staff.

common/lib/xmodule/xmodule/error_module.py

+"""
+Modules that get shown to the users when an error has occured while
+loading or rendering other modules
+"""
+
 import hashlib
 import logging
 import json
@@ -22,12 +27,19 @@ log = logging.getLogger(__name__)
 
 
 class ErrorFields(object):
+    """
+    XBlock fields used by the ErrorModules
+    """
     contents = String(scope=Scope.content)
     error_msg = String(scope=Scope.content)
     display_name = String(scope=Scope.settings)
 
 
 class ErrorModule(ErrorFields, XModule):
+    """
+    Module that gets shown to staff when there has been an error while
+    loading or rendering other modules
+    """
 
     def get_html(self):
         '''Show an error to staff.
@@ -42,6 +54,10 @@ class ErrorModule(ErrorFields, XModule):
 
 
 class NonStaffErrorModule(ErrorFields, XModule):
+    """
+    Module that gets shown to students when there has been an error while
+    loading or rendering other modules
+    """
     def get_html(self):
         '''Show an error to a student.
         TODO (vshnayder): proper style, divs, etc.
@@ -61,7 +77,7 @@ class ErrorDescriptor(ErrorFields, JSONEditingDescriptor):
     module_class = ErrorModule
 
     @classmethod
-    def _construct(self, system, contents, error_msg, location):
+    def _construct(cls, system, contents, error_msg, location):
 
         if location.name is None:
             location = location._replace(
@@ -80,7 +96,7 @@ class ErrorDescriptor(ErrorFields, JSONEditingDescriptor):
             'contents': contents,
             'display_name': 'Error: ' + location.name
         }
-        return ErrorDescriptor(
+        return cls(
             system,
             location,
             model_data,

common/lib/xmodule/xmodule/tests/test_error_module.py

+"""
+Tests for ErrorModule and NonStaffErrorModule
+"""
+import unittest
+from xmodule.tests import test_system
+import xmodule.error_module as error_module
+
+
+class TestErrorModule(unittest.TestCase):
+    """
+    Tests for ErrorModule and ErrorDescriptor
+    """
+    def setUp(self):
+        self.system = test_system()
+        self.org = "org"
+        self.course = "course"
+        self.fake_xml = "<problem />"
+        self.broken_xml = "<problem>"
+        self.error_msg = "Error"
+
+    def test_error_module_create(self):
+        descriptor = error_module.ErrorDescriptor.from_xml(
+            self.fake_xml, self.system, self.org, self.course)
+        self.assertTrue(isinstance(descriptor, error_module.ErrorDescriptor))
+
+    def test_error_module_rendering(self):
+        descriptor = error_module.ErrorDescriptor.from_xml(
+            self.fake_xml, self.system, self.org, self.course, self.error_msg)
+        module = descriptor.xmodule(self.system)
+        rendered_html = module.get_html()
+        self.assertIn(self.error_msg, rendered_html)
+        self.assertIn(self.fake_xml, rendered_html)
+
+
+class TestNonStaffErrorModule(TestErrorModule):
+    """
+    Tests for NonStaffErrorModule and NonStaffErrorDescriptor
+    """
+
+    def test_non_staff_error_module_create(self):
+        descriptor = error_module.NonStaffErrorDescriptor.from_xml(
+            self.fake_xml, self.system, self.org, self.course)
+        self.assertTrue(isinstance(descriptor, error_module.NonStaffErrorDescriptor))
+
+    def test_non_staff_error_module_rendering(self):
+        descriptor = error_module.NonStaffErrorDescriptor.from_xml(
+            self.fake_xml, self.system, self.org, self.course)
+        module = descriptor.xmodule(self.system)
+        rendered_html = module.get_html()
+        self.assertNotIn(self.error_msg, rendered_html)
+        self.assertNotIn(self.fake_xml, rendered_html)