Merge lp:~jcsackett/launchpad/button-configs-break-pickers into lp:launchpad

[j.c.sackett]

Summary
=======
An earlier branch that set up configuration options for views regarding whether or not to show the extra buttons actually broke pickers by passing in "True" where javascript expects "true". This branch fixes that.

This sort of issue should go away once we address bug 799847, in particular the part about using the JSONCache.

Pre-implementation notes
========================
None.

Implementation details
======================
popup.py sets two variables regarding the assign me button and the remove assignee button, but sets them using python booleans. They've been updated to strings, which are interpreted in the form-macros into the appropriate javascript booleans (e.g. 'false' instead of False).

Callsites have been updated to pass in the correct assignments for these variables.

Tests
=====
bin/test -vvc --layer=YUI

QA
==
Check that clicking the Choose fields (e.g. on launchpad.dev/evolution/+edit-people) properly opens the person picker.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/team.py
  lib/lp/app/widgets/popup.py

./lib/lp/registry/browser/team.py
     552: local variable 'mailing_list' is assigned to but never used

Not sure what to do regaring the lint above; it seems like the sort of code that may accomplish something via side effect, and I can't find good tests to double check. I'll try to suss it out prior to landing it.

=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py	2011-07-06 00:41:28 +0000
+++ lib/lp/app/browser/lazrjs.py	2011-07-06 14:16:29 +0000
@@ -238,7 +238,8 @@
 
     def __init__(self, context, exported_field, default_html,
                  content_box_id=None, header='Select an item',
-                 step_title='Search', remove_button_text='Remove',
+                 step_title='Search', assign_button_text='Pick Me',
+                 remove_button_text='Remove Person',
                  null_display_value='None',
                  edit_view="+edit", edit_url=None, edit_title=''):
         """Create a widget wrapper.
@@ -251,6 +252,7 @@
             Automatically generated if this is not provided.
         :param header: The large text at the top of the picker.
         :param step_title: Smaller line of text below the header.
+        :param assign_button_text: Override default button text: "Pick Me"
         :param remove_button_text: Override default button text: "Remove"
         :param null_display_value: This will be shown for a missing value
         :param edit_view: The view name to use to generate the edit_url if
@@ -265,6 +267,7 @@
         self.default_html = default_html
         self.header = header
         self.step_title = step_title
+        self.assign_button_text = assign_button_text
         self.remove_button_text = remove_button_text
         self.null_display_value = null_display_value
 
@@ -278,6 +281,7 @@
     def config(self):
         return dict(
             header=self.header, step_title=self.step_title,
+            assign_button_text=self.assign_button_text,
             remove_button_text=self.remove_button_text,
             null_display_value=self.null_display_value,
             show_remove_button=self.optional_field,

=== modified file 'lib/lp/app/javascript/lazr/picker/person_picker.js'
--- lib/lp/app/javascript/lazr/picker/person_picker.js	2011-07-06 03:20:46 +0000
+++ lib/lp/app/javascript/lazr/picker/person_picker.js	2011-07-06 14:16:29 +0000
@@ -23,6 +23,8 @@
 
         var show_assign_me_button = true;
         var show_remove_button = true;
+        var assign_button_text = 'Pick Me';
+        var remove_button_text = 'Remove Person';
 
         if (cfg !== undefined) {
             if (cfg.show_assign_me_button !== undefined) {
@@ -31,9 +33,17 @@
             if (cfg.show_remove_button !== undefined) {
                 show_remove_button = cfg.show_remove_button;
             }
+            if (cfg.assign_button_text !== undefined) {
+                assign_button_text = cfg.assign_button_text;
+            }
+            if (cfg.remove_button_text !== undefined) {
+                remove_button_text = cfg.remove_button_text;
+            }
         }
         this._show_assign_me_button = show_assign_me_button;
         this._show_remove_button = show_remove_button;
+        this._assign_me_button_text = assign_button_text;
+        this._remove_button_text = remove_button_text;
     },
 
     hide: function() {
@@ -57,15 +67,13 @@
 
     _renderPersonPickerUI: function() {
         var remove_button, assign_me_button;
-        var remove_button_text = "Remove assignee";
-        var assign_me_button_text = "Assign to me";
 
         if (this._show_remove_button) {
             remove_button = Y.Node.create(
                 '<a class="yui-picker-remove-button bg-image" ' +
                 'href="javascript:void(0)" ' +
                 'style="background-image: url(/@@/remove); padding-right: ' +
-                '1em">' + remove_button_text + '</a>');
+                '1em">' + this._remove_button_text + '</a>');
             remove_button.on('click', this.remove, this);
             this._extra_buttons.appendChild(remove_button);
             this.remove_button = remove_button;
@@ -76,7 +84,7 @@
                 '<a class="yui-picker-assign-me-button bg-image" ' +
                 'href="javascript:void(0)" ' +
                 'style="background-image: url(/@@/person)">' +
-                assign_me_button_text + '</a>');
+                this._assign_me_button_text + '</a>');
             assign_me_button.on('click', this.assign_me, this);
             this._extra_buttons.appendChild(assign_me_button);
             this.assign_me_button = assign_me_button;

=== modified file 'lib/lp/app/javascript/picker_patcher.js'
--- lib/lp/app/javascript/picker_patcher.js	2011-07-06 03:20:46 +0000
+++ lib/lp/app/javascript/picker_patcher.js	2011-07-06 14:16:29 +0000
@@ -21,6 +21,7 @@
  * @param {Object} config Object literal of config name/value pairs.
  *     config.header: a line of text at the top of the widget.
  *     config.step_title: overrides the subtitle.
+ *     config.assign_button_text: Override the default 'Assign' text.
  *     config.remove_button_text: Override the default 'Remove' text.
  *     config.null_display_value: Override the default 'None' text.
  *     config.show_remove_button: Should the remove button be shown?
@@ -40,17 +41,12 @@
 
     var show_remove_button = false;
     var show_assign_me_button = false;
-    var remove_button_text = 'Remove';
     var null_display_value = 'None';
     var show_search_box = true;
 
     resource_uri = Y.lp.client.normalize_uri(resource_uri);
 
     if (config !== undefined) {
-        if (config.remove_button_text !== undefined) {
-            remove_button_text = config.remove_button_text;
-        }
-
         if (config.null_display_value !== undefined) {
             null_display_value = config.null_display_value;
         }

=== modified file 'lib/lp/app/javascript/tests/test_picker.js'
--- lib/lp/app/javascript/tests/test_picker.js	2011-07-06 03:20:46 +0000
+++ lib/lp/app/javascript/tests/test_picker.js	2011-07-06 14:16:29 +0000
@@ -79,7 +79,6 @@
             {
                 "step_title": "Choose someone",
                 "header": "Pick Someone",
-                "remove_button_text": "Remove someone",
                 "null_display_value": "Noone",
                 "validate_callback": validate_callback
             });
@@ -280,7 +279,8 @@
         delete window.LP;
     },
 
-    create_picker: function(show_assign_me, show_remove, field_value) {
+    create_picker: function(
+        show_assign_me_button, show_remove_button, field_value) {
         if (field_value !== undefined) {
             var data_box = Y.one('#picker_id .yui3-activator-data-box');
             data_box.appendChild(Y.Node.create('<a>Content</a>'));
@@ -292,8 +292,10 @@
             "header": "Pick Someone",
             "validate_callback": null,
             "show_search_box": true,
-            "show_assign_me_button": show_assign_me,
-            "show_remove_button": show_remove
+            "show_assign_me_button": show_assign_me_button,
+            "show_remove_button": show_remove_button,
+            "assign_button_text": "Assign Moi",
+            "remove_button_text": "Remove someone"
             };
         this.picker = Y.lp.app.picker.addPickerPatcher(
                 this.vocabulary,
@@ -325,6 +327,22 @@
         this._check_button_state('.yui-picker-remove-button', is_visible);
     },
 
+    test_picker_assign_me_button_text: function() {
+        // The assign me button text is correct.
+        this.create_picker(true, true);
+        this.picker.render();
+        var assign_me_button = Y.one('.yui-picker-assign-me-button');
+        Assert.areEqual('Assign Moi', assign_me_button.get('innerHTML'));
+    },
+
+    test_picker_remove_button_text: function() {
+        // The remove button text is correct.
+        this.create_picker(true, true);
+        this.picker.render();
+        var remove_button = Y.one('.yui-picker-remove-button');
+        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
+    },
+
     test_picker_has_assign_me_button: function() {
         // The assign me button is shown.
         this.create_picker(true, true);

=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py	2011-07-06 00:41:28 +0000
+++ lib/lp/app/widgets/popup.py	2011-07-06 14:16:29 +0000
@@ -28,6 +28,11 @@
     __call__ = ViewPageTemplateFile('templates/form-picker.pt')
 
     picker_type = 'default'
+    # Provide default values for the following properties in case someone
+    # creates a vocab picker for a person instead if using the derived
+    # PersonPicker.
+    show_assign_me_button = 'false'
+    show_remove_button = 'false'
 
     popup_name = 'popup-vocabulary-picker'
 
@@ -159,6 +164,8 @@
 class PersonPickerWidget(VocabularyPickerWidget):
 
     include_create_team_link = False
+    show_assign_me_button = 'true'
+    show_remove_button = 'true'
 
     @property
     def picker_type(self):

=== modified file 'lib/lp/app/widgets/templates/form-picker-macros.pt'
--- lib/lp/app/widgets/templates/form-picker-macros.pt	2011-07-06 03:20:46 +0000
+++ lib/lp/app/widgets/templates/form-picker-macros.pt	2011-07-06 14:16:29 +0000
@@ -43,6 +43,10 @@
                 extra_no_results_message: ${view/extra_no_results_message},
                 picker_type: '${view/picker_type}'
             };
+            if (config.picker_type == 'person') {
+                config.show_assign_me_button = ${view/show_assign_me_button}
+                config.show_remove_button = ${view/show_remove_button}
+            }
             var picker = Y.lp.app.picker.create('${view/vocabulary_name}', config);
             if (config.extra_no_results_message !== null) {
                 picker.before('resultsChange', function (e) {

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-07-06 00:41:28 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-07-06 14:16:29 +0000
@@ -894,6 +894,7 @@
             assignee_content.get('id'),
             {"step_title": step_title,
              "header": "Change assignee",
+             "assign_button_text": "Assign Me",
              "remove_button_text": "Remove assignee",
              "null_display_value": "Unassigned",
              "show_remove_button": conf.user_can_unassign,

=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py	2011-07-06 00:41:28 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py	2011-07-06 14:16:29 +0000
@@ -106,6 +106,7 @@
 from lp.code.model.branchtarget import PersonBranchTarget
 from lp.code.model.sourcepackagerecipe import get_buildable_distroseries_set
 from lp.registry.interfaces.series import SeriesStatus
+from lp.services.fields import PersonChoice
 from lp.services.propertycache import cachedproperty
 from lp.soyuz.model.archive import Archive
 
@@ -840,7 +841,7 @@
             self.form_fields = self.form_fields.omit('daily_build_archive')
 
             owner_field = self.schema['owner']
-            any_owner_choice = Choice(
+            any_owner_choice = PersonChoice(
                 __name__='owner', title=owner_field.title,
                 description=(u"As an administrator you are able to reassign"
                              u" this branch to any person or team."),

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2011-07-06 00:41:28 +0000
+++ lib/lp/registry/browser/team.py	2011-07-06 14:16:29 +0000
@@ -70,6 +70,7 @@
     LaunchpadRadioWidgetWithDescription,
     )
 from lp.app.widgets.owner import HiddenUserWidget
+from lp.app.widgets.popup import PersonPickerWidget
 from lp.registry.browser.branding import BrandingChangeView
 from lp.registry.interfaces.mailinglist import (
     IMailingList,
@@ -983,8 +984,7 @@
             failed_names = [person.displayname for person in failed_joins]
             failed_list = ", ".join(failed_names)
 
-            mapping=dict(
-                this_team=target_team.displayname,
+            mapping = dict( this_team=target_team.displayname,
                 failed_list=failed_list)
 
             if len(failed_joins) == 1:
@@ -1034,6 +1034,13 @@
 
     schema = ITeamMember
     label = "Select the new member"
+    # XXX: jcsackett 5.7.2011 The assignment of 'false' to the vars below
+    # should be changed to the more appropriate False bool when we're making
+    # use of the JSON cache to setup pickers, rather than assembling
+    # javascript in a a view hacro.
+    custom_widget(
+        'newmember', PersonPickerWidget,
+        show_assign_me_button='false', show_remove_button='false')
 
     @property
     def page_title(self):