From 0aeec133fc2ecbe1e4cdb3d7c9f382cae2d809cd Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 8 Aug 2018 10:55:40 -0400 Subject: [PATCH] Render all parts of XBlock fragments in a Conditional Module [EDUCATOR-3262] --- .../lib/xmodule/xmodule/conditional_module.py | 6 +- .../xmodule/js/src/conditional/display.js | 121 +++++++++++++++++- .../xmodule/xmodule/tests/test_conditional.py | 23 ++-- .../xmodule/tests/test_conditional_logic.py | 4 +- 4 files changed, 132 insertions(+), 22 deletions(-) diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 1449b52bb9..7167a106fe 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -213,11 +213,11 @@ class ConditionalModule(ConditionalFields, XModule, StudioEditableModule): 'message': self.conditional_message} html = self.system.render_template('conditional_module.html', context) - return json.dumps({'html': [html], 'message': bool(self.conditional_message)}) + return json.dumps({'fragments': [{'content': html}], 'message': bool(self.conditional_message)}) - html = [child.render(STUDENT_VIEW).content for child in self.get_display_items()] + fragments = [child.render(STUDENT_VIEW).to_dict() for child in self.get_display_items()] - return json.dumps({'html': html}) + return json.dumps({'fragments': fragments}) def get_icon_class(self): new_class = 'other' diff --git a/common/lib/xmodule/xmodule/js/src/conditional/display.js b/common/lib/xmodule/xmodule/js/src/conditional/display.js index 4efbd37981..96d606b799 100644 --- a/common/lib/xmodule/xmodule/js/src/conditional/display.js +++ b/common/lib/xmodule/xmodule/js/src/conditional/display.js @@ -21,12 +21,11 @@ Conditional.prototype.render = function(element) { return $.postWithPrefix(this.url + "/conditional_get", (function(_this) { return function(response) { - var i, j, len, parentEl, parentId, ref; + var i, len, parentEl, parentId, ref; _this.el.html(''); - ref = response.html; - for (j = 0, len = ref.length; j < len; j++) { - i = ref[j]; - _this.el.append(i); + fragments = response.fragments; + for (i = 0, len = fragments.length; i < len; i++) { + _this.renderXBlockFragment(fragments[i]); } parentEl = $(element).parent(); parentId = parentEl.attr('id'); @@ -47,12 +46,122 @@ /* The children are rendered with a new request, so they have a different request-token. Use that token instead of @requestToken by simply not passing a token into initializeBlocks. - */ + */ return XBlock.initializeBlocks(_this.el); }; })(this)); }; + + /** + * Renders an xblock fragment into the specified element. The fragment has two attributes: + * html: the HTML to be rendered + * resources: any JavaScript or CSS resources that the HTML depends upon + * Note that the XBlock is rendered asynchronously, and so a promise is returned that + * represents this process. + * @param fragment The fragment returned from the xblock_handler + * @returns {Promise} A promise representing the rendering process + */ + Conditional.prototype.renderXBlockFragment = function(fragment) { + var html = fragment.content, + resources = fragment.resources || [], + blockView = this, + element = this.el; + // Render the HTML first as the scripts might depend upon it, and then + // asynchronously add the resources to the page. Any errors that are thrown + // by included scripts are logged to the console but are then ignored assuming + // that at least the rendered HTML will be in place. + try { + return this.addXBlockFragmentResources(resources).done(function () { + // We give XBlock fragments free-reign to add javascript and CSS to + // to the page, so XSS escaping doesn't matter much in this context + // xss-lint: disable=javascript-jquery-append + element.append(html); + }); + } catch (e) { + console.error(e, e.stack); + return $.Deferred().resolve(); + } + }; + + /** + * Dynamically loads all of an XBlock's dependent resources. This is an asynchronous + * process so a promise is returned. + * @param resources The resources to be rendered + * @returns {Promise} A promise representing the rendering process + */ + Conditional.prototype.addXBlockFragmentResources = function(resources) { + var self = this, + applyResource, + numResources, + deferred; + numResources = resources.length; + deferred = $.Deferred(); + applyResource = function (index) { + var hash, resource, value, promise; + if (index >= numResources) { + deferred.resolve(); + return; + } + resource = resources[index]; + if (!window.loadedXBlockResources) { + window.loadedXBlockResources = []; + } + if (_.indexOf(window.loadedXBlockResources, resource) < 0) { + promise = self.loadResource(resource); + window.loadedXBlockResources.push(resource); + promise.done(function () { + applyResource(index + 1); + }).fail(function () { + deferred.reject(); + }); + } else { + applyResource(index + 1); + } + }; + applyResource(0); + return deferred.promise(); + }; + + /** + * Loads the specified resource into the page. + * @param resource The resource to be loaded. + * @returns {Promise} A promise representing the loading of the resource. + */ + Conditional.prototype.loadResource = function(resource) { + // We give XBlock fragments free-reign to add javascript and CSS to + // to the page, so XSS escaping doesn't matter much in this context + var $head = $('head'), + mimetype = resource.mimetype, + kind = resource.kind, + placement = resource.placement, + data = resource.data; + if (mimetype === 'text/css') { + if (kind === 'text') { + // xss-lint: disable=javascript-jquery-append,javascript-concat-html + $head.append("'); + } else if (kind === 'url') { + // xss-lint: disable=javascript-jquery-append,javascript-concat-html + $head.append(""); + } + } else if (mimetype === 'application/javascript') { + if (kind === 'text') { + // xss-lint: disable=javascript-jquery-append,javascript-concat-html + $head.append(''); + } else if (kind === 'url') { + // This is a dependency loaded from the LMS (not ideal) + return ViewUtils.loadJavaScript(data); + } + } else if (mimetype === 'text/html') { + if (placement === 'head') { + // xss-lint: disable=javascript-jquery-append + $head.append(data); + } + } + // Return an already resolved promise for synchronous updates + return $.Deferred().resolve().promise(); + }; + return Conditional; })(); diff --git a/common/lib/xmodule/xmodule/tests/test_conditional.py b/common/lib/xmodule/xmodule/tests/test_conditional.py index 2d533d7fe2..7653486207 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional.py @@ -17,6 +17,7 @@ from xmodule.tests import DATA_DIR, get_test_system, get_test_descriptor_system from xmodule.tests.xml import factories as xml, XModuleXmlImportTest from xmodule.validation import StudioValidationMessage from xmodule.x_module import STUDENT_VIEW, AUTHOR_VIEW +from web_fragments.fragment import Fragment ORG = 'test_org' COURSE = 'conditional' # name of directory with course data @@ -85,7 +86,7 @@ class ConditionalFactory(object): # 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._xmodule.student_view.return_value = Fragment(content=u'

This is a secret

') child_descriptor.student_view = child_descriptor._xmodule.student_view child_descriptor.displayable_items.return_value = [child_descriptor] child_descriptor.runtime = descriptor_system @@ -178,16 +179,16 @@ class ConditionalModuleBasicTest(unittest.TestCase): modules['source_module'].is_attempted = "false" ajax = json.loads(modules['cond_module'].handle_ajax('', '')) print "ajax: ", ajax - html = ajax['html'] - self.assertFalse(any(['This is a secret' in item for item in html])) + fragments = ajax['fragments'] + self.assertFalse(any(['This is a secret' in item['content'] for item in fragments])) # now change state of the capa problem to make it completed modules['source_module'].is_attempted = "true" ajax = json.loads(modules['cond_module'].handle_ajax('', '')) modules['cond_module'].save() print "post-attempt ajax: ", ajax - html = ajax['html'] - self.assertTrue(any(['This is a secret' in item for item in html])) + fragments = ajax['fragments'] + self.assertTrue(any(['This is a secret' in item['content'] for item in fragments])) def test_error_as_source(self): ''' @@ -197,8 +198,8 @@ class ConditionalModuleBasicTest(unittest.TestCase): modules = ConditionalFactory.create(self.test_system, source_is_error_module=True) 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])) + fragments = ajax['fragments'] + self.assertFalse(any(['This is a secret' in item['content'] for item in fragments])) @patch('xmodule.conditional_module.log') def test_conditional_with_staff_only_source_module(self, mock_log): @@ -292,8 +293,8 @@ class ConditionalModuleXmlTest(unittest.TestCase): ajax = json.loads(module.handle_ajax('', '')) module.save() print "ajax: ", ajax - html = ajax['html'] - self.assertFalse(any(['This is a secret' in item for item in html])) + fragments = ajax['fragments'] + self.assertFalse(any(['This is a secret' in item['content'] for item in fragments])) # Now change state of the capa problem to make it completed inner_module = inner_get_module(location.replace(category="problem", name='choiceprob')) @@ -304,8 +305,8 @@ class ConditionalModuleXmlTest(unittest.TestCase): ajax = json.loads(module.handle_ajax('', '')) module.save() print "post-attempt ajax: ", ajax - html = ajax['html'] - self.assertTrue(any(['This is a secret' in item for item in html])) + fragments = ajax['fragments'] + self.assertTrue(any(['This is a secret' in item['content'] for item in fragments])) def test_conditional_module_with_empty_sources_list(self): """ diff --git a/common/lib/xmodule/xmodule/tests/test_conditional_logic.py b/common/lib/xmodule/xmodule/tests/test_conditional_logic.py index 855b9eac82..03fbe72d98 100644 --- a/common/lib/xmodule/xmodule/tests/test_conditional_logic.py +++ b/common/lib/xmodule/xmodule/tests/test_conditional_logic.py @@ -17,6 +17,6 @@ class ConditionalModuleTest(LogicTest): self.xmodule.descriptor.get_children = lambda: [] response = self.ajax_request('No', {}) - html = response['html'] + fragments = response['fragments'] - self.assertEqual(html, []) + self.assertEqual(fragments, [])