From 49217ebe8bd8eba20fb167831c8157a52b4d8b97 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 13 Dec 2013 07:47:21 -0500 Subject: [PATCH 1/3] Allow multiple client-side runtimes on a single page Make XBlock client-side runtimes proper classes, so that handlerUrl can be defined in a per-runtime way, and we can have multiple runtimes on a single page. [LMS-1630][LMS-1421][LMS-1517] --- cms/djangoapps/contentstore/views/item.py | 9 +- cms/djangoapps/contentstore/views/preview.py | 22 ++--- cms/lib/xblock/runtime.py | 2 - cms/static/coffee/spec/main.coffee | 1 + cms/static/coffee/spec/main_squire.coffee | 1 + .../coffee/src/views/module_edit.coffee | 2 +- .../coffee/src/xblock/cms.runtime.v1.coffee | 22 +++++ cms/static/js_test.yml | 3 +- cms/static/js_test_squire.yml | 2 + common/djangoapps/xmodule_modifiers.py | 14 +-- common/static/coffee/src/xblock/core.coffee | 9 +- .../coffee/src/xblock/runtime.v1.coffee | 29 ++----- lms/djangoapps/courseware/module_render.py | 7 +- .../instructor/views/instructor_dashboard.py | 3 +- lms/djangoapps/instructor/views/legacy.py | 3 +- lms/envs/common.py | 12 +-- lms/lib/xblock/runtime.py | 86 ++++++------------- .../coffee/src/xblock/lms.runtime.v1.coffee | 18 ++++ lms/static/js_test.yml | 1 + 19 files changed, 122 insertions(+), 124 deletions(-) create mode 100644 cms/static/coffee/src/xblock/cms.runtime.v1.coffee create mode 100644 lms/static/coffee/src/xblock/lms.runtime.v1.coffee diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 3d8f81a6ef..725f394d14 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -10,6 +10,7 @@ from xmodule_modifiers import wrap_xblock from django.core.exceptions import PermissionDenied from django.contrib.auth.decorators import login_required from django.http import HttpResponseBadRequest +from django.utils.translation import ugettext as _ from django.views.decorators.http import require_http_methods from xblock.fields import Scope @@ -34,7 +35,7 @@ from .helpers import _xmodule_recurse from preview import handler_prefix, get_preview_html from edxmako.shortcuts import render_to_response, render_to_string from models.settings.course_grading import CourseGradingModel -from django.utils.translation import ugettext as _ +from cms.lib.xblock.runtime import handler_url __all__ = ['orphan_handler', 'xblock_handler'] @@ -43,6 +44,12 @@ log = logging.getLogger(__name__) CREATE_IF_NOT_FOUND = ['course_info'] +# In order to allow descriptors to use a handler url, we need to +# monkey-patch the x_module library. +# TODO: Remove this code when Runtimes are no longer created by modulestores +xmodule.x_module.descriptor_global_handler_url = handler_url + + # pylint: disable=unused-argument @require_http_methods(("DELETE", "GET", "PUT", "POST")) @login_required diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 11bea40f9f..732323e924 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -33,20 +33,6 @@ __all__ = ['preview_handler'] log = logging.getLogger(__name__) -def handler_prefix(block, handler='', suffix=''): - """ - Return a url prefix for XBlock handler_url. The full handler_url - should be '{prefix}/{handler}/{suffix}?{query}'. - - Trailing `/`s are removed from the returned url. - """ - return reverse('preview_handler', kwargs={ - 'usage_id': quote_slashes(unicode(block.scope_ids.usage_id).encode('utf-8')), - 'handler': handler, - 'suffix': suffix, - }).rstrip('/?') - - @login_required def preview_handler(request, usage_id, handler, suffix=''): """ @@ -91,7 +77,11 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method An XModule ModuleSystem for use in Studio previews """ def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): - return handler_prefix(block, handler_name, suffix) + '?' + query + return reverse('preview_handler', kwargs={ + 'usage_id': quote_slashes(unicode(block.scope_ids.usage_id).encode('utf-8')), + 'handler': handler_name, + 'suffix': suffix, + }) + '?' + query def _preview_module_system(request, descriptor): @@ -123,7 +113,7 @@ def _preview_module_system(request, descriptor): # Set up functions to modify the fragment produced by student_view wrappers=( # This wrapper wraps the module in the template specified above - partial(wrap_xblock, handler_prefix, display_name_only=descriptor.location.category == 'static_tab'), + partial(wrap_xblock, 'PreviewRuntime', display_name_only=descriptor.location.category == 'static_tab'), # This wrapper replaces urls in the output that start with /static # with the correct course-specific url for the static content diff --git a/cms/lib/xblock/runtime.py b/cms/lib/xblock/runtime.py index 5c749a34c6..36437c4750 100644 --- a/cms/lib/xblock/runtime.py +++ b/cms/lib/xblock/runtime.py @@ -4,7 +4,6 @@ XBlock runtime implementations for edX Studio from django.core.urlresolvers import reverse -import xmodule.x_module from lms.lib.xblock.runtime import quote_slashes @@ -27,4 +26,3 @@ def handler_url(block, handler_name, suffix='', query='', thirdparty=False): return url -xmodule.x_module.descriptor_global_handler_url = handler_url diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 2cbc85d7c1..705ff08238 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -28,6 +28,7 @@ requirejs.config({ "tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/tiny_mce", "jquery.tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/jquery.tinymce", "xmodule": "xmodule_js/src/xmodule", + "xblock/cms.runtime.v1": "coffee/src/xblock/cms.runtime.v1", "xblock": "xmodule_js/common_static/coffee/src/xblock", "utility": "xmodule_js/common_static/js/src/utility", "accessibility": "xmodule_js/common_static/js/src/accessibility_tools", diff --git a/cms/static/coffee/spec/main_squire.coffee b/cms/static/coffee/spec/main_squire.coffee index a8e052fd2e..bc19272283 100644 --- a/cms/static/coffee/spec/main_squire.coffee +++ b/cms/static/coffee/spec/main_squire.coffee @@ -27,6 +27,7 @@ requirejs.config({ "tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/tiny_mce", "jquery.tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/jquery.tinymce", "xmodule": "xmodule_js/src/xmodule", + "xblock/cms.runtime.v1": "coffee/src/xblock/cms.runtime.v1", "xblock": "xmodule_js/common_static/coffee/src/xblock", "utility": "xmodule_js/common_static/js/src/utility", "sinon": "xmodule_js/common_static/js/vendor/sinon-1.7.1", diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index a9628b628f..a30fbe9db1 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -1,6 +1,6 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1", "js/views/feedback_notification", "js/views/metadata", "js/collections/metadata" - "js/utils/modal", "jquery.inputnumber", "xmodule", "coffee/src/main"], + "js/utils/modal", "jquery.inputnumber", "xmodule", "coffee/src/main", "xblock/cms.runtime.v1"], (Backbone, $, _, gettext, XBlock, NotificationView, MetadataView, MetadataCollection, ModalUtils) -> class ModuleEdit extends Backbone.View tagName: 'li' diff --git a/cms/static/coffee/src/xblock/cms.runtime.v1.coffee b/cms/static/coffee/src/xblock/cms.runtime.v1.coffee new file mode 100644 index 0000000000..4eddd6ef3a --- /dev/null +++ b/cms/static/coffee/src/xblock/cms.runtime.v1.coffee @@ -0,0 +1,22 @@ +define ["jquery", "xblock/runtime.v1", "URI"], ($, XBlock, URI) -> + @PreviewRuntime = {} + + class PreviewRuntime.v1 extends XBlock.Runtime.v1 + handlerUrl: (element, handlerName, suffix, query, thirdparty) -> + uri = URI("/preview/xblock").segment($(@element).data('usage-id')) + .segment('handler') + .segment(handlerName) + if suffix? then uri.segment(suffix) + if query? then uri.search(query) + uri.toString() + + @StudioRuntime = {} + + class StudioRuntime.v1 extends XBlock.Runtime.v1 + handlerUrl: (element, handlerName, suffix, query, thirdparty) -> + uri = URI("/xblock").segment($(@element).data('usage-id')) + .segment('handler') + .segment(handlerName) + if suffix? then uri.segment(suffix) + if query? then uri.search(query) + uri.toString() diff --git a/cms/static/js_test.yml b/cms/static/js_test.yml index 17d4805be0..ce247967a9 100644 --- a/cms/static/js_test.yml +++ b/cms/static/js_test.yml @@ -59,7 +59,8 @@ lib_paths: - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/jquery.smooth-scroll.min.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js - - xmodule_js/common_static/coffee/src/xblock + - xmodule_js/common_static/coffee/src/xblock/ + - xmodule_js/common_static/js/vendor/URI.min.js # Paths to source JavaScript files src_paths: diff --git a/cms/static/js_test_squire.yml b/cms/static/js_test_squire.yml index 8f57291b3d..c8a5ff439d 100644 --- a/cms/static/js_test_squire.yml +++ b/cms/static/js_test_squire.yml @@ -54,6 +54,8 @@ lib_paths: - xmodule_js/src/xmodule.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js - xmodule_js/common_static/js/test/i18n.js + - xmodule_js/common_static/coffee/src/xblock/ + - xmodule_js/common_static/js/vendor/URI.min.js # Paths to source JavaScript files src_paths: diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index f7b314d775..1b095bf917 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -15,6 +15,7 @@ from xblock.fragment import Fragment from xmodule.seq_module import SequenceModule from xmodule.vertical_module import VerticalModule from xmodule.x_module import shim_xmodule_js, XModuleDescriptor, XModule +from lms.lib.xblock.runtime import quote_slashes log = logging.getLogger(__name__) @@ -29,26 +30,28 @@ def wrap_fragment(fragment, new_content): return wrapper_frag -def wrap_xblock(handler_prefix, block, view, frag, context, display_name_only=False): # pylint: disable=unused-argument +def wrap_xblock(runtime_class, block, view, frag, context, display_name_only=False, extra_data=None): # pylint: disable=unused-argument """ Wraps the results of rendering an XBlock view in a standard
with identifying data so that the appropriate javascript module can be loaded onto it. - :param handler_prefix: A function that takes a block and returns the url prefix for - the javascript handler_url. This prefix should be able to have {handler_name}/{suffix}?{query} - appended to it to return a valid handler_url + :param runtime_class: The name of the javascript runtime class to use to load this block :param block: An XBlock (that may be an XModule or XModuleDescriptor) :param view: The name of the view that rendered the fragment being wrapped :param frag: The :class:`Fragment` to be wrapped :param context: The context passed to the view being rendered :param display_name_only: If true, don't render the fragment content at all. Instead, just render the `display_name` of `block` + :param extra_data: A dictionary with extra data values to be set on the wrapper """ + if extra_data is None: + extra_data = {} # If any mixins have been applied, then use the unmixed class class_name = getattr(block, 'unmixed_class', block.__class__).__name__ data = {} + data.update(extra_data) css_classes = ['xblock', 'xblock-' + view] if isinstance(block, (XModule, XModuleDescriptor)): @@ -65,9 +68,10 @@ def wrap_xblock(handler_prefix, block, view, frag, context, display_name_only=Fa if frag.js_init_fn: data['init'] = frag.js_init_fn + data['runtime-class'] = runtime_class data['runtime-version'] = frag.js_init_version - data['handler-prefix'] = handler_prefix(block) data['block-type'] = block.scope_ids.block_type + data['usage-id'] = quote_slashes(str(block.scope_ids.usage_id)) template_context = { 'content': block.display_name if display_name_only else frag.content, diff --git a/common/static/coffee/src/xblock/core.coffee b/common/static/coffee/src/xblock/core.coffee index 71fbfd291e..9c5cc0961c 100644 --- a/common/static/coffee/src/xblock/core.coffee +++ b/common/static/coffee/src/xblock/core.coffee @@ -1,18 +1,19 @@ @XBlock = - runtime: {} + Runtime: {} initializeBlock: (element) -> $element = $(element) children = @initializeBlocks($element) + runtime = $element.data("runtime-class") version = $element.data("runtime-version") initFnName = $element.data("init") - if version? and initFnName? - runtime = @runtime["v#{version}"](element, children) + if runtime? and version? and initFnName? + runtime = new window[runtime]["v#{version}"](element, children) initFn = window[initFnName] block = initFn(runtime, element) ? {} else elementTag = $('
').append($element.clone()).html(); - console.log("Block #{elementTag} is missing data-runtime-version or data-init, and can't be initialized") + console.log("Block #{elementTag} is missing data-runtime, data-runtime-version or data-init, and can't be initialized") block = {} block.element = element diff --git a/common/static/coffee/src/xblock/runtime.v1.coffee b/common/static/coffee/src/xblock/runtime.v1.coffee index efb9944bd2..b3f28242ab 100644 --- a/common/static/coffee/src/xblock/runtime.v1.coffee +++ b/common/static/coffee/src/xblock/runtime.v1.coffee @@ -1,24 +1,5 @@ -@XBlock.runtime.v1 = (element, children) -> - childMap = {} - $.each children, (idx, child) -> - childMap[child.name] = child - - return { - # Generate the handler url for the specified handler. - # - # element is the html element containing the xblock requesting the url - # handlerName is the name of the handler - # suffix is the optional url suffix to include in the handler url - # query is an optional query-string (note, this should not include a preceding ? or &) - handlerUrl: (element, handlerName, suffix, query) -> - handlerPrefix = $(element).data("handler-prefix") - suffix = if suffix? then "/#{suffix}" else '' - query = if query? then "?#{query}" else '' - "#{handlerPrefix}/#{handlerName}#{suffix}#{query}" - - # A list of xblock children of this element - children: children - - # A map of name -> child for the xblock children of this element - childMap: childMap - } +class XBlock.Runtime.v1 + constructor: (@element, @children) -> + @childMap = {} + $.each @children, (idx, child) => + @childMap[child.name] = child diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 207e46f638..952d3734df 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -21,7 +21,7 @@ from courseware.access import has_access, get_user_role from courseware.masquerade import setup_masquerade from courseware.model_data import FieldDataCache, DjangoKeyValueStore from lms.lib.xblock.field_data import LmsFieldData -from lms.lib.xblock.runtime import LmsModuleSystem, handler_prefix, unquote_slashes +from lms.lib.xblock.runtime import LmsModuleSystem, unquote_slashes from edxmako.shortcuts import render_to_string from psychometrics.psychoanalyze import make_psychometrics_data_update_handler from student.models import anonymous_id_for_user, user_by_anonymous_id @@ -339,7 +339,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours # Wrap the output display in a single div to allow for the XModule # javascript to be bound correctly if wrap_xmodule_display is True: - block_wrappers.append(partial(wrap_xblock, partial(handler_prefix, course_id))) + block_wrappers.append(partial(wrap_xblock, 'LmsRuntime', extra_data={'course-id': course_id})) # TODO (cpennington): When modules are shared between courses, the static # prefix is going to have to be specific to the module, not the directory @@ -379,7 +379,8 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours # As we have the time to manually test more modules, we can add to the list # of modules that get the per-course anonymized id. is_pure_xblock = isinstance(descriptor, XBlock) and not isinstance(descriptor, XModuleDescriptor) - is_lti_module = not is_pure_xblock and issubclass(descriptor.module_class, LTIModule) + module_class = getattr(descriptor, 'module_class', None) + is_lti_module = not is_pure_xblock and issubclass(module_class, LTIModule) if is_pure_xblock or is_lti_module: anonymous_student_id = anonymous_id_for_user(user, course_id) else: diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 2000c83900..ef58761c54 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -24,7 +24,6 @@ from django_comment_client.utils import has_forum_access from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR from student.models import CourseEnrollment from bulk_email.models import CourseAuthorization -from lms.lib.xblock.runtime import handler_prefix from .tools import get_units_with_due_date, title_or_url @@ -206,7 +205,7 @@ def _section_send_email(course_id, access, course): ScopeIds(None, None, None, 'i4x://dummy_org/dummy_course/html/dummy_name') ) fragment = course.system.render(html_module, 'studio_view') - fragment = wrap_xblock(partial(handler_prefix, course_id), html_module, 'studio_view', fragment, None) + fragment = wrap_xblock('LmsRuntime', html_module, 'studio_view', fragment, None, extra_data={"course-id": course_id}) email_editor = fragment.content section_data = { 'section_key': 'send_email', diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index c671b28922..0fabf5d376 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -61,7 +61,6 @@ import track.views from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from django.utils.translation import ugettext as _u -from lms.lib.xblock.runtime import handler_prefix from microsite_configuration.middleware import MicrositeConfiguration @@ -848,7 +847,7 @@ def instructor_dashboard(request, course_id): ScopeIds(None, None, None, 'i4x://dummy_org/dummy_course/html/dummy_name') ) fragment = html_module.render('studio_view') - fragment = wrap_xblock(partial(handler_prefix, course_id), html_module, 'studio_view', fragment, None) + fragment = wrap_xblock('LmsRuntime', html_module, 'studio_view', fragment, None, extra_data={"course-id": course_id}) email_editor = fragment.content # Enable instructor email only if the following conditions are met: diff --git a/lms/envs/common.py b/lms/envs/common.py index 5bc8f27d32..7ffd64b4ea 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -754,6 +754,7 @@ main_vendor_js = [ 'js/vendor/ova/ova.js', 'js/vendor/ova/catch/js/catch.js', 'js/vendor/ova/catch/js/handlebars-1.1.2.js' + 'js/vendor/URI.min.js' ] discussion_js = sorted(rooted_glob(COMMON_ROOT / 'static', 'coffee/src/discussion/**/*.js')) @@ -819,17 +820,18 @@ PIPELINE_CSS = { } +common_js = set(rooted_glob(COMMON_ROOT / 'static', 'coffee/src/**/*.js')) - set(courseware_js + discussion_js + staff_grading_js + open_ended_js + notes_js + instructor_dash_js) +project_js = set(rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/**/*.js')) - set(courseware_js + discussion_js + staff_grading_js + open_ended_js + notes_js + instructor_dash_js) + + + # test_order: Determines the position of this chunk of javascript on # the jasmine test page PIPELINE_JS = { 'application': { # Application will contain all paths not in courseware_only_js - 'source_filenames': sorted( - set(rooted_glob(COMMON_ROOT / 'static', 'coffee/src/**/*.js') + - rooted_glob(PROJECT_ROOT / 'static', 'coffee/src/**/*.js')) - - set(courseware_js + discussion_js + staff_grading_js + open_ended_js + notes_js + instructor_dash_js) - ) + [ + 'source_filenames': sorted(common_js) + sorted(project_js) + [ 'js/form.ext.js', 'js/my_courses_dropdown.js', 'js/toggle_login_modal.js', diff --git a/lms/lib/xblock/runtime.py b/lms/lib/xblock/runtime.py index 74066a6af1..a339a0a98a 100644 --- a/lms/lib/xblock/runtime.py +++ b/lms/lib/xblock/runtime.py @@ -58,63 +58,6 @@ def unquote_slashes(text): return re.sub(r'(;;|;_)', _unquote_slashes, text) -def handler_url(course_id, block, handler, suffix='', query='', thirdparty=False): - """ - Return an XBlock handler url for the specified course, block and handler. - - If handler is an empty string, this function is being used to create a - prefix of the general URL, which is assumed to be followed by handler name - and suffix. - - If handler is specified, then it is checked for being a valid handler - function, and ValueError is raised if not. - - """ - view_name = 'xblock_handler' - if handler: - # Be sure this is really a handler. - func = getattr(block, handler, None) - if not func: - raise ValueError("{!r} is not a function name".format(handler)) - if not getattr(func, "_is_xblock_handler", False): - raise ValueError("{!r} is not a handler name".format(handler)) - - if thirdparty: - view_name = 'xblock_handler_noauth' - - url = reverse(view_name, kwargs={ - 'course_id': course_id, - 'usage_id': quote_slashes(unicode(block.scope_ids.usage_id).encode('utf-8')), - 'handler': handler, - 'suffix': suffix, - }) - - # If suffix is an empty string, remove the trailing '/' - if not suffix: - url = url.rstrip('/') - - # If there is a query string, append it - if query: - url += '?' + query - - return url - - -def handler_prefix(course_id, block): - """ - Returns a prefix for use by the Javascript handler_url function. - - The prefix is a valid handler url after the handler name is slash-appended - to it. - """ - # This depends on handler url having the handler_name as the final piece of the url - # so that leaving an empty handler_name really does leave the opportunity to append - # the handler_name on the frontend - - # This is relied on by the xblock/runtime.v1.coffee frontend handlerUrl function - return handler_url(course_id, block, '').rstrip('/?') - - class LmsHandlerUrls(object): """ A runtime mixin that provides a handler_url function that routes @@ -127,7 +70,34 @@ class LmsHandlerUrls(object): # pylint: disable=no-member def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False): """See :method:`xblock.runtime:Runtime.handler_url`""" - return handler_url(self.course_id, block, handler_name, suffix='', query='', thirdparty=thirdparty) + view_name = 'xblock_handler' + if handler_name: + # Be sure this is really a handler. + func = getattr(block, handler_name, None) + if not func: + raise ValueError("{!r} is not a function name".format(handler_name)) + if not getattr(func, "_is_xblock_handler", False): + raise ValueError("{!r} is not a handler name".format(handler_name)) + + if thirdparty: + view_name = 'xblock_handler_noauth' + + url = reverse(view_name, kwargs={ + 'course_id': self.course_id, + 'usage_id': quote_slashes(unicode(block.scope_ids.usage_id).encode('utf-8')), + 'handler': handler_name, + 'suffix': suffix, + }) + + # If suffix is an empty string, remove the trailing '/' + if not suffix: + url = url.rstrip('/') + + # If there is a query string, append it + if query: + url += '?' + query + + return url class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract-method diff --git a/lms/static/coffee/src/xblock/lms.runtime.v1.coffee b/lms/static/coffee/src/xblock/lms.runtime.v1.coffee new file mode 100644 index 0000000000..9a999727a3 --- /dev/null +++ b/lms/static/coffee/src/xblock/lms.runtime.v1.coffee @@ -0,0 +1,18 @@ +@LmsRuntime = {} + +class LmsRuntime.v1 extends XBlock.Runtime.v1 + handlerUrl: (element, handlerName, suffix, query, thirdparty) -> + courseId = $(@element).data("course-id") + usageId = $(@element).data("usage-id") + handlerAuth = if thirdparty then "handler_noauth" else "handler" + + uri = URI('/courses').segment(courseId) + .segment('xblock') + .segment(usageId) + .segment(handlerAuth) + .segment(handlerName) + + if suffix? then uri.segment(suffix) + if query? then uri.search(query) + + uri.toString() diff --git a/lms/static/js_test.yml b/lms/static/js_test.yml index 6fa88250ff..4cec8c06e7 100644 --- a/lms/static/js_test.yml +++ b/lms/static/js_test.yml @@ -40,6 +40,7 @@ lib_paths: - xmodule_js/common_static/js/vendor/jquery.cookie.js - xmodule_js/common_static/js/vendor/flot/jquery.flot.js - xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js + - xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js - xmodule_js/common_static/coffee/src/xblock - xmodule_js/src/capa/ From f6731342de5b7bea22d9b450b484a53c5234e4e2 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 13 Dec 2013 07:50:33 -0500 Subject: [PATCH 2/3] Make Studio load XBlock fragment js and css on the client-side [LMS-1421][LMS-1517] --- .../contentstore/tests/test_contentstore.py | 9 ++- cms/djangoapps/contentstore/tests/utils.py | 7 ++ cms/djangoapps/contentstore/views/item.py | 80 +++++++++++++------ cms/djangoapps/contentstore/views/preview.py | 12 +-- .../coffee/spec/views/module_edit_spec.coffee | 57 ++++++++++++- .../coffee/src/views/module_edit.coffee | 34 +++++++- cms/static/sass/elements/_xmodules.scss | 2 +- cms/static/sass/views/_static-pages.scss | 6 +- cms/static/sass/views/_unit.scss | 2 +- .../coffee/spec/xblock/core_spec.coffee | 19 +++-- .../coffee/spec/xblock/runtime.v1_spec.coffee | 6 +- lms/lib/xblock/test/test_runtime.py | 30 ++++--- 12 files changed, 197 insertions(+), 67 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index d25ffe0376..d28a3ab3de 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1,7 +1,8 @@ #pylint: disable=E1101 -import shutil +import json import mock +import shutil from textwrap import dedent @@ -503,7 +504,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): This verifies that a video caption url is as we expect it to be """ resp = self._test_preview(Location('i4x', 'edX', 'toy', 'video', 'sample_video', None)) - self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"') + self.assertEquals(resp.status_code, 200) + content = json.loads(resp.content) + self.assertIn('data-caption-asset-path="/c4x/edX/toy/asset/subs_"', content['html']) def _test_preview(self, location): """ Preview test case. """ @@ -514,7 +517,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): locator = loc_mapper().translate_location( course_items[0].location.course_id, location, True, True ) - resp = self.client.get_html(locator.url_reverse('xblock')) + resp = self.client.get_fragment(locator.url_reverse('xblock')) self.assertEqual(resp.status_code, 200) # TODO: uncomment when preview no longer has locations being returned. # _test_no_locations(self, resp) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 19d1f174d2..39acda3d8a 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -57,6 +57,13 @@ class AjaxEnabledTestClient(Client): """ return self.get(path, data or {}, follow, HTTP_ACCEPT="application/json", **extra) + def get_fragment(self, path, data=None, follow=False, **extra): + """ + Convenience method for client.get which sets the accept type to application/x-fragment+json + """ + return self.get(path, data or {}, follow, HTTP_ACCEPT="application/x-fragment+json", **extra) + + @override_settings(MODULESTORE=TEST_MODULESTORE) class CourseTestCase(ModuleStoreTestCase): diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 725f394d14..8d80d9e0bb 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -1,27 +1,31 @@ """Views for items (modules).""" +import hashlib import logging from uuid import uuid4 +from collections import OrderedDict from functools import partial from static_replace import replace_static_urls from xmodule_modifiers import wrap_xblock +from django.conf import settings from django.core.exceptions import PermissionDenied from django.contrib.auth.decorators import login_required -from django.http import HttpResponseBadRequest +from django.http import HttpResponseBadRequest, HttpResponse from django.utils.translation import ugettext as _ from django.views.decorators.http import require_http_methods from xblock.fields import Scope +from xblock.fragment import Fragment from xblock.core import XBlock +import xmodule.x_module from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore import Location -from xmodule.x_module import prefer_xmodules from util.json_request import expect_json, JsonResponse from util.string_utils import str_to_bool @@ -32,8 +36,8 @@ from ..utils import get_modulestore from .access import has_course_access from .helpers import _xmodule_recurse -from preview import handler_prefix, get_preview_html -from edxmako.shortcuts import render_to_response, render_to_string +from contentstore.views.preview import get_preview_fragment +from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel from cms.lib.xblock.runtime import handler_url @@ -50,6 +54,16 @@ CREATE_IF_NOT_FOUND = ['course_info'] xmodule.x_module.descriptor_global_handler_url = handler_url +def hash_resource(resource): + """ + Hash a :class:`xblock.fragment.FragmentResource + """ + md5 = hashlib.md5() + for data in resource: + md5.update(data) + return md5.hexdigest() + + # pylint: disable=unused-argument @require_http_methods(("DELETE", "GET", "PUT", "POST")) @login_required @@ -95,7 +109,42 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid old_location = loc_mapper().translate_locator_to_location(locator) if request.method == 'GET': - if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + accept_header = request.META.get('HTTP_ACCEPT', 'application/json') + + if 'application/x-fragment+json' in accept_header: + component = modulestore().get_item(old_location) + + # Wrap the generated fragment in the xmodule_editor div so that the javascript + # can bind to it correctly + component.runtime.wrappers.append(partial(wrap_xblock, 'StudioRuntime')) + + try: + editor_fragment = component.render('studio_view') + # catch exceptions indiscriminately, since after this point they escape the + # dungeon and surface as uneditable, unsaveable, and undeletable + # component-goblins. + except Exception as exc: # pylint: disable=W0703 + log.debug("Unable to render studio_view for %r", component, exc_info=True) + editor_fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)})) + + modulestore().save_xmodule(component) + + preview_fragment = get_preview_fragment(request, component) + + hashed_resources = OrderedDict() + for resource in editor_fragment.resources + preview_fragment.resources: + hashed_resources[hash_resource(resource)] = resource + + return JsonResponse({ + 'html': render_to_string('component.html', { + 'preview': preview_fragment.content, + 'editor': editor_fragment.content, + 'label': component.display_name or component.scope_ids.block_type, + }), + 'resources': hashed_resources.items() + }) + + elif 'application/json' in accept_header: fields = request.REQUEST.get('fields', '').split(',') if 'graderType' in fields: # right now can't combine output of this w/ output of _get_module_info, but worthy goal @@ -104,25 +153,8 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid rsp = _get_module_info(locator) return JsonResponse(rsp) else: - component = modulestore().get_item(old_location) - # Wrap the generated fragment in the xmodule_editor div so that the javascript - # can bind to it correctly - component.runtime.wrappers.append(partial(wrap_xblock, handler_prefix)) + return HttpResponse(status=406) - try: - content = component.render('studio_view').content - # catch exceptions indiscriminately, since after this point they escape the - # dungeon and surface as uneditable, unsaveable, and undeletable - # component-goblins. - except Exception as exc: # pylint: disable=W0703 - log.debug("Unable to render studio_view for %r", component, exc_info=True) - content = render_to_string('html_error.html', {'message': str(exc)}) - - return render_to_response('component.html', { - 'preview': get_preview_html(request, component), - 'editor': content, - 'label': component.display_name or component.category, - }) elif request.method == 'DELETE': delete_children = str_to_bool(request.REQUEST.get('recurse', 'False')) delete_all_versions = str_to_bool(request.REQUEST.get('all_versions', 'False')) @@ -288,7 +320,7 @@ def _create_item(request): data = None template_id = request.json.get('boilerplate') if template_id is not None: - clz = XBlock.load_class(category, select=prefer_xmodules) + clz = parent.runtime.load_block_type(category) if clz is not None: template = clz.get_template(template_id) if template is not None: diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 732323e924..53de39dcf2 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -1,11 +1,12 @@ import logging +import hashlib from functools import partial from django.conf import settings from django.core.urlresolvers import reverse from django.http import Http404, HttpResponseBadRequest from django.contrib.auth.decorators import login_required -from edxmako.shortcuts import render_to_response, render_to_string +from edxmako.shortcuts import render_to_string from xmodule_modifiers import replace_static_urls, wrap_xblock from xmodule.error_module import ErrorDescriptor @@ -15,6 +16,7 @@ from xmodule.x_module import ModuleSystem from xblock.runtime import KvsFieldData from xblock.django.request import webob_to_django_response, django_to_webob_request from xblock.exceptions import NoSuchHandlerError +from xblock.fragment import Fragment from lms.lib.xblock.field_data import LmsFieldData from lms.lib.xblock.runtime import quote_slashes, unquote_slashes @@ -143,15 +145,15 @@ def _load_preview_module(request, descriptor): return descriptor -def get_preview_html(request, descriptor): +def get_preview_fragment(request, descriptor): """ Returns the HTML returned by the XModule's student_view, specified by the descriptor and idx. """ module = _load_preview_module(request, descriptor) try: - content = module.render("student_view").content + fragment = module.render("student_view") except Exception as exc: # pylint: disable=W0703 log.debug("Unable to render student_view for %r", module, exc_info=True) - content = render_to_string('html_error.html', {'message': str(exc)}) - return content + fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)})) + return fragment diff --git a/cms/static/coffee/spec/views/module_edit_spec.coffee b/cms/static/coffee/spec/views/module_edit_spec.coffee index 36716668d3..e9e3db2a29 100644 --- a/cms/static/coffee/spec/views/module_edit_spec.coffee +++ b/cms/static/coffee/spec/views/module_edit_spec.coffee @@ -1,4 +1,4 @@ -define ["coffee/src/views/module_edit", "js/models/module_info", "xmodule"], (ModuleEdit, ModuleModel) -> +define ["jquery", "coffee/src/views/module_edit", "js/models/module_info", "xmodule"], ($, ModuleEdit, ModuleModel) -> describe "ModuleEdit", -> beforeEach -> @@ -24,7 +24,7 @@ define ["coffee/src/views/module_edit", "js/models/module_info", "xmodule"], (Mo
""" - spyOn($.fn, 'load').andReturn(@moduleData) + spyOn($, 'ajax').andReturn(@moduleData) @moduleEdit = new ModuleEdit( el: $(".component") @@ -56,14 +56,63 @@ define ["coffee/src/views/module_edit", "js/models/module_info", "xmodule"], (Mo beforeEach -> spyOn(@moduleEdit, 'loadDisplay') spyOn(@moduleEdit, 'delegateEvents') + spyOn($.fn, 'append') + spyOn($, 'getScript') + + window.loadedXBlockResources = undefined + @moduleEdit.render() + $.ajax.mostRecentCall.args[0].success( + html: '
Response html
' + resources: [ + ['hash1', {kind: 'text', mimetype: 'text/css', data: 'inline-css'}], + ['hash2', {kind: 'url', mimetype: 'text/css', data: 'css-url'}], + ['hash3', {kind: 'text', mimetype: 'application/javascript', data: 'inline-js'}], + ['hash4', {kind: 'url', mimetype: 'application/javascript', data: 'js-url'}], + ['hash5', {placement: 'head', mimetype: 'text/html', data: 'head-html'}], + ['hash6', {placement: 'not-head', mimetype: 'text/html', data: 'not-head-html'}], + ] + ) it "loads the module preview and editor via ajax on the view element", -> - expect(@moduleEdit.$el.load).toHaveBeenCalledWith("/xblock/#{@moduleEdit.model.id}", jasmine.any(Function)) - @moduleEdit.$el.load.mostRecentCall.args[1]() + expect($.ajax).toHaveBeenCalledWith( + url: "/xblock/#{@moduleEdit.model.id}" + type: "GET" + headers: + Accept: 'application/x-fragment+json' + success: jasmine.any(Function) + ) expect(@moduleEdit.loadDisplay).toHaveBeenCalled() expect(@moduleEdit.delegateEvents).toHaveBeenCalled() + it "loads inline css from fragments", -> + expect($('head').append).toHaveBeenCalledWith("") + + it "loads css urls from fragments", -> + expect($('head').append).toHaveBeenCalledWith("") + + it "loads inline js from fragments", -> + expect($('head').append).toHaveBeenCalledWith("") + + it "loads js urls from fragments", -> + expect($.getScript).toHaveBeenCalledWith("js-url") + + it "loads head html", -> + expect($('head').append).toHaveBeenCalledWith("head-html") + + it "doesn't load body html", -> + expect($.fn.append).not.toHaveBeenCalledWith('not-head-html') + + it "doesn't reload resources", -> + count = $('head').append.callCount + $.ajax.mostRecentCall.args[0].success( + html: '
Response html 2
' + resources: [ + ['hash1', {kind: 'text', mimetype: 'text/css', data: 'inline-css'}], + ] + ) + expect($('head').append.callCount).toBe(count) + describe "loadDisplay", -> beforeEach -> spyOn(XBlock, 'initializeBlock') diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index a30fbe9db1..3ef164d5d1 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -75,9 +75,37 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1", render: -> if @model.id - @$el.load(@model.url(), => - @loadDisplay() - @delegateEvents() + $.ajax( + url: @model.url() + type: 'GET' + headers: + Accept: 'application/x-fragment+json' + success: (data) => + @$el.html(data.html) + + for value in data.resources + do (value) => + hash = value[0] + if not window.loadedXBlockResources? + window.loadedXBlockResources = [] + + if hash not in window.loadedXBlockResources + resource = value[1] + switch resource.mimetype + when "text/css" + switch resource.kind + when "text" then $('head').append("") + when "url" then $('head').append("") + when "application/javascript" + switch resource.kind + when "text" then $('head').append("") + when "url" then $.getScript(resource.data) + when "text/html" + switch resource.placement + when "head" then $('head').append(resource.data) + window.loadedXBlockResources.push(hash) + @loadDisplay() + @delegateEvents() ) clickSaveButton: (event) => diff --git a/cms/static/sass/elements/_xmodules.scss b/cms/static/sass/elements/_xmodules.scss index 8ffccfef2d..c889f0fde4 100644 --- a/cms/static/sass/elements/_xmodules.scss +++ b/cms/static/sass/elements/_xmodules.scss @@ -16,7 +16,7 @@ .xmodule_VideoModule { // display mode - &.xmodule_display { + &.xblock-student_view { // full screen .video-controls .add-fullscreen { diff --git a/cms/static/sass/views/_static-pages.scss b/cms/static/sass/views/_static-pages.scss index 5acd9f536d..dd2a897d2f 100644 --- a/cms/static/sass/views/_static-pages.scss +++ b/cms/static/sass/views/_static-pages.scss @@ -183,16 +183,16 @@ border-left: 1px solid $mediumGrey; border-right: 1px solid $mediumGrey; - .xmodule_display { + .xblock-student_view { display: none; } } - .new .xmodule_display { + .new .xblock-student_view { background: $yellow; } - .xmodule_display { + .xblock-student_view { @include transition(background-color $tmg-s3 linear 0s); padding: 20px 20px 22px; font-size: 24px; diff --git a/cms/static/sass/views/_unit.scss b/cms/static/sass/views/_unit.scss index 1cd74e12a1..5f8b9a897d 100644 --- a/cms/static/sass/views/_unit.scss +++ b/cms/static/sass/views/_unit.scss @@ -420,7 +420,7 @@ body.course.unit,.view-unit { } } - .xmodule_display { + .xblock-student_view { padding: 2*$baseline $baseline $baseline; overflow-x: auto; diff --git a/common/static/coffee/spec/xblock/core_spec.coffee b/common/static/coffee/spec/xblock/core_spec.coffee index 3db4190790..efc6d15807 100644 --- a/common/static/coffee/spec/xblock/core_spec.coffee +++ b/common/static/coffee/spec/xblock/core_spec.coffee @@ -2,9 +2,9 @@ describe "XBlock", -> beforeEach -> setFixtures """
-
+
-
+
@@ -13,8 +13,11 @@ describe "XBlock", -> describe "initializeBlock", -> beforeEach -> - XBlock.runtime.vA = jasmine.createSpy().andReturn('runtimeA') - XBlock.runtime.vZ = jasmine.createSpy().andReturn('runtimeZ') + window.TestRuntime = {} + @runtimeA = {name: 'runtimeA'} + @runtimeZ = {name: 'runtimeZ'} + TestRuntime.vA = jasmine.createSpy().andReturn(@runtimeA) + TestRuntime.vZ = jasmine.createSpy().andReturn(@runtimeZ) window.initFnA = jasmine.createSpy() window.initFnZ = jasmine.createSpy() @@ -28,12 +31,12 @@ describe "XBlock", -> @missingInitBlock = XBlock.initializeBlock($('#missing-init')[0]) it "loads the right runtime version", -> - expect(XBlock.runtime.vA).toHaveBeenCalledWith($('#vA')[0], @fakeChildren) - expect(XBlock.runtime.vZ).toHaveBeenCalledWith($('#vZ')[0], @fakeChildren) + expect(TestRuntime.vA).toHaveBeenCalledWith($('#vA')[0], @fakeChildren) + expect(TestRuntime.vZ).toHaveBeenCalledWith($('#vZ')[0], @fakeChildren) it "loads the right init function", -> - expect(window.initFnA).toHaveBeenCalledWith('runtimeA', $('#vA')[0]) - expect(window.initFnZ).toHaveBeenCalledWith('runtimeZ', $('#vZ')[0]) + expect(window.initFnA).toHaveBeenCalledWith(@runtimeA, $('#vA')[0]) + expect(window.initFnZ).toHaveBeenCalledWith(@runtimeZ, $('#vZ')[0]) it "loads when missing versions", -> expect(@missingVersionBlock.element).toBe($('#missing-version')) diff --git a/common/static/coffee/spec/xblock/runtime.v1_spec.coffee b/common/static/coffee/spec/xblock/runtime.v1_spec.coffee index e9c71e9b32..f13d69154c 100644 --- a/common/static/coffee/spec/xblock/runtime.v1_spec.coffee +++ b/common/static/coffee/spec/xblock/runtime.v1_spec.coffee @@ -1,4 +1,4 @@ -describe "XBlock.runtime.v1", -> +describe "XBlock.Runtime.v1", -> beforeEach -> setFixtures """
@@ -10,9 +10,7 @@ describe "XBlock.runtime.v1", -> @element = $('.xblock')[0] - @runtime = XBlock.runtime.v1(@element, @children) - it "provides a handler url", -> - expect(@runtime.handlerUrl(@element, 'foo')).toBe('/xblock/fake-usage-id/handler/foo') + @runtime = new XBlock.Runtime.v1(@element, @children) it "provides a list of children", -> expect(@runtime.children).toBe(@children) diff --git a/lms/lib/xblock/test/test_runtime.py b/lms/lib/xblock/test/test_runtime.py index 5b3efc171c..14987a87d3 100644 --- a/lms/lib/xblock/test/test_runtime.py +++ b/lms/lib/xblock/test/test_runtime.py @@ -6,7 +6,7 @@ from ddt import ddt, data from mock import Mock from unittest import TestCase from urlparse import urlparse -from lms.lib.xblock.runtime import quote_slashes, unquote_slashes, handler_url +from lms.lib.xblock.runtime import quote_slashes, unquote_slashes, LmsModuleSystem TEST_STRINGS = [ '', @@ -41,23 +41,31 @@ class TestHandlerUrl(TestCase): def setUp(self): self.block = Mock() self.course_id = "org/course/run" + self.runtime = LmsModuleSystem( + static_url='/static', + track_function=Mock(), + get_module=Mock(), + render_template=Mock(), + replace_urls=str, + course_id=self.course_id, + ) def test_trailing_characters(self): - self.assertFalse(handler_url(self.course_id, self.block, 'handler').endswith('?')) - self.assertFalse(handler_url(self.course_id, self.block, 'handler').endswith('/')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler').endswith('?')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler').endswith('/')) - self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix').endswith('?')) - self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix').endswith('/')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler', 'suffix').endswith('?')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler', 'suffix').endswith('/')) - self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix', 'query').endswith('?')) - self.assertFalse(handler_url(self.course_id, self.block, 'handler', 'suffix', 'query').endswith('/')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler', 'suffix', 'query').endswith('?')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler', 'suffix', 'query').endswith('/')) - self.assertFalse(handler_url(self.course_id, self.block, 'handler', query='query').endswith('?')) - self.assertFalse(handler_url(self.course_id, self.block, 'handler', query='query').endswith('/')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler', query='query').endswith('?')) + self.assertFalse(self.runtime.handler_url(self.block, 'handler', query='query').endswith('/')) def _parsed_query(self, query_string): """Return the parsed query string from a handler_url generated with the supplied query_string""" - return urlparse(handler_url(self.course_id, self.block, 'handler', query=query_string)).query + return urlparse(self.runtime.handler_url(self.block, 'handler', query=query_string)).query def test_query_string(self): self.assertIn('foo=bar', self._parsed_query('foo=bar')) @@ -66,7 +74,7 @@ class TestHandlerUrl(TestCase): def _parsed_path(self, handler_name='handler', suffix=''): """Return the parsed path from a handler_url with the supplied handler_name and suffix""" - return urlparse(handler_url(self.course_id, self.block, handler_name, suffix=suffix)).path + return urlparse(self.runtime.handler_url(self.block, handler_name, suffix=suffix)).path def test_suffix(self): self.assertTrue(self._parsed_path(suffix="foo").endswith('foo')) From 569c5def77ddb392cedf2336d1df015b7a4cf641 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 4 Feb 2014 11:44:42 -0500 Subject: [PATCH 3/3] Make uses of quote_slashes always operate on encoded strings --- cms/lib/xblock/runtime.py | 2 +- common/djangoapps/xmodule_modifiers.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cms/lib/xblock/runtime.py b/cms/lib/xblock/runtime.py index 36437c4750..e0e2491028 100644 --- a/cms/lib/xblock/runtime.py +++ b/cms/lib/xblock/runtime.py @@ -16,7 +16,7 @@ def handler_url(block, handler_name, suffix='', query='', thirdparty=False): raise NotImplementedError("edX Studio doesn't support third-party xblock handler urls") url = reverse('component_handler', kwargs={ - 'usage_id': quote_slashes(str(block.scope_ids.usage_id)), + 'usage_id': quote_slashes(unicode(block.scope_ids.usage_id).encode('utf-8')), 'handler': handler_name, 'suffix': suffix, }).rstrip('/') diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 1b095bf917..f8e51b7733 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -71,12 +71,12 @@ def wrap_xblock(runtime_class, block, view, frag, context, display_name_only=Fal data['runtime-class'] = runtime_class data['runtime-version'] = frag.js_init_version data['block-type'] = block.scope_ids.block_type - data['usage-id'] = quote_slashes(str(block.scope_ids.usage_id)) + data['usage-id'] = quote_slashes(unicode(block.scope_ids.usage_id).encode('utf-8')) template_context = { 'content': block.display_name if display_name_only else frag.content, 'classes': css_classes, - 'data_attributes': ' '.join('data-{}="{}"'.format(key, value) for key, value in data.items()), + 'data_attributes': ' '.join(u'data-{}="{}"'.format(key, value) for key, value in data.items()), } return wrap_fragment(frag, render_to_string('xblock_wrapper.html', template_context))