launchpad-reviewers team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/launchpad/revert into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~gary/launchpad/revert/+merge/73413

Revert https://code.launchpad.net/~gary/launchpad/bug724025/+merge/72776 .  qa-bad.
-- 
https://code.launchpad.net/~gary/launchpad/revert/+merge/73413
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/revert into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-08-29 18:21:28 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-08-30 16:46:16 +0000
@@ -247,7 +247,6 @@
 from lp.bugs.interfaces.bugwatch import BugWatchActivityStatus
 from lp.bugs.interfaces.cve import ICveSet
 from lp.bugs.interfaces.malone import IMaloneApplication
-from lp.code.interfaces.branchcollection import IAllBranches
 from lp.registry.interfaces.distribution import (
     IDistribution,
     IDistributionSet,
@@ -883,17 +882,10 @@
     @cachedproperty
     def linked_branches(self):
         """Filter out the bug_branch links to non-visible private branches."""
-        linked_branches = list(
-            self.context.bug.getVisibleLinkedBranches(
-                self.user, eager_load=True))
-        # This is an optimization for when we look at the merge proposals.
-        # Note that, like all of these sorts of Storm cache optimizations, it
-        # only helps if [launchpad] storm_cache_size in launchpad-lazr.conf is
-        # pretty big--and as of this writing, it isn't for developer
-        # instances.
-        list(getUtility(IAllBranches).getMergeProposals(
-            for_branches=[link.branch for link in linked_branches],
-            eager_load=True))
+        linked_branches = []
+        for linked_branch in self.context.bug.linked_branches:
+            if check_permission('launchpad.View', linked_branch.branch):
+                linked_branches.append(linked_branch)
         return linked_branches
 
     @property

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2011-08-29 18:21:28 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2011-08-30 16:46:16 +0000
@@ -124,51 +124,6 @@
                 self.factory.makeBugAttachment(bug=task.bug)
         self.assertThat(task, browses_under_limit)
 
-    def test_rendered_query_counts_reduced_with_branches(self):
-        f = self.factory
-        owner = f.makePerson()
-        ds = f.makeDistroSeries()
-        sourcepackagenames = [
-            f.makeSourcePackageName('testsourcepackagename%d' % i)
-            for i in range(10)]
-        sourcepackages = [
-            f.makeSourcePackage(
-                sourcepackagename=name, distroseries=ds, publish=True)
-            for name in sourcepackagenames]
-        bug = f.makeBug()
-        bugtasks = []
-        for sp in sourcepackages:
-            bugtask = f.makeBugTask(bug=bug, owner=owner, target=sp)
-            bugtasks.append(bugtask)
-        task = bugtasks[0]
-        url = canonical_url(task)
-        recorder = QueryCollector()
-        recorder.register()
-        self.addCleanup(recorder.unregister)
-        self.invalidate_caches(task)
-        self.getUserBrowser(url, owner)
-        # At least 20 of these should be removed.
-        self.assertThat(recorder, HasQueryCount(LessThan(100)))
-        count_with_no_branches = recorder.count
-        self.invalidate_caches(task)
-        with person_logged_in(owner):
-            for sp in sourcepackages:
-                target_branch = f.makePackageBranch(
-                    sourcepackage=sp, owner=owner)
-                source_branch = f.makeBranchTargetBranch(
-                    target_branch.target, owner=owner)
-                bug.linkBranch(source_branch, owner)
-                f.makeBranchMergeProposal(
-                    target_branch=target_branch,
-                    registrant=owner,
-                    source_branch=source_branch)
-        self.getUserBrowser(url, owner)
-        # Ideally this should be much fewer, but this tries to keep a win of
-        # removing more than half of these.
-        self.assertThat(recorder, HasQueryCount(
-            LessThan(count_with_no_branches + 45),
-            ))
-
     def test_interesting_activity(self):
         # The interesting_activity property returns a tuple of interesting
         # `BugActivityItem`s.

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-08-29 18:21:28 +0000
+++ lib/lp/bugs/model/bug.py	2011-08-30 16:46:16 +0000
@@ -1295,11 +1295,11 @@
             notify(ObjectDeletedEvent(bug_branch, user=user))
             bug_branch.destroySelf()
 
-    def getVisibleLinkedBranches(self, user, eager_load=False):
+    def getVisibleLinkedBranches(self, user):
         """Return all the branches linked to the bug that `user` can see."""
         all_branches = getUtility(IAllBranches)
         linked_branches = list(all_branches.visibleByUser(
-            user).linkedToBugs([self]).getBranches(eager_load=eager_load))
+            user).linkedToBugs([self]).getBranches())
         if len(linked_branches) == 0:
             return EmptyResultSet()
         else: