Launchpad itself

Merge lp:~allenap/launchpad/no-display-name-from-api-bug-491334 into lp:launchpad

  1. no-display-name-from-api-bug-491334
  2. Merge into devel
Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~allenap/launchpad/no-display-name-from-api-bug-491334
Merge into: lp:launchpad
Diff against target: 94 lines (+59/-5)
2 files modified
lib/canonical/launchpad/javascript/bugs/subscriber.js (+2/-1)
lib/canonical/launchpad/javascript/bugs/tests/test_subscriber.js (+57/-4)
To merge this branch: bzr merge lp:~allenap/launchpad/no-display-name-from-api-bug-491334
Reviewer Review Type Date Requested Status
Graham Binns (community) js Approve
Review via email: mp+15808@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Add tests to check that the API is not consulted for the subscriber's
display name if no user is logged in. Additionally, check that it is
consulted when the user is logged in, but that it could not be
obtained by other means.

Also, I changed the test_display_name_load_event() test which I at
first thought would never fail, but in fact it would. However, it was
using test.wait() and test.resume() incorrectly because it passed the
*output* of running Y.Assert.areSame() to test.resume() instead of a
function that then does the assertion. In any case, event dispatch
here is synchronous, so I've removed the test.wait() and test.resume()
altogether, making the test simpler.

No lint.

Test: firefox lib/canonical/launchpad/javascript/bugs/tests/subscriber.html

Revision history for this message
Graham Binns (gmb) :
review: Approve (js)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/javascript/bugs/subscriber.js'
--- lib/canonical/launchpad/javascript/bugs/subscriber.js 2009-12-02 10:15:38 +0000
+++ lib/canonical/launchpad/javascript/bugs/subscriber.js 2009-12-08 12:13:13 +0000
@@ -336,7 +336,8 @@
336 this.set_truncated_display_name();336 this.set_truncated_display_name();
337 this.fire('displaynameload');337 this.fire('displaynameload');
338 } else {338 } else {
339 if (LP !== undefined && LP.client.links.me !== undefined) {339 if (window.LP !== undefined &&
340 window.LP.client.links.me !== undefined) {
340 var client = new LP.client.Launchpad();341 var client = new LP.client.Launchpad();
341 this.get_display_name_from_api(client);342 this.get_display_name_from_api(client);
342 }343 }
343344
=== modified file 'lib/canonical/launchpad/javascript/bugs/tests/test_subscriber.js'
--- lib/canonical/launchpad/javascript/bugs/tests/test_subscriber.js 2009-11-25 19:21:59 +0000
+++ lib/canonical/launchpad/javascript/bugs/tests/test_subscriber.js 2009-12-08 12:13:13 +0000
@@ -148,6 +148,62 @@
148}));148}));
149149
150/*150/*
151 * Subscriber class that stubs out API calls.
152 */
153function APIStubSubscriber(config) {
154 APIStubSubscriber.superclass.constructor.apply(this, arguments);
155}
156Y.extend(APIStubSubscriber, Y.lp.Subscriber, {
157 get_display_name_from_api: function(client) {
158 this.set('display_name', 'From API');
159 this.set_truncated_display_name();
160 this.fire('displaynameload');
161 }
162});
163
164/*
165 * Test that the API is consulted when the display_name cannot be
166 * worked out from a given Node or the DOM.
167 */
168suite.add(new Y.Test.Case({
169 name: 'Subscriber Name From API',
170
171 setUp: function() {
172 // LP is global.
173 window.LP = {
174 client: {
175 links: {},
176 // Empty client constructor.
177 Launchpad: function() {}
178 }
179 };
180 },
181
182 tearDown: function() {
183 delete window.LP;
184 },
185
186 test_display_name_from_api: function() {
187 // The API should be consulted when the user is logged in. Set
188 // the link to "me" to something other than undefined to
189 // indicate that there is a logged-in user.
190 LP.client.links.me = 'not-undefined';
191 var subscriber = new APIStubSubscriber({});
192 Y.Assert.areEqual(
193 'From API', subscriber.get('display_name'),
194 'The display name should be "From API".');
195 },
196
197 test_display_name_when_not_logged_in: function() {
198 // The API should not be consulted when no user is logged in.
199 var subscriber = new APIStubSubscriber({});
200 Y.Assert.areEqual(
201 '', subscriber.get('display_name'),
202 'The display name should be the empty string.');
203 }
204}));
205
206/*
151 * Test that the displaynameload event fires.207 * Test that the displaynameload event fires.
152 */208 */
153suite.add(new Y.Test.Case({209suite.add(new Y.Test.Case({
@@ -158,13 +214,10 @@
158 var event_fired = false;214 var event_fired = false;
159 var subscriber = new Y.lp.Subscriber();215 var subscriber = new Y.lp.Subscriber();
160 subscriber.on('displaynameload', function(e) {216 subscriber.on('displaynameload', function(e) {
161 event_fired = true;217 Y.assert(true);
162 test.resume(Y.Assert.areSame(
163 true, event_fired, 'The event should have been fired.'));
164 });218 });
165 subscriber.set('uri', '/~tester');219 subscriber.set('uri', '/~tester');
166 subscriber.initializer();220 subscriber.initializer();
167 test.wait(2000);
168 }221 }
169}));222}));
170223