Ian Booth has proposed merging 
lp:~wallyworld/launchpad/personpicker-remove-assignee-link-2 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #800654 in Launchpad itself: "Assign Me link on person picker visible 
when it should be hidden"
  https://bugs.launchpad.net/launchpad/+bug/800654

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/personpicker-remove-assignee-link-2/+merge/65487

A fix was done for bug 798775 which hid the "Remove Me" button when the field 
value was empty. The fix involved correcting the css class name used to hide 
the button. The same fix was done for the "Assign Me" button. However, a 
separate, unrelated logic error in the if statement used to determine if the 
"Assign Me" should be hidden meant that the "Assign Me" button is always visible

The tests written for the last fix were also flawed and have been fixed here 
also.

== Implementation ==

Use the correct check to see if the "Assign Me" button should be hidden.

== Tests ==

Fix the test_picker.js tests.
-- 
https://code.launchpad.net/~wallyworld/launchpad/personpicker-remove-assignee-link-2/+merge/65487
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~wallyworld/launchpad/personpicker-remove-assignee-link-2 into lp:launchpad.
=== modified file 'lib/lp/app/javascript/picker.js'
--- lib/lp/app/javascript/picker.js	2011-06-21 15:00:03 +0000
+++ lib/lp/app/javascript/picker.js	2011-06-22 12:28:30 +0000
@@ -97,7 +97,7 @@
 
         if (assign_me_button) {
             if (link !== null
-                && link.get('href').indexOf(LP.links.me + '/') !== -1) {
+                && link.get('href').match(LP.links.me+"$")==LP.links.me) {
                 assign_me_button.addClass('yui3-picker-hidden');
             } else {
                 assign_me_button.removeClass('yui3-picker-hidden');

=== modified file 'lib/lp/app/javascript/tests/test_picker.js'
--- lib/lp/app/javascript/tests/test_picker.js	2011-06-21 15:00:03 +0000
+++ lib/lp/app/javascript/tests/test_picker.js	2011-06-22 12:28:30 +0000
@@ -242,13 +242,13 @@
                 "description": "description-" + i,
                 "api_uri": "~/fred-" + i};
         }
+        this.ME = '/~me';
         window.LP = {
             links: {
-                me: '/~me'
+                me: this.ME
             },
             cache: {}
         };
-        this.ME_HREF = '/~me/';
 
         // We patch Launchpad client to return some fake data for the patch
         // operation.
@@ -258,7 +258,7 @@
                 // our setup assumes success, so we just do the success
                 // callback.
                 var entry_repr = {
-                  'lp_html': {'test_link': '<a href="/~me/">Content</a>'}
+                  'lp_html': {'test_link': '<a href="/~me">Content</a>'}
                 };
                 var result = new Y.lp.client.Entry(
                     null, entry_repr, "a_self_link");
@@ -335,7 +335,7 @@
     test_picker_no_assign_me_button_if_value_is_me: function() {
         // The assign me button is not shown if the picker is created for a
         // field where the value is "me".
-        this.create_picker(true, true, this.ME_HREF);
+        this.create_picker(true, true, this.ME);
         this.picker.render();
         this._check_assign_me_button_state(false);
     },
@@ -364,7 +364,7 @@
     test_picker_has_remove_button_if_value: function() {
         // The remove button is shown if the picker is created for a field
         // which has a value.
-        this.create_picker(true, true, this.ME_HREF);
+        this.create_picker(true, true, this.ME);
         this.picker.render();
         this._check_remove_button_state(true);
     },
@@ -372,7 +372,7 @@
     test_picker_no_remove_button_unless_configured: function() {
         // The remove button is only rendered if show_remove_button setting is
         // true.
-        this.create_picker(true, false, this.ME_HREF);
+        this.create_picker(true, false, this.ME);
         this.picker.render();
         Assert.isNull(Y.one('.yui-picker-remove-button'));
     },
@@ -380,7 +380,7 @@
     test_picker_remove_button_clicked: function() {
         // The remove button is hidden once a picker value has been removed.
         // And the assign me button is shown.
-        this.create_picker(true, true, this.ME_HREF);
+        this.create_picker(true, true, this.ME);
         this.picker.render();
         var remove = Y.one('.yui-picker-remove-button');
         remove.simulate('click');

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to