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
=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-11-08 14:39:22 +0000
+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-11-14 21:40:27 +0000
@@ -45,7 +45,7 @@
45 * Since the portlet loads via XHR and inline subscribing45 * Since the portlet loads via XHR and inline subscribing
46 * depends on that portlet being loaded, setup a custom46 * depends on that portlet being loaded, setup a custom
47 * event object, to provide a hook for initializing subscription47 * event object, to provide a hook for initializing subscription
48 * link callbacks after a bugs:portletloaded event.48 * link callbacks after custom events.
49 */49 */
50var PortletTarget = function() {};50var PortletTarget = function() {};
51Y.augment(PortletTarget, Y.Event.Target);51Y.augment(PortletTarget, Y.Event.Target);
@@ -54,6 +54,9 @@
54 setup_subscription_link_handlers();54 setup_subscription_link_handlers();
55 load_subscribers_from_duplicates();55 load_subscribers_from_duplicates();
56});56});
57Y.bugs.portlet.subscribe('bugs:dupeportletloaded', function() {
58 setup_unsubscribe_icon_handlers();
59});
57/*60/*
58 * If the subscribers portlet fails to load, clear any61 * If the subscribers portlet fails to load, clear any
59 * click handlers, so the normal subscribe page can be reached.62 * click handlers, so the normal subscribe page can be reached.
@@ -64,6 +67,13 @@
64 click_handler.detach();67 click_handler.detach();
65 }68 }
66});69});
70/* If the dupe subscribers portlet fails to load,
71 * be sure to try to handle any unsub icons that may
72 * exist for others.
73 */
74Y.bugs.portlet.subscribe('bugs:dupeportletloadfailed', function(handlers) {
75 setup_unsubscribe_icon_handlers();
76});
6777
68/*78/*
69 * Subscribing someone else requires loading a grayed out79 * Subscribing someone else requires loading a grayed out
@@ -305,7 +315,6 @@
305 subscription.get('link').addClass('js-action');315 subscription.get('link').addClass('js-action');
306 }316 }
307317
308 setup_unsubscribe_icon_handlers(subscription);
309 setup_subscribe_someone_else_handler(subscription);318 setup_subscribe_someone_else_handler(subscription);
310}319}
311320
@@ -315,7 +324,15 @@
315 * @method setup_unsubscribe_icon_handlers324 * @method setup_unsubscribe_icon_handlers
316 * @param subscription {Object} A Y.lp.Subscription object.325 * @param subscription {Object} A Y.lp.Subscription object.
317 */326 */
318function setup_unsubscribe_icon_handlers(subscription) {327function setup_unsubscribe_icon_handlers() {
328 var subscription = new Y.lp.Subscription({
329 link: Y.get('.menu-link-subscription'),
330 spinner: Y.get('#sub-unsub-spinner'),
331 subscriber: new Y.lp.Subscriber({
332 uri: LP.client.links.me,
333 subscriber_ids: subscriber_ids
334 })
335 });
319 var unsubscribe_icons = Y.all('.unsub-icon');336 var unsubscribe_icons = Y.all('.unsub-icon');
320 if (unsubscribe_icons) {337 if (unsubscribe_icons) {
321 unsubscribe_icons.on('click', function(e) {338 unsubscribe_icons.on('click', function(e) {
@@ -1728,6 +1745,9 @@
1728 function hide_spinner() {1745 function hide_spinner() {
1729 Y.get('#subscribers-portlet-dupe-spinner').setStyle(1746 Y.get('#subscribers-portlet-dupe-spinner').setStyle(
1730 'display', 'none');1747 'display', 'none');
1748 // Fire a custom event to signal failure, so that
1749 // any remaining unsub icons can be hooked up.
1750 Y.bugs.portlet.fire('bugs:dupeportletloadfailed');
1731 }1751 }
17321752
1733 function on_success(transactionid, response, args) {1753 function on_success(transactionid, response, args) {
@@ -1739,6 +1759,10 @@
1739 'innerHTML',1759 'innerHTML',
1740 dupe_subscribers_container.get('innerHTML') +1760 dupe_subscribers_container.get('innerHTML') +
1741 response.responseText);1761 response.responseText);
1762
1763 // Fire a custom portlet loaded event to notify when
1764 // it's safe to setup dupe subscriber link callbacks.
1765 Y.bugs.portlet.fire('bugs:dupeportletloaded');
1742 }1766 }
17431767
1744 var config = {on: {success: on_success,1768 var config = {on: {success: on_success,
17451769
=== modified file 'lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py'
--- lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-11-08 22:43:09 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_inline_subscriber.py 2009-11-14 21:40:27 +0000
@@ -221,10 +221,12 @@
221 client.open(url=BUG_URL % 5)221 client.open(url=BUG_URL % 5)
222 client.asserts.assertNode(222 client.asserts.assertNode(
223 xpath=(u'//div[@id="subscribers-links"]'223 xpath=(u'//div[@id="subscribers-links"]'
224 '/div/a[@name="Sample Person"]'))224 '/div/a[@name="Sample Person"]'),
225 timeout=FOR_ELEMENT)
225 client.asserts.assertNode(226 client.asserts.assertNode(
226 xpath=(u'//div[@id="subscribers-from-duplicates"]'227 xpath=(u'//div[@id="subscribers-from-duplicates"]'
227 '/div/a[@name="Sample Person"]'))228 '/div/a[@name="Sample Person"]'),
229 timeout=FOR_ELEMENT)
228 # The first click unsubscribes the direct subscription, leaving230 # The first click unsubscribes the direct subscription, leaving
229 # the duplicate subscription.231 # the duplicate subscription.
230 client.click(xpath=SUBSCRIPTION_LINK)232 client.click(xpath=SUBSCRIPTION_LINK)