launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04280
[Merge] lp:~wallyworld/launchpad/picker-widget-meta into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/picker-widget-meta into lp:launchpad with lp:~wallyworld/launchpad/vocab-meta as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/picker-widget-meta/+merge/67961
This branch is 2 of 3. Once reviewed, I will be landing the 3rd branch only.
The end goal is to dynamically update the "Remove" link text on the person picker to say "Remove team" is the current field value is a team and "Remove person" if the current field value is a person. The text can be specified in the picker config if required, as is the case for the bug assignee picker.
== Implementation ==
This branch changes the picker widget implementations is popup.py and lazr.py to use the new picker meta attribute. It also does a little yak shaving to make some parameters more sensible and to clean up some javascript in a page template.
The widgets affected are InlineEditPickerWidget and VocabularyPickerWidget, PersonPickerWidget. Essentially, they have been changed to output in the json config passed to the javascript, the "selected_value_meta" for the current value of the widget when the page is loaded. This allows the javascript to correctly set the initial value of the "Remove" text. A new subclass of InlineEditPickerWidget was created - InlinePersonEditPickerWidget. Any existing person fields (found in blueprints and source package recipes) were updated to use this new widget.
The config parameters used to override the text used for the "assign me" and "remove" links on the person picker are now:
- assign_me_text
- remove_person_text
- remove_team_text
== Tests ==
Add tests to TestVocaularyPickerEidget:
test_widget_personvalue_meta
test_widget_teamvalue_meta
Add TestInlinePersonEditPickerWidget to test_inlineeditpickerwidget:
test_person_selected_value_meta
test_team_selected_value_meta
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/browser/lazrjs.py
lib/lp/app/browser/tests/test_inlineeditpickerwidget.py
lib/lp/app/widgets/popup.py
lib/lp/app/widgets/templates/form-picker-macros.pt
lib/lp/app/widgets/tests/test_popup.py
lib/lp/blueprints/browser/specification.py
lib/lp/code/browser/sourcepackagerecipe.py
./lib/lp/blueprints/browser/specification.py
203: E251 no spaces around keyword / parameter equals
--
https://code.launchpad.net/~wallyworld/launchpad/picker-widget-meta/+merge/67961
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/picker-widget-meta into lp:launchpad.
=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py 2011-07-06 14:13:04 +0000
+++ lib/lp/app/browser/lazrjs.py 2011-07-15 03:16:31 +0000
@@ -8,6 +8,7 @@
'BooleanChoiceWidget',
'EnumChoiceWidget',
'InlineEditPickerWidget',
+ 'InlinePersonEditPickerWidget',
'InlineMultiCheckboxWidget',
'standard_text_html_representation',
'TextAreaEditorWidget',
@@ -35,6 +36,7 @@
from canonical.launchpad.webapp.publisher import canonical_url
from canonical.launchpad.webapp.vocabulary import IHugeVocabulary
from lp.app.browser.stringformatter import FormattersAPI
+from lp.app.browser.vocabulary import get_person_picker_entry_meta
from lp.services.propertycache import cachedproperty
@@ -238,8 +240,9 @@
def __init__(self, context, exported_field, default_html,
content_box_id=None, header='Select an item',
- step_title='Search', assign_button_text='Pick Me',
- remove_button_text='Remove Person',
+ step_title='Search', assign_me_text='Pick me',
+ remove_person_text='Remove person',
+ remove_team_text='Remove team',
null_display_value='None',
edit_view="+edit", edit_url=None, edit_title=''):
"""Create a widget wrapper.
@@ -252,8 +255,9 @@
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 assign_me_text: Override default button text: "Pick me"
+ :param remove_person_text: Override default link text: "Remove person"
+ :param remove_team_text: Override default link text: "Remove team"
: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
one is not specified.
@@ -267,8 +271,9 @@
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.assign_me_text = assign_me_text
+ self.remove_person_text = remove_person_text
+ self.remove_team_text = remove_team_text
self.null_display_value = null_display_value
# JSON encoded attributes.
@@ -278,11 +283,17 @@
self.exported_field.vocabularyName)
@property
+ def selected_value_meta(self):
+ return None
+
+ @property
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,
+ selected_value_meta=self.selected_value_meta,
+ assign_me_text=self.assign_me_text,
+ remove_person_text=self.remove_person_text,
+ remove_team_text=self.remove_team_text,
null_display_value=self.null_display_value,
show_remove_button=self.optional_field,
show_assign_me_button=self.show_assign_me_button,
@@ -310,6 +321,13 @@
return user and user in vocabulary
+class InlinePersonEditPickerWidget(InlineEditPickerWidget):
+ @property
+ def selected_value_meta(self):
+ val = getattr(self.context, self.exported_field.__name__)
+ return get_person_picker_entry_meta(val)
+
+
class InlineMultiCheckboxWidget(WidgetBase):
"""Wrapper for the lazr-js multicheckbox widget."""
@@ -358,7 +376,7 @@
linkify_items = attribute_type == "reference"
if header is None:
- header = self.exported_field.title+":"
+ header = self.exported_field.title + ":"
self.header = header,
self.empty_display_value = empty_display_value
self.label = label
@@ -366,7 +384,7 @@
self.label_close_tag = "</%s>" % label_tag
self.items = selected_items
self.items_open_tag = ("<%s id='%s'>" %
- (items_tag, self.content_box_id+"-items"))
+ (items_tag, self.content_box_id + "-items"))
self.items_close_tag = "</%s>" % items_tag
self.linkify_items = linkify_items
@@ -376,7 +394,6 @@
else:
vocabulary = exported_field.vocabularyName
-
if isinstance(vocabulary, basestring):
vocabulary = getVocabularyRegistry().get(context, vocabulary)
@@ -522,7 +539,7 @@
@property
def config(self):
return dict(
- contentBox='#'+self.content_box_id,
+ contentBox='#' + self.content_box_id,
value=self.current_value,
title=self.header,
items=[
@@ -578,7 +595,7 @@
@property
def config(self):
return dict(
- contentBox='#'+self.content_box_id,
+ contentBox='#' + self.content_box_id,
value=self.value,
title=self.header,
items=self.items)
=== modified file 'lib/lp/app/browser/tests/test_inlineeditpickerwidget.py'
--- lib/lp/app/browser/tests/test_inlineeditpickerwidget.py 2011-03-10 07:13:39 +0000
+++ lib/lp/app/browser/tests/test_inlineeditpickerwidget.py 2011-07-15 03:16:31 +0000
@@ -2,6 +2,7 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the InlineEditPickerWidget."""
+from zope.interface.declarations import implements
__metaclass__ = type
@@ -9,7 +10,10 @@
from zope.schema import Choice
from canonical.testing.layers import DatabaseFunctionalLayer
-from lp.app.browser.lazrjs import InlineEditPickerWidget
+from lp.app.browser.lazrjs import (
+ InlineEditPickerWidget,
+ InlinePersonEditPickerWidget,
+ )
from lp.testing import (
login_person,
TestCaseWithFactory,
@@ -33,8 +37,8 @@
self.assertTrue(widget.config['show_search_box'])
def test_normal_vocabulary_is_not_searchable(self):
- # Make sure that when given a field for a normal vocabulary, the picker
- # is set to show the search box.
+ # Make sure that when given a field for a normal vocabulary, the
+ # picker is set to show the search box.
widget = self.getWidget(vocabulary='UserTeamsParticipation')
self.assertFalse(widget.config['show_search_box'])
@@ -56,3 +60,34 @@
widget = self.getWidget(vocabulary='TargetPPAs', required=True)
login_person(self.factory.makePerson())
self.assertFalse(widget.config['show_assign_me_button'])
+
+
+class TestInlinePersonEditPickerWidget(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def getWidget(self, widget_value, **kwargs):
+ class ITest(Interface):
+ test_field = Choice(**kwargs)
+
+ class Test:
+ implements(ITest)
+
+ def __init__(self):
+ self.test_field = widget_value
+
+ context = Test()
+ return InlinePersonEditPickerWidget(
+ context, ITest['test_field'], None, edit_url='fake')
+
+ def test_person_selected_value_meta(self):
+ # The widget has the correct meta value for a person value.
+ widget_value = self.factory.makePerson()
+ widget = self.getWidget(widget_value, vocabulary='ValidPersonOrTeam')
+ self.assertEquals('person', widget.config['selected_value_meta'])
+
+ def test_team_selected_value_meta(self):
+ # The widget has the correct meta value for a team value.
+ widget_value = self.factory.makeTeam()
+ widget = self.getWidget(widget_value, vocabulary='ValidPersonOrTeam')
+ self.assertEquals('team', widget.config['selected_value_meta'])
=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py 2011-07-06 18:20:57 +0000
+++ lib/lp/app/widgets/popup.py 2011-07-15 03:16:31 +0000
@@ -18,6 +18,7 @@
from canonical.launchpad.webapp import canonical_url
from lp.app.browser.stringformatter import FormattersAPI
+from lp.app.browser.vocabulary import get_person_picker_entry_meta
from lp.services.features import getFeatureFlag
from lp.services.propertycache import cachedproperty
@@ -33,6 +34,9 @@
# PersonPicker.
show_assign_me_button = 'false'
show_remove_button = 'false'
+ assign_me_text = 'Pick me'
+ remove_person_text = 'Remove person'
+ remove_team_text = 'Remove team'
popup_name = 'popup-vocabulary-picker'
@@ -101,10 +105,31 @@
class="%(cssClass)s" />""" % d
@property
+ def selected_value_meta(self):
+ return None
+
+ @property
def show_widget_id(self):
return 'show-widget-%s' % self.input_id.replace('.', '-')
@property
+ def config(self):
+ return dict(
+ picker_type=self.picker_type,
+ selected_value_meta=self.selected_value_meta,
+ header=self.header_text, step_title=self.step_title_text,
+ extra_no_results_message=self.extra_no_results_message,
+ assign_me_text=self.assign_me_text,
+ remove_person_text=self.remove_person_text,
+ remove_team_text=self.remove_team_text,
+ show_remove_button=self.show_remove_button,
+ show_assign_me_button=self.show_assign_me_button)
+
+ @property
+ def json_config(self):
+ return simplejson.dumps(self.config)
+
+ @property
def extra_no_results_message(self):
"""Extra message when there are no results.
@@ -177,6 +202,11 @@
picker_type = 'default'
return picker_type
+ @property
+ def selected_value_meta(self):
+ val = self._getFormValue()
+ return get_person_picker_entry_meta(val)
+
def chooseLink(self):
link = super(PersonPickerWidget, self).chooseLink()
if self.include_create_team_link:
=== modified file 'lib/lp/app/widgets/templates/form-picker-macros.pt'
--- lib/lp/app/widgets/templates/form-picker-macros.pt 2011-07-06 14:13:04 +0000
+++ lib/lp/app/widgets/templates/form-picker-macros.pt 2011-07-15 03:16:31 +0000
@@ -37,16 +37,7 @@
// The vocabulary picker, created when used for the first time.
function make_picker() {
- var config = {
- header: ${view/header_text},
- step_title: ${view/step_title_text},
- 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 config = ${view/json_config};
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/app/widgets/tests/test_popup.py'
--- lib/lp/app/widgets/tests/test_popup.py 2011-07-06 18:41:07 +0000
+++ lib/lp/app/widgets/tests/test_popup.py 2011-07-15 03:16:31 +0000
@@ -59,14 +59,15 @@
picker_widget = VocabularyPickerWidget(
bound_field, self.vocabulary, self.request)
+ widget_config = simplejson.loads(picker_widget.json_config)
self.assertEqual(
'ValidTeamOwner', picker_widget.vocabulary_name)
self.assertEqual(
simplejson.dumps(self.vocabulary.displayname),
- picker_widget.header_text)
+ widget_config['header'])
self.assertEqual(
simplejson.dumps(self.vocabulary.step_title),
- picker_widget.step_title_text)
+ widget_config['step_title'])
self.assertEqual(
'show-widget-field-test_valid-item', picker_widget.show_widget_id)
self.assertEqual(
@@ -124,11 +125,35 @@
# A vocabulary widget does not show the extra buttons by default.
picker_widget = VocabularyPickerWidget(
bound_field, self.vocabulary, self.request)
- self.assertEqual('false', picker_widget.show_assign_me_button)
- self.assertEqual('false', picker_widget.show_remove_button)
+ self.assertEqual('false',
+ picker_widget.config['show_assign_me_button'])
+ self.assertEqual('false',
+ picker_widget.config['show_assign_me_button'])
# A person picker widget does show them by default.
person_picker_widget = PersonPickerWidget(
bound_field, self.vocabulary, self.request)
- self.assertEqual('true', person_picker_widget.show_assign_me_button)
- self.assertEqual('true', person_picker_widget.show_remove_button)
+ self.assertEqual('true',
+ person_picker_widget.config['show_assign_me_button'])
+ self.assertEqual('true',
+ person_picker_widget.config['show_assign_me_button'])
+
+ def test_widget_personvalue_meta(self):
+ # The person picker has the correct meta value for a person value.
+ person = self.factory.makePerson()
+ bound_field = ITest['test_valid.item'].bind(person)
+ person_picker_widget = PersonPickerWidget(
+ bound_field, self.vocabulary, self.request)
+ person_picker_widget.setRenderedValue(person)
+ self.assertEqual('person',
+ person_picker_widget.config['selected_value_meta'])
+
+ def test_widget_teamvalue_meta(self):
+ # The person picker has the correct meta value for a team value.
+ team = self.factory.makeTeam()
+ bound_field = ITest['test_valid.item'].bind(team)
+ person_picker_widget = PersonPickerWidget(
+ bound_field, self.vocabulary, self.request)
+ person_picker_widget.setRenderedValue(team)
+ self.assertEqual('team',
+ person_picker_widget.config['selected_value_meta'])
=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py 2011-06-21 01:08:52 +0000
+++ lib/lp/blueprints/browser/specification.py 2011-07-15 03:16:31 +0000
@@ -99,7 +99,7 @@
from lp.app.browser.lazrjs import (
BooleanChoiceWidget,
EnumChoiceWidget,
- InlineEditPickerWidget,
+ InlinePersonEditPickerWidget,
TextAreaEditorWidget,
TextLineEditorWidget,
)
@@ -576,7 +576,7 @@
@property
def approver_widget(self):
- return InlineEditPickerWidget(
+ return InlinePersonEditPickerWidget(
self.context, ISpecification['approver'],
format_link(self.context.approver),
header='Change approver', edit_view='+people',
@@ -584,7 +584,7 @@
@property
def drafter_widget(self):
- return InlineEditPickerWidget(
+ return InlinePersonEditPickerWidget(
self.context, ISpecification['drafter'],
format_link(self.context.drafter),
header='Change drafter', edit_view='+people',
@@ -592,7 +592,7 @@
@property
def assignee_widget(self):
- return InlineEditPickerWidget(
+ return InlinePersonEditPickerWidget(
self.context, ISpecification['assignee'],
format_link(self.context.assignee),
header='Change assignee', edit_view='+people',
=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2011-07-08 15:17:42 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2011-07-15 03:16:31 +0000
@@ -84,6 +84,7 @@
from lp.app.browser.lazrjs import (
BooleanChoiceWidget,
InlineEditPickerWidget,
+ InlinePersonEditPickerWidget,
TextAreaEditorWidget,
TextLineEditorWidget,
)
@@ -201,7 +202,7 @@
has_upload = ppa.checkArchivePermission(recipe.owner)
show_request_build = has_upload
- show_request_build= (show_request_build and
+ show_request_build = (show_request_build and
check_permission('launchpad.Edit', recipe))
return Link(
'+request-daily-build', 'Build now',
@@ -262,12 +263,12 @@
enhanced_picker_enabled = bool(
getFeatureFlag('disclosure.picker_enhancements.enabled'))
if enhanced_picker_enabled:
- vocabulary='UserTeamsParticipationPlusSelfSimpleDisplay'
+ vocabulary = 'UserTeamsParticipationPlusSelfSimpleDisplay'
else:
- vocabulary='UserTeamsParticipationPlusSelf'
+ vocabulary = 'UserTeamsParticipationPlusSelf'
field = copy_field(
ISourcePackageRecipe['owner'], vocabularyName=vocabulary)
- return InlineEditPickerWidget(
+ return InlinePersonEditPickerWidget(
self.context, field,
format_link(self.context.owner),
header='Change owner',
