From ab011314eff48b6a308f57dcbb3989d3eaffd53a Mon Sep 17 00:00:00 2001
From: bradmerlin <melinbrad@gmail.com>
Date: Mon, 28 Aug 2017 17:23:17 +0200
Subject: [PATCH] Add student_view_data to HTML XBlock
to allow the HTML to be downloadable via the Course Blocks API.
Feature flag ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA must be set to enable this feature.
---
common/lib/xmodule/xmodule/html_module.py | 17 +++++-
.../xmodule/xmodule/tests/test_html_module.py | 59 ++++++++++++++++++-
.../course_api/blocks/tests/test_views.py | 2 +-
.../transformers/tests/test_student_view.py | 2 +-
lms/envs/common.py | 3 +
5 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 02063444c05..5f569e7fe19 100644
--- a/common/lib/xmodule/xmodule/html_module.py
+++ b/common/lib/xmodule/xmodule/html_module.py
@@ -6,6 +6,7 @@ import sys
import textwrap
from datetime import datetime
+from django.conf import settings
from fs.errors import ResourceNotFoundError
from lxml import etree
from path import Path as path
@@ -69,6 +70,8 @@ class HtmlBlock(object):
scope=Scope.settings
)
+ ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA = 'ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA'
+
@XBlock.supports("multi_device")
def student_view(self, _context):
"""
@@ -76,13 +79,25 @@ class HtmlBlock(object):
"""
return Fragment(self.get_html())
+ def student_view_data(self, context=None): # pylint: disable=unused-argument
+ """
+ Return a JSON representation of the student_view of this XBlock.
+ """
+ if getattr(settings, 'FEATURES', {}).get(self.ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA, False):
+ return {'enabled': True, 'html': self.get_html()}
+ else:
+ return {
+ 'enabled': False,
+ 'message': 'To enable, set FEATURES["{}"]'.format(self.ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA)
+ }
+
def get_html(self):
""" Returns html required for rendering XModule. """
# When we switch this to an XBlock, we can merge this with student_view,
# but for now the XModule mixin requires that this method be defined.
# pylint: disable=no-member
- if self.system.anonymous_student_id:
+ if self.data is not None and getattr(self.system, 'anonymous_student_id', None) is not None:
return self.data.replace("%%USER_ID%%", self.system.anonymous_student_id)
return self.data
diff --git a/common/lib/xmodule/xmodule/tests/test_html_module.py b/common/lib/xmodule/xmodule/tests/test_html_module.py
index ec2413907cd..318d9df440e 100644
--- a/common/lib/xmodule/xmodule/tests/test_html_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_html_module.py
@@ -1,6 +1,9 @@
import unittest
-
from mock import Mock
+import ddt
+
+from django.test.utils import override_settings
+
from opaque_keys.edx.locator import CourseLocator
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
@@ -24,6 +27,60 @@ def instantiate_descriptor(**field_data):
)
+@ddt.ddt
+class HtmlModuleCourseApiTestCase(unittest.TestCase):
+ """
+ Test the HTML XModule's student_view_data method.
+ """
+
+ @ddt.data(
+ dict(),
+ dict(FEATURES={}),
+ dict(FEATURES=dict(ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA=False))
+ )
+ def test_disabled(self, settings):
+ """
+ Ensure that student_view_data does not return html if the ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA feature flag
+ is not set.
+ """
+ descriptor = Mock()
+ field_data = DictFieldData({'data': '<h1>Some HTML</h1>'})
+ module_system = get_test_system()
+ module = HtmlModule(descriptor, module_system, field_data, Mock())
+
+ with override_settings(**settings):
+ self.assertEqual(module.student_view_data(), dict(
+ enabled=False,
+ message='To enable, set FEATURES["ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA"]',
+ ))
+
+ @ddt.data(
+ '<h1>Some content</h1>', # Valid HTML
+ '',
+ None,
+ '<h1>Some content</h', # Invalid HTML
+ '<script>alert()</script>', # Does not escape tags
+ '<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7">', # Images allowed
+ 'short string ' * 100, # May contain long strings
+ )
+ @override_settings(FEATURES=dict(ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA=True))
+ def test_common_values(self, html):
+ """
+ Ensure that student_view_data will return HTML data when enabled,
+ can handle likely input,
+ and doesn't modify the HTML in any way.
+
+ This means that it does NOT protect against XSS, escape HTML tags, etc.
+
+ Note that the %%USER_ID%% substitution is tested below.
+ """
+ descriptor = Mock()
+ field_data = DictFieldData({'data': html})
+ module_system = get_test_system()
+ module = HtmlModule(descriptor, module_system, field_data, Mock())
+ self.assertEqual(module.student_view_data(), dict(enabled=True, html=html))
+
+
class HtmlModuleSubstitutionTestCase(unittest.TestCase):
descriptor = Mock()
diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py
index 5c518fa7e27..70a6e491820 100644
--- a/lms/djangoapps/course_api/blocks/tests/test_views.py
+++ b/lms/djangoapps/course_api/blocks/tests/test_views.py
@@ -22,7 +22,7 @@ class TestBlocksView(SharedModuleStoreTestCase):
Test class for BlocksView
"""
requested_fields = ['graded', 'format', 'student_view_multi_device', 'children', 'not_a_field', 'due']
- BLOCK_TYPES_WITH_STUDENT_VIEW_DATA = ['video', 'discussion']
+ BLOCK_TYPES_WITH_STUDENT_VIEW_DATA = ['video', 'discussion', 'html']
@classmethod
def setUpClass(cls):
diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py
index b37ec88e80f..f20e76c40a3 100644
--- a/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py
+++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py
@@ -44,7 +44,7 @@ class TestStudentViewTransformer(ModuleStoreTestCase):
# verify html data
html_block_key = self.course_key.make_usage_key('html', 'toyhtml')
- self.assertIsNone(
+ self.assertIsNotNone(
self.block_structure.get_transformer_block_field(
html_block_key, StudentViewTransformer, StudentViewTransformer.STUDENT_VIEW_DATA,
)
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 61787e10777..1477a1b429e 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -413,6 +413,9 @@ FEATURES = {
# Set to enable Enterprise integration
'ENABLE_ENTERPRISE_INTEGRATION': False,
+
+ # Whether HTML XBlocks/XModules return HTML content with the Course Blocks API student_view_data
+ 'ENABLE_HTML_XBLOCK_STUDENT_VIEW_DATA': False,
}
# Settings for the course reviews tool template and identification key, set either to None to disable course reviews
--
GitLab