Merge pull request #1531 from cpennington/lms-xblock-handlers

Use XBlock handlers in the LMS ...

cms/djangoapps/contentstore/views/preview.py

@@ -1,22 +1,23 @@
 import logging
-import sys
 from functools import partial
 
 from django.conf import settings
-from django.http import HttpResponse, Http404, HttpResponseBadRequest, HttpResponseForbidden
 from django.core.urlresolvers import reverse
+from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden
 from django.contrib.auth.decorators import login_required
 from mitxmako.shortcuts import render_to_response, render_to_string
 
 from xmodule_modifiers import replace_static_urls, wrap_xblock
 from xmodule.error_module import ErrorDescriptor
-from xmodule.errortracker import exc_info_to_str
 from xmodule.exceptions import NotFoundError, ProcessingError
 from xmodule.modulestore.django import modulestore
 from xmodule.x_module import ModuleSystem
 from xblock.runtime import DbModel
+from xblock.django.request import webob_to_django_response, django_to_webob_request
+from xblock.exceptions import NoSuchHandlerError
 
-from lms.xblock.field_data import LmsFieldData
+from lms.lib.xblock.field_data import LmsFieldData
+from lms.lib.xblock.runtime import quote_slashes, unquote_slashes
 
 from util.sandboxing import can_execute_unsafe_code
 
@@ -26,30 +27,47 @@
 from .access import has_access
 from ..utils import get_course_for_item
 
-__all__ = ['preview_dispatch', 'preview_component']
+__all__ = ['preview_handler', 'preview_component']
 
 log = logging.getLogger(__name__)
 
 
-@login_required
-def preview_dispatch(request, preview_id, location, dispatch=None):
+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.
     """
-    Dispatch an AJAX action to a preview XModule
+    return reverse('preview_handler', kwargs={
+        'usage_id': quote_slashes(str(block.scope_ids.usage_id)),
+        'handler': handler,
+        'suffix': suffix,
+    }).rstrip('/?')
 
-    Expects a POST request, and passes the arguments to the module
 
-    preview_id (str): An identifier specifying which preview this module is used for
-    location: The Location of the module to dispatch to
-    dispatch: The action to execute
+@login_required
+def preview_handler(request, usage_id, handler, suffix=''):
     """
+    Dispatch an AJAX action to an xblock
+
+    usage_id: The usage-id of the block to dispatch to, passed through `quote_slashes`
+    handler: The handler to execute
+    suffix: The remaineder of the url to be passed to the handler
+    """
+
+    location = unquote_slashes(usage_id)
 
     descriptor = modulestore().get_item(location)
-    instance = load_preview_module(request, preview_id, descriptor)
+    instance = load_preview_module(request, descriptor)
     # Let the module handle the AJAX
+    req = django_to_webob_request(request)
     try:
-        ajax_return = instance.handle_ajax(dispatch, request.POST)
-        # Save any module data that has changed to the underlying KeyValueStore
-        instance.save()
+        resp = instance.handle(handler, req, suffix)
+
+    except NoSuchHandlerError:
+        log.exception("XBlock %s attempted to access missing handler %r", instance, handler)
+        raise Http404
 
     except NotFoundError:
         log.exception("Module indicating to user that request doesn't exist")
@@ -60,11 +78,11 @@ def preview_dispatch(request, preview_id, location, dispatch=None):
                     exc_info=True)
         return HttpResponseBadRequest()
 
-    except:
+    except Exception:
         log.exception("error processing ajax call")
         raise
 
-    return HttpResponse(ajax_return)
+    return webob_to_django_response(resp)
 
 
 @login_required
@@ -77,7 +95,7 @@ def preview_component(request, location):
     component = modulestore().get_item(location)
     # Wrap the generated fragment in the xmodule_editor div so that the javascript
     # can bind to it correctly
-    component.runtime.wrappers.append(wrap_xblock)
+    component.runtime.wrappers.append(partial(wrap_xblock, handler_prefix))
 
     try:
         content = component.render('studio_view').content
@@ -88,30 +106,36 @@ def preview_component(request, location):
         content = render_to_string('html_error.html', {'message': str(exc)})
 
     return render_to_response('component.html', {
-        'preview': get_preview_html(request, component, 0),
+        'preview': get_preview_html(request, component),
         'editor': content
     })
 
 
-def preview_module_system(request, preview_id, descriptor):
+class PreviewModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
+    """
+    An XModule ModuleSystem for use in Studio previews
+    """
+    def handler_url(self, block, handler_name, suffix='', query=''):
+        return handler_prefix(block, handler_name, suffix) + '?' + query
+
+
+def preview_module_system(request, descriptor):
     """
     Returns a ModuleSystem for the specified descriptor that is specialized for
     rendering module previews.
 
     request: The active django request
-    preview_id (str): An identifier specifying which preview this module is used for
     descriptor: An XModuleDescriptor
     """
 
     course_id = get_course_for_item(descriptor.location).location.course_id
 
-    return ModuleSystem(
+    return PreviewModuleSystem(
         static_url=settings.STATIC_URL,
-        ajax_url=reverse('preview_dispatch', args=[preview_id, descriptor.location.url(), '']).rstrip('/'),
         # TODO (cpennington): Do we want to track how instructors are using the preview problems?
         track_function=lambda event_type, event: None,
         filestore=descriptor.runtime.resources_fs,
-        get_module=partial(load_preview_module, request, preview_id),
+        get_module=partial(load_preview_module, request),
         render_template=render_from_lms,
         debug=True,
         replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id),
@@ -124,7 +148,7 @@ def preview_module_system(request, preview_id, 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, display_name_only=descriptor.location.category == 'static_tab'),
+            partial(wrap_xblock, handler_prefix, 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
@@ -138,28 +162,27 @@ def preview_module_system(request, preview_id, descriptor):
     )
 
 
-def load_preview_module(request, preview_id, descriptor):
+def load_preview_module(request, descriptor):
     """
     Return a preview XModule instantiated from the supplied descriptor.
 
     request: The active django request
-    preview_id (str): An identifier specifying which preview this module is used for
     descriptor: An XModuleDescriptor
     """
     student_data = DbModel(SessionKeyValueStore(request))
     descriptor.bind_for_student(
-        preview_module_system(request, preview_id, descriptor),
+        preview_module_system(request, descriptor),
         LmsFieldData(descriptor._field_data, student_data),  # pylint: disable=protected-access
     )
     return descriptor
 
 
-def get_preview_html(request, descriptor, idx):
+def get_preview_html(request, descriptor):
     """
     Returns the HTML returned by the XModule's student_view,
     specified by the descriptor and idx.
     """
-    module = load_preview_module(request, str(idx), descriptor)
+    module = load_preview_module(request, descriptor)
     try:
         content = module.render("student_view").content
     except Exception as exc:                          # pylint: disable=W0703

cms/envs/common.py

@@ -28,7 +28,7 @@
 from lms.envs.common import USE_TZ, TECH_SUPPORT_EMAIL, PLATFORM_NAME, BUGS_EMAIL
 from path import path
 
-from lms.xblock.mixin import LmsBlockMixin
+from lms.lib.xblock.mixin import LmsBlockMixin
 from cms.xmodule_namespace import CmsBlockMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
 from xmodule.x_module import XModuleMixin

cms/urls.py

@@ -31,8 +31,8 @@
     url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'),
     url(r'^reorder_static_tabs', 'contentstore.views.reorder_static_tabs', name='reorder_static_tabs'),
 
-    url(r'^preview/modx/(?P<preview_id>[^/]*)/(?P<location>.*?)/(?P<dispatch>[^/]*)$',
-        'contentstore.views.preview_dispatch', name='preview_dispatch'),
+    url(r'^preview/xblock/(?P<usage_id>.*?)/handler/(?P<handler>[^/]*)(?:/(?P<suffix>[^/]*))?$',
+        'contentstore.views.preview_handler', name='preview_handler'),
 
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/info/(?P<name>[^/]+)$',
         'contentstore.views.course_info', name='course_info'),

common/djangoapps/student/management/commands/sync_user_info.py

@@ -4,7 +4,7 @@
 
 from django.core.management.base import BaseCommand
 from django.contrib.auth.models import User
-import comment_client as cc
+import lms.lib.comment_client as cc
 
 
 class Command(BaseCommand):

common/djangoapps/student/models.py

@@ -27,7 +27,7 @@
 from django.forms import ModelForm, forms
 
 from course_modes.models import CourseMode
-import comment_client as cc
+import lms.lib.comment_client as cc
 from pytz import UTC
 import crum
 
@@ -41,6 +41,7 @@
 log = logging.getLogger(__name__)
 AUDIT_LOG = logging.getLogger("audit")
 
+
 class UserStanding(models.Model):
     """
     This table contains a student's account's status.

common/djangoapps/xmodule_modifiers.py

@@ -29,11 +29,14 @@ def wrap_fragment(fragment, new_content):
     return wrapper_frag
 
 
-def wrap_xblock(block, view, frag, context, display_name_only=False):  # pylint: disable=unused-argument
+def wrap_xblock(handler_prefix, block, view, frag, context, display_name_only=False):  # 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.
 
+    :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 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
@@ -63,7 +66,7 @@ def wrap_xblock(block, view, frag, context, display_name_only=False):  # pylint:
     if frag.js_init_fn:
         data['init'] = frag.js_init_fn
         data['runtime-version'] = frag.js_init_version
-        data['usage-id'] = block.scope_ids.usage_id
+        data['handler-prefix'] = handler_prefix(block)
         data['block-type'] = block.scope_ids.block_type
 
     template_context = {

common/lib/capa/capa/tests/__init__.py

@@ -33,9 +33,9 @@ def test_system():
     """
     the_system = Mock(
         spec=ModuleSystem,
+        ajax_url='/dummy-ajax-url',
         STATIC_URL='/dummy-static/',
         DEBUG=True,
-        ajax_url='courses/course_id/modx/a_location',
         track_function=Mock(),
         get_module=Mock(),
         render_template=tst_render_template,

common/lib/logsettings.py

@@ -82,7 +82,7 @@ def get_logger_config(log_dir,
             },
             'newrelic': {
                 'level': 'ERROR',
-                'class': 'newrelic_logging.NewRelicHandler',
+                'class': 'lms.lib.newrelic_logging.NewRelicHandler',
                 'formatter': 'raw',
             }
         },

common/lib/symmath/symmath/formula.py

@@ -14,6 +14,7 @@
 import re
 import logging
 import operator
+import requests
 import sympy
 from sympy.printing.latex import LatexPrinter
 from sympy.printing.str import StrPrinter
@@ -25,11 +26,9 @@
 # import sympy.physics.quantum.qubit
 
 from xml.sax.saxutils import unescape
-import sympy
 import unicodedata
 from lxml import etree
 #import subprocess
-import requests
 from copy import deepcopy
 
 log = logging.getLogger(__name__)

common/lib/xmodule/setup.py

@@ -51,6 +51,7 @@
         'docopt',
         'capa',
         'path.py',
+        'webob',
     ],
     package_data={
         'xmodule': ['js/module/*'],

common/lib/xmodule/xmodule/capa_module.py

@@ -803,7 +803,7 @@ def make_dict_of_responses(data):
         """
         Make dictionary of student responses (aka "answers")
 
-        `data` is POST dictionary (Django QueryDict).
+        `data` is POST dictionary (webob.multidict.MultiDict).
 
         The `data` dict has keys of the form 'x_y', which are mapped
         to key 'y' in the returned dict.  For example,
@@ -835,7 +835,10 @@ def make_dict_of_responses(data):
         """
         answers = dict()
 
-        for key in data:
+        # webob.multidict.MultiDict is a view of a list of tuples,
+        # so it will return a multi-value key once for each value.
+        # We only want to consider each key a single time, so we use set(data.keys())
+        for key in set(data.keys()):
             # e.g. input_resistor_1 ==> resistor_1
             _, _, name = key.partition('_')
 
@@ -857,7 +860,7 @@ def make_dict_of_responses(data):
                 name = name[:-2] if is_list_key or is_dict_key else name
 
                 if is_list_key:
-                    val = data.getlist(key)
+                    val = data.getall(key)
                 elif is_dict_key:
                     try:
                         val = json.loads(data[key])

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -217,9 +217,8 @@ def html_id(self):
         Return a string with a version of the location that is safe for use in
         html id attributes
         """
-        s = "-".join(str(v) for v in self.list()
-                     if v is not None)
-        return Location.clean_for_html(s)
+        id_string = "-".join(str(v) for v in self.list() if v is not None)
+        return Location.clean_for_html(id_string)
 
     def dict(self):
         """

common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py

@@ -213,16 +213,18 @@ def save_assessment(self, data, _system):
 
         with 'error' only present if 'success' is False, and 'hint_html' or
         'message_html' only if success is true
+
+        :param data: A `webob.multidict.MultiDict` containing the keys
+            asasssment: The sum of assessment scores
+            score_list[]: A multivalue key containing all the individual scores
         """
 
         if self.child_state != self.ASSESSING:
             return self.out_of_sync_error(data)
 
         try:
             score = int(data.get('assessment'))
-            score_list = data.getlist('score_list[]')
-            for i in xrange(0, len(score_list)):
-                score_list[i] = int(score_list[i])
+            score_list = [int(x) for x in data.getall('score_list[]')]
         except (ValueError, TypeError):
             # This is a dev_facing_error
             log.error("Non-integer score value passed to save_assessment, or no score list present.")