launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/fix-bug-picker-search into lp:launchpad.

Commit message:
Fix the bug picker to pass the bug ID as a number, not a string.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1507604 in Launchpad itself: "bug duplicate dialog error, "bug_id: got 'unicode', expected int:""
  https://bugs.launchpad.net/launchpad/+bug/1507604

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/fix-bug-picker-search/+merge/274910

Fix the bug picker to pass the bug ID as a number, not a string.

r17816 regressed this; I incorrectly adjusted the test rather than making sure the code continued to pass a number.  All of the other tests changed in the same revision were for code that really was passing a string of some kind, so are correct.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-bug-picker-search into lp:launchpad.

=== modified file 'lib/lp/bugs/javascript/bug_picker.js'
--- lib/lp/bugs/javascript/bug_picker.js	2015-10-13 11:07:08 +0000
+++ lib/lp/bugs/javascript/bug_picker.js	2015-10-19 15:51:26 +0000
@@ -130,7 +130,11 @@
      * @protected
      */
     _find_bug: function(data) {
-        var bug_id = Y.Lang.trim(data.id);
+        var bug_id = parseInt(Y.Lang.trim(data.id), 10);
+        if (isNaN(bug_id)) {
+            this.set('error', 'Please enter a valid bug number.');
+            return;
+        }
         var qs_data
             = Y.lp.client.append_qs("", "ws.accept", "application.json");
         qs_data = Y.lp.client.append_qs(qs_data, "ws.op", "getBugData");

=== modified file 'lib/lp/bugs/javascript/tests/test_bug_picker.js'
--- lib/lp/bugs/javascript/tests/test_bug_picker.js	2015-10-13 11:07:08 +0000
+++ lib/lp/bugs/javascript/tests/test_bug_picker.js	2015-10-19 15:51:26 +0000
@@ -20,7 +20,7 @@
                     this.mockio.last_request.url);
                 var expected_data =
                     'ws.accept=application.json&ws.op=getBugData&' +
-                    'bug_id=%22' + bug_id + '%22';
+                    'bug_id=' + bug_id;
                 if (Y.Lang.isValue(LP.cache.bug)) {
                     var bug_uri = encodeURIComponent('api/devel/bugs/1');
                     expected_data += '&related_bug=%22' + bug_uri + '%22';
@@ -73,8 +73,19 @@
             Y.one('.yui3-picker-search').set('value', '');
             Y.one('.lazr-search').simulate('click');
             Y.Assert.isNull(this.mockio.last_request);
-            this._assert_error_display(
-                'Please enter a valid bug number.');
+            this._assert_error_display('Please enter a valid bug number.');
+            this._assert_form_state(false);
+        },
+
+        // Attempt to enter a non-number and an error is displayed.
+        test_bug_id_not_a_number: function() {
+            this.widget = this._createWidget();
+            Y.one('.pick-bug').simulate('click');
+            this.mockio.last_request = null;
+            Y.one('.yui3-picker-search').set('value', 'some-string');
+            Y.one('.lazr-search').simulate('click');
+            Y.Assert.isNull(this.mockio.last_request);
+            this._assert_error_display('Please enter a valid bug number.');
             this._assert_form_state(false);
         },