Merge lp:~wallyworld/launchpad/request-build-popup into lp:launchpad

[Ian Booth]

[r=thumper][ui=sinzui][bug=673519] When requesting recipe builds, use a popup form if Javascript is enabled instead of an html form which requires extra page loads.

This branch introduces a popup form to allow the user to initiate recipe builds. Previously, a separate page load was used to display a standard html form.

== Implementation ==

The existing SourcePackageRecipeRequestBuildsView was used as a base class and new subclasses introduced:
- SourcePackageRecipeRequestBuildsHtmlView
- SourcePackageRecipeRequestBuildsAjaxView

The base class defines the form schema and provides the validation functionality. The individual subclasses provide slightly different implementations of the submit action, error handling and data rendering.
The html form behaves as previously. It renders the form page and relies on the underlying Launchpad form error handling to display field errors etc.

The ajax form works slightly differently. It renders just the recipe builds table (via a new +builds Zope URL). So when build(s) are requested and everything goes ok, a new version of the builds table is returned and the Javascrip ajax handler re-renders that part of the recipe page, flashing the rows of new builds. If there are errors, the ajax form returns a json data object containing the errors and sets the request status code to 400. This allows the client side Javascript to parse the json data and display the errors on the popup form. At the moment, only a single form wide error message is used, even though the json data contains field level errors. This is a limitation of the lazr.js FormOverlay implementation.

On the client side, the popup form acts modally. Client side validation is done and errors displayed on the form without a back end request - this is used to check that at least on distro series has been selected. Once the submit button is presses, the "please wait" spinner appears on the popup form. If the request went ok, the form disappears the the recipe page updated. If there are errors, the form stays visible and the errors are displayed on the form. This allows the user to correct any issues and try again. The most common case would be a user requesting an identical build to one already pending. They just need to untick that distro series and try again.

A driveby change was to make the pocket parameter of the requestBuild() method on SourcePackageRecipe default to PackagePublishingPocket.RELEASE

== Demo ==

See screenshot: http://people.canonical.com/~ianb/request-build-error.png

The screenshot shows how an error is displayed, but also gives a good indication of what the form looks like. The form is standard lazr.js functionality.

== QA ==

Simply use the form to initiate some recipe builds and ensure the overall functionality is equivalent to that of the html form.

== Tests ==

A new Windmill test was written to check the form can initiate a build and display errors as required.
bin/test -vvt test_recipe_request_build

== Lint ==

Linting changed files:
  lib/lp/code/browser/configure.zcml
  lib/lp/code/browser/sourcepackagerecipe.py
  lib/lp/code/javascript/requestbuild_overlay.js
  lib/lp/code/model/sourcepackagerecipe.py
  lib/lp/code/templates/sourcepackagerecipe-builds.pt
  lib/lp/code/templates/sourcepackagerecipe-index.pt
  lib/lp/code/windmill/tests/test_recipe_request_build.py

./lib/lp/code/templates/sourcepackagerecipe-builds.pt
      13: unbound prefix

[Curtis Hovey (sinzui)]

The check and cross button look wrong. I do not know of any other "popups" or "overlays" that use them. I only see them used when editing inline content. I do not think they mean submit or cancel. I think users believe they mean done editing or cancel editing.

I expect to see a button [Request Build] to submit the form. Overlay forms use a [Cancel] button, but I think that contradicts the _Cancel_ rule for page forms. Most popups (gosh, maybe all) place the cancel (x) icon in the top right corner. There is a rule (maybe in code) that when the (x) cancel icon is in the right corner the popup will be dismissed when the user clicks outside the popup. Overlays remain on visible when he user clicks outside. I think the unfocus rule will help us decide the buttons.

If the popup is dismissed when it loses focus, place the (x) cancel icon in the right corner. If it should remain, we use a [Cancel] button. The submit button should be [Request Build].

/me is very aware that the NE cancel icon contradicts his unity desktop conventions

review: Needs Fixing (ui)

[Ian Booth (wallyworld)]

> The check and cross button look wrong. I do not know of any other "popups" or
> "overlays" that use them. I only see them used when editing inline content. I
> do not think they mean submit or cancel. I think users believe they mean done
> editing or cancel editing.
>

They are used on a few other popup forms, mainly in the bugs area. eg Mark a Bug Report as Duplicate etc

> I expect to see a button [Request Build] to submit the form. Overlay forms use
> a [Cancel] button, but I think that contradicts the _Cancel_ rule for page
> forms. Most popups (gosh, maybe all) place the cancel (x) icon in the top
> right corner. There is a rule (maybe in code) that when the (x) cancel icon is
> in the right corner the popup will be dismissed when the user clicks outside
> the popup. Overlays remain on visible when he user clicks outside. I think the
> unfocus rule will help us decide the buttons.
>
> If the popup is dismissed when it loses focus, place the (x) cancel icon in
> the right corner. If it should remain, we use a [Cancel] button. The submit
> button should be [Request Build].
>
> /me is very aware that the NE cancel icon contradicts his unity desktop
> conventions

There's 2 lazr widgets at play here - PrettyOverlay which provides the behaviour where a popup has the close button in the top right corner and is dismissed if the user clicks outside. Then there's FormOverlay which extends PrettyOverlay and wires in form submission behaviour and doesn't wire in the "dismiss when click outside" behaviour since it allows for a proper cancel button as such (ie as used by the bug forms mentioned earlier).

I've stuck with using FormOverlay since it really is a modal form that is required here. The user clicks Request Build and they see a spinner and when the submission has been processed, the form disappears. On the other hand, if there's an error processing the form stays and an error is shown. They can then choose to correct the error and try again or click cancel to dismiss the form. What I did do though is replace the check/cross buttons with standard "Request Builds" and "Cancel" buttons.

http://people.canonical.com/~ianb/request-build-error1.png

What do you think?

[Curtis Hovey (sinzui)]

Hi Ian.

Thank you for taking the time to resolve the UI issues. I think this looks good to land.

review: Approve (ui)

[Tim Penhey (thumper)]

Remove Y.log when loading the module.
Add some comments in the form processing as discussed.
Change SLEEP to FOR_ELEMENT.

Good with these changes.

review: Approve

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2011-01-18 21:46:21 +0000
+++ lib/lp/code/browser/configure.zcml	2011-02-21 23:04:01 +0000
@@ -1228,10 +1228,17 @@
         <browser:page
             for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
             layer="lp.code.publisher.CodeLayer"
-            class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeRequestBuildsView"
+            class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeRequestBuildsHtmlView"
             name="+request-builds"
             template="../templates/sourcepackagerecipe-request-builds.pt"
             permission="launchpad.AnyPerson"/>
+        <browser:page
+            for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
+            layer="lp.code.publisher.CodeLayer"
+            class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeRequestBuildsAjaxView"
+            name="+builds"
+            template="../templates/sourcepackagerecipe-builds.pt"
+            permission="launchpad.View"/>
     </facet>
     <facet facet="branches">
         <browser:defaultView

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2011-02-18 02:39:47 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2011-02-21 23:04:01 +0000
@@ -14,6 +14,8 @@
     'SourcePackageRecipeView',
     ]
 
+import simplejson
+
 from bzrlib.plugins.builder.recipe import (
     ForbiddenInstructionError,
     RecipeParseError,
@@ -93,7 +95,6 @@
     MINIMAL_RECIPE_TEXT,
     RECIPE_BETA_FLAG,
     )
-from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 from lp.soyuz.model.archive import Archive
@@ -216,21 +217,7 @@
 
     @property
     def builds(self):
-        """A list of interesting builds.
-
-        All pending builds are shown, as well as 1-5 recent builds.
-        Recent builds are ordered by date finished (if completed) or
-        date_started (if date finished is not set due to an error building or
-        other circumstance which resulted in the build not being completed).
-        This allows started but unfinished builds to show up in the view but
-        be discarded as more recent builds become available.
-        """
-        builds = list(self.context.getPendingBuilds())
-        for build in self.context.getBuilds():
-            builds.append(build)
-            if len(builds) >= 5:
-                break
-        return builds
+        return builds_for_recipe(self.context)
 
     def dailyBuildWithoutUploadPermission(self):
         """Returns true if there are upload permissions to the daily archive.
@@ -288,6 +275,24 @@
             self.context, description, title="")
 
 
+def builds_for_recipe(recipe):
+        """A list of interesting builds.
+
+        All pending builds are shown, as well as 1-5 recent builds.
+        Recent builds are ordered by date finished (if completed) or
+        date_started (if date finished is not set due to an error building or
+        other circumstance which resulted in the build not being completed).
+        This allows started but unfinished builds to show up in the view but
+        be discarded as more recent builds become available.
+        """
+        builds = list(recipe.getPendingBuilds())
+        for build in recipe.getBuilds():
+            builds.append(build)
+            if len(builds) >= 5:
+                break
+        return builds
+
+
 class SourcePackageRecipeRequestBuildsView(LaunchpadFormView):
     """A view for requesting builds of a SourcePackageRecipe."""
 
@@ -305,24 +310,20 @@
 
     class schema(Interface):
         """Schema for requesting a build."""
+        archive = Choice(vocabulary='TargetPPAs', title=u'Archive')
         distros = List(
             Choice(vocabulary='BuildableDistroSeries'),
             title=u'Distribution series')
-        archive = Choice(vocabulary='TargetPPAs', title=u'Archive')
 
     custom_widget('distros', LabeledMultiCheckBoxWidget)
 
-    @property
-    def title(self):
-        return 'Request builds for %s' % self.context.name
-
-    label = title
-
-    @property
-    def cancel_url(self):
-        return canonical_url(self.context)
-
     def validate(self, data):
+        distros = data.get('distros', [])
+        if not len(distros):
+            self.setFieldError('distros',
+                "You need to specify at least one distro series for which "
+                "to build.")
+            return
         over_quota_distroseries = []
         for distroseries in data['distros']:
             if self.context.isOverQuota(self.user, distroseries):
@@ -333,23 +334,89 @@
                 "You have exceeded today's quota for %s." %
                 ', '.join(over_quota_distroseries))
 
-    @action('Request builds', name='request')
-    def request_action(self, action, data):
-        """User action for requesting a number of builds."""
+    def requestBuild(self, data):
+        """User action for requesting a number of builds.
+
+        We raise exceptions for most errors but if there's already a pending
+        build for a particular distroseries, we simply record that so that
+        other builds can ne queued and a message be displayed to the caller.
+        """
+        errors = {}
         for distroseries in data['distros']:
             try:
                 self.context.requestBuild(
-                    data['archive'], self.user, distroseries,
-                    PackagePublishingPocket.RELEASE, manual=True)
+                    data['archive'], self.user, distroseries, manual=True)
             except BuildAlreadyPending, e:
-                self.setFieldError(
-                    'distros',
-                    'An identical build is already pending for %s.' %
-                    e.distroseries)
-                return
+                errors['distros'] = ("An identical build is already pending "
+                    "for %s." % e.distroseries)
+        return errors
+
+
+class SourcePackageRecipeRequestBuildsHtmlView(
+        SourcePackageRecipeRequestBuildsView):
+    """Supports HTML form recipe build requests."""
+
+    @property
+    def title(self):
+        return 'Request builds for %s' % self.context.name
+
+    label = title
+
+    @property
+    def cancel_url(self):
+        return canonical_url(self.context)
+
+    @action('Request builds', name='request')
+    def request_action(self, action, data):
+        errors = self.requestBuild(data)
+        if errors:
+            [self.setFieldError(field, message)
+                for (field, message) in errors.items()]
+            return
         self.next_url = self.cancel_url
 
 
+class SourcePackageRecipeRequestBuildsAjaxView(
+        SourcePackageRecipeRequestBuildsView):
+    """Supports AJAX form recipe build requests."""
+
+    def _process_error(self, data, errors, reason):
+        """Set up the response and json data to return to the caller."""
+        self.request.response.setStatus(400, reason)
+        self.request.response.setHeader('Content-type', 'application/json')
+        return simplejson.dumps(errors)
+
+    def failure(self, action, data, errors):
+        """Called by the form if validate() finds any errors.
+
+           We simply convert the errors to json and return that data to the
+           caller for display to the user.
+        """
+        return self._process_error(data, self.widget_errors, "Validation")
+
+    @action('Request builds', name='request', failure=failure)
+    def request_action(self, action, data):
+        """User action for requesting a number of builds.
+
+        The failure handler will handle any validation errors. We still need
+        to handle errors which may occur when invoking the business logic.
+        These "expected" errors are ones which result in a predefined message
+        being displayed to the user. If the business method raises an
+        unexpected exception, that will be handled using the form's standard
+        exception processing mechanism (using response code 500).
+        """
+        errors = self.requestBuild(data)
+        # If there are errors we return a json data snippet containing the
+        # errors instead of rendering the form. These errors are processed
+        # by the caller's response handler and displayed to the user.
+        if errors:
+            return self._process_error(data, errors, "Request Build")
+
+    @property
+    def builds(self):
+        return builds_for_recipe(self.context)
+
+
 class ISourcePackageEditSchema(Interface):
     """Schema for adding or editing a recipe."""
 

=== added file 'lib/lp/code/javascript/requestbuild_overlay.js'
--- lib/lp/code/javascript/requestbuild_overlay.js	1970-01-01 00:00:00 +0000
+++ lib/lp/code/javascript/requestbuild_overlay.js	2011-02-21 23:04:01 +0000
@@ -0,0 +1,214 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * A form overlay that can request builds for a recipe..
+ *
+ * @namespace Y.lp.code.recipebuild_overlay
+ * @requires  dom, node, io-base, lazr.anim, lazr.formoverlay
+ */
+YUI.add('lp.code.requestbuild_overlay', function(Y) {
+
+var namespace = Y.namespace('lp.code.requestbuild_overlay');
+
+var lp_client;
+var request_build_overlay = null;
+var response_handler;
+
+function set_up_lp_client() {
+    if (lp_client === undefined) {
+        lp_client = new LP.client.Launchpad();
+    }
+}
+
+// This handler is used to process the results of form submission or other
+// such operation (eg get, post). It provides some boiler plate and allows the
+// developer to specify onSuccess and onError hooks. It is quite generic and
+// perhaps could be moved to an infrastructure class.
+
+RequestResponseHandler = function () {};
+RequestResponseHandler.prototype = {
+    clearProgressUI: function () {},
+    showError: function (error_msg) {},
+    getErrorHandler: function (errorCallback) {
+        var self = this;
+        return function (id, response) {
+            self.clearProgressUI();
+            // If it was a timeout...
+            if (response.status == 503) {
+                self.showError(
+                    'Timeout error, please try again in a few minutes.');
+            } else {
+                if (errorCallback != null) {
+                    errorCallback(self, id, response);
+                } else {
+                    self.showError(response.responseText);
+                }
+            }
+        };
+    },
+    getSuccessHandler: function (successCallback) {
+        var self = this;
+        return function (id, response) {
+            self.clearProgressUI();
+            successCallback(self, id, response);
+        };
+    }
+};
+
+namespace.connect_requestbuild = function() {
+
+    var request_build_handle = Y.one('#request-builds');
+    request_build_handle.addClass('js-action');
+    request_build_handle.on('click', function(e) {
+        e.preventDefault();
+        if (request_build_overlay == null) {
+            // Render the form and load the widgets to display
+            var recipe_name = LP.client.cache.context['name'];
+            request_build_overlay = new Y.lazr.FormOverlay({
+                headerContent: '<h2>Request builds for '
+                                    + recipe_name + ' </h2>',
+                form_submit_button: Y.Node.create(
+                    '<button type="submit" name="field.actions.request" ' +
+                    'value="Request builds">Request Builds</button>'),
+                form_cancel_button: Y.Node.create(
+                    '<button type="button" name="field.actions.cancel" ' +
+                    '>Cancel</button>'),
+                centered: true,
+                form_submit_callback: do_request_builds,
+                visible: false
+            });
+            request_build_overlay.render();
+        }
+        request_build_overlay.clearError();
+        var temp_spinner = [
+            '<div id="temp-spinner">',
+            '<img src="/@@/spinner"/>Loading...',
+            '</div>'].join('');
+        request_build_overlay.form_node.set("innerHTML", temp_spinner);
+        request_build_overlay.loadFormContentAndRender('+builds/++form++');
+        request_build_overlay.show();
+    });
+
+    // Wire up the processing hooks
+    response_handler = new RequestResponseHandler();
+    response_handler.clearProgressUI = function() {
+        destroy_temporary_spinner();
+    };
+    response_handler.showError = function(error) {
+        request_build_overlay.showError(error);
+        Y.log(error);
+    };
+};
+
+/*
+ * A function to return the current build records as displayed on the page
+ */
+function harvest_current_build_records() {
+    var row_classes = ['package-build', 'binary-build'];
+    var builds = new Array();
+    Y.Array.each(row_classes, function(row_class) {
+        Y.all('.'+row_class).each(function(row) {
+            var row_id = row.getAttribute('id');
+            if (builds.indexOf(row_id)<0) {
+                builds.push(row_id);
+            }
+        });
+    });
+    return builds;
+}
+
+/*
+ * Perform any client side validation
+ * Return: true if data is valid
+ */
+function validate(data) {
+    var distros = data['field.distros']
+    if (Y.Object.size(distros) == 0) {
+        response_handler.showError("You need to specify at least one " +
+                "distro series for which to build.");
+        return false;
+    }
+    return true;
+}
+
+/*
+ * The form submit function
+ */
+function do_request_builds(data) {
+    if (!validate(data))
+        return;
+    create_temporary_spinner();
+    var base_url = LP.client.cache.context.web_link;
+    var submit_url = base_url+"/+builds";
+    var current_builds = harvest_current_build_records();
+    var y_config = {
+        method: "POST",
+        headers: {'Accept': 'application/json; application/xhtml'},
+        on: {
+            failure: response_handler.getErrorHandler(
+                function(handler, id, response) {
+                    if( response.status >= 500 ) {
+                        // There's some error content we need to display.
+                        request_build_overlay.set(
+                                'form_content', response.responseText);
+                        request_build_overlay.get("form_submit_button")
+                                .setStyle('display', 'none');
+                        request_build_overlay.renderUI();
+                        //We want to force the form to be re-created
+                        request_build_overlay = null;
+                        return;
+                    }
+                    var error_info = Y.JSON.parse(response.responseText)
+                    var errors = [];
+                    for (var field_name in error_info)
+                        errors.push(error_info[field_name]);
+                    handler.showError(errors);
+                }),
+            success: response_handler.getSuccessHandler(
+                function(handler, id, response) {
+                    request_build_overlay.hide();
+                    var target = Y.one('#builds-target');
+                    target.set('innerHTML', response.responseText);
+                    var new_builds = harvest_current_build_records();
+                    Y.Array.each(new_builds, function(row_id) {
+                        if (current_builds.indexOf(row_id)>=0)
+                            return;
+                        var row = Y.one('#'+row_id);
+                        var anim = Y.lazr.anim.green_flash({node: row});
+                        anim.run();
+                    });
+                })
+        },
+        form: {
+            id: request_build_overlay.form_node,
+            useDisabled: true
+        }
+    };
+    Y.io(submit_url, y_config);
+}
+
+/*
+ * Show the temporary "Requesting..." text
+ */
+function create_temporary_spinner() {
+    // Add the temp "Requesting build..." text
+    var temp_spinner = Y.Node.create([
+        '<div id="temp-spinner">',
+        '<img src="/@@/spinner"/>Requesting build...',
+        '</div>'].join(''));
+    var request_build_handle = Y.one('.yui3-lazr-formoverlay-actions');
+    request_build_handle.insert(temp_spinner, request_build_handle);
+}
+
+/*
+ * Destroy the temporary "Requesting..." text
+ */
+function destroy_temporary_spinner() {
+    var temp_spinner = Y.one('#temp-spinner');
+    var spinner_parent = temp_spinner.get('parentNode');
+    spinner_parent.removeChild(temp_spinner);
+}
+
+}, "0.1", {"requires": [
+    "dom", "node", "io-base", "lazr.anim", "lazr.formoverlay"
+    ]});

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2011-02-18 02:39:47 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2011-02-21 23:04:01 +0000
@@ -70,6 +70,7 @@
 from lp.code.model.sourcepackagerecipedata import SourcePackageRecipeData
 from lp.registry.interfaces.distroseries import IDistroSeriesSet
 from lp.registry.model.distroseries import DistroSeries
+from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.database.stormexpr import Greatest
 from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.model.archive import Archive
@@ -244,7 +245,8 @@
         return SourcePackageRecipeBuild.getRecentBuilds(
             requester, self, distroseries).count() >= 5
 
-    def requestBuild(self, archive, requester, distroseries, pocket,
+    def requestBuild(self, archive, requester, distroseries,
+                     pocket=PackagePublishingPocket.RELEASE,
                      manual=False):
         """See `ISourcePackageRecipe`."""
         if not archive.is_ppa:

=== added file 'lib/lp/code/templates/sourcepackagerecipe-builds.pt'
--- lib/lp/code/templates/sourcepackagerecipe-builds.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-builds.pt	2011-02-21 23:04:01 +0000
@@ -0,0 +1,79 @@
+<div id="latest-builds" class="portlet">
+<h2>Latest builds</h2>
+<table id="latest-builds-listing" class="listing" style='margin-bottom: 1em;'>
+  <thead>
+    <tr>
+      <th>Status</th>
+      <th>When complete</th>
+      <th>Distribution series</th>
+      <th>Archive</th>
+    </tr>
+  </thead>
+  <tbody>
+    <tal:recipe-builds repeat="build view/builds">
+      <tal:build-view define="buildview nocall:build/@@+index">
+        <tr class="package-build" tal:attributes="id string:build-${build/url_id}">
+          <td>
+            <span tal:replace="structure build/image:icon" />
+            <a tal:content="buildview/status"
+               tal:attributes="href build/fmt:url"></a>
+          </td>
+          <td>
+            <tal:date replace="buildview/date/fmt:displaydate" />
+            <tal:estimate condition="buildview/estimate">
+              (estimated)
+            </tal:estimate>
+
+            <tal:build-log define="file build/log"
+                           tal:condition="file">
+              <a class="sprite download"
+                 tal:attributes="href build/log_url">buildlog</a>
+              (<span tal:replace="file/content/filesize/fmt:bytes" />)
+            </tal:build-log>
+          </td>
+          <td>
+            <tal:distro
+            replace="structure build/distroseries/fmt:link:mainsite" />
+          </td>
+          <td>
+            <tal:archive replace="structure build/archive/fmt:link"/>
+          </td>
+        </tr>
+        <tal:binary-builds repeat="binary buildview/binary_builds">
+          <tr tal:define="binaryview nocall:binary/@@+index"
+              class="binary-build" tal:attributes="id string:build-${binary/url_id}">
+            <td class="indent">
+              <span tal:replace="structure binary/image:icon"/>
+              <a tal:content="binary/source_package_release/title"
+                 tal:attributes="href binary/fmt:url">package - version</a>
+            </td>
+            <td>
+              <tal:date replace="binaryview/date/fmt:displaydate" />
+              <tal:estimate condition="binaryview/estimate">
+                (estimated)
+              </tal:estimate>
+
+              <tal:build-log define="file binary/log"
+                             tal:condition="file">
+                <a class="sprite download"
+                   tal:attributes="href binary/log_url">buildlog</a>
+                (<span tal:replace="file/content/filesize/fmt:bytes" />)
+              </tal:build-log>
+            </td>
+            <td class="indent">
+              <a class="sprite distribution"
+                 tal:define="archseries binary/distro_arch_series"
+                 tal:attributes="href archseries/fmt:url"
+                 tal:content="archseries/architecturetag">i386</a>
+            </td>
+          </tr>
+        </tal:binary-builds>
+      </tal:build-view>
+    </tal:recipe-builds>
+  </tbody>
+</table>
+<p tal:condition="not: view/builds">
+  This recipe has not been built yet.
+</p>
+</div>
+

=== modified file 'lib/lp/code/templates/sourcepackagerecipe-index.pt'
--- lib/lp/code/templates/sourcepackagerecipe-index.pt	2011-02-15 02:33:13 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-index.pt	2011-02-21 23:04:01 +0000
@@ -86,91 +86,41 @@
       </div>
     </div>
   </div>
+
   <div class="yui-g">
-    <div class="portlet">
-      <h2>Latest builds</h2>
-        <table class="listing" style='margin-bottom: 1em;'>
-          <thead>
-            <tr>
-              <th>Status</th>
-              <th>When complete</th>
-              <th>Distribution series</th>
-              <th>Archive</th>
-            </tr>
-          </thead>
-          <tbody>
-            <tal:recipe-builds repeat="build view/builds">
-              <tal:build-view define="buildview nocall:build/@@+index">
-            <tr>
-              <td>
-                <span tal:replace="structure build/image:icon" />
-                <a tal:content="buildview/status"
-                   tal:attributes="href build/fmt:url"></a>
-              </td>
-              <td>
-                <tal:date replace="buildview/date/fmt:displaydate" />
-                <tal:estimate condition="buildview/estimate">
-                  (estimated)
-                </tal:estimate>
-
-                <tal:build-log define="file build/log"
-                               tal:condition="file">
-                  <a class="sprite download"
-                     tal:attributes="href build/log_url">buildlog</a>
-                  (<span tal:replace="file/content/filesize/fmt:bytes" />)
-                </tal:build-log>
-              </td>
-              <td>
-                <tal:distro
-                replace="structure build/distroseries/fmt:link:mainsite" />
-              </td>
-              <td>
-                <tal:archive replace="structure build/archive/fmt:link"/>
-              </td>
-            </tr>
-            <tal:binary-builds repeat="binary buildview/binary_builds">
-              <tr tal:define="binaryview nocall:binary/@@+index"
-                  class="binary-build">
-                <td class="indent">
-                  <span tal:replace="structure binary/image:icon"/>
-                  <a tal:content="binary/source_package_release/title"
-                     tal:attributes="href binary/fmt:url">package - version</a>
-                </td>
-                <td>
-                  <tal:date replace="binaryview/date/fmt:displaydate" />
-                  <tal:estimate condition="binaryview/estimate">
-                    (estimated)
-                  </tal:estimate>
-
-                  <tal:build-log define="file binary/log"
-                                 tal:condition="file">
-                    <a class="sprite download"
-                       tal:attributes="href binary/log_url">buildlog</a>
-                    (<span tal:replace="file/content/filesize/fmt:bytes" />)
-                  </tal:build-log>
-                </td>
-                <td class="indent">
-                  <a class="sprite distribution"
-                     tal:define="archseries binary/distro_arch_series"
-                     tal:attributes="href archseries/fmt:url"
-                     tal:content="archseries/architecturetag">i386</a>
-                </td>
-              </tr>
-            </tal:binary-builds>
-            </tal:build-view>
-            </tal:recipe-builds>
-          </tbody>
-        </table>
-        <p tal:condition="not: view/builds">
-          This recipe has not been built yet.
-        </p>
-        <tal:request replace="structure context/menu:context/request_builds/fmt:link" />
-      </div>
-      <div class='portlet'>
-        <h2>Recipe contents</h2>
+    <div id="builds-target" tal:content="structure context/@@+builds" />
+  </div>
+  <div
+    tal:define="link context/menu:context/request_builds"
+    tal:condition="link/enabled"
+    >
+    <a id="request-builds"
+       class="sprite add"
+       tal:attributes="href link/url"
+       tal:content="link/text" />
+  </div>
+  <div class='portlet'>
+    <h2>Recipe contents</h2>
         <tal:widget replace="structure view/recipe_text_widget"/>
-      </div>
-    </div>
   </div>
+
+  <tal:script>
+    <script id='requestbuild-script' type="text/javascript" tal:content="string:
+      LPS.use('io-base', 'lp.code.requestbuild_overlay', function(Y) {
+        if(Y.UA.ie) {
+            return;
+        }
+
+        Y.on('load', function() {
+            var logged_in = LP.client.links['me'] !== undefined;
+            if (logged_in) {
+                Y.lp.code.requestbuild_overlay.connect_requestbuild();
+            }
+        }, window);
+      });"
+    >
+    </script>
+  </tal:script>
+</div>
 </body>
 </html>

=== added file 'lib/lp/code/windmill/tests/test_recipe_request_build.py'
--- lib/lp/code/windmill/tests/test_recipe_request_build.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/windmill/tests/test_recipe_request_build.py	2011-02-21 23:04:01 +0000
@@ -0,0 +1,96 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for requesting recipe builds."""
+
+__metaclass__ = type
+__all__ = []
+
+import transaction
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.launchpad.webapp.publisher import canonical_url
+from lp.testing.windmill.constants import (
+    FOR_ELEMENT,
+    PAGE_LOAD,
+    SLEEP,
+    )
+from lp.testing.windmill.lpuser import login_person
+from lp.app.browser.tales import PPAFormatterAPI
+from lp.code.windmill.testing import CodeWindmillLayer
+from lp.soyuz.model.processor import ProcessorFamily
+from lp.testing import WindmillTestCase
+
+
+class TestRecipeBuild(WindmillTestCase):
+    """Test setting branch status."""
+
+    layer = CodeWindmillLayer
+    suite_name = "Request recipe build"
+
+    def setUp(self):
+        super(TestRecipeBuild, self).setUp()
+        self.chef = self.factory.makePerson(
+            displayname='Master Chef', name='chef', password='test',
+            email="[e-mail]")
+        self.user = self.chef
+        self.ppa = self.factory.makeArchive(
+            displayname='Secret PPA', owner=self.chef, name='ppa')
+        self.squirrel = self.factory.makeDistroSeries(
+            displayname='Secret Squirrel', name='secret', version='100.04',
+            distribution=self.ppa.distribution)
+        naked_squirrel = removeSecurityProxy(self.squirrel)
+        naked_squirrel.nominatedarchindep = self.squirrel.newArch(
+            'i386', ProcessorFamily.get(1), False, self.chef,
+            supports_virtualized=True)
+        chocolate = self.factory.makeProduct(name='chocolate')
+        cake_branch = self.factory.makeProductBranch(
+            owner=self.chef, name='cake', product=chocolate)
+        self.recipe = self.factory.makeSourcePackageRecipe(
+            owner=self.chef, distroseries=self.squirrel, name=u'cake_recipe',
+            description=u'This recipe builds a foo for disto bar, with my'
+            ' Secret Squirrel changes.', branches=[cake_branch],
+            daily_build_archive=self.ppa)
+        transaction.commit()
+        login_person(self.chef, "[e-mail]", "test", self.client)
+
+    def makeRecipeBuild(self):
+        """Create and return a specific recipe."""
+        build = self.factory.makeSourcePackageRecipeBuild(recipe=self.recipe)
+        return build
+
+    def test_recipe_build_request(self):
+        """Request a recipe build."""
+
+        client = self.client
+        client.open(url=canonical_url(self.recipe))
+        client.waits.forElement(
+            id=u'request-builds', timeout=PAGE_LOAD)
+
+        # Request a new build.
+        client.click(id=u'request-builds')
+        client.waits.forElement(id=u'field.archive')
+        client.click(name=u'field.actions.request')
+
+        # Ensure it shows up.
+        client.waits.forElement(
+            xpath = (u'//tr[contains(@class, "package-build")]/td[4]'
+                     '/a[@href="%s"]') % PPAFormatterAPI(self.ppa).url(),
+            timeout=FOR_ELEMENT)
+
+        # And try the same one again.
+        client.click(id=u'request-builds')
+        client.waits.forElement(id=u'field.archive')
+        client.click(name=u'field.actions.request')
+
+        # And check that there's an error.
+        client.waits.forElement(
+            xpath = (
+                u'//div[contains(@class, "yui3-lazr-formoverlay-errors")]'
+                '/ul/li'), timeout=FOR_ELEMENT)
+        client.asserts.assertText(
+            xpath = (
+                u'//div[contains(@class, "yui3-lazr-formoverlay-errors")]'
+                '/ul/li'),
+            validator=u'An identical build is already pending for %s %s.'
+                        % (self.ppa.distribution.name, self.squirrel.name))

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2011-02-04 09:12:13 +0000
+++ lib/lp/soyuz/model/archive.py	2011-02-21 23:04:01 +0000
@@ -1969,6 +1969,9 @@
         # Avoiding circular imports.
         from lp.soyuz.model.archivepermission import ArchivePermission
 
+        # If there's no user logged in, then there's no archives.
+        if user is None:
+            return []
         store = Store.of(user)
         direct_membership = store.find(
             Archive,