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

[Colin Watson]

Also return Git-based merge proposals from IHasRequestedReviews.getRequestedReviews.

Also return Git-based merge proposals from IHasRequestedReviews.getRequestedReviews.

I made similar changes to nearby code in https://code.launchpad.net/~cjwatson/launchpad/git-activereviews/+merge/258910, but apparently missed the existence of getRequestedReviews.

[William Grant (wgrant)]

review: Approve (code)

[William Grant (wgrant)]

Actually, getMergeProposalsForReviewer sorts, and this will likely drop that sort. Is that a problem?

review: Needs Information

[Colin Watson (cjwatson)]

Hm. Possibly, and fixing that is difficult with the current structure (CodeReviewComment.vote isn't in the UNIONed result set). I'm thinking that the best way to tackle this might be to do something like what we did with Snap, and move the queries for merge proposals on collections to the BranchMergeProposal model so that we can use a simple OR here rather than a UNION. We would have to think carefully about how to design the bits that remain on *Collection - there'd have to be some reasonable way to extract symmetric vs. asymmetric expressions - but we'd probably still end up with a net reduction of code as well as being able to support this case. Does that sound reasonable?

[Colin Watson (cjwatson)]

The Snap disentangling that I mentioned back then was https://git.launchpad.net/launchpad/commit?id=aa00ff2075040f0487d4c09df401b3ae92ed885c.

Unmerged revisions

17739. By Colin Watson on 2015-09-15

Also return Git-based merge proposals from IHasRequestedReviews.getRequestedReviews.

=== modified file 'lib/lp/code/model/hasbranches.py'
--- lib/lp/code/model/hasbranches.py	2015-05-12 17:30:40 +0000
+++ lib/lp/code/model/hasbranches.py	2015-09-15 14:34:38 +0000
@@ -84,9 +84,14 @@
         if not status:
             status = (BranchMergeProposalStatus.NEEDS_REVIEW,)
 
-        visible_branches = getUtility(IAllBranches).visibleByUser(
-            visible_by_user)
-        return visible_branches.getMergeProposalsForReviewer(self, status)
+        def _getProposals(collection):
+            collection = collection.visibleByUser(visible_by_user)
+            return collection.getMergeProposalsForReviewer(self, status)
+
+        bzr_collection = removeSecurityProxy(getUtility(IAllBranches))
+        git_collection = removeSecurityProxy(getUtility(IAllGitRepositories))
+        return _getProposals(bzr_collection).union(
+            _getProposals(git_collection))
 
     def getOwnedAndRequestedReviews(self, status=None, visible_by_user=None,
                                     project=None, eager_load=False):

=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2015-04-22 14:52:10 +0000
+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2015-09-15 14:34:38 +0000
@@ -470,6 +470,29 @@
     ...     registrant=target_owner, source_branch=target_branch)
     >>> proposal.nominateReviewer(branch_owner, target_owner)
     <CodeReviewVoteReference at ...>
+
+Create a similar pair of proposals for Git.
+
+    >>> [source_ref] = factory.makeGitRefs(
+    ...     owner=branch_owner, target=blob, name=u"foo",
+    ...     paths=[u"refs/heads/master"])
+    >>> [target_ref] = factory.makeGitRefs(
+    ...     owner=target_owner, target=blob, name=u"bar",
+    ...     paths=[u"refs/heads/fix"])
+    >>> proposal = factory.makeBranchMergeProposalForGit(
+    ...     target_ref=target_ref,
+    ...     set_state=BranchMergeProposalStatus.NEEDS_REVIEW,
+    ...     registrant=branch_owner, source_ref=source_ref)
+    >>> proposal.nominateReviewer(target_owner, branch_owner)
+    <CodeReviewVoteReference at ...>
+
+    >>> proposal = factory.makeBranchMergeProposalForGit(
+    ...     target_ref=source_ref,
+    ...     set_state=BranchMergeProposalStatus.NEEDS_REVIEW,
+    ...     registrant=target_owner, source_ref=target_ref)
+    >>> proposal.nominateReviewer(branch_owner, target_owner)
+    <CodeReviewVoteReference at ...>
+
     >>> logout()
 
     >>> proposals = webservice.named_get('/~target', 'getRequestedReviews'
@@ -477,4 +500,4 @@
     >>> for proposal in proposals['entries']:
     ...     print_proposal(proposal)
     http://.../~source/blob/foo/+merge/4 - Needs review
-
+    http://.../~source/blob/+git/foo/+merge/6 - Needs review