launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02967
[Merge] lp:~wallyworld/launchpad/inline-recipe-distro-series-edit into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/inline-recipe-distro-series-edit into lp:launchpad with lp:~wallyworld/launchpad/inline-multicheckbox-widget as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/inline-recipe-distro-series-edit/+merge/52940
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.
--
https://code.launchpad.net/~wallyworld/launchpad/inline-recipe-distro-series-edit/+merge/52940
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/inline-recipe-distro-series-edit into lp:launchpad.
=== modified file 'lib/lp/app/doc/lazr-js-widgets.txt'
--- lib/lp/app/doc/lazr-js-widgets.txt 2011-03-16 02:53:05 +0000
+++ lib/lp/app/doc/lazr-js-widgets.txt 2011-03-16 02:53:08 +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-02 01:47:19 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2011-03-16 02:53:08 +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,
@@ -295,6 +302,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",
+ )
+
+
+@component.adapter(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.
@@ -345,26 +389,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))
@@ -377,7 +421,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)
@@ -511,18 +555,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,
@@ -576,9 +615,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'])
@@ -672,7 +711,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)
@@ -723,7 +762,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,
}
@@ -744,8 +783,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
@@ -789,7 +828,7 @@
label = title
schema = ISourcePackageEditSchema
- custom_widget('distros', LabeledMultiCheckBoxWidget)
+ custom_widget('distroseries', LabeledMultiCheckBoxWidget)
def setUpFields(self):
super(SourcePackageRecipeEditView, self).setUpFields()
@@ -813,7 +852,7 @@
@property
def initial_values(self):
return {
- 'distros': self.context.distroseries,
+ 'distroseries': self.context.distroseries,
'recipe_text': self.context.recipe_text,
}
@@ -837,7 +876,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/configure.zcml'
--- lib/lp/code/configure.zcml 2011-03-07 07:27:56 +0000
+++ lib/lp/code/configure.zcml 2011-03-16 02:53:08 +0000
@@ -997,6 +997,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-11 23:54:32 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py 2011-03-16 02:53:08 +0000
@@ -47,6 +47,7 @@
Choice,
Datetime,
Int,
+ List,
Text,
TextLine,
)
@@ -181,26 +182,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 +200,11 @@
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(
+ Reference(IDistroSeries), 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 +227,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-16 02:53:08 +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-15 14:32:17 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py 2011-03-16 02:53:08 +0000
@@ -174,6 +174,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-16 02:53:08 +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>
=== 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-16 02:53:08 +0000
@@ -0,0 +1,79 @@
+# 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 canonical.launchpad.webapp.publisher import canonical_url
+from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
+from lp.testing.windmill.constants import (
+ FOR_ELEMENT,
+ PAGE_LOAD,
+ )
+from lp.testing.windmill.lpuser import login_person
+from lp.code.windmill.testing import CodeWindmillLayer
+from lp.testing import WindmillTestCase
+
+
+class TestRecipeEdit(WindmillTestCase):
+ """Test recipe editing with inline widgets."""
+
+ layer = CodeWindmillLayer
+ suite_name = "Request recipe build"
+
+ def setUp(self):
+ super(TestRecipeEdit, self).setUp()
+ self.chef = self.factory.makePerson(
+ displayname='Master Chef', name='chef', password='test',
+ email="chef@xxxxxxxxxxx")
+ self.user = self.chef
+ self.recipe = self.factory.makeSourcePackageRecipe(
+ owner=self.chef, name=u'cake_recipe')
+ transaction.commit()
+ login_person(self.chef, "chef@xxxxxxxxxxx", "test", self.client)
+
+ def test_inline_distroseries_edit(self):
+ """Test that inline editing of distroseries works."""
+
+ client = self.client
+ client.open(url=canonical_url(self.recipe))
+ 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(self.recipe)
+ saved_recipe = store.find(
+ SourcePackageRecipe,
+ SourcePackageRecipe.name==u'cake_recipe').one()
+ self.assertEqual(len(list(saved_recipe.distroseries)), 2)
+ distroseries=sorted(
+ saved_recipe.distroseries, key=lambda ds: ds.displayname)
+ self.assertEqual(distroseries[0], hoary)
