Merge pull request #1680 from edx/christina/preview

Change preview view method to use RESTful URL.

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -454,31 +454,36 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
     @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/toy/.*'])
     def test_module_preview_in_whitelist(self):
-        '''
+        """
         Tests the ajax callback to render an XModule
-        '''
-        direct_store = modulestore('direct')
-        import_from_xml(direct_store, 'common/test/data/', ['toy'])
-
-        # also try a custom response which will trigger the 'is this course in whitelist' logic
-        problem_module_location = Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None])
-        url = reverse('preview_component', kwargs={'location': problem_module_location.url()})
-        resp = self.client.get_html(url)
-        self.assertEqual(resp.status_code, 200)
+        """
+        resp = self._test_preview(Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]))
+        # These are the data-ids of the xblocks contained in the vertical.
+        # Ultimately, these must be converted to new locators.
+        self.assertContains(resp, 'i4x://edX/toy/video/sample_video')
+        self.assertContains(resp, 'i4x://edX/toy/video/separate_file_video')
+        self.assertContains(resp, 'i4x://edX/toy/video/video_with_end_time')
+        self.assertContains(resp, 'i4x://edX/toy/poll_question/T1_changemind_poll_foo_2')
 
     def test_video_module_caption_asset_path(self):
-        '''
+        """
         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_"')
+
+    def _test_preview(self, location):
+        """ Preview test case. """
         direct_store = modulestore('direct')
-        import_from_xml(direct_store, 'common/test/data/', ['toy'])
+        _, course_items = import_from_xml(direct_store, 'common/test/data/', ['toy'])
 
         # also try a custom response which will trigger the 'is this course in whitelist' logic
-        video_module_location = Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None])
-        url = reverse('preview_component', kwargs={'location': video_module_location.url()})
-        resp = self.client.get_html(url)
+        locator = loc_mapper().translate_location(
+            course_items[0].location.course_id, location, False, True
+        )
+        resp = self.client.get_html(locator.url_reverse('xblock'))
         self.assertEqual(resp.status_code, 200)
-        self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"')
+        return resp
 
     def test_delete(self):
         direct_store = modulestore('direct')

cms/djangoapps/contentstore/views/component.py

@@ -249,12 +249,9 @@ def edit_unit(request, location):
         )
 
     components = [
-        [
-            component.location.url(),
-            loc_mapper().translate_location(
-                course.location.course_id, component.location, False, True
-            )
-        ]
+        loc_mapper().translate_location(
+            course.location.course_id, component.location, False, True
+        )
         for component
         in item.get_children()
     ]

cms/djangoapps/contentstore/views/item.py

@@ -3,7 +3,9 @@
 import logging
 from uuid import uuid4
 
+from functools import partial
 from static_replace import replace_static_urls
+from xmodule_modifiers import wrap_xblock
 
 from django.core.exceptions import PermissionDenied
 from django.contrib.auth.decorators import login_required
@@ -27,6 +29,8 @@ from xmodule.modulestore.locator import BlockUsageLocator
 from student.models import CourseEnrollment
 from django.http import HttpResponseBadRequest
 from xblock.fields import Scope
+from preview import handler_prefix, get_preview_html
+from mitxmako.shortcuts import render_to_response, render_to_string
 
 __all__ = ['orphan_handler', 'xblock_handler']
 
@@ -51,6 +55,7 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid=
         all children and "all_versions" to delete from all (mongo) versions.
     GET
         json: returns representation of the xblock (locator id, data, and metadata).
+        html: returns HTML for rendering the xblock (which includes both the "preview" view and the "editor" view)
     PUT or POST
         json: if xblock location is specified, update the xblock instance. The json payload can contain
               these fields, all optional:
@@ -76,8 +81,27 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid=
         old_location = loc_mapper().translate_locator_to_location(location)
 
         if request.method == 'GET':
-            rsp = _get_module_info(location)
-            return JsonResponse(rsp)
+            if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
+                rsp = _get_module_info(location)
+                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))
+
+                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
+                    content = render_to_string('html_error.html', {'message': str(exc)})
+
+                return render_to_response('component.html', {
+                    'preview': get_preview_html(request, component),
+                    'editor': content
+                })
         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'))

cms/djangoapps/contentstore/views/preview.py

@@ -3,7 +3,7 @@ from functools import partial
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
-from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden
+from django.http import Http404, HttpResponseBadRequest
 from django.contrib.auth.decorators import login_required
 from mitxmako.shortcuts import render_to_response, render_to_string
 
@@ -24,10 +24,9 @@ from util.sandboxing import can_execute_unsafe_code
 import static_replace
 from .session_kv_store import SessionKeyValueStore
 from .helpers import render_from_lms
-from .access import has_access
 from ..utils import get_course_for_item
 
-__all__ = ['preview_handler', 'preview_component']
+__all__ = ['preview_handler']
 
 log = logging.getLogger(__name__)
 
@@ -53,13 +52,13 @@ def preview_handler(request, usage_id, handler, suffix=''):
 
     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
+    suffix: The remainder of the url to be passed to the handler
     """
 
     location = unquote_slashes(usage_id)
 
     descriptor = modulestore().get_item(location)
-    instance = load_preview_module(request, descriptor)
+    instance = _load_preview_module(request, descriptor)
     # Let the module handle the AJAX
     req = django_to_webob_request(request)
     try:
@@ -85,32 +84,6 @@ def preview_handler(request, usage_id, handler, suffix=''):
     return webob_to_django_response(resp)
 
 
-@login_required
-def preview_component(request, location):
-    "Return the HTML preview of a component"
-    # TODO (vshnayder): change name from id to location in coffee+html as well.
-    if not has_access(request.user, location):
-        return HttpResponseForbidden()
-
-    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(partial(wrap_xblock, handler_prefix))
-
-    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
-        content = render_to_string('html_error.html', {'message': str(exc)})
-
-    return render_to_response('component.html', {
-        'preview': get_preview_html(request, component),
-        'editor': content
-    })
-
-
 class PreviewModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
     """
     An XModule ModuleSystem for use in Studio previews
@@ -119,7 +92,7 @@ class PreviewModuleSystem(ModuleSystem):  # pylint: disable=abstract-method
         return handler_prefix(block, handler_name, suffix) + '?' + query
 
 
-def preview_module_system(request, descriptor):
+def _preview_module_system(request, descriptor):
     """
     Returns a ModuleSystem for the specified descriptor that is specialized for
     rendering module previews.
@@ -135,7 +108,7 @@ def preview_module_system(request, descriptor):
         # 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),
+        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),
@@ -162,7 +135,7 @@ def preview_module_system(request, descriptor):
     )
 
 
-def load_preview_module(request, descriptor):
+def _load_preview_module(request, descriptor):
     """
     Return a preview XModule instantiated from the supplied descriptor.
 
@@ -171,7 +144,7 @@ def load_preview_module(request, descriptor):
     """
     student_data = DbModel(SessionKeyValueStore(request))
     descriptor.bind_for_student(
-        preview_module_system(request, descriptor),
+        _preview_module_system(request, descriptor),
         LmsFieldData(descriptor._field_data, student_data),  # pylint: disable=protected-access
     )
     return descriptor
@@ -182,7 +155,7 @@ 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, descriptor)
+    module = _load_preview_module(request, descriptor)
     try:
         content = module.render("student_view").content
     except Exception as exc:                          # pylint: disable=W0703

cms/djangoapps/contentstore/views/tabs.py

@@ -125,12 +125,9 @@ def edit_tabs(request, org, course, coursename):
         static_tabs.append(modulestore('direct').get_item(static_tab_loc))
 
     components = [
-        [
-            static_tab.location.url(),
-            loc_mapper().translate_location(
-                course_item.location.course_id, static_tab.location, False, True
-            )
-        ]
+        loc_mapper().translate_location(
+            course_item.location.course_id, static_tab.location, False, True
+        )
         for static_tab
         in static_tabs
     ]

cms/static/coffee/spec/views/module_edit_spec.coffee

-define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) ->
+define ["coffee/src/views/module_edit", "js/models/module_info", "xmodule"], (ModuleEdit, ModuleModel) ->
 
     describe "ModuleEdit", ->
       beforeEach ->
-        @stubModule = jasmine.createSpy("Module")
-        @stubModule.id = 'stub-id'
-        @stubModule.get = (param)->
-            if param == 'old_id'
-              return 'stub-old-id'
+        @stubModule = new ModuleModel
+            id: "stub-id"
 
         setFixtures """
         <li class="component" id="stub-id">
@@ -62,7 +59,7 @@ define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) ->
             @moduleEdit.render()
 
           it "loads the module preview and editor via ajax on the view element", ->
-            expect(@moduleEdit.$el.load).toHaveBeenCalledWith("/preview_component/#{@moduleEdit.model.get('old_id')}", jasmine.any(Function))
+            expect(@moduleEdit.$el.load).toHaveBeenCalledWith("/xblock/#{@moduleEdit.model.id}", jasmine.any(Function))
             @moduleEdit.$el.load.mostRecentCall.args[1]()
             expect(@moduleEdit.loadDisplay).toHaveBeenCalled()
             expect(@moduleEdit.delegateEvents).toHaveBeenCalled()

cms/static/coffee/src/views/module_edit.coffee

@@ -69,15 +69,13 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
           payload
           (data) =>
               @model.set(id: data.locator)
-              @model.set(old_id: data.id)
-              @$el.data('id', data.id)
               @$el.data('locator', data.locator)
               @render()
       )
 
     render: ->
-      if @model.get('old_id')
-        @$el.load("/preview_component/#{@model.get('old_id')}", =>
+      if @model.id
+        @$el.load(@model.url(), =>
           @loadDisplay()
           @delegateEvents()
         )

cms/static/coffee/src/views/tabs.coffee

@@ -6,8 +6,7 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views
     initialize: =>
       @$('.component').each((idx, element) =>
           model = new ModuleModel({
-              id: $(element).data('locator'),
-              old_id:$(element).data('id')
+              id: $(element).data('locator')
           })
 
           new ModuleEditView(

cms/static/coffee/src/views/unit.coffee

@@ -63,7 +63,6 @@ define ["jquery", "jquery.ui", "gettext", "backbone",
       @$('.component').each (idx, element) =>
         model = new ModuleModel
             id: $(element).data('locator')
-            old_id: $(element).data('id')
         new ModuleEditView
           el: element,
           onDelete: @deleteComponent,

cms/templates/edit-tabs.html

@@ -61,8 +61,8 @@ require(["backbone", "coffee/src/views/tabs"], function(Backbone, TabsEditView)
 
       <div class="tab-list">
         <ol class='components'>
-          % for id, locator in components:
-          <li class="component" data-id="${id}" data-locator="${locator}"/>
+          % for locator in components:
+          <li class="component" data-locator="${locator}"/>
           % endfor
 
           <li class="new-component-item">

cms/templates/unit.html

@@ -48,8 +48,8 @@ require(["domReady!", "jquery", "js/models/module_info", "coffee/src/views/unit"
         <article class="unit-body window">
         <p class="unit-name-input"><label>${_("Display Name:")}</label><input type="text" value="${unit.display_name_with_default | h}" class="unit-display-name-input" /></p>
           <ol class="components">
-            % for id, locator in components:
-            <li class="component" data-id="${id}" data-locator="${locator}"/>
+            % for locator in components:
+            <li class="component" data-locator="${locator}"/>
             % endfor
             <li class="new-component-item adding">
               <div class="new-component">

cms/urls.py

@@ -14,7 +14,6 @@ urlpatterns = patterns('',  # nopep8
     url(r'^$', 'contentstore.views.howitworks', name='homepage'),
     url(r'^edit/(?P<location>.*?)$', 'contentstore.views.edit_unit', name='edit_unit'),
     url(r'^subsection/(?P<location>.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'),
-    url(r'^preview_component/(?P<location>.*?)$', 'contentstore.views.preview_component', name='preview_component'),
 
     url(r'^transcripts/upload$', 'contentstore.views.upload_transcripts', name='upload_transcripts'),
     url(r'^transcripts/download$', 'contentstore.views.download_transcripts', name='download_transcripts'),