launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/preload-people-branch-index into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/preload-people-branch-index/+merge/150482

Preload subscribers in Branch:+branch-portlet-subscriber-content (which is called in from Branch:+index), which drops the query count from <number of subscribers>*2+5 to 9. Clean up a little lint, but my previous branch that touched a lot of the same files already scrubbed them pretty hard. Taken with the previous branch, this one is still net-negative.
-- 
https://code.launchpad.net/~stevenk/launchpad/preload-people-branch-index/+merge/150482
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/preload-people-branch-index into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchsubscription.py'
--- lib/lp/code/browser/branchsubscription.py	2013-01-16 06:41:43 +0000
+++ lib/lp/code/browser/branchsubscription.py	2013-02-26 03:39:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -24,6 +24,7 @@
 from lp.app.interfaces.services import IService
 from lp.code.enums import BranchSubscriptionNotificationLevel
 from lp.code.interfaces.branchsubscription import IBranchSubscription
+from lp.registry.interfaces.person import IPersonSet
 from lp.services.webapp import (
     canonical_url,
     LaunchpadView,
@@ -46,10 +47,7 @@
 
 
 class BranchPortletSubscribersContent(LaunchpadView):
-    """View for the contents for the subscribers portlet.
-
-    This view is strictly for use with ajax.
-    """
+    """View for the contents for the subscribers portlet."""
 
     def subscriptions(self):
         """Return a decorated list of branch subscriptions."""
@@ -57,6 +55,9 @@
         # Cache permissions so private subscribers can be rendered.
         # The security adaptor will do the job also but we don't want or need
         # the expense of running several complex SQL queries.
+        person_ids = [sub.personID for sub in self.context.subscriptions]
+        list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+            person_ids, need_validity=True))
         if self.user is not None:
             subscribers = [
                 subscription.person
@@ -96,18 +97,17 @@
 
     cancel_url = next_url
 
-    def add_notification_message(self, initial,
-                                 notification_level, max_diff_lines,
-                                 review_level):
+    def add_notification_message(self, initial, notification_level,
+                                 max_diff_lines, review_level):
         if notification_level in self.LEVELS_REQUIRING_LINES_SPECIFICATION:
             lines_message = '<li>%s</li>' % max_diff_lines.description
         else:
             lines_message = ''
 
         format_str = '%%s<ul><li>%%s</li>%s<li>%%s</li></ul>' % lines_message
-        message = structured(format_str, initial,
-                             notification_level.description,
-                             review_level.description)
+        message = structured(
+            format_str, initial, notification_level.description,
+            review_level.description)
         self.request.response.addNotification(message)
 
     def optional_max_diff_lines(self, notification_level, max_diff_lines):
@@ -274,7 +274,7 @@
     def initialize(self):
         self.branch = self.context.branch
         self.person = self.context.person
-        LaunchpadEditFormView.initialize(self)
+        super(BranchSubscriptionEditView, self).initialize()
 
     @action("Change", name="change")
     def change_action(self, action, data):

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2013-02-25 05:23:12 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2013-02-26 03:39:21 +0000
@@ -536,6 +536,18 @@
             view.landing_candidates
         self.assertThat(recorder, HasQueryCount(Equals(5)))
 
+    def test_query_count_subscriber_content(self):
+        branch = self.factory.makeBranch()
+        for i in range(10):
+            self.factory.makeBranchSubscription(branch=branch)
+        Store.of(branch).flush()
+        Store.of(branch).invalidate()
+        view = create_initialized_view(
+            branch, '+branch-portlet-subscriber-content')
+        with StormStatementRecorder() as recorder:
+            view.render()
+        self.assertThat(recorder, HasQueryCount(Equals(9)))
+
 
 class TestBranchViewPrivateArtifacts(BrowserTestCase):
     """ Tests that branches with private team artifacts can be viewed.

=== modified file 'lib/lp/code/interfaces/branchsubscription.py'
--- lib/lp/code/interfaces/branchsubscription.py	2013-01-07 02:40:55 +0000
+++ lib/lp/code/interfaces/branchsubscription.py	2013-02-26 03:39:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Bug subscription interfaces."""
@@ -38,6 +38,7 @@
     export_as_webservice_entry()
 
     id = Int(title=_('ID'), readonly=True, required=True)
+    personID = Int(title=_('Person ID'), required=True, readonly=True)
     person = exported(
         PersonChoice(
             title=_('Person'), required=True, vocabulary='ValidPersonOrTeam',