launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/picker-2char-search-term-768148 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #768148 in Launchpad itself: "person picker does not handle exact matches well - including not permitting exact matches for usernames < 3 characters long."
  https://bugs.launchpad.net/launchpad/+bug/768148

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/picker-2char-search-term-768148/+merge/75936

A small change to allow person picker to be used to search for Launchpad users with 2 character user names.

== Implementation ==

For person picker instances, set the min_search_chars attribute to 2.
I thought about modifying the ValidPersonOrTeam vocab to limit the search to just exact username matches if the search term is only 2 characters. This would avoid lots of "bogus" matches from fti etc due to the shortness of the search term. However, this would lead to surprises for the user and with the ranking of offered by the new picker search functionality, exact username matches appear first so it's easy to find and select what one is looking for.


== Tests ==

Add new yui test to test_personpicker.js
  - test_min_search_chars

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/picker/person_picker.js
  lib/lp/app/javascript/picker/tests/test_personpicker.js

-- 
https://code.launchpad.net/~wallyworld/launchpad/picker-2char-search-term-768148/+merge/75936
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/picker-2char-search-term-768148 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js	2011-08-10 13:33:26 +0000
+++ lib/lp/app/javascript/picker/person_picker.js	2011-09-19 00:43:26 +0000
@@ -26,6 +26,7 @@
         var assign_me_text = 'Pick me';
         var remove_person_text = 'Remove person';
         var remove_team_text = 'Remove team';
+        var min_search_chars = 2;
         if (cfg !== undefined) {
             if (cfg.show_assign_me_button !== undefined) {
                 show_assign_me_button = cfg.show_assign_me_button;
@@ -42,7 +43,12 @@
             if (cfg.remove_team_text !== undefined) {
                 remove_team_text = cfg.remove_team_text;
             }
+            if (cfg.min_search_chars !== undefined) {
+                min_search_chars = cfg.min_search_chars;
+            }
         }
+        this.set('min_search_chars', min_search_chars);
+
         /* Only show assign-me when the user is logged-in. */
         show_assign_me_button = (
             show_assign_me_button && Y.Lang.isValue(LP.links.me));

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js	2011-08-10 13:33:26 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js	2011-09-19 00:43:26 +0000
@@ -125,6 +125,12 @@
         this._check_button_state('.yui-picker-remove-button', is_visible);
     },
 
+    test_min_search_chars: function() {
+        // The minimum search term is 2 characters.
+        this.create_picker(this._picker_params(true, true));
+        Assert.areEqual(2, this.picker.get('min_search_chars'));
+    },
+
     test_search_field_focus: function () {
         // The search field has focus when the picker is shown.
         this.create_picker(this._picker_params(true, true));