launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/phantom-overlay-bug-820828 into lp:launchpad with lp:~allenap/launchpad/lazr-anim-stuff as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #820828 in Launchpad itself: "An empty overlay is displayed on the +localpackagediff page when no user is logged in."
  https://bugs.launchpad.net/launchpad/+bug/820828

For more details, see:
https://code.launchpad.net/~allenap/launchpad/phantom-overlay-bug-820828/+merge/71044

PersonPicker attempts to show the "Assign me" button even when the
user is not logged-in. As it does so, it refers to LP.links.me. This
is not set for anonymous users and an exception is thrown.

This branch ensures that the button is not rendered when the user is
not logged-in.

-- 
https://code.launchpad.net/~allenap/launchpad/phantom-overlay-bug-820828/+merge/71044
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/phantom-overlay-bug-820828 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js	2011-07-27 00:01:47 +0000
+++ lib/lp/app/javascript/picker/person_picker.js	2011-08-10 13:40:15 +0000
@@ -9,7 +9,9 @@
 /*
  * Extend the picker into the PersonPicker
  */
-var PersonPicker = function() {
+var PersonPicker;
+
+PersonPicker = function() {
     PersonPicker.superclass.constructor.apply(this, arguments);
     this._extra_buttons = Y.Node.create('<div class="extra-form-buttons"/>');
     Y.after(this._renderPersonPickerUI, this, 'renderUI');
@@ -41,6 +43,9 @@
                 remove_team_text = cfg.remove_team_text;
             }
         }
+        /* Only show assign-me when the user is logged-in. */
+        show_assign_me_button = (
+            show_assign_me_button && Y.Lang.isValue(LP.links.me));
         this._show_assign_me_button = show_assign_me_button;
         this._show_remove_button = show_remove_button;
         this._assign_me_text = assign_me_text;
@@ -78,7 +83,7 @@
         }
 
         if (this.assign_me_button) {
-            if (LP.links.me.match('~'+selected_value + "$") ||
+            if (LP.links.me.match('~' + selected_value + "$") ||
                 LP.links.me === selected_value) {
                 this.assign_me_button.addClass('yui3-picker-hidden');
             } else {

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.html'
--- lib/lp/app/javascript/picker/tests/test_personpicker.html	2011-08-10 13:40:00 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.html	2011-08-10 13:40:15 +0000
@@ -17,7 +17,9 @@
     <script type="text/javascript" src="../../anim/anim.js"></script>
     <script type="text/javascript" src="../../lazr/lazr.js"></script>
     <script type="text/javascript" src="../../overlay/overlay.js"></script>
+    <script type="text/javascript" src="../../effects/effects.js"></script>
     <script type="text/javascript" src="../../extras/extras.js"></script>
+    <script type="text/javascript" src="../../expander.js"></script>
     <script type="text/javascript" src="../picker.js"></script>
     <script type="text/javascript" src="../picker_patcher.js"></script>
 

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js	2011-07-22 00:50:21 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js	2011-08-10 13:40:15 +0000
@@ -168,6 +168,15 @@
         Assert.areEqual('Assign Moi', assign_me_button.get('innerHTML'));
     },
 
+    test_picker_assign_me_button_not_shown_when_not_logged_in: function() {
+        // The assign me button is hidden when the user is not logged-in.
+        delete window.LP.links.me;  // Log-out.
+        this.create_picker(this._picker_params(true, true));
+        this.picker.render();
+        var assign_me_button = Y.one('.yui-picker-assign-me-button');
+        Assert.isNull(assign_me_button);
+    },
+
     test_picker_remove_person_button_text: function() {
         // The remove button text is correct.
         this.create_picker(this._picker_params(true, true, "fred", "person"));