Code review comment for lp:~gmb/launchpad/subscriber-portlet-timeout-bug-393476

Revision history for this message
Graham Binns (gmb) wrote :

This branch fixes bug 393476 (subscribers portlet times out) by loading
the subscribers from duplicates separately from the main set of
subscribers.

Most bugs have a relatively low number of direct subscribers, but a bug
with a large number of duplicates can have thousands of indirect ones.
Loading these indirect subscribers can sometimes time out because of the
large number of subscribers and because in order to display them we need
to iterate over the result set before rendering the page.

My solution is to load the subscribers from duplicates separately.
Whilst we could spend time optimising the query and the iterations over
the result set, there will come a point where, for bugs with large
numbers of indirect subscribers, there will simply be too many to
handle. In those cases it's important that we still render the list of
direct subscribers (because that's usually the most interesting list of
subscribers on a bug page) and, most importantly, that we set up the
subscribe and unsubscribe links properly (at the moment the timeout
causes the AJAX controls to never get set up).

== Changes ==

=== lib/canonical/launchpad/javascript/bugs/bugtask-index.js ===

 - I've added two functions here. load_subscribers_from_duplicates()
   does the asynchronous loading of subs from dupes and
   Y.bugs.load_subscribers_portlet() contains the subs portlet setup
   code that was previously inline in the portlet template.
 - load_subscribers_from_duplicates() is now called once the
   portletloaded event is fired, meaning that it only happens once the
   direct subscribers have been loaded and the links have been set up
   correctly.

=== lib/lp/bugs/browser/bugsubscription.py ===

 - I've refactored the existing BugPortletSubcribersContents view into
   two views: BugPortletSubcribersContents and
   BugPortletDuplicateSubcribersContents. The second deals with the
   subs from dupes list.
 - I've updated the methods used in these views to turn them into
   @properties, per the coding standards.

=== lib/lp/bugs/browser/configure.zcml ===

 - I've added an entry for the BugPortletDuplicateSubcribersContents
   view.

=== lib/lp/bugs/templates/bug-portlet-dupe-subscribers-content.pt ===

 - I've added this template that displays only the list of subscribers
   from duplicates.

=== lib/lp/bugs/templates/bug-portlet-subscribers-content.pt ===

 - I've updated the existing template to remove the subs from dupes
   code.

=== lib/lp/bugs/templates/bug-portlet-subscribers.pt ===

 - I've removed the inline JS that didn't need to be inline, fixing bug
   369874 as a drive-by.

== 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/browser/bugsubscription.py
  lib/lp/bugs/browser/configure.zcml
  lib/lp/bugs/templates/bug-portlet-dupe-subscribers-content.pt
  lib/lp/bugs/templates/bug-portlet-subscribers-content.pt
  lib/lp/bugs/templates/bug-portlet-subscribers.pt

== JSLint notices ==
jslint: No problem found in '/home/graham/canonical/lp-branches/subscriber-portlet-timeout-bug-393476/lib/canonical/launchpad/javascript/bugs/bugtask-index.js'.

jslint: 1 file to lint.

== Pylint notices ==

lib/lp/bugs/browser/bugsubscription.py
    15: [F0401] Unable to import 'lazr.delegates' (No module named delegates)
    16: [F0401] Unable to import 'lazr.lifecycle.event' (No module named lifecycle)

« Back to merge proposal