diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py
index 02063444c0..5f569e7fe1 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 ec2413907c..318d9df440 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': '
Some HTML
'})
+ 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(
+ 'Some content
', # Valid HTML
+ '',
+ None,
+ 'Some contentalert()', # Does not escape tags
+ '
', # 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 5c518fa7e2..70a6e49182 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 b37ec88e80..ffef8e1e15 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
@@ -1,9 +1,9 @@
"""
Tests for StudentViewTransformer.
"""
+import ddt
# pylint: disable=protected-access
-
from openedx.core.djangoapps.content.block_structure.factory import BlockStructureFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import ToyCourseFactory
@@ -11,6 +11,7 @@ from xmodule.modulestore.tests.factories import ToyCourseFactory
from ..student_view import StudentViewTransformer
+@ddt.ddt
class TestStudentViewTransformer(ModuleStoreTestCase):
"""
Test proper behavior for StudentViewTransformer
@@ -21,20 +22,27 @@ class TestStudentViewTransformer(ModuleStoreTestCase):
self.course_usage_key = self.store.make_course_usage_key(self.course_key)
self.block_structure = BlockStructureFactory.create_from_modulestore(self.course_usage_key, self.store)
- def test_transform(self):
+ @ddt.data(
+ 'video', 'html', ['video', 'html'], [],
+ )
+ def test_transform(self, requested_student_view_data):
# collect phase
StudentViewTransformer.collect(self.block_structure)
self.block_structure._collect_requested_xblock_fields()
# transform phase
- StudentViewTransformer('video').transform(usage_info=None, block_structure=self.block_structure)
+ StudentViewTransformer(requested_student_view_data).transform(
+ usage_info=None,
+ block_structure=self.block_structure,
+ )
- # verify video data
+ # verify video data returned iff requested
video_block_key = self.course_key.make_usage_key('video', 'sample_video')
- self.assertIsNotNone(
+ self.assertEqual(
self.block_structure.get_transformer_block_field(
video_block_key, StudentViewTransformer, StudentViewTransformer.STUDENT_VIEW_DATA,
- )
+ ) is not None,
+ 'video' in requested_student_view_data
)
self.assertFalse(
self.block_structure.get_transformer_block_field(
@@ -42,12 +50,13 @@ class TestStudentViewTransformer(ModuleStoreTestCase):
)
)
- # verify html data
+ # verify html data returned iff requested
html_block_key = self.course_key.make_usage_key('html', 'toyhtml')
- self.assertIsNone(
+ self.assertEqual(
self.block_structure.get_transformer_block_field(
html_block_key, StudentViewTransformer, StudentViewTransformer.STUDENT_VIEW_DATA,
- )
+ ) is not None,
+ 'html' in requested_student_view_data
)
self.assertTrue(
self.block_structure.get_transformer_block_field(
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 61787e1077..1477a1b429 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
diff --git a/openedx/core/djangoapps/content/block_structure/block_structure.py b/openedx/core/djangoapps/content/block_structure/block_structure.py
index 04a823c442..ff14e62f4e 100644
--- a/openedx/core/djangoapps/content/block_structure/block_structure.py
+++ b/openedx/core/djangoapps/content/block_structure/block_structure.py
@@ -312,7 +312,7 @@ class FieldData(object):
if self._is_own_field(field_name):
return super(FieldData, self).__delattr__(field_name)
else:
- delattr(self.fields, field_name)
+ del self.fields[field_name]
def _is_own_field(self, field_name):
"""