From 47a3172bf1fc379b8c84a23a9829eb5179d1a036 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 8 Oct 2014 16:33:26 -0400 Subject: [PATCH 1/5] Enable parent/child tests using the acid XBlock. Also fix those tests to correctly wait until the ajax portions have finished before evaluating the page contents. --- common/test/acceptance/pages/xblock/acid.py | 17 +++++++++++++-- .../tests/lms/test_lms_acid_xblock.py | 21 +++++++++++++------ .../tests/studio/test_studio_acid_xblock.py | 2 -- requirements/edx/github.txt | 2 +- 4 files changed, 31 insertions(+), 11 deletions(-) diff --git a/common/test/acceptance/pages/xblock/acid.py b/common/test/acceptance/pages/xblock/acid.py index 55c29124c4..a05d689f88 100644 --- a/common/test/acceptance/pages/xblock/acid.py +++ b/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. diff --git a/common/test/acceptance/tests/lms/test_lms_acid_xblock.py b/common/test/acceptance/tests/lms/test_lms_acid_xblock.py index 4d3958d400..2aaacdb486 100644 --- a/common/test/acceptance/tests/lms/test_lms_acid_xblock.py +++ b/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) diff --git a/common/test/acceptance/tests/studio/test_studio_acid_xblock.py b/common/test/acceptance/tests/studio/test_studio_acid_xblock.py index 6d04fa861d..1d6b54872d 100644 --- a/common/test/acceptance/tests/studio/test_studio_acid_xblock.py +++ b/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() diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index aed5c5ccc6..c12c8293d9 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -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 From 4a43427c6d36ad7cdc315508a892166a59f12d44 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 15 Oct 2014 11:34:54 -0400 Subject: [PATCH 2/5] Add block.name to xblock javascript frontend --- common/djangoapps/xmodule_modifiers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 6edb621317..ee8f6a7a78 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -93,6 +93,9 @@ 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, From 41f3c040c8cef09c99bbaa837bd7398246a8cd40 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 15 Oct 2014 11:35:34 -0400 Subject: [PATCH 3/5] Add block type to the available attributes in the xblock javascript frontend --- common/static/coffee/src/xblock/core.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/common/static/coffee/src/xblock/core.coffee b/common/static/coffee/src/xblock/core.coffee index 885d9af599..bafcff97c9 100644 --- a/common/static/coffee/src/xblock/core.coffee +++ b/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) From 4d653d5359e466b176ecef46f9efef11cb2d7e60 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 15 Oct 2014 11:37:43 -0400 Subject: [PATCH 4/5] Set xmodule_runtime on all XModule-like things (including pure XBlocks using the XModuleMixin) Expose xmodule_runtime as .runtime if it's set (otherwise, fall back to the supplied runtime). This causes all blocks to act like XModules when they have a ModuleSystem, and like XModuleDescriptors if they only have a DescriptorSystem. --- common/lib/xmodule/xmodule/x_module.py | 35 +++++++++++++++++++------- requirements/edx/github.txt | 2 +- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 92776eabd8..709d81046e 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/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") diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index c12c8293d9..3b40143eef 100644 --- a/requirements/edx/github.txt +++ b/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@v0.5.0#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.1.5#egg=js_test_tool From fee681be683da42037141310313cb04e2b00cc53 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 15 Oct 2014 14:00:54 -0400 Subject: [PATCH 5/5] Escape xblock wrapper data attributes and css classes for safe html --- common/djangoapps/xmodule_modifiers.py | 7 ++++--- common/templates/xblock_wrapper.html | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index ee8f6a7a78..2349f1e3be 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/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) @@ -100,7 +101,7 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, '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()), } diff --git a/common/templates/xblock_wrapper.html b/common/templates/xblock_wrapper.html index 2bb3ba97e9..74574907df 100644 --- a/common/templates/xblock_wrapper.html +++ b/common/templates/xblock_wrapper.html @@ -1,4 +1,4 @@ -
+
% if js_pass_parameters: