Merge lp:~cjwatson/launchpad/git-activereviews into lp:launchpad

[Colin Watson]

Extend merge proposal listings to cover Git, and add GitRef:+activereviews.

Extend merge proposal listings to cover Git, and add GitRef:+activereviews.

We should probably have GitRepository:+activereviews at some point too, but that can come in a separate branch.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/browser/branchmergeproposallisting.py'
--- lib/lp/code/browser/branchmergeproposallisting.py	2014-12-08 00:32:30 +0000
+++ lib/lp/code/browser/branchmergeproposallisting.py	2015-05-13 09:36:31 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Base class view for branch merge proposal listings."""
@@ -40,10 +40,6 @@
     BranchMergeProposalStatus,
     CodeReviewVote,
     )
-from lp.code.interfaces.branchcollection import (
-    IAllBranches,
-    IBranchCollection,
-    )
 from lp.code.interfaces.branchmergeproposal import (
     BRANCH_MERGE_PROPOSAL_FINAL_STATES,
     IBranchMergeProposal,
@@ -168,7 +164,7 @@
 
     @cachedproperty
     def proposals(self):
-        """Return a list of BranchListingItems."""
+        """Return a list of BranchMergeProposalListingItems."""
         proposals = self._proposals_for_current_batch
         return [self._createItem(proposal) for proposal in proposals]
 
@@ -287,12 +283,11 @@
 
     def getProposals(self):
         """Get the proposals for the view."""
-        collection = IBranchCollection(self.context)
-        collection = collection.visibleByUser(self.user)
-        proposals = collection.getMergeProposals(
-            [BranchMergeProposalStatus.CODE_APPROVED,
-             BranchMergeProposalStatus.NEEDS_REVIEW], eager_load=True)
-        return proposals
+        return self.context.getMergeProposals(
+            status=(
+                BranchMergeProposalStatus.CODE_APPROVED,
+                BranchMergeProposalStatus.NEEDS_REVIEW),
+            visible_by_user=self.user, eager_load=True)
 
     def _getReviewGroup(self, proposal, votes, reviewer):
         """One of APPROVED, MINE, TO_DO, CAN_DO, ARE_DOING, OTHER or WIP.
@@ -324,8 +319,8 @@
             return self.WIP
 
         if (reviewer is not None and
-            (proposal.source_branch.owner == reviewer or
-             (reviewer.inTeam(proposal.source_branch.owner) and
+            (proposal.merge_source.owner == reviewer or
+             (reviewer.inTeam(proposal.merge_source.owner) and
               proposal.registrant == reviewer))):
             return self.MINE
 
@@ -437,15 +432,13 @@
     def _getReviewer(self):
         return self.context
 
-    def _getCollection(self):
-        return getUtility(IAllBranches)
-
-    def getProposals(self):
+    def getProposals(self, project=None):
         """See `ActiveReviewsView`."""
-        collection = self._getCollection().visibleByUser(self.user)
-        return collection.getMergeProposalsForPerson(
-            self._getReviewer(), [BranchMergeProposalStatus.CODE_APPROVED,
-            BranchMergeProposalStatus.NEEDS_REVIEW], eager_load=True)
+        return self._getReviewer().getOwnedAndRequestedReviews(
+            status=(
+                BranchMergeProposalStatus.CODE_APPROVED,
+                BranchMergeProposalStatus.NEEDS_REVIEW),
+            visible_by_user=self.user, project=project, eager_load=True)
 
 
 class PersonProductActiveReviewsView(PersonActiveReviewsView):
@@ -459,8 +452,9 @@
     def _getReviewer(self):
         return self.context.person
 
-    def _getCollection(self):
-        return getUtility(IAllBranches).inProduct(self.context.product)
+    def getProposals(self):
+        return super(PersonProductActiveReviewsView, self).getProposals(
+            project=self.context.product)
 
     @property
     def no_proposal_message(self):

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2015-04-28 16:48:22 +0000
+++ lib/lp/code/browser/configure.zcml	2015-05-13 09:36:31 +0000
@@ -844,6 +844,12 @@
         name="+register-merge"
         permission="launchpad.AnyPerson"
         template="../templates/gitref-register-merge.pt"/>
+    <browser:page
+        for="lp.code.interfaces.gitref.IGitRef"
+        class="lp.code.browser.branchmergeproposallisting.BranchActiveReviewsView"
+        permission="zope.Public"
+        name="+activereviews"
+        template="../templates/active-reviews.pt"/>
     <browser:menus
         classes="GitRefContextMenu"
         module="lp.code.browser.gitref"/>

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2013-04-29 00:10:09 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2015-05-13 09:36:31 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for BranchMergeProposal listing views."""
@@ -12,6 +12,7 @@
 from testtools.content_type import UTF8_TEXT
 from testtools.matchers import Equals
 import transaction
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
@@ -23,8 +24,14 @@
     BranchMergeProposalStatus,
     CodeReviewVote,
     )
+from lp.code.interfaces.gitref import IGitRef
+from lp.code.interfaces.gitrepository import (
+    GIT_FEATURE_FLAG,
+    IGitRepositorySet,
+    )
 from lp.registry.model.personproduct import PersonProduct
 from lp.services.database.sqlbase import flush_database_caches
+from lp.services.features.testing import FeatureFixture
 from lp.testing import (
     ANONYMOUS,
     BrowserTestCase,
@@ -45,7 +52,55 @@
 _default = object()
 
 
-class TestProposalVoteSummary(TestCaseWithFactory):
+class BzrMixin:
+    """Mixin for Bazaar-based tests."""
+
+    def _makeBranch(self, target=None, **kwargs):
+        if target is not None:
+            # This only handles projects at the moment.
+            kwargs["product"] = target
+        return self.factory.makeBranch(**kwargs)
+
+    def _makePackageBranch(self, **kwargs):
+        return self.factory.makePackageBranch(**kwargs)
+
+    def _makeStackedOnBranchChain(self, target=None, **kwargs):
+        if target is not None:
+            # This only handles projects at the moment.
+            kwargs["product"] = target
+        return self.factory.makeStackedOnBranchChain(**kwargs)
+
+    def _makeBranchMergeProposal(self, target=None, merge_target=None,
+                                 **kwargs):
+        # This only handles projects at the moment.
+        return self.factory.makeBranchMergeProposal(
+            product=target, target_branch=merge_target, **kwargs)
+
+
+class GitMixin:
+    """Mixin for Git-based tests."""
+
+    def setUp(self, user=ANONYMOUS):
+        super(GitMixin, self).setUp(user=user)
+        self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+
+    def _makeBranch(self, **kwargs):
+        return self.factory.makeGitRefs(**kwargs)[0]
+
+    def _makePackageBranch(self, **kwargs):
+        dsp = self.factory.makeDistributionSourcePackage()
+        return self.factory.makeGitRefs(target=dsp, **kwargs)[0]
+
+    def _makeStackedOnBranchChain(self, depth=None, **kwargs):
+        # Git doesn't have stacked branches.  Just make an ordinary reference.
+        return self._makeBranch(**kwargs)
+
+    def _makeBranchMergeProposal(self, merge_target=None, **kwargs):
+        return self.factory.makeBranchMergeProposalForGit(
+            target_ref=merge_target, **kwargs)
+
+
+class TestProposalVoteSummaryMixin:
     """The vote summary shows a summary of the current votes."""
 
     layer = DatabaseFunctionalLayer
@@ -53,7 +108,8 @@
     def setUp(self):
         # Use an admin so we don't have to worry about launchpad.Edit
         # permissions on the merge proposals for adding comments.
-        TestCaseWithFactory.setUp(self, user="[e-mail]")
+        super(TestProposalVoteSummaryMixin, self).setUp(
+            user="[e-mail]")
 
     def _createComment(self, proposal, reviewer=None, vote=None,
                        comment=_default):
@@ -69,7 +125,7 @@
     def _get_vote_summary(self, proposal):
         """Return the vote summary string for the proposal."""
         view = create_initialized_view(
-            proposal.source_branch.owner, '+merges', rootsite='code')
+            proposal.merge_source.owner, '+merges', rootsite='code')
         batch_navigator = view.proposals
         # There will only be one item in the list of proposals.
         [listing_item] = batch_navigator.proposals
@@ -78,14 +134,14 @@
 
     def test_no_votes_or_comments(self):
         # If there are no votes or comments, then we show that.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         summary, comment_count = self._get_vote_summary(proposal)
         self.assertEqual([], summary)
         self.assertEqual(0, comment_count)
 
     def test_no_votes_with_comments(self):
         # The comment count is shown.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         self._createComment(proposal)
         summary, comment_count = self._get_vote_summary(proposal)
         self.assertEqual([], summary)
@@ -93,7 +149,7 @@
 
     def test_vote_without_comment(self):
         # If there are no comments we don't show a count.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         self._createComment(
             proposal, vote=CodeReviewVote.APPROVE, comment=None)
         summary, comment_count = self._get_vote_summary(proposal)
@@ -104,7 +160,7 @@
 
     def test_vote_with_comment(self):
         # A vote with a comment counts as a vote and a comment.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         self._createComment(proposal, vote=CodeReviewVote.APPROVE)
         summary, comment_count = self._get_vote_summary(proposal)
         self.assertEqual(
@@ -114,7 +170,7 @@
 
     def test_disapproval(self):
         # Shown as Disapprove: <count>.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         self._createComment(proposal, vote=CodeReviewVote.DISAPPROVE)
         summary, comment_count = self._get_vote_summary(proposal)
         self.assertEqual(
@@ -124,7 +180,7 @@
 
     def test_abstain(self):
         # Shown as Abstain: <count>.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         transaction.commit()
         self._createComment(proposal, vote=CodeReviewVote.ABSTAIN)
         summary, comment_count = self._get_vote_summary(proposal)
@@ -135,7 +191,7 @@
 
     def test_vote_ranking(self):
         # Votes go from best to worst.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         self._createComment(proposal, vote=CodeReviewVote.DISAPPROVE)
         self._createComment(proposal, vote=CodeReviewVote.APPROVE)
         summary, comment_count = self._get_vote_summary(proposal)
@@ -158,7 +214,7 @@
 
     def test_multiple_votes_for_type(self):
         # Multiple votes of a type are aggregated in the summary.
-        proposal = self.factory.makeBranchMergeProposal()
+        proposal = self._makeBranchMergeProposal()
         self._createComment(proposal, vote=CodeReviewVote.DISAPPROVE)
         self._createComment(proposal, vote=CodeReviewVote.APPROVE)
         self._createComment(proposal, vote=CodeReviewVote.DISAPPROVE)
@@ -178,6 +234,16 @@
         self.assertEqual(4, comment_count)
 
 
+class TestProposalVoteSummaryBzr(
+    TestProposalVoteSummaryMixin, BzrMixin, TestCaseWithFactory):
+    """Test the vote summary for Bazaar."""
+
+
+class TestProposalVoteSummaryGit(
+    TestProposalVoteSummaryMixin, GitMixin, TestCaseWithFactory):
+    """Test the vote summary for Git."""
+
+
 class TestMerges(BrowserTestCase):
 
     layer = DatabaseFunctionalLayer
@@ -193,7 +259,7 @@
         package = self.factory.makeDistributionSourcePackage()
         self.getViewBrowser(package, '+merges', rootsite='code')
 
-    def test_query_count(self):
+    def test_query_count_bzr(self):
         product = self.factory.makeProduct()
         target = self.factory.makeBranch(
             product=product, information_type=InformationType.USERDATA)
@@ -209,24 +275,51 @@
                 product, '+merges', rootsite='code', user=product.owner)
         self.assertThat(recorder, HasQueryCount(Equals(41)))
 
-    def test_productseries(self):
+    def test_query_count_git(self):
+        self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+        product = self.factory.makeProduct()
+        [target] = self.factory.makeGitRefs(
+            target=product, information_type=InformationType.USERDATA)
+        for i in range(7):
+            [source] = self.factory.makeGitRefs(
+                target=product, information_type=InformationType.USERDATA)
+            self.factory.makeBranchMergeProposalForGit(
+                source_ref=source, target_ref=target)
+        flush_database_caches()
+        with StormStatementRecorder() as recorder:
+            self.getViewBrowser(
+                product, '+merges', rootsite='code', user=product.owner)
+        self.assertThat(recorder, HasQueryCount(Equals(38)))
+
+    def test_productseries_bzr(self):
         target = self.factory.makeBranch()
-        unique_name = target.unique_name
         with person_logged_in(target.product.owner):
             target.product.development_focus.branch = target
+            identity = target.identity
         self.factory.makeBranchMergeProposal(target_branch=target)
         view = self.getViewBrowser(target, '+merges', rootsite='code')
-        self.assertIn(unique_name, view.contents)
-
-
-class ActiveReviewGroupsTest(TestCaseWithFactory):
-    """Tests for groupings used in for active reviews."""
+        self.assertIn(identity, view.contents)
+
+    def test_product_git(self):
+        self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+        [target] = self.factory.makeGitRefs()
+        with person_logged_in(target.target.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                target.target, target.repository)
+            identity = target.identity
+        self.factory.makeBranchMergeProposalForGit(target_ref=target)
+        view = self.getViewBrowser(target, '+merges', rootsite='code')
+        self.assertIn(identity, view.contents)
+
+
+class ActiveReviewGroupsTestMixin:
+    """Tests for groupings used for active reviews."""
 
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(ActiveReviewGroupsTest, self).setUp()
-        self.bmp = self.factory.makeBranchMergeProposal(
+        super(ActiveReviewGroupsTestMixin, self).setUp()
+        self.bmp = self._makeBranchMergeProposal(
             set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
 
     def assertReviewGroupForReviewer(self, reviewer, group):
@@ -247,21 +340,21 @@
 
     def test_approved(self):
         # If the proposal is approved, then the group is approved.
-        self.bmp = self.factory.makeBranchMergeProposal(
+        self.bmp = self._makeBranchMergeProposal(
             set_state=BranchMergeProposalStatus.CODE_APPROVED)
         self.assertReviewGroupForReviewer(None, ActiveReviewsView.APPROVED)
 
     def test_work_in_progress(self):
         # If the proposal is in progress, then the group is wip.
-        self.bmp = self.factory.makeBranchMergeProposal(
+        self.bmp = self._makeBranchMergeProposal(
             set_state=BranchMergeProposalStatus.WORK_IN_PROGRESS)
         self.assertReviewGroupForReviewer(None, ActiveReviewsView.WIP)
 
-    def test_source_branch_owner(self):
-        # If the reviewer is the owner of the source branch, then the review
+    def test_merge_source_owner(self):
+        # If the reviewer is the owner of the merge source, then the review
         # is MINE.  This occurs whether or not the user is the logged in or
         # not.
-        reviewer = self.bmp.source_branch.owner
+        reviewer = self.bmp.merge_source.owner
         self.assertReviewGroupForReviewer(reviewer, ActiveReviewsView.MINE)
 
     def test_proposal_registrant(self):
@@ -271,13 +364,17 @@
         self.assertReviewGroupForReviewer(reviewer, ActiveReviewsView.OTHER)
 
         team = self.factory.makeTeam(self.bmp.registrant)
-        removeSecurityProxy(self.bmp.source_branch).owner = team
+        naked_merge_source = removeSecurityProxy(self.bmp.merge_source)
+        if IGitRef.providedBy(naked_merge_source):
+            naked_merge_source.repository.owner = team
+        else:
+            naked_merge_source.owner = team
         self.assertReviewGroupForReviewer(reviewer, ActiveReviewsView.MINE)
 
-    def test_target_branch_owner(self):
-        # For the target branch owner, it is to_do since they are the default
+    def test_merge_target_owner(self):
+        # For the merge target owner, it is to_do since they are the default
         # reviewer.
-        reviewer = self.bmp.target_branch.owner
+        reviewer = self.bmp.merge_target.owner
         self.assertReviewGroupForReviewer(reviewer, ActiveReviewsView.TO_DO)
 
     def test_group_pending_review(self):
@@ -299,7 +396,7 @@
     def test_review_done(self):
         # If the logged in user has a completed review, then the review is
         # ARE_DOING.
-        reviewer = self.bmp.target_branch.owner
+        reviewer = self.bmp.merge_target.owner
         login_person(reviewer)
         self.bmp.createComment(
             reviewer, 'subject', vote=CodeReviewVote.APPROVE)
@@ -307,7 +404,17 @@
             reviewer, ActiveReviewsView.ARE_DOING)
 
 
-class TestBranchMergeProposalListingItem(TestCaseWithFactory):
+class ActiveReviewGroupsTestBzr(
+    ActiveReviewGroupsTestMixin, BzrMixin, TestCaseWithFactory):
+    """Tests for groupings used for active reviews for Bazaar."""
+
+
+class ActiveReviewGroupsTestGit(
+    ActiveReviewGroupsTestMixin, GitMixin, TestCaseWithFactory):
+    """Tests for groupings used for active reviews for Git."""
+
+
+class TestBranchMergeProposalListingItemMixin:
     """Tests specifically relating to the BranchMergeProposalListingItem."""
 
     layer = DatabaseFunctionalLayer
@@ -358,7 +465,17 @@
         self.assertEqual(bmp.date_created, item.sort_key)
 
 
-class ActiveReviewSortingTest(TestCaseWithFactory):
+class TestBranchMergeProposalListingItemBzr(
+    TestBranchMergeProposalListingItemMixin, BzrMixin, TestCaseWithFactory):
+    """Test BranchMergeProposalListingItem for Bazaar."""
+
+
+class TestBranchMergeProposalListingItemGit(
+    TestBranchMergeProposalListingItemMixin, GitMixin, TestCaseWithFactory):
+    """Test BranchMergeProposalListingItem for Git."""
+
+
+class ActiveReviewSortingTestMixin:
     """Test the sorting of the active review groups."""
 
     layer = DatabaseFunctionalLayer
@@ -366,14 +483,14 @@
     def test_oldest_first(self):
         # The oldest requested reviews should be first.
         product = self.factory.makeProduct()
-        bmp1 = self.factory.makeBranchMergeProposal(product=product)
-        login_person(bmp1.source_branch.owner)
+        bmp1 = self._makeBranchMergeProposal(target=product)
+        login_person(bmp1.merge_source.owner)
         bmp1.requestReview(datetime(2009, 6, 1, tzinfo=pytz.UTC))
-        bmp2 = self.factory.makeBranchMergeProposal(product=product)
-        login_person(bmp2.source_branch.owner)
+        bmp2 = self._makeBranchMergeProposal(target=product)
+        login_person(bmp2.merge_source.owner)
         bmp2.requestReview(datetime(2009, 3, 1, tzinfo=pytz.UTC))
-        bmp3 = self.factory.makeBranchMergeProposal(product=product)
-        login_person(bmp3.source_branch.owner)
+        bmp3 = self._makeBranchMergeProposal(target=product)
+        login_person(bmp3.merge_source.owner)
         bmp3.requestReview(datetime(2009, 1, 1, tzinfo=pytz.UTC))
         login(ANONYMOUS)
         view = create_initialized_view(
@@ -383,8 +500,18 @@
             [item.context for item in view.review_groups[view.OTHER]])
 
 
-class ActiveReviewsWithPrivateBranches(TestCaseWithFactory):
-    """Test the sorting of the active review groups."""
+class ActiveReviewSortingTestBzr(
+    ActiveReviewSortingTestMixin, BzrMixin, TestCaseWithFactory):
+    """Test the sorting of the active review groups for Bazaar."""
+
+
+class ActiveReviewSortingTestGit(
+    ActiveReviewSortingTestMixin, GitMixin, TestCaseWithFactory):
+    """Test the sorting of the active review groups for Git."""
+
+
+class ActiveReviewsWithPrivateBranchesMixin:
+    """Test reviews of private branches."""
 
     layer = DatabaseFunctionalLayer
 
@@ -392,32 +519,42 @@
         # Merge proposals against private branches are visible to
         # the branch owner.
         product = self.factory.makeProduct()
-        branch = self.factory.makeBranch(
-            product=product, information_type=InformationType.USERDATA)
+        branch = self._makeBranch(
+            target=product, information_type=InformationType.USERDATA)
         with person_logged_in(removeSecurityProxy(branch).owner):
-            mp = self.factory.makeBranchMergeProposal(target_branch=branch)
+            mp = self._makeBranchMergeProposal(merge_target=branch)
             view = create_initialized_view(
                 branch, name='+activereviews', rootsite='code')
             self.assertEqual([mp], list(view.getProposals()))
 
 
-class PersonActiveReviewsPerformance(TestCaseWithFactory):
+class ActiveReviewsWithPrivateBranchesBzr(
+    ActiveReviewsWithPrivateBranchesMixin, BzrMixin, TestCaseWithFactory):
+    """Test reviews of private Bazaar branches."""
+
+
+class ActiveReviewsWithPrivateBranchesGit(
+    ActiveReviewsWithPrivateBranchesMixin, GitMixin, TestCaseWithFactory):
+    """Test reviews of references in private Git repositories."""
+
+
+class PersonActiveReviewsPerformanceMixin:
     """Test the performance of the person's active reviews page."""
 
     layer = LaunchpadFunctionalLayer
 
     def setupBMP(self, bmp):
         self.factory.makePreviewDiff(merge_proposal=bmp)
-        login_person(bmp.source_branch.owner)
+        login_person(bmp.merge_source.owner)
         bmp.requestReview()
 
-    def createUserBMP(self, reviewer=None, target_branch_owner=None):
-        target_branch = None
-        if target_branch_owner is not None:
-            target_branch = self.factory.makePackageBranch(
-                owner=target_branch_owner)
-        bmp = self.factory.makeBranchMergeProposal(
-            reviewer=reviewer, target_branch=target_branch)
+    def createUserBMP(self, reviewer=None, merge_target_owner=None):
+        merge_target = None
+        if merge_target_owner is not None:
+            merge_target = self._makePackageBranch(
+                owner=merge_target_owner)
+        bmp = self._makeBranchMergeProposal(
+            reviewer=reviewer, merge_target=merge_target)
         self.setupBMP(bmp)
         return bmp
 
@@ -431,10 +568,9 @@
             # Create one of the two types of BMP which will be displayed
             # on a person's +activereviews page:
             # - A BMP for which the person is the reviewer.
-            # - A BMP for which the person is the owner of the target
-            # branch.
+            # - A BMP for which the person is the owner of the merge target.
             if i % 2 == 0:
-                self.createUserBMP(target_branch_owner=user)
+                self.createUserBMP(merge_target_owner=user)
             else:
                 self.createUserBMP(reviewer=user)
         login_person(user)
@@ -457,9 +593,9 @@
         self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
 
     def createProductBMP(self, product):
-        target_branch = self.factory.makeStackedOnBranchChain(product=product)
-        bmp = self.factory.makeBranchMergeProposal(
-            product=product, target_branch=target_branch)
+        merge_target = self._makeStackedOnBranchChain(target=product)
+        bmp = self._makeBranchMergeProposal(
+            target=product, merge_target=merge_target)
         self.setupBMP(bmp)
         return bmp
 
@@ -491,3 +627,13 @@
             base_bmps + added_bmps)
         self.assertEqual(base_bmps + added_bmps, view2.proposal_count)
         self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
+
+
+class PersonActiveReviewsPerformanceBzr(
+    PersonActiveReviewsPerformanceMixin, BzrMixin, TestCaseWithFactory):
+    """Test the performance of the person's active reviews page for Bazaar."""
+
+
+class PersonActiveReviewsPerformanceGit(
+    PersonActiveReviewsPerformanceMixin, GitMixin, TestCaseWithFactory):
+    """Test the performance of the person's active reviews page for Git."""

=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py	2015-04-24 12:58:46 +0000
+++ lib/lp/code/interfaces/gitref.py	2015-05-13 09:36:31 +0000
@@ -43,11 +43,12 @@
     BranchMergeProposalStatus,
     GitObjectType,
     )
+from lp.code.interfaces.hasbranches import IHasMergeProposals
 from lp.registry.interfaces.person import IPerson
 from lp.services.webapp.interfaces import ITableBatchNavigator
 
 
-class IGitRef(Interface):
+class IGitRef(IHasMergeProposals):
     """A reference in a Git repository."""
 
     # XXX cjwatson 2015-01-19 bug=760849: "beta" is a lie to get WADL

=== modified file 'lib/lp/code/interfaces/hasbranches.py'
--- lib/lp/code/interfaces/hasbranches.py	2013-01-07 02:40:55 +0000
+++ lib/lp/code/interfaces/hasbranches.py	2015-05-13 09:36:31 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interface definitions for IHas<code related bits>."""
@@ -56,7 +56,7 @@
             title=_('Limit the branches to those modified since this date.'),
             required=False))
     @call_with(visible_by_user=REQUEST_USER)
-    @operation_returns_collection_of(Interface) # Really IBranch.
+    @operation_returns_collection_of(Interface)  # Really IBranch.
     @export_read_operation()
     @operation_for_version('beta')
     def getBranches(status=None, visible_by_user=None,
@@ -88,7 +88,7 @@
             title=_("A list of merge proposal statuses to filter by."),
             value_type=Choice(vocabulary=BranchMergeProposalStatus)))
     @call_with(visible_by_user=REQUEST_USER)
-    @operation_returns_collection_of(Interface) # Really IBranchMergeProposal.
+    @operation_returns_collection_of(Interface)  # Really IBranchMergeProposal.
     @export_read_operation()
     @operation_for_version('beta')
     def getMergeProposals(status=None, visible_by_user=None):
@@ -115,7 +115,7 @@
             title=_("A list of merge proposal statuses to filter by."),
             value_type=Choice(vocabulary=BranchMergeProposalStatus)))
     @call_with(visible_by_user=REQUEST_USER)
-    @operation_returns_collection_of(Interface) # Really IBranchMergeProposal.
+    @operation_returns_collection_of(Interface)  # Really IBranchMergeProposal.
     @export_read_operation()
     @operation_for_version('beta')
     def getRequestedReviews(status=None, visible_by_user=None):
@@ -130,6 +130,21 @@
         :returns: A list of `IBranchMergeProposal`.
         """
 
+    def getOwnedAndRequestedReviews(status=None, visible_by_user=None,
+                                    project=None):
+        """Returns merge proposals for branches owned by a person, or where
+        that person was asked to review.
+
+        This does not include merge proposals that were requested from
+        teams that the person is part of. If status is not passed then
+        it will return proposals that are in the "Needs Review" state.
+
+        :param status: A list of statuses to filter with.
+        :param visible_by_user: Normally the user who is asking.
+        :param project: Limit results to branches for this `IProduct`.
+        :returns: A list of `IBranchMergeProposal`.
+        """
+
 
 class IHasCodeImports(Interface):
     """Some things can have code imports that target them.
@@ -151,7 +166,7 @@
             schema=Interface)
         )
     @call_with(registrant=REQUEST_USER)
-    @export_factory_operation(Interface, []) # Really ICodeImport.
+    @export_factory_operation(Interface, [])  # Really ICodeImport.
     @operation_for_version('beta')
     def newCodeImport(registrant=None, branch_name=None, rcs_type=None,
                       url=None, cvs_root=None, cvs_module=None, owner=None):

=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2015-04-22 16:42:57 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2015-05-13 09:36:31 +0000
@@ -92,7 +92,10 @@
 from lp.registry.interfaces.product import IProduct
 from lp.registry.model.person import Person
 from lp.services.config import config
-from lp.services.database.bulk import load_related
+from lp.services.database.bulk import (
+    load,
+    load_related,
+    )
 from lp.services.database.constants import (
     DEFAULT,
     UTC_NOW,
@@ -1055,20 +1058,30 @@
         from lp.code.model.branch import Branch
         from lp.code.model.branchcollection import GenericBranchCollection
         from lp.code.model.gitcollection import GenericGitCollection
+        from lp.code.model.gitref import GitRef
         from lp.code.model.gitrepository import GitRepository
 
         ids = set()
         source_branch_ids = set()
-        source_git_repository_ids = set()
+        git_ref_keys = set()
         person_ids = set()
         for mp in branch_merge_proposals:
             ids.add(mp.id)
             if mp.source_branchID is not None:
                 source_branch_ids.add(mp.source_branchID)
             if mp.source_git_repositoryID is not None:
-                source_git_repository_ids.add(mp.source_git_repositoryID)
+                git_ref_keys.add(
+                    (mp.source_git_repositoryID, mp.source_git_path))
+                git_ref_keys.add(
+                    (mp.target_git_repositoryID, mp.target_git_path))
+                if mp.prerequisite_git_repositoryID is not None:
+                    git_ref_keys.add(
+                        (mp.prerequisite_git_repositoryID,
+                         mp.prerequisite_git_path))
             person_ids.add(mp.registrantID)
             person_ids.add(mp.merge_reporterID)
+        git_repository_ids = set(
+            repository_id for repository_id, _ in git_ref_keys)
 
         branches = load_related(
             Branch, branch_merge_proposals, (
@@ -1078,9 +1091,11 @@
             GitRepository, branch_merge_proposals, (
                 "target_git_repositoryID", "prerequisite_git_repositoryID",
                 "source_git_repositoryID"))
+        load(GitRef, git_ref_keys)
         # The stacked on branches are used to check branch visibility.
         GenericBranchCollection.preloadVisibleStackedOnBranches(
             branches, user)
+        GenericGitCollection.preloadVisibleRepositories(repositories, user)
 
         if len(branches) == 0 and len(repositories) == 0:
             return
@@ -1098,21 +1113,24 @@
             cache.preview_diff = previewdiff
 
         # Add source branch/repository owners' to the list of pre-loaded
-        # persons.
+        # persons.  We need the target repository owner as well; unlike
+        # branches, repository unique names aren't trigger-maintained.
         person_ids.update(
             branch.ownerID for branch in branches
             if branch.id in source_branch_ids)
         person_ids.update(
             repository.owner_id for repository in repositories
-            if repository.id in source_git_repository_ids)
+            if repository.id in git_repository_ids)
 
         # Pre-load Person and ValidPersonCache.
         list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
             person_ids, need_validity=True))
 
         # Pre-load branches'/repositories' data.
-        GenericBranchCollection.preloadDataForBranches(branches)
-        GenericGitCollection.preloadDataForRepositories(repositories)
+        if branches:
+            GenericBranchCollection.preloadDataForBranches(branches)
+        if repositories:
+            GenericGitCollection.preloadDataForRepositories(repositories)
 
 
 class BranchMergeProposalGetter:

=== modified file 'lib/lp/code/model/gitcollection.py'
--- lib/lp/code/model/gitcollection.py	2015-04-22 16:42:57 +0000
+++ lib/lp/code/model/gitcollection.py	2015-05-13 09:36:31 +0000
@@ -9,6 +9,7 @@
     ]
 
 from functools import partial
+from operator import attrgetter
 
 from lazr.uri import (
     InvalidURIError,
@@ -178,6 +179,20 @@
         return []
 
     @staticmethod
+    def preloadVisibleRepositories(repositories, user=None):
+        """Preload visibility for the given list of repositories."""
+        if len(repositories) == 0:
+            return
+        expressions = [
+            GitRepository.id.is_in(map(attrgetter("id"), repositories))]
+        if user is None:
+            collection = AnonymousGitCollection(filter_expressions=expressions)
+        else:
+            collection = VisibleGitCollection(
+                user=user, filter_expressions=expressions)
+        return list(collection.getRepositories())
+
+    @staticmethod
     def preloadDataForRepositories(repositories):
         """Preload repositories' cached associated targets."""
         load_related(Distribution, repositories, ['distribution_id'])

=== modified file 'lib/lp/code/model/hasbranches.py'
--- lib/lp/code/model/hasbranches.py	2013-03-04 04:17:17 +0000
+++ lib/lp/code/model/hasbranches.py	2015-05-13 09:36:31 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Mixin classes to implement methods for IHas<code related bits>."""
@@ -11,7 +11,10 @@
     'HasRequestedReviewsMixin',
     ]
 
+from functools import partial
+
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.code.enums import BranchMergeProposalStatus
 from lp.code.interfaces.branch import DEFAULT_BRANCH_STATUS_IN_LISTING
@@ -20,6 +23,11 @@
     IBranchCollection,
     )
 from lp.code.interfaces.branchtarget import IBranchTarget
+from lp.code.interfaces.gitcollection import (
+    IAllGitRepositories,
+    IGitCollection,
+    )
+from lp.services.database.decoratedresultset import DecoratedResultSet
 
 
 class HasBranchesMixin:
@@ -44,14 +52,28 @@
     def getMergeProposals(self, status=None, visible_by_user=None,
                           eager_load=False):
         """See `IHasMergeProposals`."""
+        # Circular import.
+        from lp.code.model.branchmergeproposal import BranchMergeProposal
+
         if not status:
             status = (
                 BranchMergeProposalStatus.CODE_APPROVED,
                 BranchMergeProposalStatus.NEEDS_REVIEW,
                 BranchMergeProposalStatus.WORK_IN_PROGRESS)
 
-        collection = IBranchCollection(self).visibleByUser(visible_by_user)
-        return collection.getMergeProposals(status, eager_load=eager_load)
+        def _getProposals(interface):
+            collection = removeSecurityProxy(interface(self))
+            collection = collection.visibleByUser(visible_by_user)
+            return collection.getMergeProposals(status, eager_load=False)
+
+        proposals = _getProposals(IBranchCollection).union(
+            _getProposals(IGitCollection))
+        if not eager_load:
+            return proposals
+        else:
+            loader = partial(
+                BranchMergeProposal.preloadDataForBMPs, user=visible_by_user)
+            return DecoratedResultSet(proposals, pre_iter_hook=loader)
 
 
 class HasRequestedReviewsMixin:
@@ -66,6 +88,34 @@
             visible_by_user)
         return visible_branches.getMergeProposalsForReviewer(self, status)
 
+    def getOwnedAndRequestedReviews(self, status=None, visible_by_user=None,
+                                    project=None, eager_load=False):
+        """See `IHasRequestedReviews`."""
+        # Circular import.
+        from lp.code.model.branchmergeproposal import BranchMergeProposal
+
+        if not status:
+            status = (BranchMergeProposalStatus.NEEDS_REVIEW,)
+
+        def _getProposals(collection):
+            collection = collection.visibleByUser(visible_by_user)
+            return collection.getMergeProposalsForPerson(
+                self, status, eager_load=False)
+
+        bzr_collection = removeSecurityProxy(getUtility(IAllBranches))
+        git_collection = removeSecurityProxy(getUtility(IAllGitRepositories))
+        if project is not None:
+            bzr_collection = bzr_collection.inProduct(project)
+            git_collection = git_collection.inProject(project)
+        proposals = _getProposals(bzr_collection).union(
+            _getProposals(git_collection))
+        if not eager_load:
+            return proposals
+        else:
+            loader = partial(
+                BranchMergeProposal.preloadDataForBMPs, user=visible_by_user)
+            return DecoratedResultSet(proposals, pre_iter_hook=loader)
+
 
 class HasCodeImportsMixin:
 

=== modified file 'lib/lp/code/templates/branchmergeproposal-listing.pt'
--- lib/lp/code/templates/branchmergeproposal-listing.pt	2009-09-14 02:04:02 +0000
+++ lib/lp/code/templates/branchmergeproposal-listing.pt	2015-05-13 09:36:31 +0000
@@ -35,10 +35,10 @@
         <td>
           <a tal:attributes="href proposal/fmt:url">
             <strong>
-              <tal:source-branch replace="proposal/source_branch/bzr_identity"/>
+              <tal:merge-source replace="proposal/merge_source/identity"/>
             </strong>
             &rArr;
-            <tal:source-branch replace="proposal/target_branch/bzr_identity"/>
+            <tal:merge-target replace="proposal/merge_target/identity"/>
           </a>
         </td>
         <td tal:attributes="class string:mergestatus${proposal/queue_status/name}"

=== modified file 'lib/lp/code/templates/branchmergeproposal-macros.pt'
--- lib/lp/code/templates/branchmergeproposal-macros.pt	2010-01-10 21:40:41 +0000
+++ lib/lp/code/templates/branchmergeproposal-macros.pt	2015-05-13 09:36:31 +0000
@@ -45,10 +45,10 @@
       <td>
         <a tal:attributes="href proposal/fmt:url">
           <strong>
-            <tal:source-branch replace="proposal/source_branch/bzr_identity"/>
+            <tal:merge-source replace="proposal/merge_source/identity"/>
           </strong>
           &rArr;
-          <tal:source-branch replace="proposal/target_branch/bzr_identity"/>
+          <tal:merge-target replace="proposal/merge_target/identity"/>
         </a>
       </td>
       <td>