diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 58cf3be70b..2b773e991d 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/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') diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 327e75c7f4..70d7bef7ce 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/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() ] diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 97bfde3b82..d33d00377b 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/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')) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 6e86a6485b..3b2ec85326 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/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 diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 277445e3b9..40ea9bfd6b 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/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 ] diff --git a/cms/static/coffee/spec/views/module_edit_spec.coffee b/cms/static/coffee/spec/views/module_edit_spec.coffee index 22d1052fa3..36716668d3 100644 --- a/cms/static/coffee/spec/views/module_edit_spec.coffee +++ b/cms/static/coffee/spec/views/module_edit_spec.coffee @@ -1,12 +1,9 @@ -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 """
  • @@ -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() diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index a13e572887..729a17615e 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/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() ) diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 0f72e8bddb..a85b3b7863 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/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( diff --git a/cms/static/coffee/src/views/unit.coffee b/cms/static/coffee/src/views/unit.coffee index 075b56d1b0..e6ba0e1382 100644 --- a/cms/static/coffee/src/views/unit.coffee +++ b/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, diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index f6b8e7b77a..54a30217ea 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -61,8 +61,8 @@ require(["backbone", "coffee/src/views/tabs"], function(Backbone, TabsEditView)
      - % for id, locator in components: -
    1. + % for locator in components: +
    2. % endfor
    3. diff --git a/cms/templates/unit.html b/cms/templates/unit.html index cc7827c7d3..e83dd45da0 100644 --- a/cms/templates/unit.html +++ b/cms/templates/unit.html @@ -48,8 +48,8 @@ require(["domReady!", "jquery", "js/models/module_info", "coffee/src/views/unit"

        - % for id, locator in components: -
      1. + % for locator in components: +
      2. % endfor
      3. diff --git a/cms/urls.py b/cms/urls.py index 99e9cbfaba..343e9f4d04 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -14,7 +14,6 @@ urlpatterns = patterns('', # nopep8 url(r'^$', 'contentstore.views.howitworks', name='homepage'), url(r'^edit/(?P.*?)$', 'contentstore.views.edit_unit', name='edit_unit'), url(r'^subsection/(?P.*?)$', 'contentstore.views.edit_subsection', name='edit_subsection'), - url(r'^preview_component/(?P.*?)$', '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'),