Merge pull request #4853 from edx/christina/stud-2093

Add a request-token to identify which xblock html was rendered as part o...

cms/djangoapps/contentstore/views/item.py

@@ -11,7 +11,7 @@ import json
 from collections import OrderedDict
 from functools import partial
 from static_replace import replace_static_urls
-from xmodule_modifiers import wrap_xblock
+from xmodule_modifiers import wrap_xblock, request_token
 
 from django.core.exceptions import PermissionDenied
 from django.contrib.auth.decorators import login_required
@@ -206,7 +206,12 @@ def xblock_view_handler(request, usage_key_string, view_name):
 
         # wrap the generated fragment in the xmodule_editor div so that the javascript
         # can bind to it correctly
-        xblock.runtime.wrappers.append(partial(wrap_xblock, 'StudioRuntime', usage_id_serializer=unicode))
+        xblock.runtime.wrappers.append(partial(
+            wrap_xblock,
+            'StudioRuntime',
+            usage_id_serializer=unicode,
+            request_token=request_token(request),
+        ))
 
         if view_name == STUDIO_VIEW:
             try:

cms/djangoapps/contentstore/views/preview.py

@@ -9,7 +9,7 @@ from django.http import Http404, HttpResponseBadRequest
 from django.contrib.auth.decorators import login_required
 from edxmako.shortcuts import render_to_string
 
-from xmodule_modifiers import replace_static_urls, wrap_xblock, wrap_fragment
+from xmodule_modifiers import replace_static_urls, wrap_xblock, wrap_fragment, request_token
 from xmodule.x_module import PREVIEW_VIEWS, STUDENT_VIEW, AUTHOR_VIEW
 from xmodule.error_module import ErrorDescriptor
 from xmodule.exceptions import NotFoundError, ProcessingError
@@ -123,7 +123,13 @@ def _preview_module_system(request, descriptor):
 
     wrappers = [
         # This wrapper wraps the module in the template specified above
-        partial(wrap_xblock, 'PreviewRuntime', display_name_only=display_name_only, usage_id_serializer=unicode),
+        partial(
+            wrap_xblock,
+            'PreviewRuntime',
+            display_name_only=display_name_only,
+            usage_id_serializer=unicode,
+            request_token=request_token(request)
+        ),
 
         # This wrapper replaces urls in the output that start with /static
         # with the correct course-specific url for the static content

common/djangoapps/xmodule_modifiers.py

@@ -6,6 +6,7 @@ import datetime
 import json
 import logging
 import static_replace
+import uuid
 
 from django.conf import settings
 from django.utils.timezone import UTC
@@ -32,7 +33,19 @@ def wrap_fragment(fragment, new_content):
     return wrapper_frag
 
 
-def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, display_name_only=False, extra_data=None):  # pylint: disable=unused-argument
+def request_token(request):
+    """
+    Return a unique token for the supplied request.
+    This token will be the same for all calls to `request_token`
+    made on the same request object.
+    """
+    if not hasattr(request, '_xblock_token'):
+        request._xblock_token = uuid.uuid1().get_hex()
+
+    return request._xblock_token
+
+
+def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, request_token, display_name_only=False, extra_data=None):  # pylint: disable=unused-argument
     """
     Wraps the results of rendering an XBlock view in a standard <section> with identifying
     data so that the appropriate javascript module can be loaded onto it.
@@ -44,6 +57,8 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer,
     :param context: The context passed to the view being rendered
     :param usage_id_serializer: A function to serialize the block's usage_id for use by the
         front-end Javascript Runtime.
+    :param request_token: An identifier that is unique per-request, so that only xblocks
+        rendered as part of this request will have their javascript initialized.
     :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
@@ -56,7 +71,7 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer,
 
     data = {}
     data.update(extra_data)
-    css_classes = ['xblock', 'xblock-' + view]
+    css_classes = ['xblock', 'xblock-{}'.format(view)]
 
     if isinstance(block, (XModule, XModuleDescriptor)):
         if view in PREVIEW_VIEWS:
@@ -76,6 +91,7 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer,
         data['runtime-version'] = frag.js_init_version
         data['block-type'] = block.scope_ids.block_type
         data['usage-id'] = usage_id_serializer(block.scope_ids.usage_id)
+        data['request-token'] = request_token
 
     template_context = {
         'content': block.display_name if display_name_only else frag.content,

common/lib/xmodule/xmodule/js/src/conditional/display.coffee

@@ -32,4 +32,6 @@ class @Conditional
           else
             $(element).show()
 
-        XBlock.initializeBlocks @el
+        # The children are rendered with a new request, so they have a different request-token.
+        # Use that token instead of @requestToken by simply not passing a token into initializeBlocks.
+        XBlock.initializeBlocks(@el)

common/lib/xmodule/xmodule/js/src/sequence/display.coffee

 class @Sequence
   constructor: (element) ->
+    @requestToken = $(element).data('request-token')
     @el = $(element).find('.sequence')
     @contents = @$('.seq_contents')
     @content_container = @$('#seq_content')
@@ -102,7 +103,7 @@ class @Sequence
       current_tab = @contents.eq(new_position - 1)
       @content_container.html(current_tab.text()).attr("aria-labelledby", current_tab.attr("aria-labelledby"))
 
-      XBlock.initializeBlocks(@content_container)
+      XBlock.initializeBlocks(@content_container, @requestToken)
 
       window.update_schematics() # For embedded circuit simulator exercises in 6.002x
 

common/lib/xmodule/xmodule/public/js/split_test_staff.js

@@ -18,7 +18,7 @@ function ABTestSelector(runtime, elem) {
             var child_group_id = $(this).data('group-id').toString();
             if(child_group_id === group_id) {
                 _this.content_container.html($(this).text());
-                XBlock.initializeBlocks(_this.content_container);
+                XBlock.initializeBlocks(_this.content_container, $(elem).data('request-token'));
             }
         });
     }

common/static/coffee/spec/xblock/core_spec.coffee

@@ -2,9 +2,21 @@ describe "XBlock", ->
   beforeEach ->
     setFixtures """
       <div>
-        <div class='xblock' id='vA' data-runtime-version="A" data-runtime-class="TestRuntime" data-init="initFnA" data-name="a-name"/>
+        <div class='xblock'
+             id='vA'
+             data-runtime-version="A"
+             data-runtime-class="TestRuntime"
+             data-init="initFnA"
+             data-name="a-name"
+        />
         <div>
-          <div class='xblock' id='vZ' data-runtime-version="Z" data-runtime-class="TestRuntime" data-init="initFnZ"/>
+          <div class='xblock'
+               id='vZ'
+               data-runtime-version="Z"
+               data-runtime-class="TestRuntime"
+               data-init="initFnZ"
+               data-request-token="req-token-z"
+          />
         </div>
         <div class='xblock' id='missing-version' data-init='initFnA' data-name='no-version'/>
         <div class='xblock' id='missing-init' data-runtime-version="A" data-name='no-init'/>
@@ -25,8 +37,11 @@ describe "XBlock", ->
       @fakeChildren = ['list', 'of', 'children']
       spyOn(XBlock, 'initializeBlocks').andReturn(@fakeChildren)
 
-      @vABlock = XBlock.initializeBlock($('#vA')[0])
-      @vZBlock = XBlock.initializeBlock($('#vZ')[0])
+      @vANode = $('#vA')[0]
+      @vZNode = $('#vZ')[0]
+
+      @vABlock = XBlock.initializeBlock(@vANode, 'req-token-a')
+      @vZBlock = XBlock.initializeBlock(@vZNode)
       @missingVersionBlock = XBlock.initializeBlock($('#missing-version')[0])
       @missingInitBlock = XBlock.initializeBlock($('#missing-init')[0])
 
@@ -35,8 +50,8 @@ describe "XBlock", ->
       expect(TestRuntime.vZ).toHaveBeenCalledWith()
 
     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, @vANode)
+      expect(window.initFnZ).toHaveBeenCalledWith(@runtimeZ, @vZNode)
 
     it "loads when missing versions", ->
       expect(@missingVersionBlock.element).toBe($('#missing-version'))
@@ -53,15 +68,29 @@ describe "XBlock", ->
       expect(@vZBlock.name).toBeUndefined()
 
     it "attaches the element to the block", ->
-      expect(@vABlock.element).toBe($('#vA')[0])
-      expect(@vZBlock.element).toBe($('#vZ')[0])
+      expect(@vABlock.element).toBe(@vANode)
+      expect(@vZBlock.element).toBe(@vZNode)
       expect(@missingVersionBlock.element).toBe($('#missing-version')[0])
       expect(@missingInitBlock.element).toBe($('#missing-init')[0])
 
+    it "passes through the request token", ->
+      expect(XBlock.initializeBlocks).toHaveBeenCalledWith($(@vANode), 'req-token-a')
+      expect(XBlock.initializeBlocks).toHaveBeenCalledWith($(@vZNode), 'req-token-z')
+
+
   describe "initializeBlocks", ->
-    it "initializes children", ->
+    beforeEach ->
       spyOn(XBlock, 'initializeBlock')
 
+      @vANode = $('#vA')[0]
+      @vZNode = $('#vZ')[0]
+
+    it "initializes children", ->
       XBlock.initializeBlocks($('#jasmine-fixtures'))
-      expect(XBlock.initializeBlock).toHaveBeenCalledWith($('#vA')[0])
-      expect(XBlock.initializeBlock).toHaveBeenCalledWith($('#vZ')[0])
+      expect(XBlock.initializeBlock).toHaveBeenCalledWith(@vANode, undefined)
+      expect(XBlock.initializeBlock).toHaveBeenCalledWith(@vZNode, undefined)
+
+    it "only initializes matching request tokens", ->
+      XBlock.initializeBlocks($('#jasmine-fixtures'), 'req-token-z')
+      expect(XBlock.initializeBlock).not.toHaveBeenCalledWith(@vANode, jasmine.any(Object))
+      expect(XBlock.initializeBlock).toHaveBeenCalledWith(@vZNode, 'req-token-z')

common/static/coffee/src/xblock/core.coffee

 @XBlock =
   Runtime: {}
 
-  initializeBlock: (element) ->
+  ###
+  Initialize the javascript for a single xblock element, and for all of it's
+  xblock children that match requestToken. If requestToken is omitted, use the
+  data-request-token attribute from element, or use the request-tokens specified on
+  the children themselves.
+  ###
+  initializeBlock: (element, requestToken) ->
     $element = $(element)
-    children = @initializeBlocks($element)
+    requestToken = requestToken or $element.data('request-token')
+    children = @initializeBlocks($element, requestToken)
     runtime = $element.data("runtime-class")
     version = $element.data("runtime-version")
     initFnName = $element.data("init")
@@ -26,7 +33,17 @@
     $element.addClass("xblock-initialized")
     block
 
-  initializeBlocks: (element) ->
-    $(element).immediateDescendents(".xblock").map((idx, elem) =>
-      @initializeBlock elem
+  ###
+  Initialize all XBlocks inside element that were rendered with requestToken.
+  If requestToken is omitted, and element has a 'data-request-token' attribute, use that.
+  If neither is available, then use the request tokens of the immediateDescendent xblocks.
+  ###
+  initializeBlocks: (element, requestToken) ->
+    requestToken = requestToken or $(element).data('request-token')
+    if requestToken
+      selector = ".xblock[data-request-token='#{requestToken}']"
+    else
+      selector = ".xblock"
+    $(element).immediateDescendents(selector).map((idx, elem) =>
+      @initializeBlock(elem, requestToken)
     ).toArray()

lms/djangoapps/courseware/module_render.py

@@ -37,7 +37,14 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.modulestore.django import modulestore, ModuleI18nService
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.util.duedate import get_extended_due_date
-from xmodule_modifiers import replace_course_urls, replace_jump_to_id_urls, replace_static_urls, add_staff_markup, wrap_xblock
+from xmodule_modifiers import (
+    replace_course_urls,
+    replace_jump_to_id_urls,
+    replace_static_urls,
+    add_staff_markup,
+    wrap_xblock,
+    request_token
+)
 from xmodule.lti_module import LTIModule
 from xmodule.x_module import XModuleDescriptor
 
@@ -218,16 +225,26 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours
 
     user_location = getattr(request, 'session', {}).get('country_code')
 
-    return get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id,
-                                              track_function, xqueue_callback_url_prefix,
-                                              position, wrap_xmodule_display, grade_bucket_type,
-                                              static_asset_path, user_location)
+    return get_module_for_descriptor_internal(
+        user=user,
+        descriptor=descriptor,
+        field_data_cache=field_data_cache,
+        course_id=course_id,
+        track_function=track_function,
+        xqueue_callback_url_prefix=xqueue_callback_url_prefix,
+        position=position,
+        wrap_xmodule_display=wrap_xmodule_display,
+        grade_bucket_type=grade_bucket_type,
+        static_asset_path=static_asset_path,
+        user_location=user_location,
+        request_token=request_token(request),
+    )
 
 
 def get_module_system_for_user(user, field_data_cache,
                                # Arguments preceding this comment have user binding, those following don't
                                descriptor, course_id, track_function, xqueue_callback_url_prefix,
-                               position=None, wrap_xmodule_display=True, grade_bucket_type=None,
+                               request_token, position=None, wrap_xmodule_display=True, grade_bucket_type=None,
                                static_asset_path='', user_location=None):
     """
     Helper function that returns a module system and student_data bound to a user and a descriptor.
@@ -243,6 +260,7 @@ def get_module_system_for_user(user, field_data_cache,
 
     Arguments:
         see arguments for get_module()
+        request_token (str): A token unique to the request use by xblock initialization
 
     Returns:
         (LmsModuleSystem, KvsFieldData):  (module system, student_data) bound to, primarily, the user and descriptor
@@ -307,10 +325,20 @@ def get_module_system_for_user(user, field_data_cache,
         """
         # TODO: fix this so that make_xqueue_callback uses the descriptor passed into
         # inner_get_module, not the parent's callback.  Add it as an argument....
-        return get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id,
-                                                  track_function, make_xqueue_callback,
-                                                  position, wrap_xmodule_display, grade_bucket_type,
-                                                  static_asset_path, user_location)
+        return get_module_for_descriptor_internal(
+            user=user,
+            descriptor=descriptor,
+            field_data_cache=field_data_cache,
+            course_id=course_id,
+            track_function=track_function,
+            xqueue_callback_url_prefix=xqueue_callback_url_prefix,
+            position=position,
+            wrap_xmodule_display=wrap_xmodule_display,
+            grade_bucket_type=grade_bucket_type,
+            static_asset_path=static_asset_path,
+            user_location=user_location,
+            request_token=request_token,
+        )
 
     def handle_grade_event(block, event_type, event):
         user_id = event.get('user_id', user.id)
@@ -377,9 +405,18 @@ def get_module_system_for_user(user, field_data_cache,
         )
 
         (inner_system, inner_student_data) = get_module_system_for_user(
-            real_user, field_data_cache_real_user,  # These have implicit user bindings, rest of args considered not to
-            module.descriptor, course_id, track_function, xqueue_callback_url_prefix, position, wrap_xmodule_display,
-            grade_bucket_type, static_asset_path, user_location
+            user=real_user,
+            field_data_cache=field_data_cache_real_user,  # These have implicit user bindings, rest of args considered not to
+            descriptor=module.descriptor,
+            course_id=course_id,
+            track_function=track_function,
+            xqueue_callback_url_prefix=xqueue_callback_url_prefix,
+            position=position,
+            wrap_xmodule_display=wrap_xmodule_display,
+            grade_bucket_type=grade_bucket_type,
+            static_asset_path=static_asset_path,
+            user_location=user_location,
+            request_token=request_token
         )
         # rebinds module to a different student.  We'll change system, student_data, and scope_ids
         module.descriptor.bind_for_student(
@@ -402,9 +439,11 @@ def get_module_system_for_user(user, field_data_cache,
     # javascript to be bound correctly
     if wrap_xmodule_display is True:
         block_wrappers.append(partial(
-            wrap_xblock, 'LmsRuntime',
+            wrap_xblock,
+            'LmsRuntime',
             extra_data={'course-id': course_id.to_deprecated_string()},
-            usage_id_serializer=lambda usage_id: quote_slashes(usage_id.to_deprecated_string())
+            usage_id_serializer=lambda usage_id: quote_slashes(usage_id.to_deprecated_string()),
+            request_token=request_token,
         ))
 
     # TODO (cpennington): When modules are shared between courses, the static
@@ -531,13 +570,16 @@ def get_module_system_for_user(user, field_data_cache,
 
 
 def get_module_for_descriptor_internal(user, descriptor, field_data_cache, course_id,  # pylint: disable=invalid-name
-                                       track_function, xqueue_callback_url_prefix,
+                                       track_function, xqueue_callback_url_prefix, request_token,
                                        position=None, wrap_xmodule_display=True, grade_bucket_type=None,
                                        static_asset_path='', user_location=None):
     """
     Actually implement get_module, without requiring a request.
 
     See get_module() docstring for further details.
+
+    Arguments:
+        request_token (str): A unique token for this request, used to isolate xblock rendering
     """
 
     # Do not check access when it's a noauth request.
@@ -547,9 +589,18 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
             return None
 
     (system, student_data) = get_module_system_for_user(
-        user, field_data_cache,  # These have implicit user bindings, the rest of args are considered not to
-        descriptor, course_id, track_function, xqueue_callback_url_prefix, position, wrap_xmodule_display,
-        grade_bucket_type, static_asset_path, user_location
+        user=user,
+        field_data_cache=field_data_cache,  # These have implicit user bindings, the rest of args are considered not to
+        descriptor=descriptor,
+        course_id=course_id,
+        track_function=track_function,
+        xqueue_callback_url_prefix=xqueue_callback_url_prefix,
+        position=position,
+        wrap_xmodule_display=wrap_xmodule_display,
+        grade_bucket_type=grade_bucket_type,
+        static_asset_path=static_asset_path,
+        user_location=user_location,
+        request_token=request_token
     )
 
     descriptor.bind_for_student(system, LmsFieldData(descriptor._field_data, student_data))  # pylint: disable=protected-access

lms/djangoapps/courseware/tests/test_module_render.py

@@ -802,12 +802,13 @@ class TestAnonymousStudentId(ModuleStoreTestCase, LoginEnrollmentTestCase):
             descriptor.module_class = xblock_class.module_class
 
         return render.get_module_for_descriptor_internal(
-            self.user,
-            descriptor,
-            Mock(spec=FieldDataCache),
-            course_id,
-            Mock(),  # Track Function
-            Mock(),  # XQueue Callback Url Prefix
+            user=self.user,
+            descriptor=descriptor,
+            field_data_cache=Mock(spec=FieldDataCache),
+            course_id=course_id,
+            track_function=Mock(),  # Track Function
+            xqueue_callback_url_prefix=Mock(),  # XQueue Callback Url Prefix
+            request_token='request_token',
         ).xmodule_runtime.anonymous_student_id
 
     @ddt.data(*PER_STUDENT_ANONYMIZED_DESCRIPTORS)

lms/djangoapps/instructor/views/instructor_dashboard.py

 """
 Instructor Dashboard Views
 """
-from django.views.decorators.http import require_POST
 
-from django.contrib.auth.decorators import login_required
 import logging
 import datetime
+import uuid
 import pytz
+
+from django.contrib.auth.decorators import login_required
+from django.views.decorators.http import require_POST
 from django.utils.translation import ugettext as _
 from django_future.csrf import ensure_csrf_cookie
 from django.views.decorators.cache import cache_control
@@ -308,6 +310,7 @@ def _section_data_download(course_key, access):
 
 def _section_send_email(course_key, access, course):
     """ Provide data for the corresponding bulk email section """
+    # This HtmlDescriptor is only being used to generate a nice text editor.
     html_module = HtmlDescriptor(
         course.system,
         DictFieldData({'data': ''}),
@@ -317,7 +320,10 @@ def _section_send_email(course_key, access, course):
     fragment = wrap_xblock(
         'LmsRuntime', html_module, 'studio_view', fragment, None,
         extra_data={"course-id": course_key.to_deprecated_string()},
-        usage_id_serializer=lambda usage_id: quote_slashes(usage_id.to_deprecated_string())
+        usage_id_serializer=lambda usage_id: quote_slashes(usage_id.to_deprecated_string()),
+        # Generate a new request_token here at random, because this module isn't connected to any other
+        # xblock rendering.
+        request_token=uuid.uuid1().get_hex()
     )
     email_editor = fragment.content
     section_data = {

lms/djangoapps/instructor/views/legacy.py

@@ -26,7 +26,7 @@ from django.core.urlresolvers import reverse
 from django.core.mail import send_mail
 from django.utils import timezone
 
-from xmodule_modifiers import wrap_xblock
+from xmodule_modifiers import wrap_xblock, request_token
 import xmodule.graders as xmgraders
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
@@ -985,7 +985,8 @@ def instructor_dashboard(request, course_id):
         fragment = wrap_xblock(
             'LmsRuntime', html_module, 'studio_view', fragment, None,
             extra_data={"course-id": course_key.to_deprecated_string()},
-            usage_id_serializer=lambda usage_id: quote_slashes(usage_id.to_deprecated_string())
+            usage_id_serializer=lambda usage_id: quote_slashes(usage_id.to_deprecated_string()),
+            request_token=request_token(request),
         )
         email_editor = fragment.content
 

lms/djangoapps/instructor_task/tasks_helper.py

@@ -372,9 +372,17 @@ def _get_module_instance_for_task(course_id, student, module_descriptor, xmodule
     xqueue_callback_url_prefix = xmodule_instance_args.get('xqueue_callback_url_prefix', '') \
         if xmodule_instance_args is not None else ''
 
-    return get_module_for_descriptor_internal(student, module_descriptor, field_data_cache, course_id,
-                                              make_track_function(), xqueue_callback_url_prefix,
-                                              grade_bucket_type=grade_bucket_type)
+    return get_module_for_descriptor_internal(
+        user=student,
+        descriptor=module_descriptor,
+        field_data_cache=field_data_cache,
+        course_id=course_id,
+        track_function=make_track_function(),
+        xqueue_callback_url_prefix=xqueue_callback_url_prefix,
+        grade_bucket_type=grade_bucket_type,
+        # This module isn't being used for front-end rendering
+        request_token=None,
+    )
 
 
 @transaction.autocommit