Fix StudentViewTransformer collect, modify LMS handler_url.

Small, localized changes:

* Shifts some of the HtmlModule logic around, moving student view
  handler logic to HtmlBlock (a replacement for HtmlFields, as we move
  just a little closer to making it a real XBlock).

* Removes has_multi_device_support() from capa, so that we don't have
  to mess around with student state. To determine whether all response
  types in that particular problem support mobile, we make use of
  self.problem_types (originally built for content library filtering).

The bigger changes are to handler URL generation:

* The LMS now also monkey-patches
  xmodule.x_module.descriptor_global_handler_url and
  xmodule.x_module.descriptor_global_local_resource_url so that we can
  get LMS XBlock URLs from the DescriptorSystem. That functionality is
  needed in the block transforms collect() phase for certain XModules
  like Video. For instance, say we want to generate the transcripts
  URLs. The collect phase is run asynchronously, without a user context.

* The URL handler monkey-patching is now done in the startup.py files
  for LMS and Studio. Studio used to do this in the import of
  cms/djangoapps/contentstore/views/item.py. This was mostly just
  because it seemed like a sane and consistent place to put it.

* LmsHandlerUrls was removed, its handler_url and local_resource_url
  methods were moved to be top level functions. The only reason that
  class existed seems to be to give a place to store course_id state,
  and that can now be derived from the block location.

* To avoid the Module -> Descriptor ProxyAttribute magic that we do
  (which explodes with an UndefinedContext error because there is no
  user involved), when examining the block's handler method in
  handler_url, I made a few changes:

** Check the .__class__ to see if the handler was defined, instead of the
   block itself.

** The above required me to relax the check for _is_xblock_handler on the
   function, since that will no longer be defined.

90% of this goes away when we kill XModules and do the refactoring we've
wanted to do for a while.

cms/djangoapps/contentstore/views/item.py

@@ -50,7 +50,6 @@ from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primar
 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, local_resource_url
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import LibraryUsageLocator
 from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW
@@ -67,12 +66,6 @@ CREATE_IF_NOT_FOUND = ['course_info']
 NEVER = lambda x: False
 ALWAYS = lambda x: True
 
-# 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
-xmodule.x_module.descriptor_global_local_resource_url = local_resource_url
-
 
 def hash_resource(resource):
     """

cms/lib/xblock/runtime.py

@@ -9,7 +9,6 @@ def handler_url(block, handler_name, suffix='', query='', thirdparty=False):
     """
     Handler URL function for Studio
     """
-
     if thirdparty:
         raise NotImplementedError("edX Studio doesn't support third-party xblock handler urls")
 

cms/startup.py

@@ -10,6 +10,8 @@ settings.INSTALLED_APPS  # pylint: disable=pointless-statement
 from openedx.core.lib.django_startup import autostartup
 from monkey_patch import django_utils_translation
 
+import xmodule.x_module
+import cms.lib.xblock.runtime
 
 def run():
     """
@@ -24,6 +26,13 @@ def run():
     if settings.FEATURES.get('USE_CUSTOM_THEME', False):
         enable_theme()
 
+    # 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
+    # https://openedx.atlassian.net/wiki/display/PLAT/Convert+from+Storage-centric+runtimes+to+Application-centric+runtimes
+    xmodule.x_module.descriptor_global_handler_url = cms.lib.xblock.runtime.handler_url
+    xmodule.x_module.descriptor_global_local_resource_url = cms.lib.xblock.runtime.local_resource_url
+
 
 def add_mimetypes():
     """

common/lib/capa/capa/capa_problem.py

@@ -574,15 +574,6 @@ class LoncapaProblem(object):
             log.warning("Could not find matching input for id: %s", input_id)
             return {}
 
-    @property
-    def has_multi_device_support(self):
-        """
-        Returns whether this capa problem has multi-device support.
-        """
-        return all(
-            responder.multi_device_support for responder in self.responders.values()
-        )
-
     # ======= Private Methods Below ========
 
     def _process_includes(self):

common/lib/xmodule/xmodule/capa_module.py

@@ -230,9 +230,11 @@ class CapaDescriptor(CapaFields, RawDescriptor):
         Returns whether the given view has support for the given functionality.
         """
         if functionality == "multi_device":
-            return self.lcp.has_multi_device_support
-        else:
-            return False
+            return all(
+                responsetypes.registry.get_class_for_tag(tag).multi_device_support
+                for tag in self.problem_types
+            )
+        return False
 
     # Proxy to CapaModule for access to any of its attributes
     answer_available = module_attr('answer_available')

common/lib/xmodule/xmodule/html_module.py

@@ -20,6 +20,7 @@ from xmodule.x_module import XModule, DEPRECATION_VSCOMPAT_EVENT
 from xmodule.xml_module import XmlDescriptor, name_to_pathname
 from xblock.core import XBlock
 from xblock.fields import Scope, String, Boolean, List
+from xblock.fragment import Fragment
 
 log = logging.getLogger("edx.courseware")
 
@@ -27,7 +28,12 @@ log = logging.getLogger("edx.courseware")
 _ = lambda text: text
 
 
-class HtmlFields(object):
+class HtmlBlock(object):
+    """
+    This will eventually subclass XBlock and merge HtmlModule and HtmlDescriptor
+    into one. For now, it's a place to put the pieces that are already sharable
+    between the two (field information and XBlock handlers).
+    """
     display_name = String(
         display_name=_("Display Name"),
         help=_("This name appears in the horizontal navigation at the top of the page."),
@@ -54,8 +60,21 @@ class HtmlFields(object):
         scope=Scope.settings
     )
 
+    @XBlock.supports("multi_device")
+    def student_view(self, _context):
+        return Fragment(self.get_html())
+
+    def get_html(self):
+        """
+        When we switch this to an XBlock, we can merge this with student_view,
+        but for now the XModule mixin requires that this method be defined.
+        """
+        if self.system.anonymous_student_id:
+            return self.data.replace("%%USER_ID%%", self.system.anonymous_student_id)
+        return self.data  
+
 
-class HtmlModuleMixin(HtmlFields, XModule):
+class HtmlModuleMixin(HtmlBlock, XModule):
     """
     Attributes and methods used by HtmlModules internally.
     """
@@ -73,23 +92,14 @@ class HtmlModuleMixin(HtmlFields, XModule):
     js_module_name = "HTMLModule"
     css = {'scss': [resource_string(__name__, 'css/html/display.scss')]}
 
-    def get_html(self):
-        if self.system.anonymous_student_id:
-            return self.data.replace("%%USER_ID%%", self.system.anonymous_student_id)
-        return self.data
-
 
 @edxnotes
 class HtmlModule(HtmlModuleMixin):
     """
     Module for putting raw html in a course
     """
-    @XBlock.supports("multi_device")
-    def student_view(self, context):
-        return super(HtmlModule, self).student_view(context)
-
 
-class HtmlDescriptor(HtmlFields, XmlDescriptor, EditingDescriptor):  # pylint: disable=abstract-method
+class HtmlDescriptor(HtmlBlock, XmlDescriptor, EditingDescriptor):  # pylint: disable=abstract-method
     """
     Module for putting raw html in a course
     """

common/lib/xmodule/xmodule/video_module/video_module.py

@@ -765,11 +765,13 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
         """
         return edxval_api.get_video_info_for_course_and_profiles(unicode(course_id), video_profile_names)
 
-    def student_view_data(self, context):
+    def student_view_data(self, context=None):
         """
         Returns a JSON representation of the student_view of this XModule.
         The contract of the JSON content is between the caller and the particular XModule.
         """
+        context = context or {}
+
         # If the "only_on_web" field is set on this video, do not return the rest of the video's data
         # in this json view, since this video is to be accessed only through its web view."
         if self.only_on_web:

common/lib/xmodule/xmodule/x_module.py

@@ -36,7 +36,6 @@ from opaque_keys.edx.asides import AsideUsageKeyV1, AsideDefinitionKeyV1
 from xmodule.exceptions import UndefinedContext
 import dogstats_wrapper as dog_stats_api
 
-
 log = logging.getLogger(__name__)
 
 XMODULE_METRIC_NAME = 'edxapp.xmodule'
@@ -1207,17 +1206,21 @@ class ConfigurableFragmentWrapper(object):  # pylint: disable=abstract-method
 
 # This function exists to give applications (LMS/CMS) a place to monkey-patch until
 # we can refactor modulestore to split out the FieldData half of its interface from
-# the Runtime part of its interface. This function matches the Runtime.handler_url interface
+# the Runtime part of its interface. This function mostly matches the
+# Runtime.handler_url interface.
+#
+# The monkey-patching happens in (lms|cms)/startup.py
 def descriptor_global_handler_url(block, handler_name, suffix='', query='', thirdparty=False):  # pylint: disable=invalid-name, unused-argument
     """
     See :meth:`xblock.runtime.Runtime.handler_url`.
     """
     raise NotImplementedError("Applications must monkey-patch this function before using handler_url for studio_view")
 
-
 # This function exists to give applications (LMS/CMS) a place to monkey-patch until
 # we can refactor modulestore to split out the FieldData half of its interface from
 # the Runtime part of its interface. This function matches the Runtime.local_resource_url interface
+#
+# The monkey-patching happens in (lms|cms)/startup.py
 def descriptor_global_local_resource_url(block, uri):  # pylint: disable=invalid-name, unused-argument
     """
     See :meth:`xblock.runtime.Runtime.local_resource_url`.
@@ -1821,6 +1824,7 @@ class CombinedSystem(object):
         DescriptorSystem, instead. This allows XModuleDescriptors that are bound as XModules
         to still function as XModuleDescriptors.
         """
+        # First we try a lookup in the module system...
         try:
             return getattr(self._module_system, name)
         except AttributeError:

lms/djangoapps/course_api/blocks/transformers/student_view.py

@@ -17,25 +17,45 @@ class StudentViewTransformer(BlockStructureTransformer):
         """
         Collect student_view_multi_device and student_view_data values for each block
         """
-        # TODO
-        # File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/x_module.py", line 1125, in _xmodule
-        #     raise UndefinedContext()
+        for block_key in block_structure.topological_traversal():
+            block = block_structure.get_xblock(block_key)
 
-        # for block_key in block_structure.topological_traversal():
-        #     block = block_structure.get_xblock(block_key)
-        #     block_structure.set_transformer_block_data(
-        #         block_key,
-        #         cls,
-        #         cls.STUDENT_VIEW_MULTI_DEVICE,
-        #         block.has_support(getattr(block, 'student_view', None), 'multi_device'),
-        #     )
-        #     if getattr(block, 'student_view_data', None):
-        #         block_structure.set_transformer_block_data(
-        #             block_key,
-        #             cls,
-        #             cls.STUDENT_VIEW_DATA,
-        #             block.student_view_data(),
-        #         )
+            # We're iterating through descriptors (not bound to a user) that are
+            # given to us by the modulestore. The reason we look at 
+            # block.__class__ is to avoid the XModuleDescriptor -> XModule
+            # proxying that would happen if we just examined block directly,
+            # since it's likely that student_view() is going to be defined on
+            # the XModule side.
+            #
+            # If that proxying happens, this method will throw an
+            # UndefinedContext exception, because we haven't initialized any of
+            # the user-specific context.
+            #
+            # This isn't a problem for pure XBlocks, because it's all in one
+            # class, and there's no proxying. So basically, if you encounter a
+            # problem where your particular XModule explodes here (and don't
+            # have the time to convert it to an XBlock), please try refactoring
+            # so that you declare your student_view() method in a common
+            # ancestor class of both your Descriptor and Module classes. As an
+            # example, I changed the name of HtmlFields to HtmlBlock and moved
+            # student_view() from HtmlModuleMixin to HtmlBlock.
+            student_view = getattr(block.__class__, 'student_view', None)
+            supports_multi_device = block.has_support(student_view, 'multi_device')
+
+            block_structure.set_transformer_block_data(
+                block_key,
+                cls,
+                cls.STUDENT_VIEW_MULTI_DEVICE,
+                supports_multi_device,
+            )
+            if getattr(block, 'student_view_data', None):
+                student_view_data = block.student_view_data()
+                block_structure.set_transformer_block_data(
+                    block_key,
+                    cls,
+                    cls.STUDENT_VIEW_DATA,
+                    student_view_data,
+                )
 
     def transform(self, user_info, block_structure):
         """

lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py

+from openedx.core.lib.block_cache.block_structure import BlockStructureFactory
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+
+from ..student_view import StudentViewTransformer
+
+
+class TestStudentViewTransformer(ModuleStoreTestCase):
+    
+    def setUp(self):
+        super(TestStudentViewTransformer, self).setUp()
+        self.course_key = self.create_toy_course()
+        self.course_root_loc = self.store.make_course_usage_key(self.course_key)
+        self.block_structure = BlockStructureFactory.create_from_modulestore(
+            self.course_root_loc, self.store
+        )
+
+    def test_collect(self):
+        StudentViewTransformer.collect(self.block_structure)

lms/djangoapps/course_api/blocks/views.py

@@ -128,7 +128,7 @@ class CourseBlocks(DeveloperErrorViewMixin, ListAPIView):
         blocks = get_course_blocks(
             params.cleaned_data['user'],
             params.cleaned_data['usage_key'],
-            transformers=LMS_COURSE_TRANSFORMERS | {blocks_api_transformer},
+            transformers=LMS_COURSE_TRANSFORMERS + [blocks_api_transformer],
         )
 
         return Response(

lms/djangoapps/lms_xblock/runtime.py

@@ -66,64 +66,67 @@ def unquote_slashes(text):
     return re.sub(r'(;;|;_)', _unquote_slashes, text)
 
 
-class LmsHandlerUrls(object):
-    """
-    A runtime mixin that provides a handler_url function that routes
-    to the LMS' xblock handler view.
-
-    This must be mixed in to a runtime that already accepts and stores
-    a course_id
-    """
-    # pylint: disable=unused-argument
-    def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False):
-        """See :method:`xblock.runtime:Runtime.handler_url`"""
-        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': unicode(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
-
-        # If third-party, return fully-qualified url
-        if thirdparty:
-            scheme = "https" if settings.HTTPS == "on" else "http"
-            url = '{scheme}://{host}{path}'.format(
-                scheme=scheme,
-                host=settings.SITE_NAME,
-                path=url
-            )
-
-        return url
-
-    def local_resource_url(self, block, uri):
-        """
-        local_resource_url for Studio
-        """
-        path = reverse('xblock_resource_url', kwargs={
-            'block_type': block.scope_ids.block_type,
-            'uri': uri,
-        })
-        return '//{}{}'.format(settings.SITE_NAME, path)
+def handler_url(block, handler_name, suffix='', query='', thirdparty=False):
+    """
+    This method matches the signature for `xblock.runtime:Runtime.handler_url()`
+
+    See :method:`xblock.runtime:Runtime.handler_url`
+    """
+    view_name = 'xblock_handler'
+    if handler_name:
+        # Be sure this is really a handler.
+        #
+        # We're checking the .__class__ instead of the block itself to avoid
+        # auto-proxying from Descriptor -> Module, in case descriptors want
+        # to ask for handler URLs without a student context.
+        func = getattr(block.__class__, handler_name, None)
+        if not func:
+            raise ValueError("{!r} is not a function name".format(handler_name))
+
+        # Is the following necessary? ProxyAttribute causes an UndefinedContext error
+        # if trying this without the module system.
+        #
+        #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': unicode(block.location.course_key),
+        '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
+
+    # If third-party, return fully-qualified url
+    if thirdparty:
+        scheme = "https" if settings.HTTPS == "on" else "http"
+        url = '{scheme}://{host}{path}'.format(
+            scheme=scheme,
+            host=settings.SITE_NAME,
+            path=url
+        )
+
+    return url
+
+def local_resource_url(block, uri):
+    """
+    local_resource_url for Studio
+    """
+    path = reverse('xblock_resource_url', kwargs={
+        'block_type': block.scope_ids.block_type,
+        'uri': uri,
+    })
+    return '//{}{}'.format(settings.SITE_NAME, path)
 
 
 class LmsPartitionService(PartitionService):
@@ -190,7 +193,7 @@ class UserTagsService(object):
         )
 
 
-class LmsModuleSystem(LmsHandlerUrls, ModuleSystem):  # pylint: disable=abstract-method
+class LmsModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
     """
     ModuleSystem specialized to the LMS
     """
@@ -210,6 +213,30 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem):  # pylint: disable=abstract
         self.request_token = kwargs.pop('request_token', None)
         super(LmsModuleSystem, self).__init__(**kwargs)
 
+    def handler_url(self, *args, **kwargs):
+        """
+        Implement the XBlock runtime handler_url interface.
+
+        This is mostly just proxying to the module level `handler_url` function
+        defined higher up in this file.
+
+        We're doing this indirection because the module level `handler_url`
+        logic is also needed by the `DescriptorSystem`. The particular
+        `handler_url` that a `DescriptorSystem` needs will be different when
+        running an LMS process or a CMS/Studio process. That's accomplished by
+        monkey-patching a global. It's a long story, but please know that you
+        can't just refactor and fold that logic into here without breaking
+        things.
+
+        https://openedx.atlassian.net/wiki/display/PLAT/Convert+from+Storage-centric+runtimes+to+Application-centric+runtimes
+
+        See :method:`xblock.runtime:Runtime.handler_url`
+        """
+        return handler_url(*args, **kwargs)
+
+    def local_resource_url(self, *args, **kwargs):
+        return local_resource_url(*args, **kwargs)
+
     def wrap_aside(self, block, aside, view, frag, context):
         """
         Creates a div which identifies the aside, points to the original block,

lms/djangoapps/lms_xblock/test/test_runtime.py

@@ -8,7 +8,7 @@ from ddt import ddt, data
 from mock import Mock
 from unittest import TestCase
 from urlparse import urlparse
-from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.locations import BlockUsageLocator, CourseLocator, SlashSeparatedCourseKey
 from lms.djangoapps.lms_xblock.runtime import quote_slashes, unquote_slashes, LmsModuleSystem
 from xblock.fields import ScopeIds
 
@@ -39,12 +39,31 @@ class TestQuoteSlashes(TestCase):
         self.assertNotIn('/', quote_slashes(test_string))
 
 
+class BlockMock(Mock):
+    """Mock class that we fill with our "handler" methods."""
+
+    def handler(self, _context):
+        pass
+
+    def handler1(self, _context):
+        pass
+
+    def handler_a(self, _context):
+        pass
+
+    @property
+    def location(self):
+        """Create a functional BlockUsageLocator for testing URL generation."""
+        course_key = CourseLocator(org="mockx", course="100", run="2015")
+        return BlockUsageLocator(course_key, block_type='mock_type', block_id="mock_id")
+
+
 class TestHandlerUrl(TestCase):
     """Test the LMS handler_url"""
 
     def setUp(self):
         super(TestHandlerUrl, self).setUp()
-        self.block = Mock(name='block', scope_ids=ScopeIds(None, None, None, 'dummy'))
+        self.block = BlockMock(name='block', scope_ids=ScopeIds(None, None, None, 'dummy'))
         self.course_key = SlashSeparatedCourseKey("org", "course", "run")
         self.runtime = LmsModuleSystem(
             static_url='/static',

lms/envs/devstack.py

@@ -229,3 +229,14 @@ except ImportError:
 MODULESTORE = convert_module_store_setting_if_needed(MODULESTORE)
 
 SECRET_KEY = '85920908f28904ed733fe576320db18cabd7b6cd'
+
+
+#####################################################################
+# Mobile
+FEATURES['ENABLE_RENDER_XBLOCK_API'] = True
+
+
+CACHES['lms.course_blocks'] = {
+        'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
+        'KEY_FUNCTION': 'util.memcache.safe_key',
+}

lms/startup.py

@@ -16,6 +16,9 @@ from monkey_patch import django_utils_translation
 import analytics
 
 
+import xmodule.x_module
+import lms_xblock.runtime
+
 log = logging.getLogger(__name__)
 
 
@@ -54,6 +57,13 @@ def run():
         set_runtime_service('credit', CreditService())
         set_runtime_service('instructor', InstructorService())
 
+    # In order to allow modules 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
+    # https://openedx.atlassian.net/wiki/display/PLAT/Convert+from+Storage-centric+runtimes+to+Application-centric+runtimes
+    xmodule.x_module.descriptor_global_handler_url = lms_xblock.runtime.handler_url
+    xmodule.x_module.descriptor_global_local_resource_url = lms_xblock.runtime.local_resource_url
+
 
 def add_mimetypes():
     """