launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/mp-review-type-broken-847682 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #847682 in Launchpad itself: "Can no longer set review type in "Request another review" person picker"
  https://bugs.launchpad.net/launchpad/+bug/847682

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/mp-review-type-broken-847682/+merge/75672

Ensure merge proposal review type is able to be set when selecting a reviewer.

== Implementation ==

A trivial fix. When a != expression was converted at some stage to !==, it broke because the value undefined was no longer being implicitly cast to null. Use Y.lang.isValue() instead.

== Tests ==

Add a test:
  - test_footer_node_preserved_without_extra_no_results_message()

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/picker/picker_patcher.js
  lib/lp/app/javascript/picker/tests/test_picker_patcher.js


-- 
https://code.launchpad.net/~wallyworld/launchpad/mp-review-type-broken-847682/+merge/75672
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/mp-review-type-broken-847682 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
--- lib/lp/app/javascript/picker/picker_patcher.js	2011-08-22 12:13:22 +0000
+++ lib/lp/app/javascript/picker/picker_patcher.js	2011-09-16 05:38:19 +0000
@@ -366,7 +366,7 @@
         user_has_searched = false;
     });
 
-    if (config.extra_no_results_message !== null) {
+    if (Y.Lang.isValue(config.extra_no_results_message)) {
         picker.before('resultsChange', function (e) {
             var new_results = e.details[0].newVal;
             if (new_results.length === 0) {

=== modified file 'lib/lp/app/javascript/picker/tests/test_picker_patcher.js'
--- lib/lp/app/javascript/picker/tests/test_picker_patcher.js	2011-08-24 15:41:10 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker_patcher.js	2011-09-16 05:38:19 +0000
@@ -177,6 +177,8 @@
     },
 
     test_extra_no_results_message: function () {
+        // If "extra_no_results_message" is defined, it is rendered in the
+        // footer slot when there are no results.
         this.create_picker(
             undefined, {'extra_no_results_message': 'message'});
         this.picker.set('results', []);
@@ -184,6 +186,17 @@
         Assert.areEqual('message', footer_slot.get('text'));
     },
 
+    test_footer_node_preserved_without_extra_no_results_message: function () {
+        // If "extra_no_results_message" is not defined, the footer slot node
+        // should be preserved.
+        this.create_picker();
+        var footer_node = Y.Node.create("<span>foobar</span>");
+        this.picker.set('footer_slot', footer_node);
+        this.picker.set('results', []);
+        var footer_slot = this.picker.get('footer_slot');
+        Assert.areEqual('foobar', footer_slot.get('text'));
+    },
+
     test_confirmation_yes: function() {
         // The picker saves the selected value if the user answers
         // "Yes" to a confirmation request.