diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index bc5384cfa801..4fb13e8519e9 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -10,20 +10,70 @@ from xmodule.x_module import XModule, STUDENT_VIEW from xmodule.seq_module import SequenceDescriptor -from xblock.fields import Scope, ReferenceList +from xmodule.studio_editable import StudioEditableModule, StudioEditableDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.validation import StudioValidation, StudioValidationMessage +from xblock.fields import Scope, ReferenceList, String +from xblock.fragment import Fragment log = logging.getLogger('edx.' + __name__) +# Make '_' a no-op so we can scrape strings +_ = lambda text: text + class ConditionalFields(object): has_children = True - show_tag_list = ReferenceList(help="List of urls of children that are references to external modules", scope=Scope.content) - sources_list = ReferenceList(help="List of sources upon which this module is conditional", scope=Scope.content) - - -class ConditionalModule(ConditionalFields, XModule): + display_name = String( + display_name=_("Display Name"), + help=_("This name appears in the horizontal navigation at the top of the page."), + scope=Scope.settings, + default=_('Conditional') + ) + + show_tag_list = ReferenceList( + help=_("List of urls of children that are references to external modules"), + scope=Scope.content + ) + + sources_list = ReferenceList( + display_name=_("Source Components"), + help=_("The component location IDs of all source components that are used to determine whether a learner is " + "shown the content of this conditional module. Copy the component location ID of a component from its " + "Settings dialog in Studio."), + scope=Scope.content + ) + + conditional_attr = String( + display_name=_("Conditional Attribute"), + help=_("The attribute of the source components that determines whether a learner is shown the content of this " + "conditional module."), + scope=Scope.content, + default='correct', + values=lambda: [{'display_name': xml_attr, 'value': xml_attr} + for xml_attr in ConditionalModule.conditions_map.keys()] + ) + + conditional_value = String( + display_name=_("Conditional Value"), + help=_("The value that the conditional attribute of the source components must match before a learner is shown " + "the content of this conditional module."), + scope=Scope.content, + default='True' + ) + + conditional_message = String( + display_name=_("Blocked Content Message"), + help=_("The message that is shown to learners when not all conditions are met to show the content of this " + "conditional module. Include {link} in the text of your message to give learners a direct link to " + "required units. For example, 'You must complete {link} before you can access this unit'."), + scope=Scope.content, + default=_('You must complete {link} before you can access this unit.') + ) + + +class ConditionalModule(ConditionalFields, XModule, StudioEditableModule): """ Blocks child module from showing unless certain conditions are met. @@ -95,27 +145,15 @@ class ConditionalModule(ConditionalFields, XModule): 'voted': 'voted' # poll_question attr } - def _get_condition(self): - # Get first valid condition. - for xml_attr, attr_name in self.conditions_map.iteritems(): - xml_value = self.descriptor.xml_attributes.get(xml_attr) - if xml_value: - return xml_value, attr_name - raise Exception( - 'Error in conditional module: no known conditional found in {!r}'.format( - self.descriptor.xml_attributes.keys() - ) - ) - @lazy def required_modules(self): return [self.system.get_module(descriptor) for descriptor in self.descriptor.get_required_module_descriptors()] def is_condition_satisfied(self): - xml_value, attr_name = self._get_condition() + attr_name = self.conditions_map[self.conditional_attr] - if xml_value and self.required_modules: + if self.conditional_value and self.required_modules: for module in self.required_modules: if not hasattr(module, attr_name): # We don't throw an exception here because it is possible for @@ -130,7 +168,7 @@ def is_condition_satisfied(self): if callable(attr): attr = attr() - if xml_value != str(attr): + if self.conditional_value != str(attr): break else: return True @@ -147,18 +185,31 @@ def get_html(self): 'depends': ';'.join(self.required_html_ids) }) + def author_view(self, context): + """ + Renders the Studio preview by rendering each child so that they can all be seen and edited. + """ + fragment = Fragment() + root_xblock = context.get('root_xblock') + is_root = root_xblock and root_xblock.location == self.location + if is_root: + # User has clicked the "View" link. Show a preview of all possible children: + self.render_children(context, fragment, can_reorder=True, can_add=True) + # else: When shown on a unit page, don't show any sort of preview - + # just the status of this block in the validation area. + + return fragment + def handle_ajax(self, _dispatch, _data): """This is called by courseware.moduleodule_render, to handle an AJAX call. """ if not self.is_condition_satisfied(): - defmsg = "{link} must be attempted before this will become visible." - message = self.descriptor.xml_attributes.get('message', defmsg) context = {'module': self, - 'message': message} + 'message': self.conditional_message} html = self.system.render_template('conditional_module.html', context) - return json.dumps({'html': [html], 'message': bool(message)}) + return json.dumps({'html': [html], 'message': bool(self.conditional_message)}) html = [child.render(STUDENT_VIEW).content for child in self.get_display_items()] @@ -177,8 +228,16 @@ def get_icon_class(self): new_class = c return new_class + def validate(self): + """ + Message for either error or warning validation message/s. + + Returns message and type. Priority given to error type message. + """ + return self.descriptor.validate() -class ConditionalDescriptor(ConditionalFields, SequenceDescriptor): + +class ConditionalDescriptor(ConditionalFields, SequenceDescriptor, StudioEditableDescriptor): """Descriptor for conditional xmodule.""" _tag_name = 'conditional' @@ -197,6 +256,7 @@ def __init__(self, *args, **kwargs): Create an instance of the conditional module. """ super(ConditionalDescriptor, self).__init__(*args, **kwargs) + # Convert sources xml_attribute to a ReferenceList field type so Location/Locator # substitution can be done. if not self.sources_list: @@ -233,6 +293,14 @@ def get_required_module_descriptors(self): def definition_from_xml(cls, xml_object, system): children = [] show_tag_list = [] + definition = {} + for conditional_attr in ConditionalModule.conditions_map.iterkeys(): + conditional_value = xml_object.get(conditional_attr) + if conditional_value is not None: + definition.update({ + 'conditional_attr': conditional_attr, + 'conditional_value': str(conditional_value), + }) for child in xml_object: if child.tag == 'show': locations = ConditionalDescriptor.parse_sources(child) @@ -247,7 +315,11 @@ def definition_from_xml(cls, xml_object, system): msg = "Unable to load child when parsing Conditional." log.exception(msg) system.error_tracker(msg) - return {'show_tag_list': show_tag_list}, children + definition.update({ + 'show_tag_list': show_tag_list, + 'conditional_message': xml_object.get('message', '') + }) + return definition, children def definition_to_xml(self, resource_fs): xml_object = etree.Element(self._tag_name) @@ -264,4 +336,36 @@ def definition_to_xml(self, resource_fs): # Locations may have been changed to Locators. stringified_sources_list = map(lambda loc: loc.to_deprecated_string(), self.sources_list) self.xml_attributes['sources'] = ';'.join(stringified_sources_list) + self.xml_attributes[self.conditional_attr] = self.conditional_value + self.xml_attributes['message'] = self.conditional_message return xml_object + + def validate(self): + validation = super(ConditionalDescriptor, self).validate() + if not self.sources_list: + conditional_validation = StudioValidation(self.location) + conditional_validation.add( + StudioValidationMessage( + StudioValidationMessage.NOT_CONFIGURED, + _(u"This component has no source components configured yet."), + action_class='edit-button', + action_label=_(u"Configure list of sources") + ) + ) + validation = StudioValidation.copy(validation) + validation.summary = conditional_validation.messages[0] + return validation + + @property + def non_editable_metadata_fields(self): + non_editable_fields = super(ConditionalDescriptor, self).non_editable_metadata_fields + non_editable_fields.extend([ + ConditionalDescriptor.due, + ConditionalDescriptor.is_practice_exam, + ConditionalDescriptor.is_proctored_enabled, + ConditionalDescriptor.is_time_limited, + ConditionalDescriptor.default_time_limit_minutes, + ConditionalDescriptor.show_tag_list, + ConditionalDescriptor.exam_review_rules, + ]) + return non_editable_fields diff --git a/common/lib/xmodule/xmodule/js/src/conditional/display.coffee b/common/lib/xmodule/xmodule/js/src/conditional/display.coffee index c8c2ce97ac47..f726fc872967 100644 --- a/common/lib/xmodule/xmodule/js/src/conditional/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/conditional/display.coffee @@ -11,7 +11,8 @@ class @Conditional return @url = @el.data('url') - @render(element) + if @url + @render(element) render: (element) -> $.postWithPrefix "#{@url}/conditional_get", (response) => diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index d392de182987..8aae66b1ea44 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -2,6 +2,7 @@ import unittest from fs.memoryfs import MemoryFS +from lxml import etree from mock import Mock, patch from xblock.field_data import DictFieldData @@ -11,8 +12,9 @@ from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, CourseLocationManager from xmodule.conditional_module import ConditionalDescriptor from xmodule.tests import DATA_DIR, get_test_system, get_test_descriptor_system -from xmodule.x_module import STUDENT_VIEW - +from xmodule.tests.xml import factories as xml, XModuleXmlImportTest +from xmodule.validation import StudioValidationMessage +from xmodule.x_module import STUDENT_VIEW, AUTHOR_VIEW ORG = 'test_org' COURSE = 'conditional' # name of directory with course data @@ -37,6 +39,13 @@ def render_template(self, template, context): raise Exception("Shouldn't be called") +class ConditionalModuleFactory(xml.XmlImportFactory): + """ + Factory for generating ConditionalModule for testing purposes + """ + tag = 'conditional' + + class ConditionalFactory(object): """ A helper class to create a conditional module and associated source and child modules @@ -94,6 +103,8 @@ def load_item(usage_id, for_parent=None): # pylint: disable=unused-argument cond_location = Location("edX", "conditional_test", "test_run", "conditional", "SampleConditional", None) field_data = DictFieldData({ 'data': '', + 'conditional_attr': 'attempted', + 'conditional_value': 'true', 'xml_attributes': {'attempted': 'true'}, 'children': [child_descriptor.location], }) @@ -146,9 +157,9 @@ def test_get_html(self): def test_handle_ajax(self): modules = ConditionalFactory.create(self.test_system) + modules['cond_module'].save() modules['source_module'].is_attempted = "false" ajax = json.loads(modules['cond_module'].handle_ajax('', '')) - modules['cond_module'].save() print "ajax: ", ajax html = ajax['html'] self.assertFalse(any(['This is a secret' in item for item in html])) @@ -167,8 +178,8 @@ def test_error_as_source(self): 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('', '')) modules['cond_module'].save() + ajax = json.loads(modules['cond_module'].handle_ajax('', '')) html = ajax['html'] self.assertFalse(any(['This is a secret' in item for item in html])) @@ -304,3 +315,106 @@ def test_conditional_module_parse_sources(self): conditional.parse_sources(conditional.xml_attributes), ['i4x://HarvardX/ER22x/poll_question/T15_poll', 'i4x://HarvardX/ER22x/poll_question/T16_poll'] ) + + def test_conditional_module_parse_attr_values(self): + root = '' + xml_object = etree.XML(root) + definition = ConditionalDescriptor.definition_from_xml(xml_object, Mock())[0] + expected_definition = { + 'show_tag_list': [], + 'conditional_attr': 'attempted', + 'conditional_value': 'false', + 'conditional_message': '' + } + + self.assertEqual(definition, expected_definition) + + def test_presence_attributes_in_xml_attributes(self): + modules = ConditionalFactory.create(self.test_system) + modules['cond_module'].save() + modules['cond_module'].definition_to_xml(Mock()) + expected_xml_attributes = { + 'attempted': 'true', + 'message': 'You must complete {link} before you can access this unit.', + 'sources': '' + } + self.assertDictEqual(modules['cond_module'].xml_attributes, expected_xml_attributes) + + +class ConditionalModuleStudioTest(XModuleXmlImportTest): + """ + Unit tests for how conditional test interacts with Studio. + """ + + def setUp(self): + super(ConditionalModuleStudioTest, self).setUp() + course = xml.CourseFactory.build() + sequence = xml.SequenceFactory.build(parent=course) + conditional = ConditionalModuleFactory( + parent=sequence, + attribs={ + 'group_id_to_child': '{"0": "i4x://edX/xml_test_course/html/conditional_0"}' + } + ) + xml.HtmlFactory(parent=conditional, url_name='conditional_0', text='This is a secret HTML') + + self.course = self.process_xml(course) + self.sequence = self.course.get_children()[0] + self.conditional = self.sequence.get_children()[0] + + self.module_system = get_test_system() + self.module_system.descriptor_runtime = self.course._runtime # pylint: disable=protected-access + + user = Mock(username='ma', email='ma@edx.org', is_staff=False, is_active=True) + self.conditional.bind_for_student( + self.module_system, + user.id + ) + + def test_render_author_view(self,): + """ + Test the rendering of the Studio author view. + """ + + def create_studio_context(root_xblock, is_unit_page): + """ + Context for rendering the studio "author_view". + """ + return { + 'reorderable_items': set(), + 'root_xblock': root_xblock, + 'is_unit_page': is_unit_page + } + + context = create_studio_context(self.conditional, False) + html = self.module_system.render(self.conditional, AUTHOR_VIEW, context).content + self.assertIn('This is a secret HTML', html) + + context = create_studio_context(self.sequence, True) + html = self.module_system.render(self.conditional, AUTHOR_VIEW, context).content + self.assertNotIn('This is a secret HTML', html) + + def test_non_editable_settings(self): + """ + Test the settings that are marked as "non-editable". + """ + non_editable_metadata_fields = self.conditional.non_editable_metadata_fields + self.assertIn(ConditionalDescriptor.due, non_editable_metadata_fields) + self.assertIn(ConditionalDescriptor.is_practice_exam, non_editable_metadata_fields) + self.assertIn(ConditionalDescriptor.is_time_limited, non_editable_metadata_fields) + self.assertIn(ConditionalDescriptor.default_time_limit_minutes, non_editable_metadata_fields) + self.assertIn(ConditionalDescriptor.show_tag_list, non_editable_metadata_fields) + + def test_validation_messages(self): + """ + Test the validation message for a correctly configured conditional. + """ + self.conditional.sources_list = None + validation = self.conditional.validate() + self.assertEqual( + validation.summary.text, + u"This component has no source components configured yet." + ) + self.assertEqual(validation.summary.type, StudioValidationMessage.NOT_CONFIGURED) + self.assertEqual(validation.summary.action_class, 'edit-button') + self.assertEqual(validation.summary.action_label, u"Configure list of sources") diff --git a/common/test/acceptance/tests/lms/test_conditional.py b/common/test/acceptance/tests/lms/test_conditional.py index 3b7cf2afe2e3..49dfba9462e1 100644 --- a/common/test/acceptance/tests/lms/test_conditional.py +++ b/common/test/acceptance/tests/lms/test_conditional.py @@ -47,8 +47,9 @@ def install_course_fixture(self, block_type='problem'): course_fixture.install() # Construct conditional block - conditional_metadata = {} source_block = None + conditional_attr = None + conditional_value = None if block_type == 'problem': problem_factory = StringResponseXMLFactory() problem_xml = problem_factory.build_xml( @@ -57,12 +58,9 @@ def install_course_fixture(self, block_type='problem'): answer='correct string', ), problem = XBlockFixtureDesc('problem', 'Test Problem', data=problem_xml[0]) - conditional_metadata = { - 'xml_attributes': { - 'attempted': 'True' - } - } source_block = problem + conditional_attr = 'attempted' + conditional_value = 'True' elif block_type == 'poll': poll = XBlockFixtureDesc( 'poll_question', @@ -73,11 +71,8 @@ def install_course_fixture(self, block_type='problem'): {'id': 'no', 'text': 'Of course not!'} ], ) - conditional_metadata = { - 'xml_attributes': { - 'poll_answer': 'yes' - } - } + conditional_attr = 'poll_answer' + conditional_value = 'yes' source_block = poll else: raise NotImplementedError() @@ -87,8 +82,9 @@ def install_course_fixture(self, block_type='problem'): conditional = XBlockFixtureDesc( 'conditional', 'Test Conditional', - metadata=conditional_metadata, sources_list=[source_block.locator], + conditional_attr=conditional_attr, + conditional_value=conditional_value ) result_block = XBlockFixtureDesc( 'html', 'Conditional Contents',