Code review comment for lp:~gmb/launchpad/subscribers-timeout-bug-471974

Here's an incremental diff of the changes that Gavin suggested on IRC:

=== modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js'
--- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-11-17 16:00:21 +0000
+++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-11-18 15:20:12 +0000
@@ -54,8 +54,7 @@
 Y.augment(PortletTarget, Y.Event.Target);
 Y.bugs.portlet = new PortletTarget();
 Y.bugs.portlet.subscribe('bugs:portletloaded', function() {
- setup_subscription_link_handlers();
- load_subscribers_from_duplicates();
+ load_subscriber_ids();
 });
 Y.bugs.portlet.subscribe('bugs:dupeportletloaded', function() {
     setup_unsubscribe_icon_handlers();
@@ -78,6 +77,22 @@
     setup_unsubscribe_icon_handlers();
 });

+/* If loading the subscriber IDs JSON has succeeded, set up the
+ * subscription link handlers and load the subscribers from dupes.
+ */
+Y.bugs.portlet.subscribe('bugs:portletsubscriberidsloaded', function() {
+ setup_subscription_link_handlers();
+ load_subscribers_from_duplicates();
+});
+
+/* If loading the subscriber IDs JSON fails we still need to load the
+ * subscribers from duplicates but we don't set up the subscription link
+ * handlers.
+ */
+Y.bugs.portlet.subscribe('bugs:portletsubscriberidsfailed', function() {
+ load_subscribers_from_duplicates();
+});
+
 /*
  * Subscribing someone else requires loading a grayed out
  * username into the DOM until the subscribe action completes.
@@ -1806,35 +1821,44 @@
         Y.bugs.portlet.fire('bugs:portletloaded');
     }

- function load_portlet() {
- var config = {on: {success: setup_portlet,
- failure: hide_spinner}};
- var url = Y.get(
- '#subscribers-content-link').getAttribute('href').replace(
- 'bugs.', '');
- Y.io(url, config);
- }
+ var config = {on: {success: setup_portlet,
+ failure: hide_spinner}};
+ var url = Y.get(
+ '#subscribers-content-link').getAttribute('href').replace(
+ 'bugs.', '');
+ Y.io(url, config);
+};

- function load_subscriber_ids(transactionid, response, args) {
+function load_subscriber_ids() {
+ function on_success(transactionid, response, args) {
         try {
- var subscriber_ids_node = Y.Node.create(
- response.responseText);
- var subscriber_ids_json = subscriber_ids_node.get('innerHTML');
- subscriber_ids = Y.JSON.parse(subscriber_ids_json);
+ subscriber_ids = Y.JSON.parse(response.responseText);
+ Y.log("Loaded subscriber IDs.");
+
+ // Fire a custom event to trigger the setting-up of the
+ // subscription handlers.
+ Y.bugs.portlet.fire('bugs:portletsubscriberidsloaded');
         } catch (e) {
             Y.log("Unable to load subscriber ids.", "error");
+
+ // Fire an event to signal failure. This ensures that the
+ // subscribers-from-dupes still get loaded into the portlet.
+ Y.bugs.portlet.fire('bugs:portletsubscriberidsfailed');
         }
-
- load_portlet();
- }
-
- var config = {on: {success: load_subscriber_ids,
- failure: load_portlet}};
+ }
+
+ function on_failure() {
+ // Fire an event to signal failure. This ensures that the
+ // subscribers-from-dupes still get loaded into the portlet.
+ Y.bugs.portlet.fire('bugs:portletsubscriberidsfailed');
+ }
+
+ var config = {on: {success: on_success,
+ failure: on_failure}};
     var url = Y.get(
         '#subscribers-ids-link').getAttribute('href').replace('bugs.', '');
- Y.log("Getting ids from " + url);
     Y.io(url, config);
-};
+}

 }, '0.1', {requires: ['base', 'oop', 'node', 'event', 'io-base', 'json-parse',
                       'substitute', 'widget-position-ext',

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2009-11-17 14:58:39 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2009-11-18 15:20:12 +0000
@@ -106,6 +106,10 @@
         """Return subscriber_ids in a form suitable for JavaScript use."""
         return dumps(self.subscriber_ids)

+ def render(self):
+ """Override the default render() to return only JSON."""
+ return self.subscriber_ids_js
+

 class SubscriptionAttrDecorator:
     """A BugSubscription with added attributes for HTML/JS."""

=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2009-11-17 14:58:39 +0000
+++ lib/lp/bugs/browser/configure.zcml 2009-11-18 15:20:12 +0000
@@ -1008,7 +1008,6 @@
             for="lp.bugs.interfaces.bug.IBug"
             name="+bug-portlet-subscribers-ids"
             class="lp.bugs.browser.bugsubscription.BugPortletSubcribersIds"
- template="../templates/bug-portlet-subscribers-ids.pt"
             permission="zope.Public"/>
         <browser:navigation
             module="lp.bugs.browser.bug"

=== removed file 'lib/lp/bugs/templates/bug-portlet-subscribers-ids.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers-ids.pt 2009-11-17 14:58:39 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers-ids.pt 1970-01-01 00:00:00 +0000
@@ -1,6 +0,0 @@
-<div
- xmlns="http://www.w3.org/1999/xhtml"
- xmlns:tal="http://xml.zope.org/namespaces/tal"
- xmlns:metal="http://xml.zope.org/namespaces/metal"
- xmlns:i18n="http://xml.zope.org/namespaces/i18n"
- tal:content="view/subscriber_ids_js" />