diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 538550b04b50..a2fa8240792f 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -160,8 +160,11 @@ def is_condition_satisfied(self): # 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)) + if module is not None: + # We do not want to log when module is None, and it is when requester + # does not have access to the requested required module. + 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) diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 8aae66b1ea44..17e06b0704c7 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -52,7 +52,7 @@ class ConditionalFactory(object): to allow for testing. """ @staticmethod - def create(system, source_is_error_module=False): + def create(system, source_is_error_module=False, source_visible_to_staff_only=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'. @@ -75,11 +75,13 @@ def create(system, source_is_error_module=False): source_descriptor = Mock(name='source_descriptor') source_descriptor.location = source_location + source_descriptor.visible_to_staff_only = source_visible_to_staff_only source_descriptor.runtime = descriptor_system source_descriptor.render = lambda view, context=None: descriptor_system.render(source_descriptor, view, context) # construct other descriptors: child_descriptor = Mock(name='child_descriptor') + child_descriptor.visible_to_staff_only = False child_descriptor._xmodule.student_view.return_value.content = u'
This is a secret
' child_descriptor.student_view = child_descriptor._xmodule.student_view child_descriptor.displayable_items.return_value = [child_descriptor] @@ -88,6 +90,12 @@ def create(system, source_is_error_module=False): child_descriptor.render = lambda view, context=None: descriptor_system.render(child_descriptor, view, context) child_descriptor.location = source_location.replace(category='html', name='child') + def visible_to_nonstaff_users(desc): + """ + Returns if the object is visible to nonstaff users. + """ + return not desc.visible_to_staff_only + def load_item(usage_id, for_parent=None): # pylint: disable=unused-argument """Test-only implementation of load_item that simply returns static xblocks.""" return { @@ -115,8 +123,12 @@ def load_item(usage_id, for_parent=None): # pylint: disable=unused-argument ScopeIds(None, None, cond_location, cond_location) ) cond_descriptor.xmodule_runtime = system - system.get_module = lambda desc: desc + system.get_module = lambda desc: desc if visible_to_nonstaff_users(desc) else None cond_descriptor.get_required_module_descriptors = Mock(return_value=[source_descriptor]) + cond_descriptor.required_modules = [ + system.get_module(descriptor) + for descriptor in cond_descriptor.get_required_module_descriptors() + ] # return dict: return {'cond_module': cond_descriptor, @@ -183,6 +195,19 @@ def test_error_as_source(self): html = ajax['html'] self.assertFalse(any(['This is a secret' in item for item in html])) + @patch('xmodule.conditional_module.log') + def test_conditional_with_staff_only_source_module(self, mock_log): + modules = ConditionalFactory.create( + self.test_system, + source_visible_to_staff_only=True, + ) + cond_module = modules['cond_module'] + cond_module.save() + cond_module.is_attempted = "false" + cond_module.handle_ajax('', '') + self.assertFalse(mock_log.warn.called) + self.assertIn(None, cond_module.required_modules) + class ConditionalModuleXmlTest(unittest.TestCase): """ diff --git a/lms/templates/conditional_module.html b/lms/templates/conditional_module.html index 6b6c1429eac8..bd47726c3aaf 100644 --- a/lms/templates/conditional_module.html +++ b/lms/templates/conditional_module.html @@ -1,7 +1,9 @@ -<% - +<%! from django.core.urlresolvers import reverse +from django.utils.translation import ugettext as _ +%> +<% def _message(reqm, message): return message.format(link="{url_name}".format( url = reverse('jump_to', kwargs=dict(course_id=reqm.course_id.to_deprecated_string(), @@ -9,7 +11,13 @@ url_name = reqm.display_name_with_default_escaped)) %> % if message: - % for reqm in module.required_modules: - - % endfor + % for reqm in module.required_modules: + % if reqm: + + % else: + + % endif + % endfor % endif