launchpad-reviewers team mailing list archive

["j.c.sackett" <jonathan.sackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/button-configs-break-pickers into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/button-configs-break-pickers/+merge/66946

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.
-- 
https://code.launchpad.net/~jcsackett/launchpad/button-configs-break-pickers/+merge/66946
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/button-configs-break-pickers into lp:launchpad.

=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py	2011-06-28 16:55:25 +0000
+++ lib/lp/app/widgets/popup.py	2011-07-05 18:13:32 +0000
@@ -31,8 +31,8 @@
     # 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
+    show_assign_me_button = 'false'
+    show_remove_button = 'false'
 
     popup_name = 'popup-vocabulary-picker'
 
@@ -164,8 +164,8 @@
 class PersonPickerWidget(VocabularyPickerWidget):
 
     include_create_team_link = False
-    show_assign_me_button = True
-    show_remove_button = True
+    show_assign_me_button = 'true'
+    show_remove_button = 'true'
 
     @property
     def picker_type(self):

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2011-06-28 11:26:36 +0000
+++ lib/lp/registry/browser/team.py	2011-07-05 18:13:32 +0000
@@ -1035,9 +1035,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 macro.
     custom_widget(
         'newmember', PersonPickerWidget,
-        show_assign_me_button=False, show_remove_button=False)
+        show_assign_me_button='false', show_remove_button='false')
 
     @property
     def page_title(self):