Merge lp:~wallyworld/launchpad/inline-recipe-distro-series-edit into lp:launchpad

[Ian Booth]

[r=abentley][bug=735899] Use the new inline multicheckbox selection widget to edit the source package recipe distroseries attribute.

Use the new inline multicheckbox selection widget to edit the source package recipe distroseries attribute.

== Implementation ==

Not much to tell - just wire up the new widget. Also incorporate Tim's recent changes to the lazr widget infrastructure.
To make everything glue together, the "distros" attribute of the SourcePackageRecipeEditSchema view interface was renamed to "distroseries" so that it becomes the same name as the underlying model attribute on the SourcePackageRecipe interface. The ISourcePackageRecipeEdit and ISourcePackageRecipeEditableAttributes interfaces also had to be reordered so referencing could work.

== Demo and QA ==

A screenshot of the widget in action:
http://people.canonical.com/~ianb/distroseries-popup.png
== Tests ==

New windmill test added.
/lp/code/windmill/tests/test_recipe_inline_distroseries_edit.py
bin/test -vvt test_inline_distroseries_edit

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/doc/lazr-js-widgets.txt
  lib/lp/code/configure.zcml
  lib/lp/code/browser/sourcepackagerecipe.py
  lib/lp/code/interfaces/sourcepackagerecipe.py
  lib/lp/code/javascript/requestbuild_overlay.js
  lib/lp/code/model/sourcepackagerecipe.py
  lib/lp/code/templates/sourcepackagerecipe-index.pt
  lib/lp/code/windmill/tests/test_recipe_inline_distroseries_edit.py

./lib/lp/code/javascript/requestbuild_overlay.js
     110: Line exceeds 78 characters.
     148: Line exceeds 78 characters.
     180: Line exceeds 78 characters.

[Aaron Bentley (abentley)]

This looks pretty good.

I wonder whether label_tag="dt" and items_tag="dd" could be defaults.

Sorry for using distros rather than distroseries. I was new to the problem domain.

I don't think you should have a setUp in RecipeEdit until have more than one test, because until you have more tests, you won't know what needs to vary.

I also see you setting up far more constants than you actually use, e.g. recipe name. Why not just use makeSourcePackageRecipe? (you could use "with person_logged_in(recipe.owner)".

The implementation of updateSeries is a bit sad. Is there no straightforward way to update that?

review: Needs Information

[Ian Booth (wallyworld)]

> This looks pretty good.
>
> I wonder whether label_tag="dt" and items_tag="dd" could be defaults.
>

Yeah, thought about that but with a sample size of 1 - this is the only place it's used so far - wasn't sure about whether to encode the assumption the widget would be rendered in a definition list or not. In the end, I thought it safer to go with span. We can always revisit when we get more places using the widget.

> Sorry for using distros rather than distroseries. I was new to the problem
> domain.
>

No probs.

> I don't think you should have a setUp in RecipeEdit until have more than one
> test, because until you have more tests, you won't know what needs to vary.
>

Ok. Will fix.

> I also see you setting up far more constants than you actually use, e.g.
> recipe name. Why not just use makeSourcePackageRecipe? (you could use "with
> person_logged_in(recipe.owner)".
>

Will take alook.

> The implementation of updateSeries is a bit sad. Is there no straightforward
> way to update that?

Ah, the way that is there is absolutely required if the distroseries field is exported as a CollectionField, which it was initially. However, now that the distroseries field is a List, that approach may not be required anymore. I'll have a look.

[Aaron Bentley (abentley)]

review: Approve

[Ian Booth (wallyworld)]

>
> > The implementation of updateSeries is a bit sad. Is there no
> straightforward
> > way to update that?
>
> Ah, the way that is there is absolutely required if the distroseries field is
> exported as a CollectionField, which it was initially. However, now that the
> distroseries field is a List, that approach may not be required anymore. I'll
> have a look.

The current implementation is still required even when distroseries is exported as a List. The property has a type which subclasses ResultSet and so one needs to use clear(), add(), remove() methods.

=== modified file 'lib/lp/app/doc/lazr-js-widgets.txt'
--- lib/lp/app/doc/lazr-js-widgets.txt	2011-03-22 02:47:21 +0000
+++ lib/lp/app/doc/lazr-js-widgets.txt	2011-03-22 02:47:23 +0000
@@ -219,7 +219,8 @@
 descriptions.  since the barrier to create a PPA is relatively low, we
 restrict the linkability of some fields.  The constructor provides a
 "linkify_text" parameter that defaults to True.  Set this to False to avoid
-the linkification of text.  See the IArchive['description'] editor for an example.
+the linkification of text.  See the IArchive['description'] editor for an
+example.
 
 
 InlineEditPickerWidget
@@ -350,9 +351,9 @@
 
     >>> from lp.app.browser.lazrjs import EnumChoiceWidget
 
-As with the other widgets, this one requires a context object and a Choice type
-field.  The rendering of the widget hooks up to the lazr ChoiceSource with the
-standard patch plugin.
+As with the other widgets, this one requires a context object and a Choice
+type field.  The rendering of the widget hooks up to the lazr ChoiceSource
+with the standard patch plugin.
 
 One of the different things about this widget is the styles that are added.
 Many enums have specific colour styles.  Generally these are the names of
@@ -459,22 +460,18 @@
     >>> login_person(eric)
     >>> print widget()
     <span id="edit-distroseries">
-    <BLANKLINE>
       <dt>
         Recipe distro series
-    <BLANKLINE>
           <button class="lazr-btn yui3-activator-act yui3-activator-hidden"
                   id="edit-distroseries-btn">
             Edit
           </button>
-    <BLANKLINE>
         <noscript>
           <a class="sprite edit"
              href="http://code.launchpad.dev/~eric/+recipe/cake_recipe/+edit"
              title=""></a>
         </noscript>
       </dt>
-    <BLANKLINE>
       <span class="yui3-activator-data-box">
         <dl id='edit-distroseries-items'>
     ...

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2011-03-18 10:31:56 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2011-03-22 02:47:23 +0000
@@ -24,15 +24,21 @@
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
 from lazr.restful.interface import use_template
+from lazr.restful.interfaces import (
+    IFieldHTMLRenderer,
+    IWebServiceClientRequest,
+    )
 import simplejson
 from storm.locals import Store
 from z3c.ptcompat import ViewPageTemplateFile
+from zope import component
 from zope.app.form.browser.widget import Widget
 from zope.app.form.interfaces import IView
 from zope.component import getUtility
 from zope.event import notify
 from zope.formlib import form
 from zope.interface import (
+    implementer,
     implements,
     Interface,
     providedBy,
@@ -44,6 +50,7 @@
     Text,
     TextLine,
     )
+from zope.schema.interfaces import ICollection
 from zope.schema.vocabulary import (
     SimpleTerm,
     SimpleVocabulary,
@@ -296,6 +303,43 @@
         title = "Edit the recipe name"
         return TextLineEditorWidget(self.context, name, title, 'h1')
 
+    @property
+    def distroseries_widget(self):
+        from lp.app.browser.lazrjs import InlineMultiCheckboxWidget
+        field = ISourcePackageEditSchema['distroseries']
+        return InlineMultiCheckboxWidget(
+            self.context,
+            field,
+            attribute_type="reference",
+            vocabulary='BuildableDistroSeries',
+            label="Distribution series:",
+            label_tag="dt",
+            header="Change default distribution series:",
+            empty_display_value="None",
+            selected_items=sorted(
+                self.context.distroseries, key=lambda ds: ds.displayname),
+            items_tag="dd",
+            )
+
+
[e-mail](ISourcePackageRecipe, ICollection,
+                   IWebServiceClientRequest)
+@implementer(IFieldHTMLRenderer)
+def distroseries_renderer(context, field, request):
+    """Render a distroseries collection as a set of links."""
+
+    def render(value):
+        distroseries = sorted(
+            context.distroseries, key=lambda ds: ds.displayname)
+        if not distroseries:
+            return 'None'
+        html = "<ul>"
+        html += ''.join(
+            ["<li>%s</li>" % format_link(series) for series in distroseries])
+        html += "</ul>"
+        return html
+    return render
+
 
 def builds_for_recipe(recipe):
         """A list of interesting builds.
@@ -337,7 +381,7 @@
 
         The distroseries function as defaults for requesting a build.
         """
-        initial_values = {'distros': self.context.distroseries}
+        initial_values = {'distroseries': self.context.distroseries}
         build = self.context.last_build
         if build is not None:
             initial_values['archive'] = build.archive
@@ -346,26 +390,26 @@
     class schema(Interface):
         """Schema for requesting a build."""
         archive = Choice(vocabulary='TargetPPAs', title=u'Archive')
-        distros = List(
+        distroseries = List(
             Choice(vocabulary='BuildableDistroSeries'),
             title=u'Distribution series')
 
-    custom_widget('distros', LabeledMultiCheckBoxWidget)
+    custom_widget('distroseries', LabeledMultiCheckBoxWidget)
 
     def validate(self, data):
-        distros = data.get('distros', [])
+        distros = data.get('distroseries', [])
         if not len(distros):
-            self.setFieldError('distros',
+            self.setFieldError('distroseries',
                 "You need to specify at least one distro series for which "
                 "to build.")
             return
         over_quota_distroseries = []
-        for distroseries in data['distros']:
+        for distroseries in data['distroseries']:
             if self.context.isOverQuota(self.user, distroseries):
                 over_quota_distroseries.append(str(distroseries))
         if len(over_quota_distroseries) > 0:
             self.setFieldError(
-                'distros',
+                'distroseries',
                 "You have exceeded today's quota for %s." %
                 ', '.join(over_quota_distroseries))
 
@@ -378,7 +422,7 @@
         """
         informational = {}
         builds = []
-        for distroseries in data['distros']:
+        for distroseries in data['distroseries']:
             try:
                 build = self.context.requestBuild(
                     data['archive'], self.user, distroseries, manual=True)
@@ -512,18 +556,13 @@
         'description',
         'owner',
         'build_daily',
+        'distroseries',
         ])
     daily_build_archive = Choice(vocabulary='TargetPPAs',
         title=u'Daily build archive',
         description=(
             u'If built daily, this is the archive where the package '
             u'will be uploaded.'))
-    distros = List(
-        Choice(vocabulary='BuildableDistroSeries'),
-        title=u'Default distribution series',
-        description=(
-            u'If built daily, these are the distribution versions that '
-            u'the recipe will be built for.'))
     recipe_text = has_structured_doc(
         Text(
             title=u'Recipe text', required=True,
@@ -577,9 +616,9 @@
 
     def validate(self, data):
         if data['build_daily']:
-            if len(data['distros']) == 0:
+            if len(data['distroseries']) == 0:
                 self.setFieldError(
-                    'distros',
+                    'distroseries',
                     'You must specify at least one series for daily builds.')
         try:
             parser = RecipeParser(data['recipe_text'])
@@ -673,7 +712,7 @@
     title = label = 'Create a new source package recipe'
 
     schema = ISourcePackageAddSchema
-    custom_widget('distros', LabeledMultiCheckBoxWidget)
+    custom_widget('distroseries', LabeledMultiCheckBoxWidget)
     custom_widget('owner', RecipeOwnerWidget)
     custom_widget('use_ppa', LaunchpadRadioWidget)
 
@@ -696,6 +735,11 @@
         # all confused when we want to create a new PPA.
         archive_widget._displayItemForMissingValue = False
 
+    def setUpFields(self):
+        super(SourcePackageRecipeAddView, self).setUpFields()
+        # Ensure distro series widget allows input
+        self.form_fields['distroseries'].for_input = True
+
     def getBranch(self):
         """The branch on which the recipe is built."""
         return self.context
@@ -729,7 +773,7 @@
             'name': self._find_unused_name(self.user),
             'recipe_text': MINIMAL_RECIPE_TEXT % self.context.bzr_identity,
             'owner': self.user,
-            'distros': series,
+            'distroseries': series,
             'build_daily': True,
             'use_ppa': EXISTING_PPA,
             }
@@ -750,8 +794,8 @@
             source_package_recipe = self.error_handler(
                 getUtility(ISourcePackageRecipeSource).new,
                 self.user, owner, data['name'],
-                data['recipe_text'], data['description'], data['distros'],
-                ppa, data['build_daily'])
+                data['recipe_text'], data['description'],
+                data['distroseries'], ppa, data['build_daily'])
             Store.of(source_package_recipe).flush()
         except ErrorHandled:
             return
@@ -795,11 +839,14 @@
     label = title
 
     schema = ISourcePackageEditSchema
-    custom_widget('distros', LabeledMultiCheckBoxWidget)
+    custom_widget('distroseries', LabeledMultiCheckBoxWidget)
 
     def setUpFields(self):
         super(SourcePackageRecipeEditView, self).setUpFields()
 
+        # Ensure distro series widget allows input
+        self.form_fields['distroseries'].for_input = True
+
         if check_permission('launchpad.Admin', self.context):
             # Exclude the PPA archive dropdown.
             self.form_fields = self.form_fields.omit('daily_build_archive')
@@ -819,7 +866,7 @@
     @property
     def initial_values(self):
         return {
-            'distros': self.context.distroseries,
+            'distroseries': self.context.distroseries,
             'recipe_text': self.context.recipe_text,
             }
 
@@ -843,7 +890,7 @@
             except ErrorHandled:
                 return
 
-        distros = data.pop('distros')
+        distros = data.pop('distroseries')
         if distros != self.context.distroseries:
             self.context.distroseries.clear()
             for distroseries_item in distros:

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-03-18 10:31:56 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-03-22 02:47:23 +0000
@@ -272,7 +272,7 @@
         branch = self.factory.makeAnyBranch()
         with person_logged_in(archive.owner):
             view = create_initialized_view(branch, '+new-recipe')
-        series = set(view.initial_values['distros'])
+        series = set(view.initial_values['distroseries'])
         initial_series = set([development, current])
         self.assertEqual(initial_series, series.intersection(initial_series))
         other_series = set(
@@ -452,7 +452,7 @@
         browser = self.getViewBrowser(self.makeBranch(), '+new-recipe')
         browser.getControl(name='field.name').value = 'daily'
         browser.getControl('Description').value = 'Make some food!'
-        browser.getControl(name='field.distros').value = []
+        browser.getControl(name='field.distroseries').value = []
         browser.getControl('Create Recipe').click()
         self.assertEqual(
             'You must specify at least one series for daily builds.',
@@ -780,8 +780,8 @@
             'lp://dev/~chef/ratatouille/meat',
             MatchesTagText(content, 'edit-recipe_text'))
         self.assertThat(
-            'Distribution series: Mumbly Midget',
-            MatchesTagText(content, 'distros'))
+            'Distribution series: Edit Mumbly Midget',
+            MatchesTagText(content, 'distroseries'))
         self.assertThat(
             'PPA 2', MatchesPickerText(content, 'edit-daily_build_archive'))
 
@@ -797,7 +797,7 @@
         view.request_action.success({
             'name': u'fings',
             'recipe_text': recipe.recipe_text,
-            'distros': recipe.distroseries})
+            'distroseries': recipe.distroseries})
         self.assertSqlAttributeEqualsDate(
             recipe, 'date_last_modified', UTC_NOW)
 
@@ -846,8 +846,8 @@
             'lp://dev/~chef/ratatouille/meat',
             MatchesTagText(content, 'edit-recipe_text'))
         self.assertThat(
-            'Distribution series: Mumbly Midget',
-            MatchesTagText(content, 'distros'))
+            'Distribution series: Edit Mumbly Midget',
+            MatchesTagText(content, 'distroseries'))
 
     def test_edit_recipe_forbidden_instruction(self):
         self.factory.makeDistroSeries(
@@ -1082,7 +1082,7 @@
             Base branch: lp://dev/~chef/chocolate/cake
             Debian version: {debupstream}-0~{revno}
             Daily build archive: Secret PPA Edit
-            Distribution series: Secret Squirrel
+            Distribution series: Edit Secret Squirrel
 
             Latest builds
             Status When complete Distribution series Archive

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2011-03-16 06:03:44 +0000
+++ lib/lp/code/configure.zcml	2011-03-22 02:47:23 +0000
@@ -1001,6 +1001,10 @@
         name="RECIPEBRANCHBUILD"
         permission="zope.Public"/>
 
+  <adapter
+    factory="lp.code.browser.sourcepackagerecipe.distroseries_renderer"
+    name="distroseries"/>
+
   <!-- RecipeBuildRecordSet and related classes-->
 
   <securedutility

=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py	2011-03-16 06:03:44 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py	2011-03-22 02:47:23 +0000
@@ -36,6 +36,7 @@
 from lazr.restful.fields import (
     CollectionField,
     Reference,
+    ReferenceChoice,
     )
 from lazr.restful.interface import copy_field
 from zope.interface import (
@@ -47,6 +48,7 @@
     Choice,
     Datetime,
     Int,
+    List,
     Text,
     TextLine,
     )
@@ -181,26 +183,6 @@
         """
 
 
-class ISourcePackageRecipeEdit(Interface):
-    """ISourcePackageRecipe methods that require launchpad.Edit permission."""
-
-    @mutator_for(ISourcePackageRecipeView['recipe_text'])
-    @operation_for_version("devel")
-    @operation_parameters(
-        recipe_text=copy_field(
-            ISourcePackageRecipeView['recipe_text']))
-    @export_write_operation()
-    def setRecipeText(recipe_text):
-        """Set the text of the recipe."""
-
-    def destroySelf():
-        """Remove this SourcePackageRecipe from the database.
-
-        This requires deleting any rows with non-nullable foreign key
-        references to this object.
-        """
-
-
 class ISourcePackageRecipeEditableAttributes(IHasOwner):
     """ISourcePackageRecipe attributes that can be edited.
 
@@ -219,10 +201,13 @@
             vocabulary='UserTeamsParticipationPlusSelf',
             description=_("The person or team who can edit this recipe.")))
 
-    distroseries = CollectionField(
-        Reference(IDistroSeries), title=_("The distroseries this recipe will"
-            " build a source package for"),
-        readonly=False)
+    distroseries = exported(List(
+        ReferenceChoice(schema=IDistroSeries,
+            vocabulary='BuildableDistroSeries'),
+        title=_("Default distribution series"),
+        description=_("If built daily, these are the distribution "
+            "versions that the recipe will be built for."),
+        readonly=True))
     build_daily = exported(Bool(
         title=_("Built daily"),
         description=_(
@@ -245,6 +230,34 @@
     is_stale = Bool(title=_('Recipe is stale.'))
 
 
+class ISourcePackageRecipeEdit(Interface):
+    """ISourcePackageRecipe methods that require launchpad.Edit permission."""
+
+    @mutator_for(ISourcePackageRecipeView['recipe_text'])
+    @operation_for_version("devel")
+    @operation_parameters(
+        recipe_text=copy_field(
+            ISourcePackageRecipeView['recipe_text']))
+    @export_write_operation()
+    def setRecipeText(recipe_text):
+        """Set the text of the recipe."""
+
+    @mutator_for(ISourcePackageRecipeEditableAttributes['distroseries'])
+    @operation_parameters(distroseries=copy_field(
+        ISourcePackageRecipeEditableAttributes['distroseries']))
+    @export_write_operation()
+    @operation_for_version("devel")
+    def updateSeries(distroseries):
+        """Replace this recipe's distro series."""
+
+    def destroySelf():
+        """Remove this SourcePackageRecipe from the database.
+
+        This requires deleting any rows with non-nullable foreign key
+        references to this object.
+        """
+
+
 class ISourcePackageRecipe(ISourcePackageRecipeData,
     ISourcePackageRecipeEdit, ISourcePackageRecipeEditableAttributes,
     ISourcePackageRecipeView):

=== modified file 'lib/lp/code/javascript/requestbuild_overlay.js'
--- lib/lp/code/javascript/requestbuild_overlay.js	2011-03-11 23:54:32 +0000
+++ lib/lp/code/javascript/requestbuild_overlay.js	2011-03-22 02:47:23 +0000
@@ -250,7 +250,7 @@
  * Return: true if data is valid
  */
 function validate(data) {
-    var distros = data['field.distros']
+    var distros = data['field.distroseries']
     if (Y.Object.size(distros) == 0) {
         request_build_response_handler.showError(
                 "You need to specify at least one distro series for " +
@@ -391,7 +391,7 @@
 
 function get_distroseries_nodes() {
     return request_build_overlay.form_node.all(
-            "label[for^='field.distros.']");
+            "label[for^='field.distroseries.']");
 }
 
 var DISABLED_DISTROSERIES_CHECKBOX_HTML =

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2011-03-18 06:47:15 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2011-03-22 02:47:23 +0000
@@ -187,6 +187,12 @@
     def recipe_text(self):
         return self.builder_recipe.get_recipe_text()
 
+    def updateSeries(self, distroseries):
+        if distroseries != self.distroseries:
+            self.distroseries.clear()
+            for distroseries_item in distroseries:
+                self.distroseries.add(distroseries_item)
+
     @staticmethod
     def new(registrant, owner, name, recipe, description,
             distroseries=None, daily_build_archive=None, build_daily=False,

=== modified file 'lib/lp/code/templates/sourcepackagerecipe-index.pt'
--- lib/lp/code/templates/sourcepackagerecipe-index.pt	2011-03-02 01:47:19 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-index.pt	2011-03-22 02:47:23 +0000
@@ -118,15 +118,8 @@
             <dt>Daily build archive:</dt>
             <dd tal:content="structure view/archive_picker"/>
           </dl>
-
-          <dl id="distros">
-            <dt>Distribution series:</dt>
-            <dd>
-              <ul>
-                  <li tal:repeat="curseries context/distroseries"
-                      tal:content="structure curseries/fmt:link" />
-              </ul>
-            </dd>
+          <dl id="distroseries">
+            <tal:distroseries tal:replace="structure view/distroseries_widget"/>
           </dl>
         </div>
       </div>

=== modified file 'lib/lp/code/templates/sourcepackagerecipe-new.pt'
--- lib/lp/code/templates/sourcepackagerecipe-new.pt	2011-02-14 01:48:57 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-new.pt	2011-03-22 02:47:23 +0000
@@ -99,7 +99,7 @@
               </tal:widget>
             </tal:create-ppa>
 
-            <tal:widget define="widget nocall:view/widgets/distros">
+            <tal:widget define="widget nocall:view/widgets/distroseries">
               <metal:block use-macro="context/@@launchpad_form/widget_row" />
             </tal:widget>
             <tal:widget define="widget nocall:view/widgets/recipe_text">

=== modified file 'lib/lp/code/templates/sourcepackagerecipe-request-builds.pt'
--- lib/lp/code/templates/sourcepackagerecipe-request-builds.pt	2010-03-31 19:27:07 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-request-builds.pt	2011-03-22 02:47:23 +0000
@@ -16,7 +16,7 @@
             <tal:widget define="widget nocall:view/widgets/archive">
               <metal:block use-macro="context/@@launchpad_form/widget_row" />
             </tal:widget>
-            <tal:widget define="widget nocall:view/widgets/distros">
+            <tal:widget define="widget nocall:view/widgets/distroseries">
               <metal:block use-macro="context/@@launchpad_form/widget_row" />
             </tal:widget>
           </table>

=== added file 'lib/lp/code/windmill/tests/test_recipe_inline_distroseries_edit.py'
--- lib/lp/code/windmill/tests/test_recipe_inline_distroseries_edit.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/windmill/tests/test_recipe_inline_distroseries_edit.py	2011-03-22 02:47:23 +0000
@@ -0,0 +1,71 @@
+# 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.component import getUtility
+from storm.store import Store
+
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from lp.testing import WindmillTestCase
+from lp.testing.windmill.constants import (
+    FOR_ELEMENT,
+    PAGE_LOAD,
+    )
+from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
+from lp.code.windmill.testing import CodeWindmillLayer
+
+
+class TestRecipeEdit(WindmillTestCase):
+    """Test recipe editing with inline widgets."""
+
+    layer = CodeWindmillLayer
+    suite_name = "Request recipe build"
+
+    def test_inline_distroseries_edit(self):
+        """Test that inline editing of distroseries works."""
+
+        chef = self.factory.makePerson(
+            displayname='Master Chef', name='chef', password='test',
+            email="[e-mail]")
+        recipe = self.factory.makeSourcePackageRecipe(owner=chef)
+        transaction.commit()
+
+        client, start_url = self.getClientFor(recipe, user=chef)
+        client.waits.forElement(
+            id=u'edit-distroseries-items', timeout=PAGE_LOAD)
+
+        # Edit the distro series.
+        client.click(jquery=u'("#edit-distroseries-btn")[0]')
+        client.waits.forElement(
+            jquery=u'("#edit-distroseries-save")',
+            timeout=FOR_ELEMENT)
+        # Click the checkbox to select the first distro series
+        client.click(name=u'field.distroseries.0')
+        client.waits.forElement(
+          jquery=u"('[name=\"field.distroseries.0\"][checked=\"checked\"]')",
+          timeout=FOR_ELEMENT)
+        # Save it
+        client.click(jquery=u'("#edit-distroseries-save")[0]')
+
+        # Wait for the the new one that is added.
+        client.waits.forElement(
+            jquery=u"('#edit-distroseries-items ul li a')[0]",
+            timeout=FOR_ELEMENT)
+
+        # Check that the new data was saved.
+        transaction.commit()
+        hoary = getUtility(ILaunchpadCelebrities).ubuntu['hoary']
+        store = Store.of(recipe)
+        saved_recipe = store.find(
+            SourcePackageRecipe,
+            SourcePackageRecipe.name==recipe.name).one()
+        self.assertEqual(len(list(saved_recipe.distroseries)), 2)
+        distroseries=sorted(
+            saved_recipe.distroseries, key=lambda ds: ds.displayname)
+        self.assertEqual(distroseries[0], hoary)