Add ui in studio for conditional_module.

common/lib/xmodule/xmodule/conditional_module.py

@@ -10,20 +10,68 @@ from pkg_resources import resource_string
 
 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 location IDs of the components whose attributes are used to determine whether a learner is shown "
+               "the content of this conditional module."),
+        scope=Scope.content
+    )
+
+    conditional_attr = String(
+        display_name=_("Conditional Attribute"),
+        help=_("The attribute from the course component used to determine 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 of the conditional attribute that must be true for a learner to be shown "
+               "the content of this conditional module."),
+        scope=Scope.content,
+        default='True'
+    )
+
+    conditional_message = String(
+        display_name=_("Blocked Content Message"),
+        help=_("The message learners see when not all conditions are met for this block. "
+               "You can use the {link} variable to give learners a direct link to the required module."),
+        scope=Scope.content,
+        default=_('{link} must be attempted before this will become visible.')
+    )
+
+
+class ConditionalModule(ConditionalFields, XModule, StudioEditableModule):
     """
     Blocks child module from showing unless certain conditions are met.
 
@@ -95,27 +143,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 +166,7 @@ class ConditionalModule(ConditionalFields, XModule):
                 if callable(attr):
                     attr = attr()
 
-                if xml_value != str(attr):
+                if self.conditional_value != str(attr):
                     break
             else:
                 return True
@@ -147,18 +183,31 @@ class ConditionalModule(ConditionalFields, XModule):
             '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 +226,16 @@ class ConditionalModule(ConditionalFields, XModule):
                 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 +254,7 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
         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 +291,14 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
     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 +313,11 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
                     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 +334,36 @@ class ConditionalDescriptor(ConditionalFields, SequenceDescriptor):
         # 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

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) =>

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

@@ -2,6 +2,7 @@ import json
 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 opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
 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 @@ class DummySystem(ImportSystem):
         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 @@ class ConditionalFactory(object):
         cond_location = Location("edX", "conditional_test", "test_run", "conditional", "SampleConditional", None)
         field_data = DictFieldData({
             'data': '<conditional/>',
+            'condional_attr': 'attempted',
+            'conditional_value': 'true',
             'xml_attributes': {'attempted': 'true'},
             'children': [child_descriptor.location],
         })
@@ -146,9 +157,9 @@ class ConditionalModuleBasicTest(unittest.TestCase):
 
     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 @@ class ConditionalModuleBasicTest(unittest.TestCase):
         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,105 @@ class ConditionalModuleXmlTest(unittest.TestCase):
             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 = '<conditional attempted="false"></conditional>'
+        xml_object = etree.XML(root)
+        definition = ConditionalDescriptor.definition_from_xml(xml_object, Mock())[0]
+        expected_definition = {
+            'show_tag_list': [],
+            'condional_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': '{link} must be attempted before this will become visible.',
+            '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")