← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~allenap/launchpad/dd-initseries-bug-727105-form-submission into lp:launchpad

 

Gavin Panella has proposed merging lp:~allenap/launchpad/dd-initseries-bug-727105-form-submission into lp:launchpad with lp:~allenap/launchpad/dd-initseries-bug-727105-packageset-picker-fixes as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/launchpad/dd-initseries-bug-727105-form-submission/+merge/55514

This branch completes work on +initseries, barring some polish which
we will add later on.

It does the following:

- Adds a stub submit button is rendered by the view. I could have done
  this in Javascript, but this is how it has worked out for now.

- Adds a new widget FormActionsWidget, to "progressively enhance" the
  form actions area.

- Adds another new widget DeriveDistroSeriesActionsWidget, a
  specialized FormActionsWidget specifically for calling
  deriveDistroSeries.

- Wires this new widget up to the other widgets in the page.

-- 
https://code.launchpad.net/~allenap/launchpad/dd-initseries-bug-727105-form-submission/+merge/55514
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/dd-initseries-bug-727105-form-submission into lp:launchpad.
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py	2011-03-30 11:14:44 +0000
+++ lib/lp/registry/browser/distroseries.py	2011-03-30 11:14:51 +0000
@@ -549,6 +549,10 @@
     label = 'Initialize series'
     page_title = label
 
+    @action(_(label), name='initialize')
+    def submit(self, action, data):
+        """Stub for the Javascript in the page to use."""
+
     @property
     def is_derived_series_feature_enabled(self):
         return getFeatureFlag("soyuz.derived-series-ui.enabled") is not None

=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py	2011-03-30 11:14:44 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-03-30 11:14:51 +0000
@@ -44,7 +44,7 @@
         with feature_flags():
             root = html.fromstring(view())
             self.assertEqual(
-                [], root.cssselect("#init-series-form-container"))
+                [], root.cssselect("#initseries-form-container"))
             # Instead an explanatory message is shown.
             [message] = root.cssselect("p.error.message")
             self.assertIn(
@@ -59,7 +59,7 @@
             set_feature_flag(u"soyuz.derived-series-ui.enabled", u"true")
             root = html.fromstring(view())
             self.assertNotEqual(
-                [], root.cssselect("#init-series-form-container"))
+                [], root.cssselect("#initseries-form-container"))
             # A different explanatory message is shown for clients that don't
             # process Javascript.
             [message] = root.cssselect("p.error.message")

=== modified file 'lib/lp/registry/javascript/distroseries.js'
--- lib/lp/registry/javascript/distroseries.js	2011-03-30 11:14:44 +0000
+++ lib/lp/registry/javascript/distroseries.js	2011-03-30 11:14:51 +0000
@@ -569,50 +569,223 @@
 
 
 /**
+ * A widget to encapsulate functionality around the form actions.
+ *
+ * @class FormActionsWidget
+ */
+var FormActionsWidget = function() {
+    FormActionsWidget
+        .superclass.constructor.apply(this, arguments);
+};
+
+Y.mix(FormActionsWidget, {
+
+    NAME: 'formActionsWidget',
+
+    HTML_PARSER: {
+        submitButtonNode: "input[type=submit]"
+    }
+
+});
+
+Y.extend(FormActionsWidget, Y.Widget, {
+
+    initializer: function(config) {
+        this.client = new Y.lp.client.Launchpad();
+        this.error_handler = new Y.lp.client.ErrorHandler();
+        this.error_handler.clearProgressUI = Y.bind(this.hideSpinner, this);
+        this.error_handler.showError = Y.bind(this.showError, this);
+        this.submitButtonNode = config.submitButtonNode;
+        this.spinnerNode = Y.Node.create(
+            '<img src="/@@/spinner" alt="Loading..." />');
+    },
+
+    /**
+     * Show the spinner, and hide the submit button.
+     *
+     * @method showSpinner
+     */
+    showSpinner: function() {
+        this.submitButtonNode.replace(this.spinnerNode);
+    },
+
+    /**
+     * Hide the spinner, and show the submit button again.
+     *
+     * @method hideSpinner
+     */
+    hideSpinner: function() {
+        this.spinnerNode.replace(this.submitButtonNode);
+    },
+
+    /**
+     * Display an error.
+     *
+     * @method showError
+     */
+    showError: function(error) {
+        Y.Node.create('<p class="error message" />')
+            .appendTo(this.get("contentBox"))
+            .set("text", error);
+    },
+
+    /**
+     * Remove all errors that have been previously displayed by showError.
+     *
+     * @method hideErrors
+     */
+    hideErrors: function(error) {
+        this.get("contentBox").all("p.error.message").remove();
+    }
+
+});
+
+namespace.FormActionsWidget = FormActionsWidget;
+
+
+/**
+ * A widget to encapsulate functionality around the form actions.
+ *
+ * @class DeriveDistroSeriesActionsWidget
+ */
+var DeriveDistroSeriesActionsWidget = function() {
+    DeriveDistroSeriesActionsWidget
+        .superclass.constructor.apply(this, arguments);
+};
+
+Y.mix(DeriveDistroSeriesActionsWidget, {
+
+    NAME: 'deriveDistroSeriesActionsWidget'
+
+});
+
+Y.extend(DeriveDistroSeriesActionsWidget, FormActionsWidget, {
+
+    initializer: function(config) {
+        this.context = config.context;
+        this.deriveFromChoice = config.deriveFromChoice;
+        this.architectureChoice = config.architectureChoice;
+        this.packagesetChoice = config.packagesetChoice;
+        this.packageCopyOptions = config.packageCopyOptions;
+    },
+
+    /**
+     * Display a success message then fade out and remove the form.
+     *
+     * @method success
+     */
+    success: function() {
+        var message = [
+            "The initialization of ", this.context.displayname,
+            " has been scheduled and should run shortly."
+        ].join("");
+        var messageNode = Y.Node.create("<p />")
+            .addClass("informational")
+            .addClass("message")
+            .set("text", message);
+        var form = this.get("contentBox").ancestor("form");
+        form.transition(
+            {duration: 1, height: 0, opacity: 0},
+            function() { form.remove(true); });
+        form.insert(messageNode, "after");
+    },
+
+    /**
+     * Call deriveDistroSeries via the API.
+     *
+     * @method submit
+     */
+    submit: function() {
+        var self = this;
+        var config = {
+            on: {
+                start: function() {
+                    self.hideErrors();
+                    self.showSpinner();
+                },
+                success: Y.bind(this.success, this),
+                failure: this.error_handler.getFailureHandler(),
+                end: Y.bind(this.hideSpinner, this)
+            },
+            parameters: {
+                name: this.context.name,
+                distribution: this.context.distribution_link,
+                architectures: this.architectureChoice.get("choice"),
+                packagesets: this.packagesetChoice.get("choice"),
+                rebuild: this.packageCopyOptions.get("choice") == (
+                    "Copy Source and Rebuild")
+            }
+        };
+        var parent = this.deriveFromChoice.get("value");
+        this.client.named_post(
+            parent, "deriveDistroSeries", config);
+    }
+
+});
+
+namespace.DeriveDistroSeriesActionsWidget = DeriveDistroSeriesActionsWidget;
+
+
+/**
  * Setup the widgets on the +initseries page.
  *
  * @function setup
  */
 namespace.setup = function() {
-    var form_container = Y.one("#init-series-form-container");
+    var form_container = Y.one("#initseries-form-container");
     var form_table_body = form_container.one("table.form > tbody");
-    var architecture_choice = new ArchitecturesChoiceListWidget()
-        .set("name", "field.architectures")
-        .set("label", "Architectures")
-        .set("description", (
-                 "Choose the architectures you want to " +
-                 "use from the parent series."))
-        .render(form_table_body);
-    var packageset_choice = new PackagesetPickerWidget()
-        .set("name", "field.packagesets")
-        .set("size", 5)
-        .set("multiple", true)
-        .set("label", "Package sets to copy from parent")
-        .set("description", (
-                 "The package sets that will be imported " +
-                 "into the derived distroseries."))
-        .render(form_table_body);
-    var package_copy_options = new ChoiceListWidget()
-        .set("name", "field.package_copy_options")
-        .set("type", "radio")
-        .set("label", "Copy options")
-        .set("description", (
-                 "Choose whether to rebuild all the sources you copy " +
-                 "from the parent, or to copy their binaries too."))
-        .set("choices", ["Copy Source and Rebuild",
-                         "Copy Source and Binaries"])
-        .set("choice", "Copy Source and Binaries")
-        .render(form_table_body);
+
+    // Pre-existing form fields.
     var derive_from_choice =
         form_table_body.one("[name=field.derived_from_series]");
 
+    // Widgets.
+    var architecture_choice =
+        new ArchitecturesChoiceListWidget()
+            .set("name", "field.architectures")
+            .set("label", "Architectures")
+            .set("description", (
+                     "Choose the architectures you want to " +
+                     "use from the parent series."))
+            .render(form_table_body);
+    var packageset_choice =
+        new PackagesetPickerWidget()
+            .set("name", "field.packagesets")
+            .set("size", 5)
+            .set("multiple", true)
+            .set("label", "Package sets to copy from parent")
+            .set("description", (
+                     "The package sets that will be imported " +
+                     "into the derived distroseries."))
+            .render(form_table_body);
+    var package_copy_options =
+        new ChoiceListWidget()
+            .set("name", "field.package_copy_options")
+            .set("type", "radio")
+            .set("label", "Copy options")
+            .set("description", (
+                     "Choose whether to rebuild all the sources you copy " +
+                     "from the parent, or to copy their binaries too."))
+            .set("choices", ["Copy Source and Rebuild",
+                             "Copy Source and Binaries"])
+            .set("choice", "Copy Source and Binaries")
+            .render(form_table_body);
+    var form_actions =
+        new DeriveDistroSeriesActionsWidget({
+            context: LP.cache.context,
+            srcNode: form_container.one("div.actions"),
+            deriveFromChoice: derive_from_choice,
+            architectureChoice: architecture_choice,
+            packagesetChoice: packageset_choice,
+            packageCopyOptions: package_copy_options
+        });
+
     // Wire up the distroseries select to the architectures widget.
     function update_architecture_choice() {
         architecture_choice
             .set("distroSeries", derive_from_choice.get("value"));
     }
     derive_from_choice.on("change", update_architecture_choice);
-
     // Update the architectures widget for the selected distroseries.
     update_architecture_choice();
 
@@ -622,14 +795,18 @@
             .set("distroSeries", derive_from_choice.get("value"));
     }
     derive_from_choice.on("change", update_packageset_choice);
-
     // Update the packagesets widget for the selected distroseries.
     update_packageset_choice();
 
+    // Wire up the form submission.
+    form_container.one("form").on(
+        "submit", function(event) {
+            event.halt(); form_actions.submit(); });
+
     // Show the form.
     form_container.removeClass("unseen");
 };
 
 
 }, "0.1", {"requires": ["node", "dom", "io", "widget", "lp.client",
-                        "lazr.anim", "array-extras"]});
+                        "lazr.anim", "array-extras", "transition"]});

=== modified file 'lib/lp/registry/javascript/tests/test_distroseries.js'
--- lib/lp/registry/javascript/tests/test_distroseries.js	2011-03-30 11:14:44 +0000
+++ lib/lp/registry/javascript/tests/test_distroseries.js	2011-03-30 11:14:51 +0000
@@ -36,7 +36,7 @@
         },
 
         tearDown: function() {
-            this.container.remove();
+            this.container.remove(true);
         },
 
         testRender: function() {
@@ -132,7 +132,7 @@
         },
 
         tearDown: function() {
-            this.container.remove();
+            this.container.remove(true);
         },
 
         testRenderChoices: function() {
@@ -246,7 +246,7 @@
         },
 
         tearDown: function() {
-            this.container.remove();
+            this.container.remove(true);
         },
 
         testSetDistroArchSeriesesUpdatesChoices: function() {
@@ -354,7 +354,7 @@
         },
 
         tearDown: function() {
-            this.container.remove();
+            this.container.remove(true);
         },
 
         testNameChange: function() {
@@ -548,7 +548,7 @@
         },
 
         tearDown: function() {
-            this.container.remove();
+            this.container.remove(true);
         },
 
         testSetPackageSetsUpdatesChoices: function() {
@@ -659,6 +659,168 @@
         testSelectWidget, testPackagesetPickerWidget);
     suite.add(new Y.Test.Case(testPackagesetPickerWidget));
 
+    var testFormActionsWidget = {
+        name: 'TestFormActionsWidget',
+
+        makeActionsDiv: function() {
+            var submit = Y.Node.create("<input />")
+                .set("type", "submit")
+                .set("value", "Initialize series");
+            var cancel = Y.Node.create("<a>Cancel</a>");
+            var div = Y.Node.create("<div />")
+                .addClass("actions")
+                .append(submit)
+                .append(cancel);
+            return div;
+        },
+
+        setUp: function() {
+            this.actions = this.makeActionsDiv();
+            this.widget = new initseries.FormActionsWidget(
+                {srcNode: this.actions});
+        },
+
+        tearDown: function() {
+            this.actions.remove(true);
+        },
+
+        testInitializer: function() {
+            Assert.isTrue(
+                this.actions.one("input").compareTo(
+                    this.widget.submitButtonNode));
+        },
+
+        testSpinner: function() {
+            Assert.isTrue(
+                this.actions.contains(this.widget.submitButtonNode));
+            Assert.isFalse(
+                this.actions.contains(this.widget.spinnerNode));
+            this.widget.showSpinner();
+            Assert.isFalse(
+                this.actions.contains(this.widget.submitButtonNode));
+            Assert.isTrue(
+                this.actions.contains(this.widget.spinnerNode));
+            this.widget.hideSpinner();
+            Assert.isTrue(
+                this.actions.contains(this.widget.submitButtonNode));
+            Assert.isFalse(
+                this.actions.contains(this.widget.spinnerNode));
+        },
+
+        testShowError: function() {
+            this.widget.showError("The Man From U.N.C.L.E.");
+            Assert.areEqual(
+                "The Man >From U.N.C.L.E.",
+                this.actions.one("p.error.message").get("text"));
+        },
+
+        testHideErrors: function() {
+            this.widget.showError("The Man From U.N.C.L.E.");
+            this.widget.showError("The Woman From A.U.N.T.I.E.");
+            this.widget.hideErrors();
+            Assert.isNull(this.actions.one("p.error.message"));
+        }
+
+    };
+
+    suite.add(new Y.Test.Case(testFormActionsWidget));
+
+    var testDeriveDistroSeriesActionsWidget = {
+        name: 'TestDeriveDistroSeriesActionsWidget',
+
+        setUp: function() {
+            this.actions = this.makeActionsDiv();
+            this.widget = new initseries.DeriveDistroSeriesActionsWidget({
+                srcNode: this.actions,
+                context: {
+                    name: "hagfish",
+                    displayname: "Horrid Hagfish",
+                    distribution_link: "http://example.com/deribuntu/snaggle";
+                },
+                deriveFromChoice: {
+                    get: function(name) {
+                        Assert.areEqual("value", name);
+                        return "ubuntu/hoary";
+                    }
+                },
+                architectureChoice: {
+                    get: function(name) {
+                        Assert.areEqual("choice", name);
+                        return ["i386", "sparc"];
+                    }
+                },
+                packagesetChoice: {
+                    get: function(name) {
+                        Assert.areEqual("choice", name);
+                        return ["foo", "bar"];
+                    }
+                },
+                packageCopyOptions: {
+                    get: function(name) {
+                        Assert.areEqual("choice", name);
+                        return "Copy Source and Rebuild";
+                    }
+                }
+            });
+            this.form = Y.Node.create("<form />");
+            this.form.append(this.actions);
+            this.container = Y.Node.create("<div />");
+            this.container.append(this.form);
+            this.body = Y.one("body");
+            this.body.append(this.container);
+        },
+
+        tearDown: function() {
+            this.container.remove(true);
+        },
+
+        testSuccess: function() {
+            Assert.isTrue(this.container.contains(this.form));
+            Assert.isNull(this.body.one("p.informational.message"));
+            this.widget.success();
+            Assert.areEqual(
+                ("The initialization of Horrid Hagfish " +
+                 "has been scheduled and should run shortly."),
+                this.body.one("p.informational.message").get("text"));
+            // The form is slowly evaporated.
+            this.wait(function() {
+                Assert.isFalse(
+                    this.container.contains(this.form));
+            }, 1100);
+        },
+
+        testSubmit: function() {
+            var io = false;
+            this.widget.client = {
+                named_post: function(path, operation, config) {
+                    io = true;
+                    Assert.areEqual("ubuntu/hoary", path);
+                    Assert.areEqual("deriveDistroSeries", operation);
+                    Assert.areEqual(
+                        "http://example.com/deribuntu/snaggle";,
+                        config.parameters.distribution);
+                    ArrayAssert.itemsAreEqual(
+                        ["i386", "sparc"],
+                        config.parameters.architectures);
+                    ArrayAssert.itemsAreEqual(
+                        ["foo", "bar"],
+                        config.parameters.packagesets);
+                    Assert.isTrue(config.parameters.rebuild);
+                    Assert.isObject(config.on);
+                    Assert.isFunction(config.on.success);
+                    Assert.isFunction(config.on.failure);
+                }
+            };
+            this.widget.submit();
+            Assert.isTrue(io, "No IO initiated.");
+        }
+
+    };
+
+    testDeriveDistroSeriesActionsWidget = Y.merge(
+        testFormActionsWidget, testDeriveDistroSeriesActionsWidget);
+    suite.add(new Y.Test.Case(testDeriveDistroSeriesActionsWidget));
+
     Y.Test.Runner.add(suite);
 
     Y.on('domready', function() {

=== modified file 'lib/lp/registry/templates/distroseries-initialize.pt'
--- lib/lp/registry/templates/distroseries-initialize.pt	2011-03-30 11:14:44 +0000
+++ lib/lp/registry/templates/distroseries-initialize.pt	2011-03-30 11:14:51 +0000
@@ -22,7 +22,7 @@
         please use the <code>deriveDistroSeries</code> API call via the
         web service.
       </p>
-      <div class="unseen" id="init-series-form-container">
+      <div class="unseen" id="initseries-form-container">
         <metal:form use-macro="context/@@launchpad_form/form" />
       </div>
       <script type="text/javascript">


Follow ups