Merge pull request #5590 from cpennington/xblocks-with-children

Enable pure XBlocks to use children in edx-platform

common/djangoapps/xmodule_modifiers.py

@@ -7,6 +7,7 @@ import json
 import logging
 import static_replace
 import uuid
+import markupsafe
 
 from django.conf import settings
 from django.utils.timezone import UTC
@@ -71,7 +72,7 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer,
 
     data = {}
     data.update(extra_data)
-    css_classes = ['xblock', 'xblock-{}'.format(view)]
+    css_classes = ['xblock', 'xblock-{}'.format(markupsafe.escape(view))]
 
     if isinstance(block, (XModule, XModuleDescriptor)):
         if view in PREVIEW_VIEWS:
@@ -81,7 +82,7 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer,
             # The block is acting as an XModuleDescriptor
             css_classes.append('xmodule_edit')
 
-        css_classes.append('xmodule_' + class_name)
+        css_classes.append('xmodule_' + markupsafe.escape(class_name))
         data['type'] = block.js_module_name
         shim_xmodule_js(frag)
 
@@ -93,11 +94,14 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer,
         data['usage-id'] = usage_id_serializer(block.scope_ids.usage_id)
         data['request-token'] = request_token
 
+    if block.name:
+        data['name'] = block.name
+
     template_context = {
         'content': block.display_name if display_name_only else frag.content,
         'classes': css_classes,
         'display_name': block.display_name_with_default,
-        'data_attributes': u' '.join(u'data-{}="{}"'.format(key, value)
+        'data_attributes': u' '.join(u'data-{}="{}"'.format(markupsafe.escape(key), markupsafe.escape(value))
                                      for key, value in data.iteritems()),
     }
 

common/lib/xmodule/xmodule/x_module.py

@@ -191,6 +191,26 @@ class XModuleMixin(XBlockMixin):
         default=None
     )
 
+    def __init__(self, *args, **kwargs):
+        self.xmodule_runtime = None
+        super(XModuleMixin, self).__init__(*args, **kwargs)
+
+    @property
+    def runtime(self):
+        # Handle XModule backwards compatibility. If this is a pure
+        # XBlock, and it has an xmodule_runtime defined, then we're in
+        # an XModule context, not an XModuleDescriptor context,
+        # so we should use the xmodule_runtime (ModuleSystem) as the runtime.
+        if (not isinstance(self, (XModule, XModuleDescriptor)) and
+            self.xmodule_runtime is not None):
+            return self.xmodule_runtime
+        return self._runtime
+
+    @runtime.setter
+    def runtime(self, value):
+        self._runtime = value
+
+
     @property
     def system(self):
         """
@@ -1168,9 +1188,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # p
             return super(DescriptorSystem, self).render(block, view_name, context)
 
     def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False):
-        xmodule_runtime = getattr(block, 'xmodule_runtime', None)
-        if xmodule_runtime is not None:
-            return xmodule_runtime.handler_url(block, handler_name, suffix, query, thirdparty)
+        if block.xmodule_runtime is not None:
+            return block.xmodule_runtime.handler_url(block, handler_name, suffix, query, thirdparty)
         else:
             # Currently, Modulestore is responsible for instantiating DescriptorSystems
             # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem
@@ -1182,9 +1201,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # p
         """
         See :meth:`xblock.runtime.Runtime:local_resource_url` for documentation.
         """
-        xmodule_runtime = getattr(block, 'xmodule_runtime', None)
-        if xmodule_runtime is not None:
-            return xmodule_runtime.local_resource_url(block, uri)
+        if block.xmodule_runtime is not None:
+            return block.xmodule_runtime.local_resource_url(block, uri)
         else:
             # Currently, Modulestore is responsible for instantiating DescriptorSystems
             # This means that LMS/CMS don't have a way to define a subclass of DescriptorSystem
@@ -1202,9 +1220,8 @@ class DescriptorSystem(MetricsMixin, ConfigurableFragmentWrapper, Runtime):  # p
         """
         See :meth:`xblock.runtime.Runtime:publish` for documentation.
         """
-        xmodule_runtime = getattr(block, 'xmodule_runtime', None)
-        if xmodule_runtime is not None:
-            return xmodule_runtime.publish(block, event_type, event)
+        if block.xmodule_runtime is not None:
+            return block.xmodule_runtime.publish(block, event_type, event)
 
     def add_block_as_child_node(self, block, node):
         child = etree.SubElement(node, "unknown")

common/static/coffee/src/xblock/core.coffee

@@ -34,6 +34,7 @@
 
     block.element = element
     block.name = $element.data("name")
+    block.type = $element.data("block-type")
 
     $element.trigger("xblock-initialized")
     $element.data("initialized", true)

common/templates/xblock_wrapper.html

-<div class="${' '.join(classes)}" ${data_attributes}>
+<div class="${' '.join(classes) | n}" ${data_attributes}>
 % if js_pass_parameters:
   <script type="json/xblock-args" class="xblock_json_init_args">
     ${js_init_parameters}

common/test/acceptance/pages/xblock/acid.py

@@ -3,7 +3,7 @@ PageObjects related to the AcidBlock
 """
 
 from bok_choy.page_object import PageObject
-from bok_choy.promise import Promise
+from bok_choy.promise import EmptyPromise, BrokenPromise
 from .utils import wait_for_xblock_initialization
 
 
@@ -31,9 +31,22 @@ class AcidView(PageObject):
         # and then wait to make sure that the xblock has finished initializing.
         return (
             self.q(css='{} .acid-block'.format(self.context_selector)).present and
-            wait_for_xblock_initialization(self, self.context_selector)
+            wait_for_xblock_initialization(self, self.context_selector) and
+            self._ajax_finished()
         )
 
+    def _ajax_finished(self):
+        try:
+            EmptyPromise(
+                lambda: self.browser.execute_script("return jQuery.active") == 0,
+                "AcidBlock tests still running",
+                timeout=240
+            ).fulfill()
+        except BrokenPromise:
+            return False
+        else:
+            return True
+
     def test_passed(self, test_selector):
         """
         Return whether a particular :class:`.AcidBlock` test passed.

common/test/acceptance/tests/lms/test_lms_acid_xblock.py

@@ -80,7 +80,6 @@ class XBlockAcidNoChildTest(XBlockAcidBase):
             )
         ).install()
 
-    @skip('Flakey test, TE-401')
     def test_acid_block(self):
         super(XBlockAcidNoChildTest, self).test_acid_block()
 
@@ -113,10 +112,20 @@ class XBlockAcidChildTest(XBlockAcidBase):
             )
         ).install()
 
-    def validate_acid_block_view(self, acid_block):
-        super(XBlockAcidChildTest, self).validate_acid_block_view()
-        self.assertTrue(acid_block.child_tests_passed)
+    def validate_acid_parent_block_view(self, acid_parent_block):
+        super(XBlockAcidChildTest, self).validate_acid_block_view(acid_parent_block)
+        self.assertTrue(acid_parent_block.child_tests_passed)
 
-    @skip('This will fail until we fix support of children in pure XBlocks')
     def test_acid_block(self):
-        super(XBlockAcidChildTest, self).test_acid_block()
+        """
+        Verify that all expected acid block tests pass in the lms.
+        """
+
+        self.course_info_page.visit()
+        self.tab_nav.go_to_tab('Courseware')
+
+        acid_parent_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid_parent]')
+        self.validate_acid_parent_block_view(acid_parent_block)
+
+        acid_block = AcidView(self.browser, '.xblock-student_view[data-block-type=acid]')
+        self.validate_acid_block_view(acid_block)

common/test/acceptance/tests/studio/test_studio_acid_xblock.py

@@ -202,10 +202,8 @@ class XBlockAcidChildTest(XBlockAcidParentBase):
 
         self.user = course_fix.user
 
-    @skip('This will fail until we fix support of children in pure XBlocks')
     def test_acid_block_preview(self):
         super(XBlockAcidChildTest, self).test_acid_block_preview()
 
-    @skip('This will fail until we fix support of children in pure XBlocks')
     def test_acid_block_editor(self):
         super(XBlockAcidChildTest, self).test_acid_block_editor()

requirements/edx/github.txt

@@ -20,7 +20,7 @@
 -e git+https://github.com/pmitros/django-pyfs.git@d175715e0fe3367ec0f1ee429c242d603f6e8b10#egg=djpyfs
 
 # Our libraries:
--e git+https://github.com/edx/XBlock.git@81a6d713c98d4914af96a0ca624ee7fa4903625e#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@246811773c67a84fdb17614a8e9f7ec7b1890574#egg=XBlock
 -e git+https://github.com/edx/codejail.git@66dd5a45e5072666ff9a70c768576e9ffd1daa4b#egg=codejail
 -e git+https://github.com/edx/diff-cover.git@9a44ae21369662a7d06bfc5111875fc0d119e03b#egg=diff_cover
 -e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool
@@ -28,7 +28,7 @@
 -e git+https://github.com/edx/edx-analytics-data-api-client.git@0.1.0#egg=edx-analytics-data-api-client
 -e git+https://github.com/edx/bok-choy.git@4a259e3548a19e41cc39433caf68ea58d10a27ba#egg=bok_choy
 -e git+https://github.com/edx-solutions/django-splash.git@7579d052afcf474ece1239153cffe1c89935bc4f#egg=django-splash
--e git+https://github.com/edx/acid-block.git@459aff7b63db8f2c5decd1755706c1a64fb4ebb1#egg=acid-xblock
+-e git+https://github.com/edx/acid-block.git@df1a7f0cae46567c251d507b8c72168aed8ec042#egg=acid-xblock
 -e git+https://github.com/edx/edx-ora2.git@release-2014-09-18T16.00#egg=edx-ora2
 -e git+https://github.com/edx/opaque-keys.git@295d93170b2f6e57e3a2b9ba0a52087a4e8712c5#egg=opaque-keys
 -e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease