Fix STUD-1316

This bug affected unpublishing content (and would trigger an error
message in studio). The fix was to uncouple the rendering of the studio_view
of an xblock with the student_view, so that content that wasn't in
draft mode already wouldn't be modified by early rendering of the
studio_view.

[STUD-1316]

cms/djangoapps/contentstore/features/common.py

@@ -404,3 +404,8 @@ def i_delete_draft(_step):
 @step(u'I publish the unit$')
 def publish_unit(_step):
     world.select_option('visibility-select', 'public')
+
+
+@step(u'I unpublish the unit$')
+def unpublish_unit(_step):
+    world.select_option('visibility-select', 'private')

cms/djangoapps/contentstore/features/problem-editor.feature

@@ -105,6 +105,14 @@ Feature: CMS.Problem Editor
     And I click on "delete draft"
     Then the problem display name is "Blank Common Problem"
 
+  Scenario: Problems can be made private after being made public
+    Given I have created a Blank Common Problem
+    When I publish the unit
+    And I click on "edit a draft"
+    And I click on "delete draft"
+    And I unpublish the unit
+    Then I can edit the problem
+
   # Disabled 11/13/2013 after failing in master
   # The screenshot showed that the LaTeX editor had the text "hi",
   # but Selenium timed out waiting for the text to appear.

cms/djangoapps/contentstore/features/problem-editor.py

@@ -258,12 +258,6 @@ def verify_high_level_source_links(step, visible):
                     msg="Expected not to find the latex button but it is present.")
 
     world.cancel_component(step)
-    if visible:
-        assert_true(world.is_css_present('.upload-button'),
-                    msg="Expected to find the upload button but it is not present.")
-    else:
-        assert_true(world.is_css_not_present('.upload-button'),
-                    msg="Expected not to find the upload button but it is present.")
 
 
 def verify_modified_weight():

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -510,7 +510,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         locator = loc_mapper().translate_location(
             course_items[0].location.course_id, location, True, True
         )
-        resp = self.client.get_fragment(locator.url_reverse('xblock'))
+        resp = self.client.get_fragment(locator.url_reverse('xblock', 'student_view'))
         self.assertEqual(resp.status_code, 200)
         # TODO: uncomment when preview no longer has locations being returned.
         # _test_no_locations(self, resp)

cms/djangoapps/contentstore/views/item.py

@@ -39,7 +39,7 @@ from edxmako.shortcuts import render_to_string
 from models.settings.course_grading import CourseGradingModel
 from cms.lib.xblock.runtime import handler_url
 
-__all__ = ['orphan_handler', 'xblock_handler']
+__all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler']
 
 log = logging.getLogger(__name__)
 
@@ -109,41 +109,7 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
         if request.method == 'GET':
             accept_header = request.META.get('HTTP_ACCEPT', 'application/json')
 
-            if 'application/x-fragment+json' in accept_header:
-                store = get_modulestore(old_location)
-                component = store.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, 'StudioRuntime'))
-
-                try:
-                    editor_fragment = component.render('studio_view')
-                # 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
-                    log.debug("Unable to render studio_view for %r", component, exc_info=True)
-                    editor_fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)}))
-
-                store.save_xmodule(component)
-
-                preview_fragment = get_preview_fragment(request, component)
-
-                hashed_resources = OrderedDict()
-                for resource in editor_fragment.resources + preview_fragment.resources:
-                    hashed_resources[hash_resource(resource)] = resource
-
-                return JsonResponse({
-                    'html': render_to_string('component.html', {
-                        'preview': preview_fragment.content,
-                        'editor': editor_fragment.content,
-                        'label': component.display_name or component.scope_ids.block_type,
-                    }),
-                    'resources': hashed_resources.items()
-                })
-
-            elif 'application/json' in accept_header:
+            if 'application/json' in accept_header:
                 fields = request.REQUEST.get('fields', '').split(',')
                 if 'graderType' in fields:
                     # right now can't combine output of this w/ output of _get_module_info, but worthy goal
@@ -198,6 +164,68 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
         )
 
 
+# pylint: disable=unused-argument
+@require_http_methods(("GET"))
+@login_required
+@expect_json
+def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, version_guid=None, block=None):
+    """
+    The restful handler for requests for rendered xblock views.
+
+    Returns a json object containing two keys:
+        html: The rendered html of the view
+        resources: A list of tuples where the first element is the resource hash, and
+            the second is the resource description
+    """
+    locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
+    if not has_course_access(request.user, locator):
+        raise PermissionDenied()
+    old_location = loc_mapper().translate_locator_to_location(locator)
+
+    accept_header = request.META.get('HTTP_ACCEPT', 'application/json')
+
+    if 'application/x-fragment+json' in accept_header:
+        store = get_modulestore(old_location)
+        component = store.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, 'StudioRuntime'))
+
+        if view_name == 'studio_view':
+            try:
+                fragment = component.render('studio_view')
+            # 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
+                log.debug("unable to render studio_view for %r", component, exc_info=True)
+                fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)}))
+
+            store.save_xmodule(component)
+
+        elif view_name == 'student_view':
+            fragment = get_preview_fragment(request, component)
+            fragment.content = render_to_string('component.html', {
+                'preview': fragment.content,
+                'label': component.display_name or component.scope_ids.block_type,
+            })
+        else:
+            raise Http404
+
+        hashed_resources = OrderedDict()
+        for resource in fragment.resources:
+            hashed_resources[hash_resource(resource)] = resource
+
+        return JsonResponse({
+            'html': fragment.content,
+            'resources': hashed_resources.items()
+        })
+
+    else:
+        return HttpResponse(status=406)
+
+
 def _save_item(request, usage_loc, item_location, data=None, children=None, metadata=None, nullout=None,
                grader_type=None, publish=None):
     """

cms/djangoapps/contentstore/views/tests/test_item.py

@@ -565,7 +565,11 @@ class TestEditItem(ItemTest):
         self.assertNotEqual(draft.data, published.data)
 
         # Get problem by 'xblock_handler'
-        resp = self.client.get('/xblock/' + self.problem_locator, HTTP_ACCEPT='application/x-fragment+json')
+        resp = self.client.get('/xblock/' + self.problem_locator + '/student_view', HTTP_ACCEPT='application/x-fragment+json')
+        self.assertEqual(resp.status_code, 200)
+
+        # Activate the editing view
+        resp = self.client.get('/xblock/' + self.problem_locator + '/studio_view', HTTP_ACCEPT='application/x-fragment+json')
         self.assertEqual(resp.status_code, 200)
 
         # Both published and draft content should still be different

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

@@ -55,6 +55,7 @@ define ["jquery", "coffee/src/views/module_edit", "js/models/module_info", "xmod
         describe "render", ->
           beforeEach ->
             spyOn(@moduleEdit, 'loadDisplay')
+            spyOn(@moduleEdit, 'loadEdit')
             spyOn(@moduleEdit, 'delegateEvents')
             spyOn($.fn, 'append')
             spyOn($, 'getScript')
@@ -74,15 +75,58 @@ define ["jquery", "coffee/src/views/module_edit", "js/models/module_info", "xmod
               ]
             )
 
-          it "loads the module preview and editor via ajax on the view element", ->
+          it "loads the module preview via ajax on the view element", ->
             expect($.ajax).toHaveBeenCalledWith(
-              url: "/xblock/#{@moduleEdit.model.id}"
+              url: "/xblock/#{@moduleEdit.model.id}/student_view"
+              type: "GET"
+              headers:
+                Accept: 'application/x-fragment+json'
+              success: jasmine.any(Function)
+            )
+
+            expect($.ajax).not.toHaveBeenCalledWith(
+              url: "/xblock/#{@moduleEdit.model.id}/studio_view"
               type: "GET"
               headers:
                 Accept: 'application/x-fragment+json'
               success: jasmine.any(Function)
             )
             expect(@moduleEdit.loadDisplay).toHaveBeenCalled()
+            expect(@moduleEdit.loadEdit).not.toHaveBeenCalled()
+            expect(@moduleEdit.delegateEvents).toHaveBeenCalled()
+
+          it "loads the editing view via ajax on demand", ->
+            expect($.ajax).not.toHaveBeenCalledWith(
+              url: "/xblock/#{@moduleEdit.model.id}/studio_view"
+              type: "GET"
+              headers:
+                Accept: 'application/x-fragment+json'
+              success: jasmine.any(Function)
+            )
+            expect(@moduleEdit.loadEdit).not.toHaveBeenCalled()
+
+            @moduleEdit.clickEditButton({'preventDefault': jasmine.createSpy('event.preventDefault')})
+
+            $.ajax.mostRecentCall.args[0].success(
+              html: '<div>Response html</div>'
+              resources: [
+                ['hash1', {kind: 'text', mimetype: 'text/css', data: 'inline-css'}],
+                ['hash2', {kind: 'url', mimetype: 'text/css', data: 'css-url'}],
+                ['hash3', {kind: 'text', mimetype: 'application/javascript', data: 'inline-js'}],
+                ['hash4', {kind: 'url', mimetype: 'application/javascript', data: 'js-url'}],
+                ['hash5', {placement: 'head', mimetype: 'text/html', data: 'head-html'}],
+                ['hash6', {placement: 'not-head', mimetype: 'text/html', data: 'not-head-html'}],
+              ]
+            )
+
+            expect($.ajax).toHaveBeenCalledWith(
+              url: "/xblock/#{@moduleEdit.model.id}/studio_view"
+              type: "GET"
+              headers:
+                Accept: 'application/x-fragment+json'
+              success: jasmine.any(Function)
+            )
+            expect(@moduleEdit.loadEdit).toHaveBeenCalled()
             expect(@moduleEdit.delegateEvents).toHaveBeenCalled()
 
           it "loads inline css from fragments", ->

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

@@ -18,37 +18,37 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
       @onDelete = @options.onDelete
       @render()
 
-    $component_editor: => @$el.find('.component-editor')
+    $componentEditor: => @$el.find('.component-editor')
+    $moduleEditor: => @$componentEditor().find('.module-editor')
 
     loadDisplay: ->
       XBlock.initializeBlock(@$el.find('.xblock-student_view'))
 
     loadEdit: ->
-      if not @module
-        @module = XBlock.initializeBlock(@$el.find('.xblock-studio_view'))
-        # At this point, metadata-edit.html will be loaded, and the metadata (as JSON) is available.
-        metadataEditor = @$el.find('.metadata_edit')
-        metadataData = metadataEditor.data('metadata')
-        models = [];
-        for key of metadataData
-          models.push(metadataData[key])
-        @metadataEditor = new MetadataView.Editor({
-            el: metadataEditor,
-            collection: new MetadataCollection(models)
-        })
-
-        @module.setMetadataEditor(@metadataEditor) if @module.setMetadataEditor
-
-        # Need to update set "active" class on data editor if there is one.
-        # If we are only showing settings, hide the data editor controls and update settings accordingly.
-        if @hasDataEditor()
-          @selectMode(@editorMode)
-        else
-          @hideDataEditor()
-
-        title = interpolate(gettext('<em>Editing:</em> %s'),
-          [@metadataEditor.getDisplayName()])
-        @$el.find('.component-name').html(title)
+      @module = XBlock.initializeBlock(@$el.find('.xblock-studio_view'))
+      # At this point, metadata-edit.html will be loaded, and the metadata (as JSON) is available.
+      metadataEditor = @$el.find('.metadata_edit')
+      metadataData = metadataEditor.data('metadata')
+      models = [];
+      for key of metadataData
+        models.push(metadataData[key])
+      @metadataEditor = new MetadataView.Editor({
+          el: metadataEditor,
+          collection: new MetadataCollection(models)
+      })
+
+      @module.setMetadataEditor(@metadataEditor) if @module.setMetadataEditor
+
+      # Need to update set "active" class on data editor if there is one.
+      # If we are only showing settings, hide the data editor controls and update settings accordingly.
+      if @hasDataEditor()
+        @selectMode(@editorMode)
+      else
+        @hideDataEditor()
+
+      title = interpolate(gettext('<em>Editing:</em> %s'),
+        [@metadataEditor.getDisplayName()])
+      @$el.find('.component-name').html(title)
 
     customMetadata: ->
         # Hack to support metadata fields that aren't part of the metadata editor (ie, LaTeX high level source).
@@ -56,7 +56,7 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
         # build up an object to pass back to the server on the subsequent POST.
         # Note that these values will always be sent back on POST, even if they did not actually change.
         _metadata = {}
-        _metadata[$(el).data("metadata-name")] = el.value for el in $('[data-metadata-name]',  @$component_editor())
+        _metadata[$(el).data("metadata-name")] = el.value for el in $('[data-metadata-name]',  @$componentEditor())
         return _metadata
 
     changedMetadata: ->
@@ -73,15 +73,15 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
               @render()
       ).success(callback)
 
-    render: ->
+    loadView: (viewName, target, callback) ->
       if @model.id
         $.ajax(
-          url: @model.url()
+          url: "#{decodeURIComponent(@model.url())}/#{viewName}"
           type: 'GET'
           headers:
             Accept: 'application/x-fragment+json'
           success: (data) =>
-            @$el.html(data.html)
+            $(target).html(data.html)
 
             for value in data.resources
               do (value) =>
@@ -104,10 +104,14 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
                       switch resource.placement
                         when "head" then $('head').append(resource.data)
                   window.loadedXBlockResources.push(hash)
-            @loadDisplay()
-            @delegateEvents()
+            callback()
         )
 
+    render: -> @loadView('student_view', @$el, =>
+      @loadDisplay()
+      @delegateEvents()
+    )
+
     clickSaveButton: (event) =>
       event.preventDefault()
       data = @module.save()
@@ -122,7 +126,6 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
         title: gettext('Saving&hellip;')
       saving.show()
       @model.save(data).done( =>
-        @module = null
         @render()
         @$el.removeClass('editing')
         saving.hide()
@@ -131,15 +134,18 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1",
     clickCancelButton: (event) ->
       event.preventDefault()
       @$el.removeClass('editing')
-      @$component_editor().slideUp(150)
+      @$componentEditor().slideUp(150)
       ModalUtils.hideModalCover()
 
     clickEditButton: (event) ->
       event.preventDefault()
       @$el.addClass('editing')
       ModalUtils.showModalCover(true)
-      @$component_editor().slideDown(150)
-      @loadEdit()
+      @loadView('studio_view', @$moduleEditor(), =>
+        @$componentEditor().slideDown(150)
+        @loadEdit()
+        @delegateEvents()
+      )
 
     clickModeButton: (event) ->
       event.preventDefault()

cms/templates/component.html

@@ -16,9 +16,7 @@
         </div> <!-- Editor Header -->
 
         <div class="component-edit-modes">
-            <div class="module-editor">
-                ${editor}
-            </div>
+            <div class="module-editor"/>
         </div>
         <div class="row module-actions">
             <a href="#" class="save-button action-primary action">${_("Save")}</a>

cms/templates/widgets/source-edit.html

@@ -162,15 +162,5 @@ require(["jquery", "jquery.leanModal", "codemirror/stex"], function($) {
       el.find('.hls-data').val(el.data('editor').getValue());
       el.closest('.component').find('.save-button').click();
   }
-
-  ## add upload and download links / buttons to component edit box
-  hlsmodal.closest('.component').find('.component-actions').append('<div id="link-${hlskey}" style="float:right;"></div>');
-  $('#link-${hlskey}').html('<a class="upload-button standard" id="upload-${hlskey}">upload</a>');
-  $('#upload-${hlskey}').click(function() {
-      hlsmodal.closest('.component').find('.edit-button').trigger('click'); // open up editor window
-      $('#hls-trig-${hlskey}').trigger('click'); // open up HLS editor window
-      hlsmodal.find('#hlsfile').trigger('click');
-  });
-
 }); // end require()
   </script>

cms/urls.py

@@ -79,6 +79,7 @@ urlpatterns += patterns(
     url(r'(?ix)^import/{}$'.format(parsers.URL_RE_SOURCE), 'import_handler'),
     url(r'(?ix)^import_status/{}/(?P<filename>.+)$'.format(parsers.URL_RE_SOURCE), 'import_status_handler'),
     url(r'(?ix)^export/{}$'.format(parsers.URL_RE_SOURCE), 'export_handler'),
+    url(r'(?ix)^xblock/{}/(?P<view_name>[^/]+)$'.format(parsers.URL_RE_SOURCE), 'xblock_view_handler'),
     url(r'(?ix)^xblock($|/){}$'.format(parsers.URL_RE_SOURCE), 'xblock_handler'),
     url(r'(?ix)^tabs/{}$'.format(parsers.URL_RE_SOURCE), 'tabs_handler'),
     url(r'(?ix)^settings/details/{}$'.format(parsers.URL_RE_SOURCE), 'settings_handler'),