Merge lp:~gmb/launchpad/use-subscribe-form-for-fun-and-profit-bug-687747 into lp:launchpad

[Graham Binns]

[r=deryck][ui=none][bug=687747] The new subscribe wizard will now load asynchronously and will pull its form contents from a specified URL.

This branch is the latest in a line of branches to get the new
subscription wizard widget ready to be linked up with the Launchpad
code.

In this branch, the existing code, which was pretty much a placeholder
and very linear, is replaced with lots of asynchronous, event-driven
stuff. This leaves us ready for a final branch or two that will allow us
to start using the subscription wizard.

Changes made in this branch (no lint):

== lib/lp/bugs/javascript/bug_subscription_widget.js ==

 - I've updated the existing code to make it event driven, so that
   calling the initialize_subscription_wizard function sets up a bunch
   of event handlers and then calls the load_subscription_form method in
   order to get the ball rolling.

== lib/lp/bugs/javascript/tests/test_bug_subscription_widget.html ==

 - I've imported a few more bits of JS needed to make the tests work.

== lib/lp/bugs/javascript/tests/test_bug_subscription_widget.js ==

 - I've updated the tests to reflect the event-driven nature of the new
   code and I've added tests to cover the new event handling stuff.

[Deryck Hodge (deryck)]

This looks good. Thanks for the nice testing! I'm glad to see you working on the widget's base functionality like this first, and testing well, before moving on to implementation. Nicely done.

One minor tip, if you're curious about his:

I had to disable fetchCSS when Windmill was running to prevent YUI trying to go off-site for resources, so as you may have noticed, the logger is hard to read. If you apply this diff locally (setting fetchCSS to true) you can have an easier to read test:

=== modified file 'lib/lp/bugs/javascript/tests/test_bug_subscription_widget.js'
--- lib/lp/bugs/javascript/tests/test_bug_subscription_widget.js 2010-12-14 10:42:45 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_subscription_widget.js 2010-12-17 16:12:40 +0000
@@ -4,7 +4,7 @@
     base: '../../../../canonical/launchpad/icing/yui/',
     filter: 'raw',
     combine: false,
- fetchCSS: false
+ fetchCSS: true
     }).use(
         'event', 'event-simulate', 'lazr.testing.mockio',
         'lp.bugs.bug_subscription_wizard', 'node', 'test',

Of course, don't commit this until we can work out testing better. But locally it makes the test easier on the eyes.

Cheers,
deryck

review: Approve (code)

=== modified file 'lib/lp/bugs/javascript/bug_subscription_widget.js'
--- lib/lp/bugs/javascript/bug_subscription_widget.js	2010-12-03 12:39:40 +0000
+++ lib/lp/bugs/javascript/bug_subscription_widget.js	2010-12-14 10:54:13 +0000
@@ -4,11 +4,11 @@
  * Handling of the bug subscription form overlay widget.
  *
  * @module bugs
- * @submodule bug_subscription_widget
+ * @submodule bug_subscription_wizard
  */
-YUI.add('lp.bugs.bug_subscription_widget', function(Y) {
+YUI.add('lp.bugs.bug_subscription_wizard', function(Y) {
 
-var namespace = Y.namespace('lp.bugs.bug_subscription_widget');
+var namespace = Y.namespace('lp.bugs.bug_subscription_wizard');
 
 var submit_button_html =
     '<button type="submit" name="field.actions.subscribe" ' +
@@ -18,76 +18,112 @@
     '<button type="button" name="field.actions.cancel" ' +
     'class="lazr-neg lazr-btn" >Cancel</button>';
 
+namespace.subscription_wizard = null;
+namespace.subscribe_form_body = null;
+
 namespace.create_subscription_wizard = function() {
-    // Construct the form. This is a bit hackish but it saves us from
-    // having to try to get information from TAL into JavaScript and all
-    // the horror that entails.
-    var subscribe_form_body =
-        '<div>' +
-        '    <p>Tell me when</p>' +
-        '    <table>' +
-        '       <tr>' +
-        '         <td>' +
-        '           <input type="radio" name="field.bug_notification_level" ' +
-        '               id="bug-notification-level-comments"' +
-        '               value="Discussion"' +
-        '               class="bug-notification-level" />' +
-        '         </td>' +
-        '         <td>' +
-        '           <label for="bug-notification-level-comments">' +
-        '             A change is made or a comment is added to this bug' +
-        '           </label>' +
-        '         </td>' +
-        '     </tr>' +
-        '     <tr>' +
-        '       <td>' +
-        '         <input type="radio" name="field.bug_notification_level" ' +
-        '             id="bug-notification-level-metadata" value="Details"' +
-        '             class="bug-notification-level" />' +
-        '       </td>' +
-        '       <td>' +
-        '         <label for="bug-notification-level-metadata">' +
-        '             A change is made to the bug; do not notify me about ' +
-        '             new comments.' +
-        '         </label>' +
-        '       </td>' +
-        '     </tr>' +
-        '     <tr>' +
-        '       <td>' +
-        '         <input type="radio" name="field.bug_notification_level" ' +
-        '             id="bug-notification-level-lifecycle" value="Lifecycle"' +
-        '             class="bug-notification-level" />' +
-        '       </td>' +
-        '       <td>' +
-        '         <label for="bug-notification-level-lifecycle">' +
-        '           Changes are made to the bug\'s status.' +
-        '         </label>' +
-        '       </td>' +
-        '     </tr>' +
-        '  </table>' +
-        '</div>';
-
     // Create the do-you-want-to-subscribe FormOverlay.
     var wizard_steps = [
         new Y.lazr.wizard.Step({
-            form_content: subscribe_form_body,
+            form_content: namespace.subscribe_form_body,
             form_submit_button: Y.Node.create(submit_button_html),
             form_cancel_button: Y.Node.create(cancel_button_html),
             funcLoad: function() {},
             funcCleanUp: function() {}
             })
         ];
-    var subscribe_wizard = new Y.lazr.wizard.Wizard({
+    namespace.subscription_wizard = new Y.lazr.wizard.Wizard({
         headerContent: '<h2>Subscribe to this bug</h2>',
         centered: true,
         visible: false,
         steps: wizard_steps
     });
-    subscribe_wizard.render('#subscribe-wizard');
-
-    return subscribe_wizard;
+    namespace.subscription_wizard.render('#subscribe-wizard');
+    Y.fire('subscriptionwizard:ready');
+};
+
+/**
+ * Load the subscription form from a remote source.
+ *
+ * @method load_subscription_form
+ * @param {string} url the URL to load the form from.
+ * @param {Object} io_provider an Object providing a .io() method. This
+ *     will only be used for testing purposes; if io_provider is not
+ *     passed we'll just use Y.io for everything.
+ */
+namespace.load_subscription_form = function(url, io_provider) {
+    if (io_provider === undefined) {
+        io_provider = Y;
+    }
+    function on_success(id, response) {
+        namespace.subscribe_form_body = response.responseText;
+        Y.fire('subscriptionform:loaded');
+    }
+    function on_failure(id, response) {
+        namespace.subscribe_form_body =
+            "Sorry, an error occurred while loading the form.";
+        Y.fire('subscriptionform:loaded');
+    }
+    var cfg = {
+        on: {
+            success: on_success,
+            failure: on_failure
+            },
+        }
+    io_provider.io(url, cfg);
+};
+
+/**
+ * Initialize the subscription wizard and set up event handlers.
+ *
+ * @method initialize_subscription_wizard
+ * @param {Node} target_node The node to which to link the wizard.
+ * @param {string} url the URL to load the form from.
+ * @param {Object} io_provider an Object providing a .io() method.
+ *      This is only used for testing.
+ */
+namespace.initialize_subscription_wizard = function(
+    target_node, url, io_provider)
+{
+    namespace.set_up_event_handlers(target_node);
+    namespace.load_subscription_form(url, io_provider);
+};
+
+/**
+ * Set up the event handlers for the wizard.
+ *
+ * @method set_up_event_handlers
+ * @param {Node} target_node The node to which to link the wizard.
+ */
+    // Set up the event handlers.
+namespace.set_up_event_handlers = function(target_node) {
+    Y.on(
+        'subscriptionform:loaded',
+        Y.bind(function(e) {
+            // We only create the subscription wizard once the form
+            // contents have been loaded from their remote source.
+            namespace.create_subscription_wizard()
+        }
+    ));
+    Y.on(
+        'subscriptionwizard:ready',
+        Y.bind(function(e) {
+            // Once the wizard has loaded, bind the show_wizard method
+            // to the onClick handler for the target_node.
+            target_node.on('click', namespace.show_wizard);
+        }
+    ));
+};
+
+/**
+ * Show the wizard.
+ *
+ * @method show_wizard
+ */
+namespace.show_wizard = function() {
+    namespace.subscription_wizard.show()
 };
 
 }, "0.1", {"requires": [
     "base", "io", "oop", "node", "event", "lazr.formoverlay",
-    "lazr.effects", "lazr.wizard"]});
+    "log", "lazr.effects", "lazr.wizard"]});

=== modified file 'lib/lp/bugs/javascript/tests/test_bug_subscription_widget.html'
--- lib/lp/bugs/javascript/tests/test_bug_subscription_widget.html	2010-11-04 20:05:41 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_subscription_widget.html	2010-12-14 10:54:13 +0000
@@ -8,14 +8,23 @@
   <link rel="stylesheet" href="../../../../canonical/launchpad/icing/yui/cssreset/reset.css"/>
   <link rel="stylesheet" href="../../../../canonical/launchpad/icing/yui/cssfonts/fonts.css"/>
   <link rel="stylesheet" href="../../../../canonical/launchpad/icing/yui/cssbase/base.css"/>
+  <link rel="stylesheet"
+      href="../../../../canonical/launchpad/icing/lazr/build/testing/assets/testlogger.css"/>
 
   <!-- Dependency -->
   <script type="text/javascript"
     src="../../../../canonical/launchpad/icing/lazr/build/lazr.js"></script>
   <script type="text/javascript"
+    src="../../../../canonical/launchpad/icing/lazr/build/testing/testing.js"></script>
+  <script type="text/javascript"
+    src="../../../../canonical/launchpad/icing/lazr/build/testing/mockio.js"></script>
+  <script type="text/javascript"
     src="../../../../canonical/launchpad/icing/lazr/build/overlay/overlay.js">
   </script>
   <script type="text/javascript"
+    src="../../../../canonical/launchpad/icing/lazr/build/formoverlay/formoverlay.js">
+  </script>
+  <script type="text/javascript"
     src="../../../../canonical/launchpad/icing/lazr/build/choiceedit/choiceedit.js">
   </script>
   <script type="text/javascript"

=== modified file 'lib/lp/bugs/javascript/tests/test_bug_subscription_widget.js'
--- lib/lp/bugs/javascript/tests/test_bug_subscription_widget.js	2010-12-03 12:39:40 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_subscription_widget.js	2010-12-14 10:54:13 +0000
@@ -6,18 +6,75 @@
     combine: false,
     fetchCSS: false
     }).use(
-        'event', 'lp.bugs.bug_subscription_widget', 'node', 'test',
-        'widget-stack', 'console', function(Y) {
+        'event', 'event-simulate', 'lazr.testing.mockio',
+        'lp.bugs.bug_subscription_wizard', 'node', 'test',
+        'widget-stack', 'console', 'log', function(Y) {
 
 // Local aliases
 var Assert = Y.Assert,
     ArrayAssert = Y.ArrayAssert;
 
 var suite = new Y.Test.Suite("Bug subscription widget tests");
+var subscribe_node = Y.Node.create(
+    '<a href="#" id="subscription-widget-link">Click me</a>');
+
+var subscribe_form_body =
+    '<div>' +
+    '    <p>Tell me when</p>' +
+    '    <table>' +
+    '       <tr>' +
+    '         <td>' +
+    '           <input type="radio" ' +
+    '               name="field.bug_notification_level" ' +
+    '               id="bug-notification-level-comments"' +
+    '               value="Discussion"' +
+    '               class="bug-notification-level" />' +
+    '         </td>' +
+    '         <td>' +
+    '           <label for="bug-notification-level-comments">' +
+    '             A change is made or a comment is added to ' +
+    '             this bug' +
+    '           </label>' +
+    '         </td>' +
+    '     </tr>' +
+    '     <tr>' +
+    '       <td>' +
+    '         <input type="radio"' +
+    '             name="field.bug_notification_level" ' +
+    '             id="bug-notification-level-metadata"' +
+    '             value="Details"' +
+    '             class="bug-notification-level" />' +
+    '       </td>' +
+    '       <td>' +
+    '         <label for="bug-notification-level-metadata">' +
+    '             A change is made to the bug; do not notify ' +
+    '             me about new comments.' +
+    '         </label>' +
+    '       </td>' +
+    '     </tr>' +
+    '     <tr>' +
+    '       <td>' +
+    '         <input type="radio"' +
+    '             name="field.bug_notification_level" ' +
+    '             id="bug-notifiction-level-lifecycle"' +
+    '             value="Lifecycle"' +
+    '             class="bug-notification-level" />' +
+    '       </td>' +
+    '       <td>' +
+    '         <label for="bug-notification-level-lifecycle">' +
+    '           Changes are made to the bug\'s status.' +
+    '         </label>' +
+    '       </td>' +
+    '     </tr>' +
+    '  </table>' +
+    '</div>';
+var success_response = Y.lazr.testing.MockIo.makeXhrSuccessResponse(
+    subscribe_form_body);
+var mock_io = new Y.lazr.testing.MockIo();
 
 suite.add(new Y.Test.Case({
 
-    name: 'bug_subscription_widget_basics',
+    name: 'bug_subscription_wizard_basics',
 
     setUp: function() {
         // Monkeypatch LP.client to avoid network traffic and to make
@@ -37,15 +94,25 @@
         LP.client.cache.bug = {
             self_link: "http://bugs.example.com/bugs/1234"
         };
-        this.subscribe_wizard =
-            Y.lp.bugs.bug_subscription_widget.create_subscription_wizard();
-        this.subscribe_wizard.show();
+
+        // Monkeypatch the subscribe_form_body attribute of the
+        // wizard module so that the test doesn't cause the wizard to
+        // try to load a Launchpad page.
+        Y.lp.bugs.bug_subscription_wizard.subscribe_form_body =
+            subscribe_form_body;
+        Y.lp.bugs.bug_subscription_wizard.create_subscription_wizard();
+        this.subscription_wizard =
+            Y.lp.bugs.bug_subscription_wizard.subscription_wizard;
+    },
+
+    tearDown: function() {
+        this.subscription_wizard.destroy();
     },
 
     test_bug_notification_level_values: function() {
         // The bug_notification_level field will have a value that's one
         // of [Discussion, Details, Lifecycle].
-        var form_node = this.subscribe_wizard.form_node;
+        var form_node = this.subscription_wizard.form_node;
         var notification_level_radio_buttons = form_node.queryAll(
             'input[name=field.bug_notification_level]');
         Y.each(notification_level_radio_buttons, function(obj) {
@@ -55,9 +122,150 @@
                 value == 'Details' ||
                 value == 'Lifecycle');
         });
-    }
-
-}));
+    },
+
+    test_wizard_first_step_content: function() {
+        // The form content for the first wizard step will have been
+        // loaded using the _get_subscribe_form_content function defined
+        // above.
+        var steps = this.subscription_wizard.steps;
+        Assert.areEqual(
+            subscribe_form_body,
+            this.subscription_wizard.get('steps')[0].get('form_content'));
+    },
+
+    test_wizard_is_hidden_on_creation: function() {
+        // The wizard is hidden when it is created.
+        var bounding_box =
+            this.subscription_wizard.get('boundingBox');
+        Assert.isFalse(this.subscription_wizard.get('visible'));
+        Assert.isTrue(
+            bounding_box.hasClass('yui3-lazr-wizard-hidden'),
+            "The form is hidden after cancel is clicked.");
+    }
+}));
+
+
+suite.add(new Y.Test.Case({
+
+    name: 'bug_subscription_wizard_async',
+
+    setUp: function() {
+        // Monkeypatch LP.client to avoid network traffic and to make
+        // some things work as expected.
+        window.LP = {
+            client: {
+                links: {},
+                // Empty client constructor.
+                Launchpad: function() {},
+                cache: {}
+            }
+        };
+        LP.client.Launchpad.prototype.named_post =
+            function(url, func, config) {
+                config.on.success();
+            };
+        LP.client.cache.bug = {
+            self_link: "http://bugs.example.com/bugs/1234"
+        };
+
+        // Monkeypatch the subscribe_form_body attribute of the
+        // wizard module so that the test doesn't cause the wizard to
+        // try to load a Launchpad page.
+        Y.lp.bugs.bug_subscription_wizard.subscribe_form_body =
+            subscribe_form_body;
+    },
+
+    tearDown: function() {
+        Y.lp.bugs.bug_subscription_wizard.subscription_wizard.destroy();
+    },
+
+    test_create_subscription_wizard_fires_event: function() {
+        // The create_subscription_wizard function fires a
+        // subscriptionwizard:ready event when it has completed so that
+        // the subscription wizard can then be used in a page.
+        var subscriptionwizard_ready_fired = false;
+        Y.on(
+            'subscriptionwizard:ready',
+            Y.bind(function(e) { subscriptionwizard_ready_fired = true; }));
+        Y.lp.bugs.bug_subscription_wizard.create_subscription_wizard();
+        Assert.isTrue(subscriptionwizard_ready_fired);
+    },
+
+    test_load_subscription_form_fires_event: function() {
+        // The load_subscription_form() function fires a
+        // subscriptionform:loaded event.
+        var subscriptionform_loaded_fired = false;
+        Y.on(
+            'subscriptionform:loaded',
+            Y.bind(function(e) { subscriptionform_loaded_fired = true; }));
+        Y.lp.bugs.bug_subscription_wizard.load_subscription_form(
+            'http://example.com', mock_io);
+        mock_io.simulateXhr(success_response, false);
+        Assert.isTrue(subscriptionform_loaded_fired);
+    },
+
+    test_wizard_created_on_subscriptionform_loaded_event: function() {
+        // When the subscriptionform:loaded event is fired, the wizard
+        // will be created.
+        Y.lp.bugs.bug_subscription_wizard.set_up_event_handlers(
+            subscribe_node);
+        Y.fire('subscriptionform:loaded');
+        var subscription_wizard =
+            Y.lp.bugs.bug_subscription_wizard.subscription_wizard;
+        Assert.areEqual(
+            subscribe_form_body,
+            subscription_wizard.get('steps')[0].get('form_content'))
+    },
+
+    test_load_subscription_form_loads_subscription_form: function() {
+        // The load_subscription_form() function loads the body of the
+        // subscription form form a remote URL.
+        Y.lp.bugs.bug_subscription_wizard.set_up_event_handlers(
+            subscribe_node);
+        Y.lp.bugs.bug_subscription_wizard.load_subscription_form(
+            'http://example.com', mock_io);
+        mock_io.simulateXhr(success_response, false);
+
+        // The contents of the subscribe_form_body for the wizard will
+        // have been loaded.
+        var subscription_wizard =
+            Y.lp.bugs.bug_subscription_wizard.subscription_wizard;
+        Assert.areEqual(
+            subscribe_form_body,
+            subscription_wizard.get('steps')[0].get('form_content'));
+    },
+
+    test_initialize_subscription_wizard_links_everything: function() {
+        // The initialize_subscription_wizard function sets up event
+        // handlers and links the wizard, once created, to the node that
+        // we pass to it.
+        var subscriptionform_loaded_fired = false;
+        var subscriptionwizard_ready_fired = false;
+        Y.on(
+            'subscriptionform:loaded',
+            Y.bind(function(e) { subscriptionform_loaded_fired = true; }));
+        Y.on(
+            'subscriptionwizard:ready',
+            Y.bind(function(e) { subscriptionwizard_ready_fired = true; }));
+        Y.lp.bugs.bug_subscription_wizard.initialize_subscription_wizard(
+            subscribe_node, 'http://example.com', mock_io);
+        mock_io.simulateXhr(success_response, false);
+        Y.Event.simulate(Y.Node.getDOMNode(subscribe_node), 'click');
+
+        // All the events will have fired and the node will be linked to
+        // the wizard via its onClick handler.
+        var subscription_wizard =
+            Y.lp.bugs.bug_subscription_wizard.subscription_wizard;
+        Assert.isTrue(subscriptionform_loaded_fired);
+        Assert.isTrue(subscriptionwizard_ready_fired);
+        Assert.areEqual(
+            subscribe_form_body,
+            subscription_wizard.get('steps')[0].get('form_content'));
+        Assert.isTrue(subscription_wizard.get('visible'));
+    }
+}));
+
 
 var handle_complete = function(data) {
     status_node = Y.Node.create(