Merge lp:~wallyworld/launchpad/infotype-widget-1007984 into lp:launchpad

[Ian Booth]

Improve bug info type popup widget - add error display amd progress spinner.

== Implementation ==

1. Implement the showError and handleError methods on the error handler object.
2. Wire up the spinner.
3. Render both the new value and description test when thepopup widget is clicked and reset back if there is an error.

== Tests ==

Update the relevant yui tests. In particular, add a new test to check that the behaviour when an error occurs is as expected.
Also, the tests as written had an issue in that they were failing to disable the animation on the subscribers portlet. This lead to null exceptions and browser issues when running the tests. So I added code to do the correct thing.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/choice.js
  lib/lp/bugs/javascript/bug_subscription_portlet.js
  lib/lp/bugs/javascript/information_type_choice.js
  lib/lp/bugs/javascript/tests/test_information_type_choice.html
  lib/lp/bugs/javascript/tests/test_information_type_choice.js

[Steve Kowalik (stevenk)]

Looks good. I'm glad some of the bugs are contained within ChoiceSource itself.

review: Approve (code)

=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js	2012-06-04 11:41:47 +0000
+++ lib/lp/app/javascript/choice.js	2012-06-05 08:13:22 +0000
@@ -2,7 +2,7 @@
 
 var namespace = Y.namespace('lp.app.choice');
 
-function hook_up_spinner(widget) {
+namespace.hook_up_choicesource_spinner = function(widget) {
   // ChoiceSource makes assumptions about HTML in lazr-js
   // that don't hold true here, so we need to do our own
   // spinner icon and clear it when finished.
@@ -19,7 +19,7 @@
     icon.addClass('edit');
     icon.setStyle('bottom', '0px');
   }, widget, '_uiClearWaiting');
-}
+};
 
 namespace.addBinaryChoice = function(config, resource_uri, attribute) {
   var widget = new Y.ChoiceSource(config);
@@ -28,7 +28,7 @@
     cfg: {
       patch: attribute,
       resource: resource_uri}});
-  hook_up_spinner(widget);
+  namespace.hook_up_choicesource_spinner(widget);
   widget.render();
 };
 
@@ -41,7 +41,7 @@
     cfg: {
       patch: attribute,
       resource: resource_uri}});
-  hook_up_spinner(widget);
+  namespace.hook_up_choicesource_spinner(widget);
   widget.on('save', function(e) {
       var cb = widget.get('contentBox');
       var value = widget.get('value');

=== modified file 'lib/lp/bugs/javascript/bug_subscription_portlet.js'
--- lib/lp/bugs/javascript/bug_subscription_portlet.js	2012-02-21 22:46:28 +0000
+++ lib/lp/bugs/javascript/bug_subscription_portlet.js	2012-06-05 08:13:22 +0000
@@ -58,7 +58,7 @@
 /*
  * Update the text and link at the top of the subscription portlet.
  */
-function update_subscription_status() {
+function update_subscription_status(skip_animation) {
     var status = Y.one('#current_user_subscription');
     var span = status.one('span');
     var whitespace = status.one('span+span');
@@ -113,7 +113,9 @@
             }
         }
     }
-    Y.lp.anim.green_flash({ node: status }).run();
+    if(!Y.Lang.isBoolean(skip_animation) || !skip_animation) {
+        Y.lp.anim.green_flash({ node: status }).run();
+    }
 }
 namespace.update_subscription_status = update_subscription_status;
 

=== modified file 'lib/lp/bugs/javascript/information_type_choice.js'
--- lib/lp/bugs/javascript/information_type_choice.js	2012-05-30 20:31:37 +0000
+++ lib/lp/bugs/javascript/information_type_choice.js	2012-06-05 08:13:22 +0000
@@ -9,10 +9,25 @@
 var namespace = Y.namespace('lp.bugs.information_type_choice');
 var information_type_descriptions = {};
 
-namespace.save_information_type = function(value, lp_client) {
+// For testing.
+var skip_animation = false;
+
+namespace.save_information_type = function(widget, value, lp_client) {
     var error_handler = new Y.lp.client.ErrorHandler();
+    error_handler.showError = function(error_msg) {
+        Y.lp.app.errors.display_error(
+            Y.one('#information-type'), error_msg);
+    };
+    error_handler.handleError = function(ioId, response) {
+        var orig_value = LP.cache.bug.information_type;
+        widget.set('value', orig_value);
+        update_information_type_description(orig_value);
+        return false;
+    };
     var config = {
         on: {
+            start: Y.bind(widget._uiSetWaiting),
+            end: Y.bind(widget._uiClearWaiting),
             success: function(id, response) {
                 namespace.information_type_save_success(value);
             },
@@ -35,7 +50,6 @@
 };
 
 namespace.get_information_type_banner_text = function(value) {
-    console.dir(LP.cache);
     var fallback_text = "The information on this page is private.";
     var text_template = "This page contains {info_type} information.";
 
@@ -45,7 +59,7 @@
     if (LP.cache.show_information_type_in_ui) {
         return Y.Lang.substitute(text_template, {'info_type': value});
     } else {
-        return fallback_text; 
+        return fallback_text;
     }
 };
 
@@ -54,8 +68,7 @@
     var private_type = (Y.Array.indexOf(LP.cache.private_types, value) >= 0);
     var subscription_ns = Y.lp.bugs.bugtask_index.portlets.subscription;
     var privacy_banner = Y.lp.app.banner.privacy.getPrivacyBanner();
-    subscription_ns.update_subscription_status();
-    update_information_type_description(value);
+    subscription_ns.update_subscription_status(skip_animation);
     if (private_type) {
         var banner_text = namespace.get_information_type_banner_text(value);
         privacy_banner.updateText(banner_text);
@@ -67,7 +80,9 @@
     }
 };
 
-namespace.setup_information_type_choice = function(privacy_link, lp_client) {
+namespace.setup_information_type_choice = function(privacy_link, lp_client,
+                                                   skip_anim) {
+    skip_animation = skip_anim;
     Y.Array.each(LP.cache.information_types, function(info_type) {
         information_type_descriptions[info_type.value] = info_type.description;
     });
@@ -81,6 +96,7 @@
         items: LP.cache.information_types,
         backgroundColor: '#FFFF99'
     });
+    Y.lp.app.choice.hook_up_choicesource_spinner(information_type_edit);
     information_type_edit.render();
     information_type_edit.on("save", function(e) {
         var value = information_type_edit.get('value');
@@ -89,10 +105,13 @@
         if (value === 'Private') {
             value = 'User Data';
         }
-        namespace.save_information_type(value, lp_client);
+        update_information_type_description(value);
+        namespace.save_information_type(
+            information_type_edit, value, lp_client);
     });
     privacy_link.addClass('js-action');
+    return information_type_edit;
 };
 }, "0.1", {"requires": ["base", "oop", "node", "event", "io-base",
                         "lazr.choiceedit", "lp.bugs.bugtask_index",
-                        "lp.app.banner.privacy"]});
+                        "lp.app.banner.privacy", "lp.app.choice"]});

=== modified file 'lib/lp/bugs/javascript/tests/test_information_type_choice.html'
--- lib/lp/bugs/javascript/tests/test_information_type_choice.html	2012-05-16 02:09:30 +0000
+++ lib/lp/bugs/javascript/tests/test_information_type_choice.html	2012-06-05 08:13:22 +0000
@@ -28,6 +28,8 @@
       <script type="text/javascript"
           src="../../../../../build/js/lp/app/lp.js"></script>
       <script type="text/javascript"
+          src="../../../../../build/js/lp/app/choice.js"></script>
+      <script type="text/javascript"
           src="../../../../../build/js/lp/app/testing/mockio.js"></script>
       <script type="text/javascript"
           src="../../../../../build/js/lp/app/client.js"></script>

=== modified file 'lib/lp/bugs/javascript/tests/test_information_type_choice.js'
--- lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-05-30 13:59:24 +0000
+++ lib/lp/bugs/javascript/tests/test_information_type_choice.js	2012-06-05 08:13:22 +0000
@@ -25,10 +25,12 @@
                     show_information_type_in_ui: true,
                     private_types: ['Private'],
                     information_types: [
-                        {'value': 'Public', 'description': 'Public',
-                            'name': 'Public'},
-                        {'value': 'Private', 'description': 'Private',
-                            'name': 'Private'}
+                        {'value': 'Public', 'name': 'Public',
+                            'description': 'Public Description'},
+                        {'value': 'Private', 'name': 'Private',
+                            'description': 'Private Description'},
+                        {'value': 'User Data', 'name': 'User Data',
+                            'description': 'User Data Description'}
                     ]
                 }
             };
@@ -39,7 +41,8 @@
 
             var lp_client = new Y.lp.client.Launchpad();
             var privacy_link = Y.one('#privacy-link');
-            ns.setup_information_type_choice(privacy_link, lp_client);
+            this.widget = ns.setup_information_type_choice(
+                privacy_link, lp_client, true);
         },
         tearDown: function () {
             if (this.fixture !== null) {
@@ -75,7 +78,7 @@
             var lp_client = new Y.lp.client.Launchpad({
                 io_provider: mockio
             });
-            ns.save_information_type('User Data', lp_client);
+            ns.save_information_type(this.widget, 'User Data', lp_client);
             Y.Assert.areEqual('/api/devel/bug/1', mockio.last_request.url);
             Y.Assert.areEqual(
                 'ws.op=transitionToInformationType&' +
@@ -84,10 +87,6 @@
         },
 
         test_information_type_save_success_private: function() {
-            var description_node = Y.one('#information-type-description');
-            Y.Assert.areEqual(
-                'Everyone can see this information.',
-                description_node.get('text'));
             var old_func = this._shim_privacy_banner();
             var hide_flag = false;
             var update_flag = false;
@@ -103,7 +102,6 @@
             Y.Assert.isTrue(body.hasClass('private'));
             Y.Assert.isTrue(hide_flag);
             Y.Assert.isTrue(update_flag);
-            Y.Assert.areEqual('Private', description_node.get('text'));
             this._unshim_privacy_banner(old_func);
         },
 
@@ -118,8 +116,6 @@
             var body = Y.one('body');
             Y.Assert.isTrue(body.hasClass('public'));
             Y.Assert.isTrue(flag);
-            var description_node = Y.one('#information-type-description');
-            Y.Assert.areEqual('Public', description_node.get('text'));
             this._unshim_privacy_banner(old_func);
         },
 
@@ -130,12 +126,36 @@
                 '.yui3-ichoicelist-content a[href=#Private]');
             var orig_save_information_type = ns.save_information_type;
             var function_called = false;
-            ns.save_information_type = function(value, lp_client) {
+            ns.save_information_type = function(widget, value, lp_client) {
                 Y.Assert.areEqual(
                     'User Data', value); function_called = true; };
             private_choice.simulate('click');
+            var description_node = Y.one('#information-type-description');
+            Y.Assert.areEqual(
+                'User Data Description', description_node.get('text'));
             Y.Assert.isTrue(function_called);
             ns.save_information_type = orig_save_information_type;
+        },
+
+        test_information_type_save_error: function() {
+            var mockio = new Y.lp.testing.mockio.MockIo();
+            var lp_client = new Y.lp.client.Launchpad({
+                io_provider: mockio
+            });
+            this.widget.set('value', 'User Data');
+            ns.save_information_type(this.widget, 'User Data', lp_client);
+            mockio.last_request.respond({
+                status: 500,
+                statusText: 'An error occurred'
+            });
+            // The original info type value from the cache should have been
+            // set back into the widget.
+            Y.Assert.areEqual('Public', this.widget.get('value'));
+            var description_node = Y.one('#information-type-description');
+            Y.Assert.areEqual(
+                'Public Description', description_node.get('text'));
+            // The error was displayed.
+            Y.Assert.isNotNull(Y.one('.yui3-lazr-formoverlay-errors'));
         }
     }));