Launchpad itself

Merge lp:~deryck/launchpad/dupe-portlet-click-fail into lp:launchpad

  1. dupe-portlet-click-fail
  2. Merge into devel
Proposed by Deryck Hodge
Status: Merged
Approved by: Deryck Hodge
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~deryck/launchpad/dupe-portlet-click-fail
Merge into: lp:launchpad
Diff against target: 101 lines (+31/-5)
2 files modified
lib/canonical/launchpad/javascript/bugs/bugtask-index.js (+27/-3)
lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py (+4/-2)
To merge this branch: bzr merge lp:~deryck/launchpad/dupe-portlet-click-fail
Reviewer Review Type Date Requested Status
Paul Hummer (community) Approve
Review via email: mp+14875@code.launchpad.net

Commit message

Correctly hook up unsubscribe icons for onclick events after the duplicate subscribers portlet has loaded (or failed to load.)

To post a comment you must log in.
Revision history for this message
Deryck Hodge (deryck) wrote :

This is a fix for bug #482379. The unsubscribe icons in subscription
portlets were not being hooked up for handling onclick events. So
people could not unsubscribe inline.

This happened because of an update to load duplicate subscriptions from
a second XHR request (the direct subscribers load in the first XHR). So
this branch creates a new custom event to fire after the duplicate
subscriber list has loaded. If that load fails, a failure event fires
to make sure any unsub icons for direct subscribers are still hooked up.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/javascript/bugs/bugtask-index.js
  lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py

== JSLint notices ==
jslint: No problem found in
'/data/home/deryck/canonical/lp-branches/dupe-portlet-click-fail/lib/canonical/launchpad/javascript/bugs/bugtask-index.js'.

jslint: 1 file to lint.

Revision history for this message
Paul Hummer (rockstar) wrote :

After finding out that on:complete fires before on:success and on:failure, I'm quite happy with the way this is. Land it!

review: Approve

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/bugtask-index.js'
2--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-11-08 14:39:22 +0000
3+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-11-14 21:40:27 +0000
4@@ -45,7 +45,7 @@
5 * Since the portlet loads via XHR and inline subscribing
6 * depends on that portlet being loaded, setup a custom
7 * event object, to provide a hook for initializing subscription
8- * link callbacks after a bugs:portletloaded event.
9+ * link callbacks after custom events.
10 */
11 var PortletTarget = function() {};
12 Y.augment(PortletTarget, Y.Event.Target);
13@@ -54,6 +54,9 @@
14 setup_subscription_link_handlers();
15 load_subscribers_from_duplicates();
16 });
17+Y.bugs.portlet.subscribe('bugs:dupeportletloaded', function() {
18+ setup_unsubscribe_icon_handlers();
19+});
20 /*
21 * If the subscribers portlet fails to load, clear any
22 * click handlers, so the normal subscribe page can be reached.
23@@ -64,6 +67,13 @@
24 click_handler.detach();
25 }
26 });
27+/* If the dupe subscribers portlet fails to load,
28+ * be sure to try to handle any unsub icons that may
29+ * exist for others.
30+ */
31+Y.bugs.portlet.subscribe('bugs:dupeportletloadfailed', function(handlers) {
32+ setup_unsubscribe_icon_handlers();
33+});
34
35 /*
36 * Subscribing someone else requires loading a grayed out
37@@ -305,7 +315,6 @@
38 subscription.get('link').addClass('js-action');
39 }
40
41- setup_unsubscribe_icon_handlers(subscription);
42 setup_subscribe_someone_else_handler(subscription);
43 }
44
45@@ -315,7 +324,15 @@
46 * @method setup_unsubscribe_icon_handlers
47 * @param subscription {Object} A Y.lp.Subscription object.
48 */
49-function setup_unsubscribe_icon_handlers(subscription) {
50+function setup_unsubscribe_icon_handlers() {
51+ var subscription = new Y.lp.Subscription({
52+ link: Y.get('.menu-link-subscription'),
53+ spinner: Y.get('#sub-unsub-spinner'),
54+ subscriber: new Y.lp.Subscriber({
55+ uri: LP.client.links.me,
56+ subscriber_ids: subscriber_ids
57+ })
58+ });
59 var unsubscribe_icons = Y.all('.unsub-icon');
60 if (unsubscribe_icons) {
61 unsubscribe_icons.on('click', function(e) {
62@@ -1728,6 +1745,9 @@
63 function hide_spinner() {
64 Y.get('#subscribers-portlet-dupe-spinner').setStyle(
65 'display', 'none');
66+ // Fire a custom event to signal failure, so that
67+ // any remaining unsub icons can be hooked up.
68+ Y.bugs.portlet.fire('bugs:dupeportletloadfailed');
69 }
70
71 function on_success(transactionid, response, args) {
72@@ -1739,6 +1759,10 @@
73 'innerHTML',
74 dupe_subscribers_container.get('innerHTML') +
75 response.responseText);
76+
77+ // Fire a custom portlet loaded event to notify when
78+ // it's safe to setup dupe subscriber link callbacks.
79+ Y.bugs.portlet.fire('bugs:dupeportletloaded');
80 }
81
82 var config = {on: {success: on_success,
83
84=== modified file 'lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py'
85--- lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-11-08 22:43:09 +0000
86+++ lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-11-14 21:40:27 +0000
87@@ -221,10 +221,12 @@
88 client.open(url=BUG_URL % 5)
89 client.asserts.assertNode(
90 xpath=(u'//div[@id="subscribers-links"]'
91- '/div/a[@name="Sample Person"]'))
92+ '/div/a[@name="Sample Person"]'),
93+ timeout=FOR_ELEMENT)
94 client.asserts.assertNode(
95 xpath=(u'//div[@id="subscribers-from-duplicates"]'
96- '/div/a[@name="Sample Person"]'))
97+ '/div/a[@name="Sample Person"]'),
98+ timeout=FOR_ELEMENT)
99 # The first click unsubscribes the direct subscription, leaving
100 # the duplicate subscription.
101 client.click(xpath=SUBSCRIPTION_LINK)