launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02159
lp:~gmb/launchpad/use-subscribe-form-for-fun-and-profit-bug-687747 into lp:launchpad
Graham Binns has proposed merging lp:~gmb/launchpad/use-subscribe-form-for-fun-and-profit-bug-687747 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#687747 The subscribe wizard should load its contents from $bug/+subscribe/++form++
https://bugs.launchpad.net/bugs/687747
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.
--
https://code.launchpad.net/~gmb/launchpad/use-subscribe-form-for-fun-and-profit-bug-687747/+merge/43629
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/use-subscribe-form-for-fun-and-profit-bug-687747 into lp:launchpad.
=== 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(
