From 2514dca55086811c99d8282ca80de428f67d6960 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 4 Oct 2013 08:03:25 -0400 Subject: [PATCH] Put XBlock css and javascript onto the LMS courseware page This required changing structural XModules to fully implement student_view, rather than just returning the HTML of their children in a get_html call. [LMS-223] [LMS-1170] --- .../lib/xmodule/xmodule/randomize_module.py | 7 ++--- common/lib/xmodule/xmodule/seq_module.py | 26 +++++++++---------- .../xmodule/tests/test_xblock_wrappers.py | 18 +++++++++++-- .../lib/xmodule/xmodule/timelimit_module.py | 7 ++--- common/lib/xmodule/xmodule/vertical_module.py | 25 +++++++++++------- lms/djangoapps/courseware/views.py | 21 +++++++++------ lms/templates/courseware/courseware.html | 5 +++- 7 files changed, 70 insertions(+), 39 deletions(-) diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py index b53e44768b..00baf3f140 100644 --- a/common/lib/xmodule/xmodule/randomize_module.py +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -7,6 +7,7 @@ from xmodule.seq_module import SequenceDescriptor from lxml import etree from xblock.fields import Scope, Integer +from xblock.fragment import Fragment log = logging.getLogger('mitx.' + __name__) @@ -77,12 +78,12 @@ class RandomizeModule(RandomizeFields, XModule): return [self.child_descriptor] - def get_html(self): + def student_view(self, context): if self.child is None: # raise error instead? In fact, could complain on descriptor load... - return u"
Nothing to randomize between
" + return Fragment(content=u"
Nothing to randomize between
") - return self.child.render('student_view').content + return self.child.render('student_view', context) def get_icon_class(self): return self.child.get_icon_class() if self.child else 'other' diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index bb4acfdf11..c0c9bfc1a0 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -9,6 +9,7 @@ from xmodule.x_module import XModule from xmodule.progress import Progress from xmodule.exceptions import NotFoundError from xblock.fields import Integer, Scope +from xblock.fragment import Fragment from pkg_resources import resource_string log = logging.getLogger(__name__) @@ -43,12 +44,6 @@ class SequenceModule(SequenceFields, XModule): if getattr(self.system, 'position', None) is not None: self.position = int(self.system.position) - self.rendered = False - - def get_html(self): - self.render() - return self.content - def get_progress(self): ''' Return the total progress, adding total done and total available. (assumes that each submodule uses the same "units" for progress.) @@ -66,20 +61,24 @@ class SequenceModule(SequenceFields, XModule): return json.dumps({'success': True}) raise NotFoundError('Unexpected dispatch type') - def render(self): + def student_view(self, context): # If we're rendering this sequence, but no position is set yet, # default the position to the first element if self.position is None: self.position = 1 - if self.rendered: - return ## Returns a set of all types of all sub-children contents = [] + + fragment = Fragment() + for child in self.get_display_items(): progress = child.get_progress() + rendered_child = child.render('student_view', context) + fragment.add_frag_resources(rendered_child) + childinfo = { - 'content': child.render('student_view').content, + 'content': rendered_child.content, 'title': "\n".join( grand_child.display_name for grand_child in child.get_children() @@ -98,11 +97,12 @@ class SequenceModule(SequenceFields, XModule): 'element_id': self.location.html_id(), 'item_id': self.id, 'position': self.position, - 'tag': self.location.category + 'tag': self.location.category, } - self.content = self.system.render_template('seq_module.html', params) - self.rendered = True + fragment.add_content(self.system.render_template('seq_module.html', params)) + + return fragment def get_icon_class(self): child_classes = set(child.get_icon_class() diff --git a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py index 4d677cbc53..6959b63673 100644 --- a/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py +++ b/common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py @@ -12,7 +12,7 @@ from mock import Mock from xblock.field_data import DictFieldData from xblock.fields import ScopeIds -from xmodule.x_module import ModuleSystem +from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor from xmodule.mako_module import MakoDescriptorSystem from xmodule.annotatable_module import AnnotatableDescriptor from xmodule.capa_module import CapaDescriptor @@ -143,6 +143,10 @@ class TestStudentView(TestXBlockWrapper): # Check that when an xmodule is instantiated from descriptor_cls # it generates the same thing from student_view that it does from get_html def check_student_view_leaf_node(self, descriptor_cls): + + if descriptor_cls.module_class.student_view != XModule.student_view: + raise SkipTest(descriptor_cls.__name__ + " implements student_view") + descriptor = self.leaf_module(descriptor_cls) assert_equal( descriptor._xmodule.get_html(), @@ -165,6 +169,10 @@ class TestStudentView(TestXBlockWrapper): # with only xmodule children, it generates the same html from student_view # as it does using get_html def check_student_view_container_node_xmodules_only(self, descriptor_cls): + + if descriptor_cls.module_class.student_view != XModule.student_view: + raise SkipTest(descriptor_cls.__name__ + " implements student_view") + descriptor = self.container_module(descriptor_cls, 2) assert_equal( descriptor._xmodule.get_html(), @@ -197,7 +205,10 @@ class TestStudioView(TestXBlockWrapper): # it generates the same thing from studio_view that it does from get_html def check_studio_view_leaf_node(self, descriptor_cls): if descriptor_cls in NOT_STUDIO_EDITABLE: - raise SkipTest(descriptor_cls.__name__ + "is not editable in studio") + raise SkipTest(descriptor_cls.__name__ + " is not editable in studio") + + if descriptor_cls.studio_view != XModuleDescriptor.studio_view: + raise SkipTest(descriptor_cls.__name__ + " implements studio_view") descriptor = self.leaf_descriptor(descriptor_cls) assert_equal(descriptor.get_html(), descriptor.render('studio_view').content) @@ -222,6 +233,9 @@ class TestStudioView(TestXBlockWrapper): if descriptor_cls in NOT_STUDIO_EDITABLE: raise SkipTest(descriptor_cls.__name__ + "is not editable in studio") + if descriptor_cls.studio_view != XModuleDescriptor.studio_view: + raise SkipTest(descriptor_cls.__name__ + " implements studio_view") + descriptor = self.container_descriptor(descriptor_cls, 2) assert_equal(descriptor.get_html(), descriptor.render('studio_view').content) diff --git a/common/lib/xmodule/xmodule/timelimit_module.py b/common/lib/xmodule/xmodule/timelimit_module.py index b4bde1928c..e1c4042dc0 100644 --- a/common/lib/xmodule/xmodule/timelimit_module.py +++ b/common/lib/xmodule/xmodule/timelimit_module.py @@ -9,6 +9,7 @@ from xmodule.x_module import XModule from xmodule.progress import Progress from xmodule.exceptions import NotFoundError from xblock.fields import Float, String, Boolean, Scope +from xblock.fragment import Fragment log = logging.getLogger(__name__) @@ -84,14 +85,14 @@ class TimeLimitModule(TimeLimitFields, XModule): def get_remaining_time_in_ms(self): return int((self.ending_at - time()) * 1000) - def get_html(self): + def student_view(self, context): # assumes there is one and only one child, so it only renders the first child children = self.get_display_items() if children: child = children[0] - return child.render('student_view').content + return child.render('student_view', context) else: - return u"" + return Fragment() def get_progress(self): ''' Return the total progress, adding total done and total available. diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py index d825181033..0879e3bba3 100644 --- a/common/lib/xmodule/xmodule/vertical_module.py +++ b/common/lib/xmodule/xmodule/vertical_module.py @@ -1,3 +1,4 @@ +from xblock.fragment import Fragment from xmodule.x_module import XModule from xmodule.seq_module import SequenceDescriptor from xmodule.progress import Progress @@ -17,18 +18,24 @@ class VerticalModule(VerticalFields, XModule): def __init__(self, *args, **kwargs): XModule.__init__(self, *args, **kwargs) - self.contents = None - def get_html(self): - if self.contents is None: - self.contents = [{ + def student_view(self, context): + fragment = Fragment() + contents = [] + + for child in self.get_display_items(): + rendered_child = child.render('student_view', context) + fragment.add_frag_resources(rendered_child) + + contents.append({ 'id': child.id, - 'content': child.render('student_view').content - } for child in self.get_display_items()] + 'content': rendered_child.content + }) - return self.system.render_template('vert_module.html', { - 'items': self.contents - }) + fragment.add_content(self.system.render_template('vert_module.html', { + 'items': contents + })) + return fragment def get_progress(self): # TODO: Cache progress or children array? diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 56471d25f1..75a8d8db1c 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -31,6 +31,7 @@ from django_comment_client.utils import get_discussion_title from student.models import UserTestGroup, CourseEnrollment from util.cache import cache, cache_if_anonymous +from xblock.fragment import Fragment from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, NoPathToItem @@ -327,7 +328,7 @@ def index(request, course_id, chapter=None, section=None, 'COURSE_TITLE': course.display_name_with_default, 'course': course, 'init': '', - 'content': '', + 'fragment': Fragment(), 'staff_access': staff_access, 'masquerade': masq, 'xqa_server': settings.MITX_FEATURES.get('USE_XQA_SERVER', 'http://xqa:server@content-qa.mitx.mit.edu/xqa') @@ -395,7 +396,7 @@ def index(request, course_id, chapter=None, section=None, # check here if this section *is* a timed module. if section_module.category == 'timelimit': - timer_context = update_timelimit_module(user, course_id, student_module_cache, + timer_context = update_timelimit_module(user, course_id, section_field_data_cache, section_descriptor, section_module) if 'timer_expiration_duration' in timer_context: context.update(timer_context) @@ -407,7 +408,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.render('student_view').content + context['fragment'] = section_module.render('student_view') else: # section is none, so display a message prev_section = get_current_child(chapter_module) @@ -417,11 +418,15 @@ def index(request, course_id, chapter=None, section=None, prev_section_url = reverse('courseware_section', kwargs={'course_id': course_id, 'chapter': chapter_descriptor.url_name, 'section': prev_section.url_name}) - context['content'] = render_to_string('courseware/welcome-back.html', - {'course': course, - 'chapter_module': chapter_module, - 'prev_section': prev_section, - 'prev_section_url': prev_section_url}) + context['fragment'] = Fragment(content=render_to_string( + 'courseware/welcome-back.html', + { + 'course': course, + 'chapter_module': chapter_module, + 'prev_section': prev_section, + 'prev_section_url': prev_section_url + } + )) result = render_to_response('courseware/courseware.html', context) except Exception as e: diff --git a/lms/templates/courseware/courseware.html b/lms/templates/courseware/courseware.html index 2a52b50b09..da3beba262 100644 --- a/lms/templates/courseware/courseware.html +++ b/lms/templates/courseware/courseware.html @@ -14,6 +14,7 @@ ## all needs to stay together for the Candy.js plugin to work. % endif + ${fragment.head_html()} <%block name="js_extra"> @@ -150,6 +151,8 @@ % endif +${fragment.foot_html()} + % if timer_expiration_duration: @@ -185,7 +188,7 @@ % endif
- ${content} + ${fragment.body_html()}