Merge pull request #827 from edx/christina/course-info-links3

Rewriting of links for Course Updates and Course Handouts.

cms/djangoapps/contentstore/features/course-updates.feature

@@ -45,3 +45,25 @@ Feature: Course updates
         When I modify the handout to "<ol>Test</ol>"
         Then I see the handout "Test"
         And I see a "saving" notification
+
+    Scenario: Static links are rewritten when previewing a course update
+        Given I have opened a new course in Studio
+        And I go to the course updates page
+        When I add a new update with the text "<img src='/static/my_img.jpg'/>"
+        # Can only do partial text matches because of the quotes with in quotes (and regexp step matching).
+        Then I should see the update "/c4x/MITx/999/asset/my_img.jpg"
+        And I change the update from "/static/my_img.jpg" to "<img src='/static/modified.jpg'/>"
+        Then I should see the update "/c4x/MITx/999/asset/modified.jpg"
+        And when I reload the page
+        Then I should see the update "/c4x/MITx/999/asset/modified.jpg"
+
+    Scenario: Static links are rewritten when previewing handouts
+        Given I have opened a new course in Studio
+        And I go to the course updates page
+        When I modify the handout to "<ol><img src='/static/my_img.jpg'/></ol>"
+        # Can only do partial text matches because of the quotes with in quotes (and regexp step matching).
+        Then I see the handout "/c4x/MITx/999/asset/my_img.jpg"
+        And I change the handout from "/static/my_img.jpg" to "<img src='/static/modified.jpg'/>"
+        Then I see the handout "/c4x/MITx/999/asset/modified.jpg"
+        And when I reload the page
+        Then I see the handout "/c4x/MITx/999/asset/modified.jpg"

cms/djangoapps/contentstore/features/course-updates.py

@@ -38,6 +38,16 @@ def modify_update(_step, text):
     change_text(text)
 
 
+@step(u'I change the update from "([^"]*)" to "([^"]*)"$')
+def change_existing_update(_step, before, after):
+    verify_text_in_editor_and_update('div.post-preview a.edit-button', before, after)
+
+
+@step(u'I change the handout from "([^"]*)" to "([^"]*)"$')
+def change_existing_handout(_step, before, after):
+    verify_text_in_editor_and_update('div.course-handouts a.edit-button', before, after)
+
+
 @step(u'I delete the update$')
 def click_button(_step):
     button_css = 'div.post-preview a.delete-button'
@@ -80,3 +90,10 @@ def change_text(text):
     type_in_codemirror(0, text)
     save_css = 'a.save-button'
     world.css_click(save_css)
+
+
+def verify_text_in_editor_and_update(button_css, before, after):
+    world.css_click(button_css)
+    text = world.css_find(".cm-string").html
+    assert before in text
+    change_text(after)

cms/djangoapps/contentstore/views/course.py

@@ -18,6 +18,7 @@ from mitxmako.shortcuts import render_to_response
 
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.inheritance import own_metadata
+from xmodule.contentstore.content import StaticContent
 
 from xmodule.modulestore.exceptions import (
     ItemNotFoundError, InvalidLocationError)
@@ -206,7 +207,8 @@ def course_info(request, org, course, name, provided_id=None):
         'context_course': course_module,
         'url_base': "/" + org + "/" + course + "/",
         'course_updates': json.dumps(get_course_updates(location)),
-        'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url() })
+        'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url(),
+        'base_asset_url': StaticContent.get_base_url_path_for_course_assets(location) + '/'})
 
 
 @expect_json

cms/envs/common.py

@@ -246,7 +246,7 @@ PIPELINE_JS = {
              'js/models/metadata_model.js', 'js/views/metadata_editor_view.js',
              'js/models/uploads.js', 'js/views/uploads.js',
              'js/models/textbook.js', 'js/views/textbook.js',
-             'js/views/assets.js', 'js/utility.js',
+             'js/views/assets.js', 'js/src/utility.js',
              'js/models/settings/course_grading_policy.js'],
         'output_filename': 'js/cms-application.js',
         'test_order': 0

cms/static/coffee/spec/views/course_info_spec.coffee

+courseInfoPage = """
+                 <div class="course-info-wrapper">
+                 <div class="main-column window">
+                 <article class="course-updates" id="course-update-view">
+                 <ol class="update-list" id="course-update-list"></ol>
+                 </article>
+                 </div>
+                 <div class="sidebar window course-handouts" id="course-handouts-view"></div>
+                 </div>
+                 """
+
+commonSetup = () ->
+    window.analytics = jasmine.createSpyObj('analytics', ['track'])
+    window.course_location_analytics = jasmine.createSpy()
+    window.courseUpdatesXhr = sinon.useFakeXMLHttpRequest()
+    requests = []
+    window.courseUpdatesXhr.onCreate = (xhr) -> requests.push(xhr)
+    return requests
+
+commonCleanup = () ->
+    window.courseUpdatesXhr.restore()
+    delete window.analytics
+    delete window.course_location_analytics
+
+describe "Course Updates", ->
+    courseInfoTemplate = readFixtures('course_info_update.underscore')
+
+    beforeEach ->
+        setFixtures($("<script>", {id: "course_info_update-tpl", type: "text/template"}).text(courseInfoTemplate))
+        appendSetFixtures courseInfoPage
+
+        @collection = new CMS.Models.CourseUpdateCollection()
+        @courseInfoEdit = new CMS.Views.ClassInfoUpdateView({
+            el: $('.course-updates'),
+            collection: @collection,
+            base_asset_url : 'base-asset-url/'
+        })
+
+        @courseInfoEdit.render()
+
+        @event = {
+            preventDefault : () -> 'no op'
+        }
+
+        @createNewUpdate = () ->
+            # Edit button is not in the template under test (it is in parent HTML).
+            # Therefore call onNew directly.
+            @courseInfoEdit.onNew(@event)
+            spyOn(@courseInfoEdit.$codeMirror, 'getValue').andReturn('/static/image.jpg')
+            @courseInfoEdit.$el.find('.save-button').click()
+
+        @requests = commonSetup()
+
+    afterEach ->
+        commonCleanup()
+
+    it "does not rewrite links on save", ->
+        # Create a new update, verifying that the model is created
+        # in the collection and save is called.
+        expect(@collection.isEmpty()).toBeTruthy()
+        @courseInfoEdit.onNew(@event)
+        expect(@collection.length).toEqual(1)
+        model = @collection.at(0)
+        spyOn(model, "save").andCallThrough()
+        spyOn(@courseInfoEdit.$codeMirror, 'getValue').andReturn('/static/image.jpg')
+
+        # Click the "Save button."
+        @courseInfoEdit.$el.find('.save-button').click()
+        expect(model.save).toHaveBeenCalled()
+
+        # Verify content sent to server does not have rewritten links.
+        contentSaved = JSON.parse(this.requests[0].requestBody).content
+        expect(contentSaved).toEqual('/static/image.jpg')
+
+    it "does rewrite links for preview", ->
+        # Create a new update.
+        @createNewUpdate()
+
+        # Verify the link is rewritten for preview purposes.
+        previewContents = @courseInfoEdit.$el.find('.update-contents').html()
+        expect(previewContents).toEqual('base-asset-url/image.jpg')
+
+    it "shows static links in edit mode", ->
+        @createNewUpdate()
+
+        # Click edit and verify CodeMirror contents.
+        @courseInfoEdit.$el.find('.edit-button').click()
+        expect(@courseInfoEdit.$codeMirror.getValue()).toEqual('/static/image.jpg')
+
+
+describe "Course Handouts", ->
+    handoutsTemplate = readFixtures('course_info_handouts.underscore')
+
+    beforeEach ->
+        setFixtures($("<script>", {id: "course_info_handouts-tpl", type: "text/template"}).text(handoutsTemplate))
+        appendSetFixtures courseInfoPage
+
+        @model = new CMS.Models.ModuleInfo({
+            id: 'handouts-id',
+            data: '/static/fromServer.jpg'
+        })
+
+        @handoutsEdit = new CMS.Views.ClassInfoHandoutsView({
+            el: $('#course-handouts-view'),
+            model: @model,
+            base_asset_url: 'base-asset-url/'
+        });
+
+        @handoutsEdit.render()
+
+        @requests = commonSetup()
+
+    afterEach ->
+        commonCleanup()
+
+    it "does not rewrite links on save", ->
+        # Enter something in the handouts section, verifying that the model is saved
+        # when "Save" is clicked.
+        @handoutsEdit.$el.find('.edit-button').click()
+        spyOn(@handoutsEdit.$codeMirror, 'getValue').andReturn('/static/image.jpg')
+        spyOn(@model, "save").andCallThrough()
+        @handoutsEdit.$el.find('.save-button').click()
+        expect(@model.save).toHaveBeenCalled()
+
+        contentSaved = JSON.parse(this.requests[0].requestBody).data
+        expect(contentSaved).toEqual('/static/image.jpg')
+
+    it "does rewrite links in initial content", ->
+        expect(@handoutsEdit.$preview.html().trim()).toBe('base-asset-url/fromServer.jpg')
+
+    it "does rewrite links after edit", ->
+        # Edit handouts and save.
+        @handoutsEdit.$el.find('.edit-button').click()
+        spyOn(@handoutsEdit.$codeMirror, 'getValue').andReturn('/static/image.jpg')
+        @handoutsEdit.$el.find('.save-button').click()
+
+        # Verify preview text.
+        expect(@handoutsEdit.$preview.html().trim()).toBe('base-asset-url/image.jpg')
+
+    it "shows static links in edit mode", ->
+        # Click edit and verify CodeMirror contents.
+        @handoutsEdit.$el.find('.edit-button').click()
+        expect(@handoutsEdit.$codeMirror.getValue().trim()).toEqual('/static/fromServer.jpg')
+

cms/static/js/views/course_info_edit.js

@@ -3,6 +3,26 @@
    The render here adds views for each update/handout by delegating to their collections but does not
    generate any html for the surrounding page.
 */
+
+var editWithCodeMirror = function(model, contentName, baseAssetUrl, textArea) {
+    var content = rewriteStaticLinks(model.get(contentName), baseAssetUrl, '/static/');
+    model.set(contentName, content);
+    var $codeMirror = CodeMirror.fromTextArea(textArea, {
+        mode: "text/html",
+        lineNumbers: true,
+        lineWrapping: true
+    });
+    $codeMirror.setValue(content);
+    $codeMirror.clearHistory();
+    return $codeMirror;
+};
+
+var changeContentToPreview = function (model, contentName, baseAssetUrl) {
+    var content = rewriteStaticLinks(model.get(contentName), '/static/', baseAssetUrl);
+    model.set(contentName, content);
+    return content;
+};
+
 CMS.Views.CourseInfoEdit = Backbone.View.extend({
   // takes CMS.Models.CourseInfo as model
   tagName: 'div',
@@ -11,18 +31,19 @@ CMS.Views.CourseInfoEdit = Backbone.View.extend({
     // instantiate the ClassInfoUpdateView and delegate the proper dom to it
     new CMS.Views.ClassInfoUpdateView({
         el: $('body.updates'),
-        collection: this.model.get('updates')
+        collection: this.model.get('updates'),
+        base_asset_url: this.model.get('base_asset_url')
     });
 
     new CMS.Views.ClassInfoHandoutsView({
         el: this.$('#course-handouts-view'),
-        model: this.model.get('handouts')
+        model: this.model.get('handouts'),
+        base_asset_url: this.model.get('base_asset_url')
     });
     return this;
   }
 });
 
-// ??? Programming style question: should each of these classes be in separate files?
 CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
     // collection is CourseUpdateCollection
     events: {
@@ -48,6 +69,7 @@ CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
           var self = this;
           this.collection.each(function (update) {
               try {
+                  changeContentToPreview(update, 'content', self.options['base_asset_url'])
                   var newEle = self.template({ updateModel : update });
                   $(updateEle).append(newEle);
             } catch (e) {
@@ -72,20 +94,18 @@ CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
         $(updateEle).prepend($newForm);
 
         var $textArea = $newForm.find(".new-update-content").first();
-        if (this.$codeMirror == null ) {
-            this.$codeMirror = CodeMirror.fromTextArea($textArea.get(0), {
-                mode: "text/html",
-                lineNumbers: true,
-                lineWrapping: true,
-            });
-        }
+        this.$codeMirror = CodeMirror.fromTextArea($textArea.get(0), {
+            mode: "text/html",
+            lineNumbers: true,
+            lineWrapping: true
+        });
 
         $newForm.addClass('editing');
         this.$currentPost = $newForm.closest('li');
 
         window.$modalCover.show();
         window.$modalCover.bind('click', function() {
-            self.closeEditor(self, true);
+            self.closeEditor(true);
         });
 
         $('.date').datepicker('destroy');
@@ -110,7 +130,7 @@ CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
                 ele.remove();
             }
         });
-        this.closeEditor(this);
+        this.closeEditor();
 
         analytics.track('Saved Course Update', {
             'course': course_location_analytics,
@@ -122,8 +142,10 @@ CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
         event.preventDefault();
         // change editor contents back to model values and hide the editor
         $(this.editor(event)).hide();
+        // If the model was never created (user created a new update, then pressed Cancel),
+        // we wish to remove it from the DOM.
         var targetModel = this.eventModel(event);
-        this.closeEditor(this, !targetModel.id);
+        this.closeEditor(!targetModel.id);
     },
 
     onEdit: function(event) {
@@ -134,16 +156,10 @@ CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
 
         $(this.editor(event)).show();
         var $textArea = this.$currentPost.find(".new-update-content").first();
-        if (this.$codeMirror == null ) {
-            this.$codeMirror = CodeMirror.fromTextArea($textArea.get(0), {
-                mode: "text/html",
-                lineNumbers: true,
-                lineWrapping: true,
-            });
-        }
+        var targetModel = this.eventModel(event);
+        this.$codeMirror = editWithCodeMirror(targetModel, 'content', self.options['base_asset_url'], $textArea.get(0));
 
         window.$modalCover.show();
-        var targetModel = this.eventModel(event);
         window.$modalCover.bind('click', function() {
             self.closeEditor(self);
         });
@@ -193,31 +209,35 @@ CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
             }
         });
         confirm.show();
-},
+    },
 
-    closeEditor: function(self, removePost) {
-        var targetModel = self.collection.get(self.$currentPost.attr('name'));
+    closeEditor: function(removePost) {
+        var targetModel = this.collection.get(this.$currentPost.attr('name'));
 
         if(removePost) {
-            self.$currentPost.remove();
+            this.$currentPost.remove();
         }
-
-        // close the modal and insert the appropriate data
-        self.$currentPost.removeClass('editing');
-        self.$currentPost.find('.date-display').html(targetModel.get('date'));
-        self.$currentPost.find('.date').val(targetModel.get('date'));
-        try {
-            // just in case the content causes an error (embedded js errors)
-            self.$currentPost.find('.update-contents').html(targetModel.get('content'));
-            self.$currentPost.find('.new-update-content').val(targetModel.get('content'));
-        } catch (e) {
-            // ignore but handle rest of page
+        else {
+            // close the modal and insert the appropriate data
+            this.$currentPost.removeClass('editing');
+            this.$currentPost.find('.date-display').html(targetModel.get('date'));
+            this.$currentPost.find('.date').val(targetModel.get('date'));
+
+            var content = changeContentToPreview(targetModel, 'content', this.options['base_asset_url'])
+            try {
+                // just in case the content causes an error (embedded js errors)
+                this.$currentPost.find('.update-contents').html(content);
+                this.$currentPost.find('.new-update-content').val(content);
+            } catch (e) {
+                // ignore but handle rest of page
+            }
+            this.$currentPost.find('form').hide();
+            this.$currentPost.find('.CodeMirror').remove();
         }
-        self.$currentPost.find('form').hide();
+
         window.$modalCover.unbind('click');
         window.$modalCover.hide();
         this.$codeMirror = null;
-        self.$currentPost.find('.CodeMirror').remove();
     },
 
     // Dereferencing from events to screen elements
@@ -275,8 +295,8 @@ CMS.Views.ClassInfoHandoutsView = Backbone.View.extend({
     },
 
     render: function () {
-        var updateEle = this.$el;
-        var self = this;
+        changeContentToPreview(this.model, 'data', this.options['base_asset_url'])
+
         this.$el.html(
             $(this.template( {
                 model: this.model
@@ -295,22 +315,17 @@ CMS.Views.ClassInfoHandoutsView = Backbone.View.extend({
         var self = this;
         this.$editor.val(this.$preview.html());
         this.$form.show();
-        if (this.$codeMirror == null) {
-            this.$codeMirror = CodeMirror.fromTextArea(this.$editor.get(0), {
-                mode: "text/html",
-                lineNumbers: true,
-                lineWrapping: true,
-            });
-        }
+
+        this.$codeMirror = editWithCodeMirror(self.model, 'data', self.options['base_asset_url'], this.$editor.get(0));
+
         window.$modalCover.show();
         window.$modalCover.bind('click', function() {
-            self.closeEditor(self);
+            self.closeEditor();
         });
     },
 
     onSave: function(event) {
         this.model.set('data', this.$codeMirror.getValue());
-        this.render();
         var saving = new CMS.Views.Notification.Mini({
             title: gettext('Saving') + '…'
         });
@@ -320,8 +335,9 @@ CMS.Views.ClassInfoHandoutsView = Backbone.View.extend({
                 saving.hide();
             }
         });
+        this.render();
         this.$form.hide();
-        this.closeEditor(this);
+        this.closeEditor();
 
         analytics.track('Saved Course Handouts', {
             'course': course_location_analytics
@@ -331,14 +347,14 @@ CMS.Views.ClassInfoHandoutsView = Backbone.View.extend({
 
     onCancel: function(event) {
         this.$form.hide();
-        this.closeEditor(this);
+        this.closeEditor();
     },
 
-    closeEditor: function(self) {
+    closeEditor: function() {
         this.$form.hide();
         window.$modalCover.unbind('click');
         window.$modalCover.hide();
-        self.$form.find('.CodeMirror').remove();
+        this.$form.find('.CodeMirror').remove();
         this.$codeMirror = null;
     }
 });

cms/static/js_test.yml

@@ -51,9 +51,11 @@ lib_paths:
     - xmodule_js/common_static/js/vendor/sinon-1.7.1.js
     - xmodule_js/common_static/js/vendor/jasmine-jquery.js
     - xmodule_js/common_static/js/vendor/jasmine-stealth.js
+    - xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js
     - xmodule_js/src/xmodule.js
     - xmodule_js/src
     - xmodule_js/common_static/js/test/add_ajax_prefix.js
+    - xmodule_js/common_static/js/src/utility.js
 
 # Paths to source JavaScript files
 src_paths:

cms/templates/course_info.html

@@ -39,6 +39,7 @@
       model : new CMS.Models.CourseInfo({
         courseId : '${context_course.location}',
         updates : course_updates,
+        base_asset_url : '${base_asset_url}',
         handouts : course_handouts
         })
     });

common/lib/xmodule/xmodule/js/js_test.yml

@@ -53,6 +53,7 @@ lib_paths:
     - common_static/js/vendor/sinon-1.7.1.js
     - common_static/js/vendor/analytics.js
     - common_static/js/test/add_ajax_prefix.js
+    - common_static/js/src/utility.js
 
 # Paths to spec (test) JavaScript files
 spec_paths:

common/lib/xmodule/xmodule/js/src/html/edit.coffee

@@ -101,32 +101,25 @@ class @HTMLEditingDescriptor
   # Show the Advanced (codemirror) Editor. Pulled out as a helper method for unit testing.
   showAdvancedEditor: (visualEditor) ->
     if visualEditor.isDirty()
-      content = @rewriteStaticLinks(visualEditor.getContent({no_events: 1}), @base_asset_url, '/static/')
+      content = rewriteStaticLinks(visualEditor.getContent({no_events: 1}), @base_asset_url, '/static/')
       @advanced_editor.setValue(content)
       @advanced_editor.setCursor(0)
     @advanced_editor.refresh()
     @advanced_editor.focus()
     @showingVisualEditor = false
 
-  rewriteStaticLinks: (content, from, to) ->
-    if from == null || to == null
-      return content
-
-    regex = new RegExp(from, 'g')
-    return content.replace(regex, to)    
-
   # Show the Visual (tinyMCE) Editor. Pulled out as a helper method for unit testing.
   showVisualEditor: (visualEditor) ->
     # In order for isDirty() to return true ONLY if edits have been made after setting the text,
     # both the startContent must be sync'ed up and the dirty flag set to false.
-    content = @rewriteStaticLinks(@advanced_editor.getValue(), '/static/', @base_asset_url)
+    content = rewriteStaticLinks(@advanced_editor.getValue(), '/static/', @base_asset_url)
     visualEditor.setContent(content)
     visualEditor.startContent = content
     @focusVisualEditor(visualEditor)
     @showingVisualEditor = true
 
   initInstanceCallback: (visualEditor) =>
-    visualEditor.setContent(@rewriteStaticLinks(@advanced_editor.getValue(), '/static/', @base_asset_url))
+    visualEditor.setContent(rewriteStaticLinks(@advanced_editor.getValue(), '/static/', @base_asset_url))
     @focusVisualEditor(visualEditor)
 
   focusVisualEditor: (visualEditor) =>
@@ -150,5 +143,5 @@ class @HTMLEditingDescriptor
     text = @advanced_editor.getValue()
     visualEditor = @getVisualEditor()
     if @showingVisualEditor and visualEditor.isDirty()
-      text = @rewriteStaticLinks(visualEditor.getContent({no_events: 1}), @base_asset_url, '/static/')
+      text = rewriteStaticLinks(visualEditor.getContent({no_events: 1}), @base_asset_url, '/static/')
     data: text

common/static/js/spec/utility_spec.js

+describe('utility.rewriteStaticLinks', function () {
+    it('returns "content" if "from" or "to" is null', function () {
+        expect(rewriteStaticLinks('foo', null, 'bar')).toBe('foo');
+        expect(rewriteStaticLinks('foo', 'bar', null)).toBe('foo');
+        expect(rewriteStaticLinks('foo', null, null)).toBe('foo');
+    });
+    it('does a replace of "from" to "to"', function () {
+        expect(rewriteStaticLinks('<img src="/static/foo.x"/>', '/static/', 'howdy')).toBe('<img src="howdyfoo.x"/>')
+    });
+    it('returns "content" if "from" is not found', function () {
+        expect(rewriteStaticLinks('<img src="/static/foo.x"/>', '/statix/', 'howdy')).toBe('<img src="/static/foo.x"/>')
+    });
+});

common/static/js/utility.js → common/static/js/src/utility.js

@@ -18,3 +18,13 @@ window.isExternal = function (url) {
         return true;
     return false;
 };
+
+// Utility method for replacing a portion of a string.
+window.rewriteStaticLinks = function(content, from, to) {
+    if (from === null || to === null) {
+        return content
+    }
+
+    var regex = new RegExp(from, 'g');
+    return content.replace(regex, to)
+};

common/static/js_test.yml

@@ -52,10 +52,12 @@ lib_paths:
 # Paths to source JavaScript files
 src_paths:
     - coffee/src
+    - js/src
 
 # Paths to spec (test) JavaScript files
 spec_paths:
     - coffee/spec
+    - js/spec
 
 # Regular expressions used to exclude *.js files from
 # appearing in the test runner page.

lms/envs/common.py

@@ -601,7 +601,7 @@ PIPELINE_JS = {
             'js/toggle_login_modal.js',
             'js/sticky_filter.js',
             'js/query-params.js',
-            'js/utility.js',
+            'js/src/utility.js',
         ],
         'output_filename': 'js/lms-application.js',