Merge lp:~thumper/launchpad/choice-widget into lp:launchpad

[Tim Penhey]

[r=gmb][bug=731629] Create an EnumChoiceWidget and use it on the branch page.

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.

[Graham Binns (gmb)]

review: Approve (code)

=== 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-09 21:29:50 +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-09 21:29:50 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = [
     'BooleanChoiceWidget',
+    'EnumChoiceWidget',
     'InlineEditPickerWidget',
     'standard_text_html_representation',
     'TextAreaEditorWidget',
@@ -21,6 +22,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 +408,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/doc/lazr-js-widgets.txt'
--- lib/lp/app/doc/lazr-js-widgets.txt	2011-03-01 04:48:10 +0000
+++ lib/lp/app/doc/lazr-js-widgets.txt	2011-03-09 21:29:50 +0000
@@ -326,3 +326,60 @@
 
 The edit link can be changed in exactly the same way as for the
 TextLineEditorWidget above.
+
+
+EnumChoiceWidget
+----------------
+
+This widget is primarily there allow setting of enum fields easily
+using the popup chooser.
+
+    >>> 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.
+
+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
+the enum values (the upper-case bit) with a defined prefix.  This prefix
+is passed in to the constructor of the widget.
+
+A list of items is generated from the vocabulary.
+
+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.code.interfaces.branch import IBranch
+    >>> branch = factory.makeAnyBranch()
+    >>> widget = EnumChoiceWidget(
+    ...     branch, IBranch['lifecycle_status'],
+    ...     header='Change status to', css_class_prefix='branchstatus')
+    >>> print widget()
+    <span id="edit-lifecycle_status">
+       <span class="value branchstatusDEVELOPMENT">Development</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(branch.owner)
+    >>> print widget()
+    <span id="edit-lifecycle_status">
+        <span class="value branchstatusDEVELOPMENT">Development</span>
+        <span>&nbsp;
+          <a class="editicon sprite edit" href="http://.../+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.

=== 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-09 21:29:50 +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-09 21:29:50 +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">&nbsp;
+    <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-03-03 05:18:26 +0000
+++ lib/lp/code/browser/branch.py	2011-03-09 21:29:50 +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/stories/branches/xx-branch-edit.txt'
--- lib/lp/code/stories/branches/xx-branch-edit.txt	2010-10-03 15:30:06 +0000
+++ lib/lp/code/stories/branches/xx-branch-edit.txt	2011-03-09 21:29:50 +0000
@@ -111,7 +111,7 @@
     'http://code.launchpad.dev/~name12/gnome-terminal/klingon'
 
     >>> contents = browser.contents
-    >>> status_tag = find_tag_by_id(contents, 'branch-details-status-value')
+    >>> status_tag = find_tag_by_id(contents, 'edit-lifecycle_status')
     >>> print extract_text(status_tag)
     Merged
 

=== 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-09 21:29:50 +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-09 21:29:50 +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" />&nbsp;
-        <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-09 21:29:50 +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, "[e-mail]", "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)