launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/maas-bug-934223 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #934223 in Maas: "Add a node not logged in returns unhelpful message"
  https://bugs.launchpad.net/maas/+bug/934223

For more details, see:
https://code.launchpad.net/~rvb/maas/maas-bug-934223/+merge/94019

This branch adds a special error message that is displayed when an API request is made only to find out that no one is logged in (Unauthenticated error).

I've tested that the login link is present in the error message.  I think that is enough but feel free to suggest something else if you have a better idea.

Note that I've added the Y.log statement to help us debug errors.  It won't do anything if debug is turned off in YUI or if no javascript debugger is on.
-- 
https://code.launchpad.net/~rvb/maas/maas-bug-934223/+merge/94019
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/maas-bug-934223 into lp:maas.

=== modified file 'src/maasserver/static/js/node_add.js'
--- src/maasserver/static/js/node_add.js	2012-02-21 10:50:07 +0000
+++ src/maasserver/static/js/node_add.js	2012-02-21 17:06:17 +0000
@@ -54,6 +54,7 @@
         field.set('id', field.get('id') + form_nb);
         return Y.Node.create('<p />').append(field);
     },
+
     /**
      * Hide the panel.
      *
@@ -124,9 +125,27 @@
         return addnodeform;
     },
 
-    displayFormError: function(error_message) {
+    /**
+     * Display an error message.  The passed-in parameter can be a string or 
+     * a Node (in which case it will be appended to the error node).
+     *
+     * @method displayFormError
+     */
+    displayFormError: function(error) {
+        var error_node;
+        if (typeof error === 'string') {
+            // 'error' is a string, create a simple node with the error
+            // message in it.
+            error_node = Y.Node.create('<span />')
+                .set('text', error);
+        }
+        else {
+            // We assume error is a Y.Node.
+            error_node = error;
+        }
+
         this.get(
-            'srcNode').one('.form-global-errors').set('text', error_message);
+            'srcNode').one('.form-global-errors').empty().append(error_node);
      },
 
    /**
@@ -154,9 +173,26 @@
         this.add_node = Y.one('#add-node').getContent();
         // Create panel's content.
         this.set('bodyContent', this.createForm());
-       // Prepare spinnerNode.
+        this.initializeNodes();
+    },
+
+    /**
+     * Initialize the nodes this widget will use.
+     *
+     * @method initializeNodes
+     */
+    initializeNodes: function() {
+        // Prepare spinnerNode.
         this.spinnerNode = Y.Node.create('<img />')
             .set('src', MAAS_config.uris.statics + 'img/spinner.gif');
+        // Prepare logged-off error message.
+        this.loggedOffNode = Y.Node.create('<span />')
+            .set('text', "You have been logged out, please ")
+            .append(Y.Node.create('<a />')
+                .set('text', 'login')
+                .set('href', MAAS_config.uris.login))
+            .append(Y.Node.create('<span />')
+                .set('text', ' again.'));
     },
 
     bindUI: function() {
@@ -188,6 +224,7 @@
                     self.hidePanel();
                 },
                 failure: function(id, out) {
+                    Y.log(out);
                     if (out.status === 400) {
                         try {
                             // Validation error: display the errors in the
@@ -195,12 +232,16 @@
                             self.displayFieldErrors(JSON.parse(out.response));
                         }
                         catch (e) {
-                            self.displayFormError('Unable to create Node.');
+                            self.displayFormError("Unable to create Node.");
                         }
                     }
+                    else if (out.status === 401) {
+                        // Unauthorized error: the user has been logged out.
+                        self.displayFormError(self.loggedOffNode);
+                    }
                     else {
                         // Unexpected error.
-                        self.displayFormError('Unable to create Node.');
+                        self.displayFormError("Unable to create Node.");
                     }
                 },
                 end: Y.bind(self.hideSpinner, self)

=== modified file 'src/maasserver/static/js/tests/test_node_add.html'
--- src/maasserver/static/js/tests/test_node_add.html	2012-02-03 14:39:50 +0000
+++ src/maasserver/static/js/tests/test_node_add.html	2012-02-21 17:06:17 +0000
@@ -15,6 +15,7 @@
     <!--
     var MAAS_config = {
       uris: {
+        login: '/login/',
         statics: '/static/',
         nodes_handler: '/api/nodes/'
       }

=== modified file 'src/maasserver/static/js/tests/test_node_add.js'
--- src/maasserver/static/js/tests/test_node_add.js	2012-02-21 10:50:07 +0000
+++ src/maasserver/static/js/tests/test_node_add.js	2012-02-21 17:06:17 +0000
@@ -109,6 +109,24 @@
         );
         button.simulate('click');
         Y.Assert.isTrue(fired);
+    },
+
+    testLoggedOffErrorMessage: function() {
+        var mockXhr = new Y.Base();
+        mockXhr.send = function(url, cfg) {
+            cfg.on.failure(3, {status: 401});
+        };
+        this.mockIO(mockXhr, module);
+        module.showAddNodeWidget();
+        var panel = module._add_node_singleton;
+        panel.get('srcNode').one('#id_hostname').set('value', 'host');
+        var button = panel.get('srcNode').one('.yui3-button');
+        button.simulate('click');
+        var error_message = panel.get(
+            'srcNode').one('.form-global-errors').get('innerHTML');
+        // The link to the login page is present in the error message.
+        var link_position = error_message.search(MAAS_config.uris.login);
+        Y.Assert.areNotEqual(-1, link_position);
     }
 
 }));

=== modified file 'src/maasserver/templates/maasserver/js-conf.html'
--- src/maasserver/templates/maasserver/js-conf.html	2012-02-20 11:02:48 +0000
+++ src/maasserver/templates/maasserver/js-conf.html	2012-02-21 17:06:17 +0000
@@ -7,6 +7,7 @@
 };
 var MAAS_config = {
     uris: {
+        login: '{% url "login" %}',
         statics: '{{ STATIC_URL }}',
         nodes_handler: '{% url "nodes_handler" %}',
         account_handler: '{% url "account_handler" %}'