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
1=== modified file 'lib/canonical/launchpad/javascript/bugs/subscriber.js'
2--- lib/canonical/launchpad/javascript/bugs/subscriber.js 2009-12-02 10:15:38 +0000
3+++ lib/canonical/launchpad/javascript/bugs/subscriber.js 2009-12-08 12:13:13 +0000
4@@ -336,7 +336,8 @@
5 this.set_truncated_display_name();
6 this.fire('displaynameload');
7 } else {
8- if (LP !== undefined && LP.client.links.me !== undefined) {
9+ if (window.LP !== undefined &&
10+ window.LP.client.links.me !== undefined) {
11 var client = new LP.client.Launchpad();
12 this.get_display_name_from_api(client);
13 }
14
15=== modified file 'lib/canonical/launchpad/javascript/bugs/tests/test_subscriber.js'
16--- lib/canonical/launchpad/javascript/bugs/tests/test_subscriber.js 2009-11-25 19:21:59 +0000
17+++ lib/canonical/launchpad/javascript/bugs/tests/test_subscriber.js 2009-12-08 12:13:13 +0000
18@@ -148,6 +148,62 @@
19 }));
20
21 /*
22+ * Subscriber class that stubs out API calls.
23+ */
24+function APIStubSubscriber(config) {
25+ APIStubSubscriber.superclass.constructor.apply(this, arguments);
26+}
27+Y.extend(APIStubSubscriber, Y.lp.Subscriber, {
28+ get_display_name_from_api: function(client) {
29+ this.set('display_name', 'From API');
30+ this.set_truncated_display_name();
31+ this.fire('displaynameload');
32+ }
33+});
34+
35+/*
36+ * Test that the API is consulted when the display_name cannot be
37+ * worked out from a given Node or the DOM.
38+ */
39+suite.add(new Y.Test.Case({
40+ name: 'Subscriber Name From API',
41+
42+ setUp: function() {
43+ // LP is global.
44+ window.LP = {
45+ client: {
46+ links: {},
47+ // Empty client constructor.
48+ Launchpad: function() {}
49+ }
50+ };
51+ },
52+
53+ tearDown: function() {
54+ delete window.LP;
55+ },
56+
57+ test_display_name_from_api: function() {
58+ // The API should be consulted when the user is logged in. Set
59+ // the link to "me" to something other than undefined to
60+ // indicate that there is a logged-in user.
61+ LP.client.links.me = 'not-undefined';
62+ var subscriber = new APIStubSubscriber({});
63+ Y.Assert.areEqual(
64+ 'From API', subscriber.get('display_name'),
65+ 'The display name should be "From API".');
66+ },
67+
68+ test_display_name_when_not_logged_in: function() {
69+ // The API should not be consulted when no user is logged in.
70+ var subscriber = new APIStubSubscriber({});
71+ Y.Assert.areEqual(
72+ '', subscriber.get('display_name'),
73+ 'The display name should be the empty string.');
74+ }
75+}));
76+
77+/*
78 * Test that the displaynameload event fires.
79 */
80 suite.add(new Y.Test.Case({
81@@ -158,13 +214,10 @@
82 var event_fired = false;
83 var subscriber = new Y.lp.Subscriber();
84 subscriber.on('displaynameload', function(e) {
85- event_fired = true;
86- test.resume(Y.Assert.areSame(
87- true, event_fired, 'The event should have been fired.'));
88+ Y.assert(true);
89 });
90 subscriber.set('uri', '/~tester');
91 subscriber.initializer();
92- test.wait(2000);
93 }
94 }));
95