From f3b36b8bc9542fbbaa4de30b0502962788514fe0 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Fri, 31 May 2013 15:28:54 -0400 Subject: [PATCH 1/2] Use from_descriptor instead of from_xml to make ErrorDescriptor --- .../xmodule/tests/test_error_module.py | 52 ++++++++++++++----- lms/djangoapps/courseware/module_render.py | 5 +- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index d6b6f77ae631..b329b4b5ccd6 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -4,6 +4,9 @@ import unittest from xmodule.tests import test_system import xmodule.error_module as error_module +from xmodule.modulestore import Location +from xmodule.x_module import XModuleDescriptor +from mock import MagicMock class TestErrorModule(unittest.TestCase): @@ -14,22 +17,33 @@ def setUp(self): self.system = test_system() self.org = "org" self.course = "course" - self.fake_xml = "" + self.location = Location(['i4x', self.org, self.course, None, None]) + self.valid_xml = "" self.broken_xml = "" self.error_msg = "Error" - def test_error_module_create(self): + def test_error_module_xml_rendering(self): descriptor = error_module.ErrorDescriptor.from_xml( - self.fake_xml, self.system, self.org, self.course) + self.valid_xml, self.system, self.org, self.course, self.error_msg) 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) + self.assertIn(self.valid_xml, rendered_html) + + def test_error_module_from_descriptor(self): + descriptor = MagicMock([XModuleDescriptor], + system=self.system, + location=self.location, + _model_data=self.valid_xml) + + error_descriptor = error_module.ErrorDescriptor.from_descriptor( + descriptor, self.error_msg) + self.assertTrue(isinstance(error_descriptor, error_module.ErrorDescriptor)) + module = error_descriptor.xmodule(self.system) + rendered_html = module.get_html() + self.assertIn(self.error_msg, rendered_html) + self.assertIn(self.valid_xml, rendered_html) class TestNonStaffErrorModule(TestErrorModule): @@ -39,13 +53,27 @@ class TestNonStaffErrorModule(TestErrorModule): def test_non_staff_error_module_create(self): descriptor = error_module.NonStaffErrorDescriptor.from_xml( - self.fake_xml, self.system, self.org, self.course) + self.valid_xml, self.system, self.org, self.course) self.assertTrue(isinstance(descriptor, error_module.NonStaffErrorDescriptor)) - def test_non_staff_error_module_rendering(self): + def test_from_xml_render(self): descriptor = error_module.NonStaffErrorDescriptor.from_xml( - self.fake_xml, self.system, self.org, self.course) + self.valid_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) + self.assertNotIn(self.valid_xml, rendered_html) + + def test_error_module_from_descriptor(self): + descriptor = MagicMock([XModuleDescriptor], + system=self.system, + location=self.location, + _model_data=self.valid_xml) + + error_descriptor = error_module.NonStaffErrorDescriptor.from_descriptor( + descriptor, self.error_msg) + self.assertTrue(isinstance(error_descriptor, error_module.ErrorDescriptor)) + module = error_descriptor.xmodule(self.system) + rendered_html = module.get_html() + self.assertNotIn(self.error_msg, rendered_html) + self.assertNotIn(self.valid_xml, rendered_html) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 284b746249b1..2ae7bcdc1f28 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -335,9 +335,8 @@ def can_execute_unsafe_code(): else: err_descriptor_class = NonStaffErrorDescriptor - err_descriptor = err_descriptor_class.from_xml( - str(descriptor), descriptor.system, - org=descriptor.location.org, course=descriptor.location.course, + err_descriptor = err_descriptor_class.from_descriptor( + descriptor, error_msg=exc_info_to_str(sys.exc_info()) ) From d895c64f4ac23fdab00954e125ae43a6aeddf132 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 4 Jun 2013 10:43:06 -0400 Subject: [PATCH 2/2] Use full descriptor when showing errors, not just the model data. --- common/lib/xmodule/xmodule/error_module.py | 2 +- common/lib/xmodule/xmodule/tests/test_error_module.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 8014234f69b1..7f1537017611 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -121,7 +121,7 @@ def from_json(cls, json_data, system, error_msg='Error not available'): def from_descriptor(cls, descriptor, error_msg='Error not available'): return cls._construct( descriptor.system, - descriptor._model_data, + str(descriptor), error_msg, location=descriptor.location, ) diff --git a/common/lib/xmodule/xmodule/tests/test_error_module.py b/common/lib/xmodule/xmodule/tests/test_error_module.py index b329b4b5ccd6..78e5b46a96a8 100644 --- a/common/lib/xmodule/xmodule/tests/test_error_module.py +++ b/common/lib/xmodule/xmodule/tests/test_error_module.py @@ -43,7 +43,7 @@ def test_error_module_from_descriptor(self): module = error_descriptor.xmodule(self.system) rendered_html = module.get_html() self.assertIn(self.error_msg, rendered_html) - self.assertIn(self.valid_xml, rendered_html) + self.assertIn(str(descriptor), rendered_html) class TestNonStaffErrorModule(TestErrorModule): @@ -76,4 +76,4 @@ def test_error_module_from_descriptor(self): module = error_descriptor.xmodule(self.system) rendered_html = module.get_html() self.assertNotIn(self.error_msg, rendered_html) - self.assertNotIn(self.valid_xml, rendered_html) + self.assertNotIn(str(descriptor), rendered_html)