From 99f9894f1cd11bd0e8c47cb2599ae397241fd85b Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 24 Jul 2013 08:51:30 -0400 Subject: [PATCH] Switch LMS over to using XBlock rendering commands This makes the LMS use an XBlock's student_view, rather than an XModule's get_html to render for display. However, it does not yet use wrap_child to handle instructor debug information or url rewriting. [LMS-219] --- common/lib/xmodule/xmodule/x_module.py | 3 +- lms/djangoapps/courseware/courses.py | 4 +- lms/djangoapps/courseware/tabs.py | 2 +- lms/djangoapps/courseware/tests/__init__.py | 8 +- .../courseware/tests/test_module_render.py | 122 +++++++++++++++++- .../courseware/tests/test_videoalpha_mongo.py | 12 +- .../courseware/tests/test_videoalpha_xml.py | 7 +- .../courseware/tests/test_word_cloud.py | 6 +- lms/djangoapps/courseware/views.py | 2 +- 9 files changed, 136 insertions(+), 30 deletions(-) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 65c9a0e981..d399001a6a 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -12,6 +12,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecif from xblock.core import XBlock, Scope, String, Integer, Float, ModelType from xblock.fragment import Fragment +from xblock.runtime import Runtime from xmodule.modulestore.locator import BlockUsageLocator log = logging.getLogger(__name__) @@ -870,7 +871,7 @@ class XMLParsingSystem(DescriptorSystem): self.policy = policy -class ModuleSystem(object): +class ModuleSystem(Runtime): ''' This is an abstraction such that x_modules can function independent of the courseware (e.g. import into other types of courseware, LMS, diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index ef1b786645..91ec14011e 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -163,7 +163,7 @@ def get_course_about_section(course, section_key): html = '' if about_module is not None: - html = about_module.get_html() + html = about_module.runtime.render(about_module, None, 'student_view').content return html @@ -211,7 +211,7 @@ def get_course_info_section(request, course, section_key): html = '' if info_module is not None: - html = info_module.get_html() + html = info_module.runtime.render(info_module, None, 'student_view').content return html diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 149542c344..7a77898cdd 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -371,6 +371,6 @@ def get_static_tab_contents(request, course, tab): html = '' if tab_module is not None: - html = tab_module.get_html() + html = tab_module.runtime.render(tab_module, None, 'student_view').content return html diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index 33c8d12701..3169606a7b 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -77,13 +77,15 @@ class BaseTestXmodule(ModuleStoreTestCase): data=self.DATA ) - system = get_test_system() - system.render_template = lambda template, context: context + self.runtime = get_test_system() + # Allow us to assert that the template was called in the same way from + # different code paths while maintaining the type returned by render_template + self.runtime.render_template = lambda template, context: unicode((template, sorted(context.items()))) model_data = {'location': self.item_descriptor.location} model_data.update(self.MODEL_DATA) self.item_module = self.item_descriptor.module_class( - system, self.item_descriptor, model_data + self.runtime, self.item_descriptor, model_data ) self.item_url = Location(self.item_module.location).url() diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 6b409f677b..25056ba100 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1,7 +1,7 @@ """ Test for lms courseware app, module render unit """ -from mock import MagicMock, patch +from mock import MagicMock, patch, Mock import json from django.http import Http404, HttpResponse @@ -12,8 +12,10 @@ from django.test.client import RequestFactory from django.test.utils import override_settings from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase import courseware.module_render as render -from courseware.tests.tests import LoginEnrollmentTestCase +from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_MONGO_MODULESTORE from courseware.model_data import ModelDataCache from modulestore_config import TEST_DATA_XML_MODULESTORE @@ -49,8 +51,10 @@ class ModuleRenderTestCase(LoginEnrollmentTestCase): dispatch=self.dispatch)) def test_get_module(self): - self.assertIsNone(render.get_module('dummyuser', None, - 'invalid location', None, None)) + self.assertEqual( + None, + render.get_module('dummyuser', None, 'invalid location', None, None) + ) def test_module_render_with_jump_to_id(self): """ @@ -230,7 +234,8 @@ class TestTOC(TestCase): 'url_name': 'secret:magic', 'display_name': 'secret:magic'}]) actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, None, model_data_cache) - assert reduce(lambda x, y: x and (y in actual), expected, True) + for toc_section in expected: + self.assertIn(toc_section, actual) def test_toc_toy_from_section(self): chapter = 'Overview' @@ -257,4 +262,109 @@ class TestTOC(TestCase): 'url_name': 'secret:magic', 'display_name': 'secret:magic'}]) actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, section, model_data_cache) - assert reduce(lambda x, y: x and (y in actual), expected, True) + for toc_section in expected: + self.assertIn(toc_section, actual) + + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class TestHtmlModifiers(ModuleStoreTestCase): + """ + Tests to verify that standard modifications to the output of XModule/XBlock + student_view are taking place + """ + def setUp(self): + self.user = UserFactory.create() + self.request = RequestFactory().get('/') + self.request.user = self.user + self.request.session = {} + self.course = CourseFactory.create() + self.content_string = '

This is the content

' + self.rewrite_link = 'Test rewrite' + self.course_link = 'Test course rewrite' + self.descriptor = ItemFactory.create( + category='html', + data=self.content_string + self.rewrite_link + self.course_link + ) + self.location = self.descriptor.location + self.model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + self.course.id, + self.user, + self.descriptor + ) + + def test_xmodule_display_wrapper_enabled(self): + module = render.get_module( + self.user, + self.request, + self.location, + self.model_data_cache, + self.course.id, + wrap_xmodule_display=True, + ) + result_fragment = module.runtime.render(module, None, 'student_view') + + self.assertIn('section class="xmodule_display xmodule_HtmlModule"', result_fragment.content) + + def test_xmodule_display_wrapper_disabled(self): + module = render.get_module( + self.user, + self.request, + self.location, + self.model_data_cache, + self.course.id, + wrap_xmodule_display=False, + ) + result_fragment = module.runtime.render(module, None, 'student_view') + + self.assertNotIn('section class="xmodule_display xmodule_HtmlModule"', result_fragment.content) + + def test_static_link_rewrite(self): + module = render.get_module( + self.user, + self.request, + self.location, + self.model_data_cache, + self.course.id, + ) + result_fragment = module.runtime.render(module, None, 'student_view') + + self.assertIn( + '/c4x/{org}/{course}/asset/foo_content'.format( + org=self.course.location.org, + course=self.course.location.course, + ), + result_fragment.content + ) + + def test_course_link_rewrite(self): + module = render.get_module( + self.user, + self.request, + self.location, + self.model_data_cache, + self.course.id, + ) + result_fragment = module.runtime.render(module, None, 'student_view') + + self.assertIn( + '/courses/{course_id}/bar/content'.format( + course_id=self.course.id + ), + result_fragment.content + ) + + @patch('courseware.module_render.has_access', Mock(return_value=True)) + def test_histogram(self): + module = render.get_module( + self.user, + self.request, + self.location, + self.model_data_cache, + self.course.id, + ) + result_fragment = module.runtime.render(module, None, 'student_view') + + self.assertIn( + 'Staff Debug', + result_fragment.content + ) diff --git a/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py b/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py index d5afb1a78c..30cf556b9f 100644 --- a/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py +++ b/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py @@ -34,9 +34,7 @@ class TestVideo(BaseTestXmodule): def test_videoalpha_constructor(self): """Make sure that all parameters extracted correclty from xml""" - # `get_html` return only context, cause we - # overwrite `system.render_template` - context = self.item_module.get_html() + fragment = self.runtime.render(self.item_module, None, 'student_view') expected_context = { 'data_dir': getattr(self, 'data_dir', None), 'caption_asset_path': '/c4x/MITx/999/asset/subs_', @@ -51,7 +49,7 @@ class TestVideo(BaseTestXmodule): 'youtube_streams': self.item_module.youtube_streams, 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) } - self.assertDictEqual(context, expected_context) + self.assertEqual(fragment.content, self.runtime.render_template('videoalpha.html', expected_context)) class TestVideoNonYouTube(TestVideo): @@ -78,9 +76,7 @@ class TestVideoNonYouTube(TestVideo): the template generates an empty string for the YouTube streams. """ - # `get_html` return only context, cause we - # overwrite `system.render_template` - context = self.item_module.get_html() + fragment = self.runtime.render(self.item_module, None, 'student_view') expected_context = { 'data_dir': getattr(self, 'data_dir', None), 'caption_asset_path': '/c4x/MITx/999/asset/subs_', @@ -95,4 +91,4 @@ class TestVideoNonYouTube(TestVideo): 'youtube_streams': '', 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) } - self.assertDictEqual(context, expected_context) + self.assertEqual(fragment.content, self.runtime.render_template('videoalpha.html', expected_context)) diff --git a/lms/djangoapps/courseware/tests/test_videoalpha_xml.py b/lms/djangoapps/courseware/tests/test_videoalpha_xml.py index a14fc6cac6..ae49be86dc 100644 --- a/lms/djangoapps/courseware/tests/test_videoalpha_xml.py +++ b/lms/djangoapps/courseware/tests/test_videoalpha_xml.py @@ -104,10 +104,9 @@ class VideoAlphaModuleUnitTest(unittest.TestCase): def test_videoalpha_constructor(self): """Make sure that all parameters extracted correclty from xml""" module = VideoAlphaFactory.create() + module.runtime.render_template = lambda template, context: unicode((template, sorted(context.items()))) - # `get_html` return only context, cause we - # overwrite `system.render_template` - context = module.get_html() + fragment = module.runtime.render(module, None, 'student_view') expected_context = { 'caption_asset_path': '/static/subs/', 'sub': module.sub, @@ -122,7 +121,7 @@ class VideoAlphaModuleUnitTest(unittest.TestCase): 'track': module.track, 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) } - self.assertDictEqual(context, expected_context) + self.assertEqual(fragment.content, module.runtime.render_template('videoalpha.html', expected_context)) self.assertDictEqual( json.loads(module.get_instance_state()), diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 7c214f3458..6b01ae94f4 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -242,9 +242,7 @@ class TestWordCloud(BaseTestXmodule): def test_word_cloud_constructor(self): """Make sure that all parameters extracted correclty from xml""" - # `get_html` return only context, cause we - # overwrite `system.render_template` - context = self.item_module.get_html() + fragment = self.runtime.render(self.item_module, None, 'student_view') expected_context = { 'ajax_url': self.item_module.system.ajax_url, @@ -253,4 +251,4 @@ class TestWordCloud(BaseTestXmodule): 'num_inputs': 5, # default value 'submitted': False # default value } - self.assertDictEqual(context, expected_context) + self.assertEqual(fragment.content, self.runtime.render_template('word_cloud.html', expected_context)) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index b675f4dfc3..f152c0833b 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -400,7 +400,7 @@ def index(request, course_id, chapter=None, section=None, # add in the appropriate timer information to the rendering context: context.update(check_for_active_timelimit_module(request, course_id, course)) - context['content'] = section_module.get_html() + context['content'] = section_module.runtime.render(section_module, None, 'student_view').content else: # section is none, so display a message prev_section = get_current_child(chapter_module)