launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02888
[Merge] lp:~thumper/launchpad/choice-widget into lp:launchpad
Tim Penhey has proposed merging lp:~thumper/launchpad/choice-widget into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~thumper/launchpad/choice-widget/+merge/52357
Call this a tech-debt removal branch, in that we have too much
specific javascript for simply updating an enum field.
A new widget is created to our bucket of widgets: EnumChoiceWidget.
As a proof of concept, the branch status update is changed to
use this new widget.
--
https://code.launchpad.net/~thumper/launchpad/choice-widget/+merge/52357
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thumper/launchpad/choice-widget into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
--- lib/canonical/launchpad/icing/style-3-0.css.in 2011-03-03 18:56:00 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css.in 2011-03-07 03:24:16 +0000
@@ -1618,7 +1618,7 @@
}
.branchstatusMERGED, .branchstatusMERGED a,
.branchstatusABANDONED, .branchstatusABANDONED a {
- color: #gray;
+ color: gray;
}
.branchstatusNEW, .branchstatusNEW a {
color: black;
=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py 2011-02-10 01:57:55 +0000
+++ lib/lp/app/browser/lazrjs.py 2011-03-07 03:24:16 +0000
@@ -21,6 +21,7 @@
from zope.schema.interfaces import IVocabulary
from zope.schema.vocabulary import getVocabularyRegistry
+from lazr.enum import IEnumeratedType
from lazr.restful.declarations import LAZR_WEBSERVICE_EXPORTED
from canonical.lazr.utils import get_current_browser_request
from canonical.lazr.utils import safe_hasattr
@@ -406,3 +407,38 @@
@property
def json_config(self):
return simplejson.dumps(self.config)
+
+
+class EnumChoiceWidget(EditableWidgetBase):
+ """A popup choice widget."""
+
+ __call__ = ViewPageTemplateFile('../templates/enum-choice-widget.pt')
+
+ def __init__(self, context, exported_field, header,
+ content_box_id=None, enum=None,
+ edit_view="+edit", edit_url=None,
+ css_class_prefix=''):
+ super(EnumChoiceWidget, self).__init__(
+ context, exported_field, content_box_id, edit_view, edit_url)
+ self.header = header
+ value = getattr(self.context, self.attribute_name)
+ self.css_class = "value %s%s" % (css_class_prefix, value.name)
+ self.value = value.title
+ if enum is None:
+ # Get the enum from the exported field.
+ enum = exported_field.vocabulary
+ if IEnumeratedType(enum, None) is None:
+ raise ValueError('%r does not provide IEnumeratedType' % enum)
+ self.items = vocabulary_to_choice_edit_items(enum, css_class_prefix)
+
+ @property
+ def config(self):
+ return dict(
+ contentBox='#'+self.content_box_id,
+ value=self.value,
+ title=self.header,
+ items=self.items)
+
+ @property
+ def json_config(self):
+ return simplejson.dumps(self.config)
=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js 2011-02-14 00:44:44 +0000
+++ lib/lp/app/javascript/choice.js 2011-03-07 03:24:16 +0000
@@ -2,18 +2,7 @@
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}});
+function hook_up_spinner(widget) {
// ChoiceSource makes assumptions about HTML in lazr-js
// that don't hold true here, so we need to do our own
// spinner icon and clear it when finished.
@@ -30,8 +19,46 @@
icon.addClass('edit');
icon.setStyle('bottom', '0px');
}, widget, '_uiClearWaiting');
+}
+
+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}});
+ hook_up_spinner(widget);
widget.render();
};
+namespace.addEnumChoice = function(config, resource_uri, attribute) {
+
+ var widget = new Y.ChoiceSource(config);
+ widget.plug({
+ fn: Y.lp.client.plugins.PATCHPlugin,
+ cfg: {
+ patch: attribute,
+ resource: resource_uri}});
+ hook_up_spinner(widget);
+ widget.on('save', function(e) {
+ var cb = widget.get('contentBox');
+ var value = widget.get('value');
+ Y.Array.each(config.items, function(item) {
+ if (item.value == value) {
+ cb.one('span').addClass(item.css_class);
+ } else {
+ cb.one('span').removeClass(item.css_class);
+ }
+ });
+ });
+ widget.render();
+}
+
}, "0.1", {"requires": ["lazr.choiceedit", "lp.client.plugins"]});
=== added file 'lib/lp/app/templates/enum-choice-widget.pt'
--- lib/lp/app/templates/enum-choice-widget.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/app/templates/enum-choice-widget.pt 2011-03-07 03:24:16 +0000
@@ -0,0 +1,17 @@
+<span tal:attributes="id view/content_box_id">
+ <span tal:attributes="class view/css_class"
+ tal:content="structure view/value" />
+ <span tal:condition="view/can_write">
+ <a tal:attributes="href view/edit_url"
+ class="editicon sprite edit"></a>
+ </span>
+</span>
+<script tal:condition="view/can_write"
+ tal:content="structure string:
+LPS.use('lp.app.choice', function(Y) {
+ Y.lp.app.choice.addEnumChoice(
+ ${view/json_config},
+ ${view/json_resource_uri},
+ ${view/json_attribute});
+});
+"/>
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2011-02-27 19:45:44 +0000
+++ lib/lp/code/browser/branch.py 2011-03-07 03:24:16 +0000
@@ -107,6 +107,7 @@
)
from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
from lp.app.errors import NotFoundError
+from lp.app.browser.lazrjs import EnumChoiceWidget
from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
from lp.app.widgets.suggestion import TargetBranchWidget
from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch
@@ -668,17 +669,11 @@
return list(self.context.getProductSeriesPushingTranslations())
@property
- def status_config(self):
+ def status_widget(self):
"""The config to configure the ChoiceSource JS widget."""
- return simplejson.dumps({
- 'status_widget_items': vocabulary_to_choice_edit_items(
- BranchLifecycleStatus,
- css_class_prefix='branchstatus'),
- 'status_value': self.context.lifecycle_status.title,
- 'user_can_edit_status': check_permission(
- 'launchpad.Edit', self.context),
- 'branch_path': '/' + self.context.unique_name,
- })
+ return EnumChoiceWidget(
+ self.context.branch, IBranch['lifecycle_status'],
+ header='Change status to', css_class_prefix='branchstatus')
class BranchInProductView(BranchView):
=== removed file 'lib/lp/code/javascript/branch.status.js'
--- lib/lp/code/javascript/branch.status.js 2010-07-15 10:55:27 +0000
+++ lib/lp/code/javascript/branch.status.js 1970-01-01 00:00:00 +0000
@@ -1,49 +0,0 @@
-/* Copyright 2009 Canonical Ltd. This software is licensed under the
- * GNU Affero General Public License version 3 (see the file LICENSE).
- *
- * Code for handling the update of the branch status.
- *
- * @module lp.code.branchstatus
- * @requires node, lazr.choiceedit, lp.client.plugins
- */
-
-YUI.add('lp.code.branch.status', function(Y) {
-
-var namespace = Y.namespace('lp.code.branch.status');
-
-/*
- * Connect the branch status to the javascript events.
- */
-namespace.connect_status = function(conf) {
-
- var status_content = Y.one('#branch-details-status-value');
-
- if (conf.user_can_edit_status) {
- var status_choice_edit = new Y.ChoiceSource({
- contentBox: status_content,
- value: conf.status_value,
- title: 'Change status to',
- items: conf.status_widget_items});
- status_choice_edit.showError = function(err) {
- Y.lp.app.errors.display_error(null, err);
- };
- status_choice_edit.on('save', function(e) {
- var cb = status_choice_edit.get('contentBox');
- Y.Array.each(conf.status_widget_items, function(item) {
- if (item.value == status_choice_edit.get('value')) {
- cb.one('span').addClass(item.css_class);
- } else {
- cb.one('span').removeClass(item.css_class);
- }
- });
- });
- status_choice_edit.plug({
- fn: Y.lp.client.plugins.PATCHPlugin,
- cfg: {
- patch: 'lifecycle_status',
- resource: conf.branch_path}});
- status_choice_edit.render();
- }
-};
-
-}, "0.1", {"requires": ["node", "lazr.choiceedit", "lp.client.plugins"]});
=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt 2011-02-23 22:32:15 +0000
+++ lib/lp/code/templates/branch-index.pt 2011-03-07 03:24:16 +0000
@@ -43,8 +43,6 @@
contentBox: '#portlet-subscribers'
});
subscription_portlet.render();
-
- Y.lp.code.branch.status.connect_status(${view/status_config});
}
Y.lp.code.branchmergeproposal.diff.connect_diff_links();
}, window);
=== modified file 'lib/lp/code/templates/branch-information.pt'
--- lib/lp/code/templates/branch-information.pt 2010-10-19 02:20:08 +0000
+++ lib/lp/code/templates/branch-information.pt 2011-03-07 03:24:16 +0000
@@ -22,15 +22,7 @@
<dl id="status">
<dt>Status:</dt>
- <dd>
- <span id="branch-details-status-value">
- <span tal:attributes="class string:value branchstatus${context/lifecycle_status/name}"
- tal:content="structure context/lifecycle_status/title" />
- <a href="+edit-status" tal:condition="context/required:launchpad.Edit">
- <img class="editicon" src="/@@/edit"/>
- </a>
- </span>
- </dd>
+ <dd tal:content="structure view/status_widget" />
</dl>
</div>
=== modified file 'lib/lp/code/windmill/tests/test_branch_status.py'
--- lib/lp/code/windmill/tests/test_branch_status.py 2011-02-10 04:00:00 +0000
+++ lib/lp/code/windmill/tests/test_branch_status.py 2011-03-07 03:24:16 +0000
@@ -6,19 +6,15 @@
__metaclass__ = type
__all__ = []
-import unittest
-
import transaction
-import windmill
-
+
+from storm.store import Store
+
+from lp.code.enums import BranchLifecycleStatus
+from lp.code.model.branch import Branch
from lp.code.windmill.testing import CodeWindmillLayer
from lp.testing import WindmillTestCase
-from lp.testing.windmill.constants import (
- FOR_ELEMENT,
- PAGE_LOAD,
- SLEEP,
- )
-from lp.testing.windmill.lpuser import login_person
+from lp.testing.windmill.constants import FOR_ELEMENT
class TestBranchStatus(WindmillTestCase):
@@ -37,37 +33,23 @@
client = self.client
- start_url = (
- windmill.settings['TEST_URL'] + branch.unique_name)
- client.open(url=start_url)
- client.waits.forPageLoad(timeout=PAGE_LOAD)
- login_person(eric, "eric@xxxxxxxxxxx", "test", client)
-
+ client, start_url = self.getClientFor(branch, user=eric)
# Click on the element containing the branch status.
- client.waits.forElement(
- id=u'branch-details-status-value', timeout=PAGE_LOAD)
- client.click(id=u'branch-details-status-value')
- client.waits.forElement(
- xpath=u'//div[contains(@class, "yui3-ichoicelist-content")]')
-
- # Change the status to experimental.
+ client.click(id=u'edit-lifecycle_status')
+ client.waits.forElement(
+ classname=u'yui3-ichoicelist-content', timeout=FOR_ELEMENT)
client.click(link=u'Experimental')
- client.waits.sleep(milliseconds=SLEEP)
-
- client.asserts.assertText(
- xpath=u'//span[@id="branch-details-status-value"]/span',
- validator=u'Experimental')
-
- # Reload the page and make sure the change sticks.
- client.open(url=start_url)
- client.waits.forPageLoad(timeout=PAGE_LOAD)
client.waits.forElement(
- xpath=u'//span[@id="branch-details-status-value"]/span',
+ jquery=u'("div#edit-lifecycle_status a.editicon.sprite.edit")',
timeout=FOR_ELEMENT)
- client.asserts.assertText(
- xpath=u'//span[@id="branch-details-status-value"]/span',
- validator=u'Experimental')
-
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
+ client.asserts.assertTextIn(
+ id=u'edit-lifecycle_status', validator=u'Experimental')
+ client.asserts.assertNode(
+ jquery=u'("div#edit-lifecycle_status span.value.branchstatusEXPERIMENTAL")')
+
+ transaction.commit()
+ freshly_fetched_branch = Store.of(branch).find(
+ Branch, Branch.id == branch.id).one()
+ self.assertEqual(
+ BranchLifecycleStatus.EXPERIMENTAL,
+ freshly_fetched_branch.lifecycle_status)
