Merge lp:~wallyworld/launchpad/recipe-request-builds-existing into lp:launchpad

[Ian Booth]

[r=thumper][bug=728789] Fix the mechanism used by the recipe request builds popup form to determine the currently pending builds for the recipe.

Fix the mechanism used by the recipe request builds popup form to determine the currently pending builds for the recipe. The currently pending builds are required so that the corresponding distro series checkboxes on the popup form can be disabled, preventing the user from asking for an already pending build.

== Implementation ==

The original implementation was broken - there was no web service api available to do the job and a crap implementation was done. Now, the pending_builds property on a recipe has been exported to the web service. This branch uses that api.

The web service is invoked as follows:

lp_client.get(LP.cache.context.pending_builds_collection_link, y_config);

The result is a list of build entries. We are interested in the build's "distro_series" and "archive" properties (which have been exported to the web service):
  build_record.get("distro_series_link")
  build_record.get("archive_link")

By way of example, the distro_series_link resolves to something like: "https://code.launchpad.dev/api/devel/ubuntu/warty"
We need to use the information we now have to figure out what checkboxes on the request build popup to disable. The checkboxes are labelled with the distro series name so that's what we need to match on. Strictly speaking, we would need to make yet another back end call to get the name of the distro series defined by the distro series link. However, this merge proposal uses the fact that the distro_series_link value ends with a segment corresponding to the distro series name. So we avoid having to make another back end call.

The same approach is used for the archive - the archive link value ends with a path segment which is sufficient to define which archive has been chosen from archive combo box.

I need advice as to whether the above approach is considered kosher. The alternative is to make 2 additional back end calls to fetch the distro series and archive names. However, as stated, the link values contain sufficient information so that these additional back end calls can be avoided.

In addition to the aboce changes, the invocation of the disable_existing_builds() function is now implemented using a Y.after() call so that it happens when the form is finished rendering.

== Tests==

Use the existing code.windmill.tests.test_recipe_request_build.py tests.

[Tim Penhey (thumper)]

Code looks reasonable.

review: Approve

=== modified file 'lib/lp/code/javascript/requestbuild_overlay.js'
--- lib/lp/code/javascript/requestbuild_overlay.js	2011-02-28 22:49:50 +0000
+++ lib/lp/code/javascript/requestbuild_overlay.js	2011-03-07 10:23:58 +0000
@@ -80,6 +80,9 @@
                 form_submit_callback: do_request_builds,
                 visible: false
             });
+            Y.after(function() {
+                disable_existing_builds(request_build_submit_button);
+            }, request_build_overlay, "bindUI");
             request_build_overlay.render();
             request_build_submit_button =
                     Y.Node.one("[name='field.actions.request']");
@@ -93,7 +96,6 @@
         request_build_overlay.form_node.set("innerHTML", loading_spinner);
         request_build_overlay.loadFormContentAndRender('+builds/++form++');
         request_build_submit_button.show();
-        disable_existing_builds(request_build_submit_button);
         request_build_overlay.show();
     });
 
@@ -410,13 +412,13 @@
     Y.Array.each(last_known_pending_builds, function(distroarchive) {
         var archive_parts = ppa_value.split("/");
         var archive = archive_parts.pop();
-        if( archive != distroarchive[1] )
+        if( archive.toLowerCase() != distroarchive[1].toLowerCase() )
             return;
 
         for (var i=0; i<distroseries_nodes.size(); i++) {
             var distroseries_node = distroseries_nodes.item(i);
             var distro = distroseries_node.get("text").trim();
-            if (distro == distroarchive[0] ) {
+            if (distro.toLowerCase() == distroarchive[0].toLowerCase() ) {
                 nr_matches += 1;
                 var disabled_checkbox_html = Y.Lang.substitute(
                     DISABLED_DISTROSERIES_CHECKBOX_HTML, {distro: distro});
@@ -438,19 +440,15 @@
     // so that we can attempt to prevent the user from requesting builds
     // which are already pending. It's not foolproof since a build may finish
     // or be initiated after this initial lookup, but we do handle that
-    // situation in the error handling. We also have to do this work by
-    // parsing html but it's the best we have for now.
-
-    var base_url = LP.cache.context.web_link;
-    var submit_url = base_url+"/+builds";
+    // situation in the error handling.
 
     distroseries_node_html = new Array();
     last_known_pending_builds = new Array();
     var y_config = {
-        headers: {'Accept': 'application/json; application/xhtml'},
+        headers: {'Accept': 'application/json;'},
         on: {
             success:
-                function(id, response) {
+                function(build_info) {
                     // We save the inner html of each distro series checkbox
                     // so we can restore it when required.
                     var distro_nodes = get_distroseries_nodes();
@@ -459,22 +457,29 @@
                                 distro_node.get("innerHTML"));
                     });
 
-                    // We parse the build view html to extract the pending
-                    // build info. Yuck. But there's no other api call to use
-                    // at the moment.
-                    var builds = Y.Node.create(response.responseText);
-                    var build_rows = builds.all('tr.package-build');
-                    build_rows.each(function(build_row) {
-                        var distro = build_row.one(":nth-child(3) a");
-                        var archive = build_row.one(":nth-child(4) a");
-                        var href = archive.get("href");
-                        var href_parts = href.split("/");
+                    //We have a json collection of the pending build records.
+                    var size = build_info.total_size;
+                    for( var i=0; i<size; i++ ) {
+                        var build_record = build_info.entries[i];
+                        var distro_link = build_record.get(
+                                                "distro_series_link");
+                        var archive_link = build_record.get("archive_link");
 
+                        // The very last segment of the distro series and
+                        // archive links will be the bits we want to match on.
+                        // eg for distro series, the link will be like:
+                        // https://code.launchpad.dev/api/devel/ubuntu/warty
+                        // and "warty" is what we match on.
+                        var distro_parts = distro_link.split("/");
+                        var archive_parts = archive_link.split("/");
                         last_known_pending_builds.push(
-                                [distro.get("text"), href_parts.pop()])
-                    });
+                                [distro_parts.pop(), archive_parts.pop()])
+                    }
                     var ppa_selector = request_build_overlay.form_node.one(
                             "[name='field.archive']");
+                    if (ppa_selector == null) {
+                        return;
+                    }
                     ppa_selector.on("change", function(e) {
                         ppa_changed(ppa_selector.get("value"), submit_button);
                     });
@@ -482,7 +487,8 @@
                 }
         }
     };
-    Y.io(submit_url, y_config);
+    set_up_lp_client();
+    lp_client.get(LP.cache.context.pending_builds_collection_link, y_config);
 }
 }, "0.1", {"requires": [
     "dom", "node", "io-base", "lazr.anim", "lazr.formoverlay", "lp.client"