launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/picker-navigation-wrong-batch-698022 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #698022 in Launchpad itself: ""Choose..." source package search results navigation shows wrong batch on subsequent search"
  https://bugs.launchpad.net/launchpad/+bug/698022

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/picker-navigation-wrong-batch-698022/+merge/68226

== Implementation ==

Whenever a new search is performed using a picker, the selected batch needs to be reset, otherwise the batch navigation control will not be rendered correctly when the new results are displayed. This branch adds the necessary code to the _defaultSearchUserAction handler.

== Tests ==

Add a new test to test_picker:
  test_search_event_resets_the_current_batch

-- 
https://code.launchpad.net/~wallyworld/launchpad/picker-navigation-wrong-batch-698022/+merge/68226
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/picker-navigation-wrong-batch-698022 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/picker/picker.js'
--- lib/lp/app/javascript/picker/picker.js	2011-07-18 01:25:27 +0000
+++ lib/lp/app/javascript/picker/picker.js	2011-07-18 10:07:28 +0000
@@ -695,6 +695,7 @@
         this.set(RESULTS, []);
         this.set(BATCHES, null);
         this.set(BATCH_COUNT, null);
+        this.set(SELECTED_BATCH, 0);
         this._search_input.set('value', '');
         this._results_box.set('innerHTML', '');
     },
@@ -716,6 +717,11 @@
                 {min: this.get(MIN_SEARCH_CHARS)});
             this.set(ERROR, msg);
         } else {
+            // Reset the selected batch for new searches.
+            var current_search_string = this.get(CURRENT_SEARCH_STRING);
+            if (current_search_string !== search_string) {
+                this.set(SELECTED_BATCH, 0);
+            }
             this.set(CURRENT_SEARCH_STRING, search_string);
             this.fire(SEARCH, search_string);
         }

=== modified file 'lib/lp/app/javascript/picker/tests/test_picker.js'
--- lib/lp/app/javascript/picker/tests/test_picker.js	2011-07-18 01:25:27 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker.js	2011-07-18 10:07:28 +0000
@@ -356,6 +356,37 @@
             "Widget should be in search mode.");
     },
 
+    test_search_event_resets_the_current_batch: function () {
+        this.picker.render();
+        this.picker.set('batches', [
+            {value: 'batch1', name: 'Batch 1'},
+            {value: 'batch2', name: 'Batch 2'}
+            ]);
+        this.picker.set('selected_batch', 1);
+        Assert.areEqual(1, this.picker.get('selected_batch'));
+        this.picker._search_input.set('value', 'bar');
+        simulate(
+            this.picker.get('boundingBox'), '.lazr-search.lazr-btn', 'click');
+
+        // An initial search resets the batch.
+        Assert.areEqual(0, this.picker.get('selected_batch'));
+
+        // Batch is reset if search term has changed.
+        this.picker.set('search_mode', false);
+        this.picker.set('selected_batch', 1);
+        this.picker._search_input.set('value', 'foo');
+        simulate(
+            this.picker.get('boundingBox'), '.lazr-search.lazr-btn', 'click');
+        Assert.areEqual(0, this.picker.get('selected_batch'));
+
+        // Batch is not reset if search term hasn't changed.
+        this.picker.set('search_mode', false);
+        this.picker.set('selected_batch', 1);
+        simulate(
+            this.picker.get('boundingBox'), '.lazr-search.lazr-btn', 'click');
+        Assert.areEqual(1, this.picker.get('selected_batch'));
+    },
+
     test_setting_search_mode: function () {
         // Setting search_mode adds a CSS class and disables the search box.