launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02572
[Merge] lp:~thumper/launchpad/daily-ajax into lp:launchpad
Tim Penhey has proposed merging lp:~thumper/launchpad/daily-ajax into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#673530 Inline editing for recipes
https://bugs.launchpad.net/bugs/673530
For more details, see:
https://code.launchpad.net/~thumper/launchpad/daily-ajax/+merge/49295
The primary motivation of this branch is to add a simple popup
chooser for the build frequency. I did this by creating a
BooleanChoiceWidget.
lib/canonical/launchpad/icing/style-3-0.css.in
- Since the value string of the choice is clickable, we should
make the cursor change, and underline it as the user hovers
The lazr-js-widgets documentation is updated to cover the new
widget.
I also added an in-page help link about the build schedule.
--
https://code.launchpad.net/~thumper/launchpad/daily-ajax/+merge/49295
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/daily-ajax into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
--- lib/canonical/launchpad/icing/style-3-0.css.in 2011-02-09 10:37:39 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css.in 2011-02-11 01:53:14 +0000
@@ -960,6 +960,12 @@
.widget-hd.js-action:hover {
text-decoration: underline;
}
+
+.yui3-ichoicesource-content .value:hover {
+ text-decoration: underline;
+ cursor:pointer;
+ }
+
.error.message, .warning.message, .informational.message {
border: solid #666;
border-width: 1px 2px 2px 1px;
=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py 2011-01-28 01:09:35 +0000
+++ lib/lp/app/browser/lazrjs.py 2011-02-11 01:53:14 +0000
@@ -5,6 +5,7 @@
__metaclass__ = type
__all__ = [
+ 'BooleanChoiceWidget',
'InlineEditPickerWidget',
'standard_text_html_representation',
'TextAreaEditorWidget',
@@ -61,6 +62,7 @@
if mutator_info is not None:
mutator_method, mutator_extra = mutator_info
self.mutator_method_name = mutator_method.__name__
+ self.json_attribute = simplejson.dumps(self.api_attribute)
@property
def resource_uri(self):
@@ -90,19 +92,27 @@
return False
-class TextWidgetBase(WidgetBase):
+class EditableWidgetBase(WidgetBase):
+ """Adds an edit_url property to WidgetBase."""
+
+ def __init__(self, context, exported_field, content_box_id,
+ edit_view, edit_url):
+ super(EditableWidgetBase, self).__init__(
+ context, exported_field, content_box_id)
+ if edit_url is None:
+ edit_url = canonical_url(self.context, view_name=edit_view)
+ self.edit_url = edit_url
+
+
+class TextWidgetBase(EditableWidgetBase):
"""Abstract base for the single and multiline text editor widgets."""
def __init__(self, context, exported_field, title, content_box_id,
edit_view, edit_url):
super(TextWidgetBase, self).__init__(
- context, exported_field, content_box_id)
- if edit_url is None:
- edit_url = canonical_url(self.context, view_name=edit_view)
- self.edit_url = edit_url
+ context, exported_field, content_box_id, edit_view, edit_url)
self.accept_empty = simplejson.dumps(self.optional_field)
self.title = title
- self.json_attribute = simplejson.dumps(self.api_attribute)
self.widget_css_selector = simplejson.dumps('#' + self.content_box_id)
@property
@@ -110,7 +120,19 @@
return simplejson.dumps(self.resource_uri + '/' + self.api_attribute)
-class TextLineEditorWidget(TextWidgetBase):
+class DefinedTagMixin:
+ """Mixin class to define open and closing tags."""
+
+ @property
+ def open_tag(self):
+ return '<%s id="%s">' % (self.tag, self.content_box_id)
+
+ @property
+ def close_tag(self):
+ return '</%s>' % self.tag
+
+
+class TextLineEditorWidget(TextWidgetBase, DefinedTagMixin):
"""Wrapper for the lazr-js inlineedit/editor.js widget."""
__call__ = ViewPageTemplateFile('../templates/text-line-editor.pt')
@@ -146,14 +168,6 @@
self.width = simplejson.dumps(width)
@property
- def open_tag(self):
- return '<%s id="%s">' % (self.tag, self.content_box_id)
-
- @property
- def close_tag(self):
- return '</%s>' % self.tag
-
- @property
def value(self):
text = getattr(self.context, self.attribute_name, self.default_text)
if text is None:
@@ -333,3 +347,62 @@
return ''
nomail = FormattersAPI(value).obfuscate_email()
return FormattersAPI(nomail).text_to_html(linkify_text=linkify_text)
+
+
+class BooleanChoiceWidget(EditableWidgetBase, DefinedTagMixin):
+ """A ChoiceEdit for a boolean field."""
+
+ __call__ = ViewPageTemplateFile('../templates/boolean-choice-widget.pt')
+
+ def __init__(self, context, exported_field,
+ tag, false_text, true_text, prefix=None,
+ edit_view="+edit", edit_url=None,
+ content_box_id=None, header='Select an item'):
+ """Create a widget wrapper.
+
+ :param context: The object that is being edited.
+ :param exported_field: The attribute being edited. This should be
+ a field from an interface of the form ISomeInterface['fieldname']
+ :param tag: The HTML tag to use.
+ :param false_text: The string to show for a false value.
+ :param true_text: The string to show for a true value.
+ :param prefix: Optional text to show before the value.
+ :param edit_view: The view name to use to generate the edit_url if
+ one is not specified.
+ :param edit_url: The URL to use for editing when the user isn't logged
+ in and when JS is off. Defaults to the edit_view on the context.
+ :param content_box_id: The HTML id to use for this widget. Automatically
+ generated if this is not provided.
+ :param header: The large text at the top of the choice popup.
+ """
+ super(BooleanChoiceWidget, self).__init__(
+ context, exported_field, content_box_id, edit_view, edit_url)
+ self.header = header
+ self.tag = tag
+ self.prefix = prefix
+ self.true_text = true_text
+ self.false_text = false_text
+ self.current_value = getattr(self.context, self.attribute_name)
+
+ @property
+ def value(self):
+ if self.current_value:
+ return self.true_text
+ else:
+ return self.false_text
+
+ @property
+ def config(self):
+ return dict(
+ contentBox='#'+self.content_box_id,
+ value=self.current_value,
+ title=self.header,
+ items=[
+ dict(name=self.true_text, value=True, style='', help='',
+ disabled=False),
+ dict(name=self.false_text, value=False, style='', help='',
+ disabled=False)])
+
+ @property
+ def json_config(self):
+ return simplejson.dumps(self.config)
=== modified file 'lib/lp/app/doc/lazr-js-widgets.txt'
--- lib/lp/app/doc/lazr-js-widgets.txt 2011-01-28 00:54:11 +0000
+++ lib/lp/app/doc/lazr-js-widgets.txt 2011-02-11 01:53:14 +0000
@@ -268,3 +268,61 @@
If the field is optional, a "Remove" link is shown. The "Remove" text is
customizable thought the "remove_button_text" parameter.
+
+
+BooleanChoiceWidget
+-------------------
+
+This widget provides a simple popup with two options for the user to choose
+from.
+
+ >>> from lp.app.browser.lazrjs import BooleanChoiceWidget
+
+As with the other widgets, this one requires a context object and a Bool type
+field. The rendering of the widget hooks up to the lazr ChoiceSource with the
+standard patch plugin.
+
+The surrounding tag is customisable, and a prefix may be given. The prefix is
+passed through to the ChoiceSource and is rendered as part of the widget, but
+isn't updated when the value changes.
+
+If the user does not have edit rights, the widget just renders the text based
+on the current value of the field on the object:
+
+ >>> login(ANONYMOUS)
+ >>> from lp.registry.interfaces.person import IPerson
+ >>> hide_email = IPerson['hide_email_addresses']
+ >>> widget = BooleanChoiceWidget(
+ ... eric, hide_email, 'span',
+ ... false_text="Don't hide it",
+ ... true_text="Keep it secret",
+ ... prefix="My email: ")
+ >>> print widget()
+ <span id="edit-hide_email_addresses">
+ My email: <span class="value">Don't hide it</span>
+ </span>
+
+If the user has edit rights, an edit icon is rendered and some javascript is
+rendered to hook up the widget.
+
+ >>> login_person(eric)
+ >>> print widget()
+ <span id="edit-hide_email_addresses">
+ My email: <span class="value">Don't hide it</span>
+ <span>
+
+ <a class="editicon sprite edit"
+ href="http://launchpad.dev/~eric/+edit";></a>
+ </span>
+ </span>
+ <script>
+ LPS.use('lp.app.choice', function(Y) {
+ ...
+ </script>
+
+
+Changing the edit link
+**********************
+
+The edit link can be changed in exactly the same way as for the
+TextLineEditorWidget above.
=== added file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/choice.js 2011-02-11 01:53:14 +0000
@@ -0,0 +1,21 @@
+YUI.add('lp.app.choice', function(Y) {
+
+var namespace = Y.namespace('lp.app.choice');
+
+namespace.addBinaryChoice = function(config, resource_uri, attribute) {
+
+ if (Y.UA.ie) {
+ return;
+ }
+
+ var widget = new Y.ChoiceSource(config);
+ widget.plug({
+ fn: Y.lp.client.plugins.PATCHPlugin,
+ cfg: {
+ patch: attribute,
+ resource: resource_uri}});
+ widget.render();
+};
+
+
+}, "0.1", {"requires": ["lazr.choiceedit", "lp.client.plugins"]});
=== added file 'lib/lp/app/templates/boolean-choice-widget.pt'
--- lib/lp/app/templates/boolean-choice-widget.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/app/templates/boolean-choice-widget.pt 2011-02-11 01:53:14 +0000
@@ -0,0 +1,18 @@
+<tal:open-tag replace="structure view/open_tag"/>
+<tal:prefix replace="view/prefix"/><span class="value" tal:content="view/value">Unset</span>
+ <span tal:condition="view/can_write">
+
+ <a tal:attributes="href view/edit_url"
+ class="editicon sprite edit"></a>
+ </span>
+<tal:close-tag replace="structure view/close_tag"/>
+
+<script tal:condition="view/can_write"
+ tal:content="structure string:
+LPS.use('lp.app.choice', function(Y) {
+ Y.lp.app.choice.addBinaryChoice(
+ ${view/json_config},
+ ${view/json_resource_uri},
+ ${view/json_attribute});
+});
+"/>
=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2011-02-03 05:23:55 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2011-02-11 01:53:14 +0000
@@ -69,7 +69,10 @@
LaunchpadFormView,
render_radio_widget_part,
)
-from lp.app.browser.lazrjs import InlineEditPickerWidget
+from lp.app.browser.lazrjs import (
+ BooleanChoiceWidget,
+ InlineEditPickerWidget,
+ )
from lp.app.browser.tales import format_link
from lp.app.widgets.itemswidgets import (
LabeledMultiCheckBoxWidget,
@@ -260,6 +263,15 @@
header='Change daily build archive',
step_title='Select a PPA')
+ @property
+ def daily_build_widget(self):
+ return BooleanChoiceWidget(
+ self.context, ISourcePackageRecipe['build_daily'],
+ tag='span',
+ false_text='Build on request',
+ true_text='Build daily',
+ header='Change build schedule')
+
class SourcePackageRecipeRequestBuildsView(LaunchpadFormView):
"""A view for requesting builds of a SourcePackageRecipe."""
=== added file 'lib/lp/code/help/recipe-build-frequency.html'
--- lib/lp/code/help/recipe-build-frequency.html 1970-01-01 00:00:00 +0000
+++ lib/lp/code/help/recipe-build-frequency.html 2011-02-11 01:53:14 +0000
@@ -0,0 +1,41 @@
+<html>
+ <head>
+ <title>Source package recipe build schedule</title>
+ <link rel="stylesheet" type="text/css"
+ href="/+icing/yui/cssreset/reset.css" />
+ <link rel="stylesheet" type="text/css"
+ href="/+icing/yui/cssfonts/fonts.css" />
+ <link rel="stylesheet" type="text/css"
+ href="/+icing/yui/cssbase/base.css" />
+ <style type="text/css">
+ dt { font-weight: bold }
+ dd p { margin-bottom: 0.5em }
+ </style>
+ </head>
+ <body>
+ <h1>Source package recipe build schedule</h1>
+
+ <p>There are two options for when recipes get built:</p>
+ <dl>
+ <dt>Build daily</dt>
+ <dd>
+ <p>A build will be scheduled automatically once a change in any
+ of the branches used in the recipe is detected.</p>
+ <p>If there has been a build of the recipe within the previous
+ 24 hours into the daily build PPA, the build will not be scheduled
+ until 24 hours since the last build into the daily build PPA.</p>
+ <p>If the recipe had been built within the last 24 hours into a
+ different PPA using the "Request build" action, this will not
+ delay the daily build.</p>
+ <p>If you really want the build to happen before the 24 period is up,
+ you can use the "Request build" action.</p>
+ </dd>
+ <dt>Build on request</dt>
+ <dd>
+ <p>Builds of the recipe have to be manually requested using the
+ "Request build" action.</p>
+ </dd>
+ </dl>
+
+ </body>
+</html>
=== modified file 'lib/lp/code/templates/sourcepackagerecipe-index.pt'
--- lib/lp/code/templates/sourcepackagerecipe-index.pt 2011-01-20 20:35:32 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-index.pt 2011-02-11 01:53:14 +0000
@@ -44,13 +44,12 @@
<h2>Recipe information</h2>
<div class="two-column-list">
<dl id="build_daily">
- <dt>Build schedule:</dt>
- <dd tal:condition="context/build_daily">
- Built daily
- </dd>
- <dd tal:condition="not:context/build_daily">
- Built on request
- </dd>
+ <dt>Build schedule:
+ <a href="/+help/recipe-build-frequency.html" target="help" class="sprite maybe">
+ <span class="invisible-link">Tag help</span>
+ </a>
+ </dt>
+ <dd tal:content="structure view/daily_build_widget"/>
</dl>
<dl id="owner">
=== added file 'lib/lp/code/windmill/tests/test_recipe_index.py'
--- lib/lp/code/windmill/tests/test_recipe_index.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/windmill/tests/test_recipe_index.py 2011-02-11 01:53:14 +0000
@@ -0,0 +1,48 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for branch statuses."""
+
+__metaclass__ = type
+__all__ = []
+
+import transaction
+import unittest
+
+from lp.code.windmill.testing import CodeWindmillLayer
+from lp.testing import WindmillTestCase
+from lp.testing.windmill.constants import (
+ PAGE_LOAD,
+ SLEEP,
+ )
+
+
+class TestRecipeSetDaily(WindmillTestCase):
+ """Test setting the daily build flag."""
+
+ layer = CodeWindmillLayer
+ suite_name = "Recipe daily build flag setting"
+
+ BUILD_DAILY_TEXT = u'//span[@id="edit-build_daily"]/span[@class="value"]'
+ BUILD_DAILY_POPUP = u'//div[contains(@class, "yui3-ichoicelist-content")]'
+
+ def test_inline_recipe_daily_build(self):
+ eric = self.factory.makePerson(
+ name="eric", displayname="Eric the Viking", password="test",
+ email="eric@xxxxxxxxxxx")
+ recipe = self.factory.makeSourcePackageRecipe(owner=eric)
+ transaction.commit()
+
+ client, start_url = self.getClientFor(recipe, user=eric)
+ client.click(xpath=self.BUILD_DAILY_TEXT)
+ client.waits.forElement(xpath=self.BUILD_DAILY_POPUP)
+ client.click(link=u'Build daily')
+ client.waits.sleep(milliseconds=SLEEP)
+ client.asserts.assertText(
+ xpath=self.BUILD_DAILY_TEXT, validator=u'Build daily')
+
+ # Reload the page and make sure the change has stuck.
+ client.open(url=start_url)
+ client.waits.forPageLoad(timeout=PAGE_LOAD)
+ client.asserts.assertText(
+ xpath=self.BUILD_DAILY_TEXT, validator=u'Build daily')
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2011-02-10 08:38:19 +0000
+++ lib/lp/testing/__init__.py 2011-02-11 01:53:14 +0000
@@ -158,7 +158,7 @@
from lp.testing.fixture import ZopeEventHandlerFixture
from lp.testing.karma import KarmaRecorder
from lp.testing.matchers import Provides
-from lp.testing.windmill import constants
+from lp.testing.windmill import constants, lpuser
class FakeTime:
@@ -772,6 +772,23 @@
# of things like https://launchpad.net/bugs/515494)
self.client.open(url=self.layer.appserver_root_url())
+ def getClientFor(self, obj, user=None, password='test', view_name=None):
+ """Return a new client, and the url that it has loaded."""
+ client = WindmillTestClient(self.suite_name)
+ if user is not None:
+ email = removeSecurityProxy(user).preferredemail.email
+ client.open(url=lpuser.get_basic_login_url(email, password))
+ client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+ if isinstance(obj, basestring):
+ url = obj
+ else:
+ url = canonical_url(
+ obj, view_name=view_name, force_local_path=True)
+ obj_url = self.layer.base_url + url
+ client.open(url=obj_url)
+ client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+ return client, obj_url
+
class YUIUnitTestCase(WindmillTestCase):
=== modified file 'lib/lp/testing/windmill/lpuser.py'
--- lib/lp/testing/windmill/lpuser.py 2011-02-10 04:00:00 +0000
+++ lib/lp/testing/windmill/lpuser.py 2011-02-11 01:53:14 +0000
@@ -11,6 +11,14 @@
from lp.testing.windmill import constants
+def get_basic_login_url(email, password):
+ """Return the constructed url to login a user."""
+ base_url = windmill.settings['TEST_URL']
+ basic_auth_url = base_url.replace('http://', 'http://%s:%s@')
+ basic_auth_url = basic_auth_url + '+basiclogin'
+ return basic_auth_url % (email, password)
+
+
class LaunchpadUser:
"""Object representing well-known user on Launchpad."""
@@ -32,10 +40,7 @@
current_url = client.commands.execJS(
code='windmill.testWin().location;')['result']['href']
- base_url = windmill.settings['TEST_URL']
- basic_auth_url = base_url.replace('http://', 'http://%s:%s@')
- basic_auth_url = basic_auth_url + '+basiclogin'
- client.open(url=basic_auth_url % (self.email, self.password))
+ client.open(url=get_basic_login_url(self.email, self.password))
client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
client.open(url=current_url)
client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
@@ -63,8 +68,7 @@
def login_person(person, email, password, client):
"""Create a LaunchpadUser for a person and password."""
- user = LaunchpadUser(
- person.displayname, email, password)
+ user = LaunchpadUser(person.displayname, email, password)
user.ensure_login(client)
