Refactor Studio editing JavaScript to make it easier to test

Add JavaScript tests for Studio editing and XBlock server interaction
Refactor student-facing JS views to use JS server interface.
manage.py test runs JavaScript tests
Fix data type issue for rendering peer/self/grade on page load

.gitignore

@@ -48,3 +48,4 @@ htmlcov
 # node
 node_modules
 npm-debug.log
+coverage

.travis.yml

@@ -4,7 +4,6 @@ python:
 install:
   - "pip install -r requirements/test.txt --use-mirrors"
   - "pip install -e ."
-  - "npm install"
 before_script:
   - export DISPLAY=:99.0
   - sh -e /etc/init.d/xvfb start
@@ -12,7 +11,6 @@ before_script:
 script:
   - "python manage.py test"
   - "python manage.py harvest"
-  - "npm test"
   - "python setup.py install"
 after_success:
   coveralls

README.rst

@@ -27,18 +27,12 @@ up in the XBlock workbench.
 Running Tests
 =============
 
-To run the Python test suite:
+To run the unit test suite:
 
 .. code:: bash
 
     python manage.py test
 
-To run the JavaScript test suite (after installing `node <http://nodejs.org/download/>`_)
-
-.. code:: bash
-
-    npm install && npm test
-
 
 License
 =======

apps/openassessment/xblock/openassessmentblock.py

@@ -252,6 +252,7 @@ class OpenAssessmentBlock(XBlock, SubmissionMixin, PeerAssessmentMixin, SelfAsse
         context = Context(context_dict)
         frag = Fragment(template.render(context))
         frag.add_css(load("static/css/openassessment.css"))
+        frag.add_javascript(load("static/js/src/oa_server.js"))
         frag.add_javascript(load("static/js/src/oa_base.js"))
         frag.initialize_js('OpenAssessmentBlock')
         return frag

apps/openassessment/xblock/static/js/spec/oa_edit.js

@@ -2,9 +2,49 @@
 Tests for OA XBlock editing.
 */
 
-describe("OpenAssessment editor", function() {
+describe("OpenAssessment.StudioUI", function() {
 
-    var runtime = null;
+    var runtime = {
+        notify: function(type, data) {}
+    };
+
+    // Stub server that returns dummy data or reports errors.
+    var StubServer = function() {
+        this.loadError = false;
+        this.updateError = false;
+        this.xml = '<openassessment></openassessment>';
+
+        this.errorPromise = $.Deferred(function(defer) {
+            defer.rejectWith(this, ['Test error']);
+        }).promise();
+
+        this.loadXml = function() {
+            var xml = this.xml;
+            if (!this.loadError) {
+                return $.Deferred(function(defer) {
+                    defer.resolveWith(this, [xml]);
+                }).promise();
+            }
+            else {
+                return this.errorPromise;
+            }
+        }
+
+        this.updateXml = function(xml) {
+            if (!this.updateError) {
+                this.xml = xml;
+                return $.Deferred(function(defer) {
+                    defer.resolve();
+                }).promise();
+            }
+            else {
+                return this.errorPromise;
+            }
+        }
+    };
+
+    var server = null;
+    var ui = null;
 
     beforeEach(function() {
 
@@ -12,47 +52,56 @@ describe("OpenAssessment editor", function() {
         jasmine.getFixtures().fixturesPath = 'base/fixtures'
         loadFixtures('oa_edit.html');
 
-        // Mock the runtime
-        runtime = {
-            notify: function(type, data) {},
+        // Create the stub server
+        server = new StubServer();
 
-            // Dummy handler URL returns whatever it's passed in for the handler name
-            handlerUrl: function(element, handler) {
-                return handler;
-            }
-        };
+        // Mock the runtime
         spyOn(runtime, 'notify');
 
+        // Create the object under test
+        var el = $('#openassessment-edit').get(0);
+        ui = new OpenAssessment.StudioUI(runtime, el, server);
     });
 
     it("loads the XML definition", function() {
-        // Stub AJAX calls to always return successful
-        spyOn($, 'ajax').andCallFake(function(params) {
-            params.success({
-                'success': true,
-                'xml': '<openassessment></openassessment>',
-                'msg': ''
-            });
-        });
-
-        // Initialize the editor
-        var editor = OpenAssessmentEditor(runtime, $('#openassessment-edit'));
+        // Initialize the UI
+        ui.load()
 
         // Expect that the XML definition was loaded
-        var editorContents = $('.openassessment-editor').text();
-        expect(editorContents).toEqual('<openassessment></openassessment>');
+        var contents = ui.codeBox.getValue();
+        expect(contents).toEqual('<openassessment></openassessment>');
     });
 
     it("saves the XML definition", function() {
-    expect(false).toBe(true);
+        // Update the XML
+        ui.codeBox.setValue('<openassessment>test!</openassessment>');
+
+        // Save the updated XML
+        ui.save();
+
+        // Expect the saving notification to start/end
+        expect(runtime.notify).toHaveBeenCalledWith('save', {state: 'start'});
+        expect(runtime.notify).toHaveBeenCalledWith('save', {state: 'end'});
+
+        // Expect the server's XML to have been updated
+        expect(server.xml).toEqual('<openassessment>test!</openassessment>');
+    });
+
+    it("cancels editing", function() {
+        ui.cancel();
+        expect(runtime.notify).toHaveBeenCalledWith('cancel', {});
     });
 
-    it("reverts the XML definition on cancellation", function() {
-    expect(false).toBe(true);
+    it("displays an error when server reports a load XML error", function() {
+        server.loadError = true;
+        ui.load();
+        expect(runtime.notify).toHaveBeenCalledWith('error', {msg: 'Test error'});
     });
 
-    it("displays validation errors but preserves the author's changes", function() {
-    expect(false).toBe(true);
+    it("displays an error when server reports an update XML error", function() {
+        server.updateError = true;
+        ui.save('<openassessment>test!</openassessment>');
+        expect(runtime.notify).toHaveBeenCalledWith('error', {msg: 'Test error'});
     });
 
 });

apps/openassessment/xblock/static/js/spec/oa_server.js

+/*
+Tests for OA XBlock server interactions.
+*/
+
+describe("OpenAssessment.Server", function() {
+
+    // Stub runtime implementation that returns the handler as the URL
+    var runtime = {
+        handlerUrl: function(element, handler) { return "/" + handler }
+    }
+
+    var server = null;
+
+    /**
+    Stub AJAX requests.
+
+    Args:
+        success (bool): If true, return a promise that resolves;
+            otherwise, return a promise that fails.
+
+        responseData(object): Data to pass to the caller if the AJAX
+            call completes successfully.
+    **/
+    var stubAjax = function(success, responseData) {
+        spyOn($, 'ajax').andReturn(
+            $.Deferred(function(defer) {
+                if (success) { defer.resolveWith(this, [responseData]); }
+                else { defer.reject() }
+           }).promise()
+        );
+    }
+
+    beforeEach(function() {
+        // Create the server
+        // Since the runtime is a stub implementation that ignores the element passed to it,
+        // we can set the element parameter to null.
+        server = new OpenAssessment.Server(runtime, null);
+    });
+
+    it("Renders the XBlock as HTML", function() {
+        stubAjax(true, "<div>Open Assessment</div>");
+
+        var loadedHtml = "";
+        server.render('submission').done(function(html) {
+            loadedHtml = html;
+        });
+
+        expect(loadedHtml).toEqual("<div>Open Assessment</div>");
+        expect($.ajax).toHaveBeenCalledWith({
+            url: '/render_submission', type: "POST", dataType: "html"
+        });
+    });
+
+    it("Sends a submission the XBlock", function() {
+        // Status, student ID, attempt number
+        stubAjax(true, [true, 1, 2]);
+
+        var receivedStudentId = null;
+        var receivedAttemptNum = null;
+        server.submit("This is only a test").done(
+            function(studentId, attemptNum) {
+                receivedStudentId = studentId;
+                receivedAttemptNum = attemptNum;
+            }
+        );
+
+        expect(receivedStudentId).toEqual(1);
+        expect(receivedAttemptNum).toEqual(2);
+        expect($.ajax).toHaveBeenCalledWith({
+            url: '/submit',
+            type: "POST",
+            data: {submission: "This is only a test"}
+        });
+    });
+
+    it("loads the XBlock's XML definition", function() {
+        stubAjax(true, { success: true, xml: "<openassessment />" });
+
+        var loadedXml = "";
+        server.loadXml().done(function(xml) {
+            loadedXml = xml;
+        });
+
+        expect(loadedXml).toEqual('<openassessment />');
+        expect($.ajax).toHaveBeenCalledWith({
+            url: '/xml', type: "POST", data: '""'
+        });
+    });
+
+    it("updates the XBlock's XML definition", function() {
+        stubAjax(true, { success: true });
+
+        server.updateXml('<openassessment />');
+        expect($.ajax).toHaveBeenCalledWith({
+            url: '/update_xml', type: "POST",
+            data: JSON.stringify({xml: '<openassessment />'})
+        });
+    });
+
+    it("informs the caller of an Ajax error when rendering as HTML", function() {
+        stubAjax(false, null);
+
+        var receivedMsg = "";
+        server.render('submission').fail(function(msg) {
+            receivedMsg = msg;
+        });
+
+        expect(receivedMsg).toEqual("Could not contact server.");
+    });
+
+    it("informs the caller of an Ajax error when sending a submission", function() {
+        stubAjax(false, null);
+
+        var receivedErrorCode = "";
+        var receivedErrorMsg = "";
+        server.submit('This is only a test.').fail(
+            function(errorCode, errorMsg) {
+                receivedErrorCode = errorCode;
+                receivedErrorMsg = errorMsg;
+            }
+        );
+
+        expect(receivedErrorCode).toEqual("AJAX");
+        expect(receivedErrorMsg).toEqual("Could not contact server.");
+    });
+
+    it("informs the caller of an server error when sending a submission", function() {
+        stubAjax(true, [false, "ENODATA", "Error occurred!"]);
+
+        var receivedErrorCode = "";
+        var receivedErrorMsg = "";
+        server.submit('This is only a test.').fail(
+            function(errorCode, errorMsg) {
+                receivedErrorCode = errorCode;
+                receivedErrorMsg = errorMsg;
+            }
+        );
+
+        expect(receivedErrorCode).toEqual("ENODATA");
+        expect(receivedErrorMsg).toEqual("Error occurred!");
+    });
+
+    it("informs the caller of an Ajax error when loading XML", function() {
+        stubAjax(false, null);
+
+        var receivedMsg = null;
+        server.loadXml().fail(function(msg) {
+            receivedMsg = msg;
+        });
+
+        expect(receivedMsg).toEqual("Could not contact server.");
+    });
+
+    it("informs the caller of an Ajax error when updating XML", function() {
+        stubAjax(false, null);
+
+        var receivedMsg = null;
+        server.updateXml('test').fail(function(msg) {
+            receivedMsg = msg;
+        });
+
+        expect(receivedMsg).toEqual("Could not contact server.");
+    });
+
+    it("informs the caller of a server error when loading XML", function() {
+        stubAjax(true, { success: false, msg: "Test error" });
+
+        var receivedMsg = null;
+        server.updateXml('test').fail(function(msg) {
+            receivedMsg = msg;
+        });
+
+        expect(receivedMsg).toEqual("Test error");
+    });
+
+    it("informs the caller of a server error when updating XML", function() {
+        stubAjax(true, { success: false, msg: "Test error" });
+
+        var receivedMsg = null;
+        server.loadXml().fail(function(msg) {
+            receivedMsg = msg;
+        });
+
+        expect(receivedMsg).toEqual("Test error");
+    });
+});

apps/openassessment/xblock/static/js/src/oa_base.js

@@ -123,6 +123,7 @@ function OpenAssessmentBlock(runtime, element) {
         $.ajax({
             type: "POST",
             url: renderPeerUrl,
+            dataType: "html",
             success:  function(data) {
                 $(peerListItem, element).replaceWith(data);
                 collapse($(peerListItem, element));
@@ -132,6 +133,7 @@ function OpenAssessmentBlock(runtime, element) {
         $.ajax({
             type: "POST",
             url: renderSelfUrl,
+            dataType: "html",
             success:  function(data) {
                 $(selfListItem, element).replaceWith(data);
                 collapse($(selfListItem, element));
@@ -140,6 +142,7 @@ function OpenAssessmentBlock(runtime, element) {
 
         $.ajax({
             type: "POST",
+            dataType: "html",
             url: renderGradeUrl,
             success:  function(data) {
                 $(gradeListItem, element).replaceWith(data);

apps/openassessment/xblock/static/js/src/oa_edit.js

 /* JavaScript for Studio editing view of Open Assessment XBlock */
-function OpenAssessmentEditor(runtime, element) {
 
-    function displayError(errorMsg) {
-        runtime.notify('error', {msg: errorMsg});
-    }
 
-    // Update editor with the XBlock's current content
-    function updateEditorFromXBlock(editor) {
-        $.ajax({
-            type: "POST",
-            url: runtime.handlerUrl(element, 'xml'),
-            data: "\"\"",
-            success: function(data) {
-                if (data.success) {
-                    editor.setValue(data.xml);
-                }
-
-                else {
-                    displayError(data.msg);
-                }
+/* Namespace for open assessment */
+if (typeof OpenAssessment == "undefined" || !OpenAssessment) {
+    OpenAssessment = {};
+}
+
+
+/**
+Interface for editing UI in Studio.
+The constructor initializes the DOM for editing.
+
+Args:
+    runtime (Runtime): an XBlock runtime instance.
+    element (DOM element): The DOM element representing this XBlock.
+    server (OpenAssessment.Server): The interface to the XBlock server.
+
+Returns:
+    OpenAssessment.StudioUI
+**/
+OpenAssessment.StudioUI = function(runtime, element, server) {
+    this.runtime = runtime;
+    this.server = server;
+
+    // Initialize the code box
+    this.codeBox = CodeMirror.fromTextArea(
+        $(element).find('.openassessment-editor').first().get(0),
+        {mode: "xml", lineNumbers: true, lineWrapping: true}
+    );
+
+    // Install click handlers
+    var ui = this;
+    $(element).find('.openassessment-save-button').click(
+        function(eventData) {
+            ui.save();
+    });
+
+    $(element).find('.openassessment-cancel-button').click(
+        function(eventData) {
+            ui.cancel();
+    });
+};
+
+
+OpenAssessment.StudioUI.prototype = {
+
+    /**
+    Load the XBlock XML definition from the server and display it in the UI.
+    **/
+    load: function() {
+        var ui = this;
+        this.server.loadXml().done(
+            function(xml) {
+                ui.codeBox.setValue(xml);
+            }).fail(function(msg) {
+                ui.showError(msg);
             }
+        );
+    },
+
+    /**
+    Save the updated XML definition to the server.
+    **/
+    save: function() {
+        // Notify the client-side runtime that we are starting
+        // to save so it can show the "Saving..." notification
+        this.runtime.notify('save', {state: 'start'});
+
+        // Send the updated XML to the server
+        var xml = this.codeBox.getValue();
+        var ui = this;
+        this.server.updateXml(xml).done(function() {
+            // Notify the client-side runtime that we finished saving
+            // so it can hide the "Saving..." notification.
+            ui.runtime.notify('save', {state: 'end'});
+        }).fail(function(msg) {
+            ui.showError(msg);
         });
-    }
+    },
 
-    function initializeEditor() {
-        var textAreas = $(element).find('.openassessment-editor');
-        if (textAreas.length < 1) {
-            console.warn("Could not find element for OpenAssessmentBlock XML editor");
-            return null;
-        }
-        else {
-            return CodeMirror.fromTextArea(
-                textAreas[0], {mode: "xml", lineNumbers: true, lineWrapping: true}
-            );
-        }
-    }
+    /**
+    Cancel editing.
+    **/
+    cancel: function() {
+        // Notify the client-side runtime so it will close the editing modal.
+        this.runtime.notify('cancel', {});
+    },
 
-    function initializeSaveButton(editor) {
-        saveButtons = $(element).find('.openassessment-save-button');
-        if (saveButtons.length < 1) {
-            console.warn("Could not find element for OpenAssessmentBlock save button");
-        }
-        else {
-            saveButtons.click(function (eventObject) {
-                // Notify the client-side runtime that we are starting
-                // to save so it can show the "Saving..." notification
-                runtime.notify('save', {state: 'start'});
-
-                // POST the updated description to the XBlock
-                // The server-side code is responsible for validating and persisting
-                // the updated content.
-                $.ajax({
-                    type: "POST",
-                    url: runtime.handlerUrl(element, 'update_xml'),
-                    data: JSON.stringify({ xml: editor.getValue() }),
-                    success: function(data) {
-                        // Notify the client-side runtime that we finished saving
-                        // so it can hide the "Saving..." notification.
-                        if (data.success) {
-                            runtime.notify('save', {state: 'end'});
-                        }
-
-                        // Display an error alert if any errors occurred
-                        else {
-                            displayError(data.msg);
-                        }
-                    }
-                });
-            });
-        }
-    }
+    /**
+    Display an error message to the user.
 
-    function initializeCancelButton(editor) {
-        cancelButtons = $(element).find('.openassessment-cancel-button');
-        if (cancelButtons.length < 1) {
-            console.warn("Could not find element for OpenAssessmentBlock cancel button");
-        }
-        else {
-            cancelButtons.click(function (eventObject) {
-                // Revert to the XBlock's current content
-                updateEditorFromXBlock(editor);
-
-                // Notify the client-side runtime so it will close the editing modal.
-                runtime.notify('cancel', {});
-            });
-        }
+    Args:
+        errorMsg (string): The error message to display.
+    **/
+    showError: function(errorMsg) {
+        this.runtime.notify('error', {msg: errorMsg});
     }
+};
+
+
+/* XBlock entry point for Studio view */
+function OpenAssessmentEditor(runtime, element) {
 
-    $(function ($) {
-        editor = initializeEditor();
-        if (editor) {
-            updateEditorFromXBlock(editor);
-            initializeSaveButton(editor);
-            initializeCancelButton(editor);
-        }
+    /**
+    Initialize the editing interface on page load.
+    **/
+    $(function($) {
+        var server = new OpenAssessment.Server(runtime, element);
+        var ui = new OpenAssessment.StudioUI(runtime, element, server);
+        ui.load();
     });
 }

apps/openassessment/xblock/static/js/src/oa_server.js

+/* JavaScript interface for interacting with server-side OpenAssessment XBlock */
+
+/* Namespace for open assessment */
+if (typeof OpenAssessment == "undefined" || !OpenAssessment) {
+    OpenAssessment = {};
+}
+
+
+/**
+Interface for server-side XBlock handlers.
+
+Args:
+    runtime (Runtime): An XBlock runtime instance.
+    element (DOM element): The DOM element representing this XBlock.
+
+Returns:
+    OpenAssessment.Server
+**/
+OpenAssessment.Server = function(runtime, element) {
+    this.runtime = runtime;
+    this.element = element;
+};
+
+
+OpenAssessment.Server.prototype = {
+
+    /**
+    Construct the URL for the handler, specific to one instance of the XBlock on the page.
+
+    Args:
+        handler (string): The name of the XBlock handler.
+
+    Returns:
+        URL (string)
+    **/
+    url: function(handler) {
+        return this.runtime.handlerUrl(this.element, handler);
+    },
+
+    /**
+    Render the XBlock.
+
+    Args:
+        component (string): The component to render.
+
+    Returns:
+        A JQuery promise, which resolves with the HTML of the rendered XBlock
+        and fails with an error message.
+
+    Example:
+        server.render('submission').done(
+            function(html) { console.log(html); }
+        ).fail(
+            function(err) { console.log(err); }
+        )
+    **/
+    render: function(component) {
+        var url = this.url('render_' + component);
+        return $.Deferred(function(defer) {
+            $.ajax({
+                url: url,
+                type: "POST",
+                dataType: "html"
+            }).done(function(data) {
+                defer.resolveWith(this, [data]);
+            }).fail(function(data) {
+                defer.rejectWith(this, ['Could not contact server.']);
+            })
+        }).promise();
+    },
+
+    /**
+    Send a submission to the XBlock.
+
+    Args:
+        submission (string): The text of the student's submission.
+
+    Returns:
+        A JQuery promise, which resolves with the student's ID and attempt number
+        if the call was successful and fails with an status code and error message otherwise.
+    **/
+    submit: function(submission) {
+        var url = this.url('submit');
+        return $.Deferred(function(defer) {
+            $.ajax({
+                type: "POST",
+                url: url,
+                data: {submission: submission}
+            }).done(function(data) {
+                var success = data[0];
+                if (success) {
+                    var studentId = data[1];
+                    var attemptNum = data[2];
+                    defer.resolveWith(this, [studentId, attemptNum]);
+                }
+                else {
+                    var errorNum = data[1];
+                    var errorMsg = data[2];
+                    defer.rejectWith(this, [errorNum, errorMsg]);
+                }
+            }).fail(function(data) {
+                defer.rejectWith(this, ["AJAX", "Could not contact server."]);
+            })
+        }).promise();
+    },
+
+    /**
+    Load the XBlock's XML definition from the server.
+
+    Returns:
+        A JQuery promise, which resolves with the XML definition
+        and fails with an error message.
+
+    Example:
+        server.loadXml().done(
+            function(xml) { console.log(xml); }
+        ).fail(
+            function(err) { console.log(err); }
+        );
+    **/
+    loadXml: function() {
+        var url = this.url('xml');
+        return $.Deferred(function(defer) {
+            $.ajax({
+                type: "POST", url: url, data: "\"\""
+            }).done(function(data) {
+                if (data.success) { defer.resolveWith(this, [data.xml]); }
+                else { defer.rejectWith(this, [data.msg]); }
+            }).fail(function(data) {
+                defer.rejectWith(this, ['Could not contact server.']);
+            });
+        }).promise();
+    },
+
+    /**
+    Update the XBlock's XML definition on the server.
+
+    Returns:
+        A JQuery promise, which resolves with no arguments
+        and fails with an error message.
+
+    Example usage:
+        server.updateXml(xml).done(
+            function() {}
+        ).fail(
+            function(err) { console.log(err); }
+        );
+    **/
+    updateXml: function(xml) {
+        var url = this.url('update_xml');
+        var payload = JSON.stringify({xml: xml});
+        return $.Deferred(function(defer) {
+            $.ajax({
+                type: "POST", url: url, data: payload
+            }).done(function(data) {
+                if (data.success) { defer.resolve() }
+                else { defer.rejectWith(this, [data.msg]); }
+            }).fail(function(data) {
+                defer.rejectWith(this, ['Could not contact server.']);
+            });
+        }).promise();
+    }
+};

apps/openassessment/xblock/studio_mixin.py

@@ -38,6 +38,7 @@ class StudioMixin(object):
         """
         rendered_template = get_template('openassessmentblock/oa_edit.html').render(Context({}))
         frag = Fragment(rendered_template)
+        frag.add_javascript(pkg_resources.resource_string(__name__, "static/js/src/oa_server.js"))
         frag.add_javascript(pkg_resources.resource_string(__name__, "static/js/src/oa_edit.js"))
         frag.initialize_js('OpenAssessmentEditor')
         return frag

karma.conf.js

@@ -36,15 +36,18 @@ module.exports = function(config) {
     // preprocess matching files before serving them to the browser
     // available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor
     preprocessors: {
-
+      'src/*.js': 'coverage'
     },
 
 
     // test results reporter to use
     // possible values: 'dots', 'progress'
     // available reporters: https://npmjs.org/browse/keyword/karma-reporter
-    reporters: ['progress'],
+    reporters: ['progress', 'coverage'],
 
+    coverageReporter: {
+        type : 'text'
+    },
 
     // web server port
     port: 9876,
@@ -73,4 +76,5 @@ module.exports = function(config) {
     singleRun: true
 
   });
+
 };

manage.py

@@ -15,3 +15,7 @@ if __name__ == "__main__":
     from django.core.management import execute_from_command_line
 
     execute_from_command_line(sys.argv)
+
+    # Execute JavaScript tests
+    if 'test' in sys.argv:
+        os.system('npm install && npm test')

package.json

@@ -2,6 +2,7 @@
   "devDependencies": {
     "karma": "~0.11",
     "karma-jasmine": "0.1.3",
+    "karma-coverage": "0.1.5",
     "karma-firefox-launcher": "~0.1.3"
   },
   "scripts": {