Merge pull request #1859 from MITx/fix/brian/conditional-error

cope with ErrorModule as source (or required) module, and add tests.

common/lib/xmodule/xmodule/conditional_module.py

@@ -92,9 +92,13 @@ class ConditionalModule(ConditionalFields, XModule):
         if xml_value and self.required_modules:
             for module in self.required_modules:
                 if not hasattr(module, attr_name):
-                    raise Exception('Error in conditional module: \
-                    required module {module} has no {module_attr}'.format(
-                        module=module, module_attr=attr_name))
+                    # We don't throw an exception here because it is possible for 
+                    # the descriptor of a required module to have a property but
+                    # for the resulting module to be a (flavor of) ErrorModule.
+                    # So just log and return false.
+                    log.warn('Error in conditional module: \
+                        required module {module} has no {module_attr}'.format(module=module, module_attr=attr_name))
+                    return False
 
                 attr = getattr(module, attr_name)
                 if callable(attr):
@@ -137,16 +141,15 @@ class ConditionalModule(ConditionalFields, XModule):
 
     def get_icon_class(self):
         new_class = 'other'
-        if self.is_condition_satisfied():
-            # HACK: This shouldn't be hard-coded to two types
-            # OBSOLETE: This obsoletes 'type'
-            class_priority = ['video', 'problem']
-
-            child_classes = [self.system.get_module(child_descriptor).get_icon_class()
-                             for child_descriptor in self.descriptor.get_children()]
-            for c in class_priority:
-                if c in child_classes:
-                    new_class = c
+        # HACK: This shouldn't be hard-coded to two types
+        # OBSOLETE: This obsoletes 'type'
+        class_priority = ['video', 'problem']
+
+        child_classes = [self.system.get_module(child_descriptor).get_icon_class()
+                         for child_descriptor in self.descriptor.get_children()]
+        for c in class_priority:
+            if c in child_classes:
+                new_class = c
         return new_class
 
 

common/lib/xmodule/xmodule/tests/test_conditional.py

+
 import json
-from path import path
 import unittest
 from fs.memoryfs import MemoryFS
-
-from lxml import etree
+from ast import literal_eval
 from mock import Mock, patch
-from collections import defaultdict
 
-from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
-from xmodule.xml_module import is_pointer_tag
-from xmodule.errortracker import make_error_tracker
+from xmodule.error_module import NonStaffErrorDescriptor
 from xmodule.modulestore import Location
 from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
-from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.conditional_module import ConditionalModule
 
-from .test_export import DATA_DIR
+from xmodule.tests.test_export import DATA_DIR
 
 ORG = 'test_org'
-COURSE = 'conditional'  	# name of directory with course data
+COURSE = 'conditional'      # name of directory with course data
 
 from . import test_system
 
@@ -47,10 +43,118 @@ class DummySystem(ImportSystem):
     def render_template(self, template, context):
             raise Exception("Shouldn't be called")
 
+class ConditionalFactory(object):
+    """
+    A helper class to create a conditional module and associated source and child modules
+    to allow for testing.
+    """
+    @staticmethod
+    def create(system, source_is_error_module=False):
+        """
+        return a dict of modules: the conditional with a single source and a single child.
+        Keys are 'cond_module', 'source_module', and 'child_module'.
+
+        if the source_is_error_module flag is set, create a real ErrorModule for the source.
+        """
+        # construct source descriptor and module:
+        source_location = Location(["i4x", "edX", "conditional_test", "problem", "SampleProblem"])
+        if source_is_error_module:
+            # Make an error descriptor and module
+            source_descriptor = NonStaffErrorDescriptor.from_xml('some random xml data', 
+                                                                 system,
+                                                                 org=source_location.org, 
+                                                                 course=source_location.course,
+                                                                 error_msg='random error message')
+            source_module = source_descriptor.xmodule(system)
+        else:
+            source_descriptor = Mock()
+            source_descriptor.location = source_location
+            source_module = Mock()
+
+        # construct other descriptors:
+        child_descriptor = Mock()
+        cond_descriptor = Mock()
+        cond_descriptor.get_required_module_descriptors = lambda: [source_descriptor, ]
+        cond_descriptor.get_children = lambda: [child_descriptor, ]
+        cond_descriptor.xml_attributes = {"attempted": "true"}
+
+        # create child module:
+        child_module = Mock()
+        child_module.get_html = lambda: '<p>This is a secret</p>'
+        child_module.displayable_items = lambda: [child_module]
+        module_map = {source_descriptor: source_module, child_descriptor: child_module}
+        system.get_module = lambda descriptor: module_map[descriptor]
+
+        # construct conditional module:
+        cond_location = Location(["i4x", "edX", "conditional_test", "conditional", "SampleConditional"])
+        model_data = {'data': '<conditional/>'}
+        cond_module = ConditionalModule(system, cond_location, cond_descriptor, model_data)
+
+        # return dict:
+        return {'cond_module': cond_module,
+                'source_module': source_module,
+                'child_module': child_module }
+
+
+class ConditionalModuleBasicTest(unittest.TestCase):
+    """
+    Make sure that conditional module works, using mocks for
+    other modules.
+    """
+
+    def setUp(self):
+        self.test_system = test_system()
+
+    def test_icon_class(self):
+        '''verify that get_icon_class works independent of condition satisfaction''' 
+        modules = ConditionalFactory.create(self.test_system)
+        for attempted in ["false", "true"]:
+            for icon_class in [ 'other', 'problem', 'video']:
+                modules['source_module'].is_attempted = attempted
+                modules['child_module'].get_icon_class = lambda: icon_class
+                self.assertEqual(modules['cond_module'].get_icon_class(), icon_class)
+
+
+    def test_get_html(self):
+        modules = ConditionalFactory.create(self.test_system)
+        # because test_system returns the repr of the context dict passed to render_template,
+        # we reverse it here
+        html = modules['cond_module'].get_html()
+        html_dict = literal_eval(html)
+        self.assertEqual(html_dict['element_id'], 'i4x-edX-conditional_test-conditional-SampleConditional')
+        self.assertEqual(html_dict['id'], 'i4x://edX/conditional_test/conditional/SampleConditional')
+        self.assertEqual(html_dict['depends'], 'i4x-edX-conditional_test-problem-SampleProblem')
+
+    def test_handle_ajax(self):
+        modules = ConditionalFactory.create(self.test_system)
+        modules['source_module'].is_attempted = "false"
+        ajax = json.loads(modules['cond_module'].handle_ajax('', ''))
+        print "ajax: ", ajax
+        html = ajax['html']
+        self.assertFalse(any(['This is a secret' in item for item in html]))
+
+        # now change state of the capa problem to make it completed
+        modules['source_module'].is_attempted = "true"
+        ajax = json.loads(modules['cond_module'].handle_ajax('', ''))
+        print "post-attempt ajax: ", ajax
+        html = ajax['html']
+        self.assertTrue(any(['This is a secret' in item for item in html]))
 
+    def test_error_as_source(self):
+        '''
+        Check that handle_ajax works properly if the source is really an ErrorModule,
+        and that the condition is not satisfied.
+        '''
+        modules = ConditionalFactory.create(self.test_system, source_is_error_module=True)
+        ajax = json.loads(modules['cond_module'].handle_ajax('', ''))
+        html = ajax['html']
+        self.assertFalse(any(['This is a secret' in item for item in html]))
 
-class ConditionalModuleTest(unittest.TestCase):
 
+class ConditionalModuleXmlTest(unittest.TestCase):
+    """
+    Make sure ConditionalModule works, by loading data in from an XML-defined course.
+    """
     @staticmethod
     def get_system(load_error_modules=True):
         '''Get a dummy system'''
@@ -106,7 +210,7 @@ class ConditionalModuleTest(unittest.TestCase):
         html_expect = "{'ajax_url': 'courses/course_id/modx/a_location', 'element_id': 'i4x-HarvardX-ER22x-conditional-condone', 'id': 'i4x://HarvardX/ER22x/conditional/condone', 'depends': 'i4x-HarvardX-ER22x-problem-choiceprob'}"
         self.assertEqual(html, html_expect)
 
-        gdi =  module.get_display_items()
+        gdi = module.get_display_items()
         print "gdi=", gdi
 
         ajax = json.loads(module.handle_ajax('', ''))
@@ -121,3 +225,4 @@ class ConditionalModuleTest(unittest.TestCase):
         print "post-attempt ajax: ", ajax
         html = ajax['html']
         self.assertTrue(any(['This is a secret' in item for item in html]))
+