Merge lp:~james-w/launchpad/get-requested-reviews into lp:launchpad

[James Westby]

Export a method on IPerson to get the reviews they have been requested to do.

[James Westby (james-w)]

Hi,

This extends IPerson with a mixin similar to IHasMergeProposals that
doesn't get the owned merge proposals, but those that person has
been requested to review.

Thanks to Barry for his help.

Thanks,

James

[Eleanor Berger (intellectronica)]

(16:27:14) thekorn: james_w, not that I have review powers, but there is a typo in xx-branchmergeproposal.txt: `Gettting` ;)
(16:28:44) intellectronica: james_w: we don't use line continuation in the launchpad codebase. instead use parentheses to group different constituents of an expression appearing on different lines
(16:29:45) intellectronica: i can see that the same problem appears in an existing lines. no idea why the author of that line decided to format it like that. if you feel like fixing that too, i won't complain :) (but don't feel obliged)
https://code.edge.launchpad.net/launchpad/+activereviews
(16:31:52) intellectronica: james_w: the tuple on line 103 of the diff is formatted a bit funny. why not just put it on one line?
(16:32:56) intellectronica: james_w: also, maybe rename 'collection' in the same function to something more meaningful, like 'user_visible_branches'?
(16:34:08) intellectronica: james_w: using title capitalization, isn't "Getting Merge Proposals a Person has been Asked to Review" the correct form?
jamalta james_w
(16:35:23) intellectronica: james_w: why the final comma on line 129?
(16:35:53) intellectronica: james_w: other than that it all looks good

review: Needs Fixing

[Tim Penhey (thumper)]

This looks pretty good to me. Thanks James.

review: Approve

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2010-02-01 19:36:23 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2010-02-04 16:54:18 +0000
@@ -42,7 +42,8 @@
 from lp.code.interfaces.codereviewcomment import ICodeReviewComment
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
 from lp.code.interfaces.diff import IPreviewDiff
-from lp.code.interfaces.hasbranches import IHasBranches, IHasMergeProposals
+from lp.code.interfaces.hasbranches import (
+    IHasBranches, IHasMergeProposals, IHasRequestedReviews)
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distributionmirror import IDistributionMirror
 from lp.registry.interfaces.distributionsourcepackage import (
@@ -116,11 +117,11 @@
 IBranchMergeProposal['votes'].value_type.schema = ICodeReviewVoteReference
 
 IHasBranches['getBranches'].queryTaggedValue(
-    LAZR_WEBSERVICE_EXPORTED)['return_type'].value_type.schema = \
-        IBranch
+    LAZR_WEBSERVICE_EXPORTED)['return_type'].value_type.schema = IBranch
 IHasMergeProposals['getMergeProposals'].queryTaggedValue(
-    LAZR_WEBSERVICE_EXPORTED)['return_type'].value_type.schema = \
-        IBranchMergeProposal
+    LAZR_WEBSERVICE_EXPORTED)['return_type'].value_type.schema = IBranchMergeProposal
+IHasRequestedReviews['getRequestedReviews'].queryTaggedValue(
+    LAZR_WEBSERVICE_EXPORTED)['return_type'].value_type.schema = IBranchMergeProposal
 
 # IBugTask
 

=== modified file 'lib/lp/code/interfaces/hasbranches.py'
--- lib/lp/code/interfaces/hasbranches.py	2009-06-30 16:56:07 +0000
+++ lib/lp/code/interfaces/hasbranches.py	2010-02-04 16:54:18 +0000
@@ -7,6 +7,7 @@
 __all__ = [
     'IHasBranches',
     'IHasMergeProposals',
+    'IHasRequestedReviews',
     ]
 
 
@@ -71,3 +72,33 @@
         :param visible_by_user: Normally the user who is asking.
         :returns: A list of `IBranchMergeProposal`.
         """
+
+
+class IHasRequestedReviews(Interface):
+    """IPersons can have reviews requested of them in merge proposals.
+
+    This interface defines the common methods for getting these merge proposals
+    for a particular person.
+    """
+
+    # In order to minimise dependancies the returns_collection is defined as
+    # Interface here and defined fully in the circular imports file.
+
+    @operation_parameters(
+        status=List(
+            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.
+    @export_read_operation()
+    def getRequestedReviews(status=None, visible_by_user=None):
+        """Returns merge proposals that a review was requested from the person.
+
+        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.
+        :returns: A list of `IBranchMergeProposal`.
+        """

=== modified file 'lib/lp/code/model/hasbranches.py'
--- lib/lp/code/model/hasbranches.py	2009-06-30 16:56:07 +0000
+++ lib/lp/code/model/hasbranches.py	2010-02-04 16:54:18 +0000
@@ -7,12 +7,15 @@
 __all__ = [
     'HasBranchesMixin',
     'HasMergeProposalsMixin',
+    'HasRequestedReviewsMixin',
     ]
 
+from zope.component import getUtility
 
 from lp.code.enums import BranchMergeProposalStatus
 from lp.code.interfaces.branch import DEFAULT_BRANCH_STATUS_IN_LISTING
-from lp.code.interfaces.branchcollection import IBranchCollection
+from lp.code.interfaces.branchcollection import (
+    IAllBranches, IBranchCollection)
 
 
 class HasBranchesMixin:
@@ -41,3 +44,17 @@
 
         collection = IBranchCollection(self).visibleByUser(visible_by_user)
         return collection.getMergeProposals(status)
+
+
+class HasRequestedReviewsMixin:
+    """A mixin implementation class for `IHasRequestedReviews`."""
+
+    def getRequestedReviews(self, status=None, visible_by_user=None):
+        """See `IHasRequestedReviews`."""
+        if not status:
+            status = (BranchMergeProposalStatus.NEEDS_REVIEW,)
+
+        visible_branches = getUtility(IAllBranches).visibleByUser(
+            visible_by_user)
+        proposals = visible_branches.getMergeProposalsForPerson(self, status)
+        return proposals

=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2009-12-06 23:33:54 +0000
+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2010-02-04 16:54:18 +0000
@@ -456,3 +456,20 @@
     >>> print_proposals(webservice, url='/widgets',
     ...     status=[BranchMergeProposalStatus.REJECTED.title])
     http://.../~source/fooix/fix-it/+merge/... - Rejected
+
+== Getting Merge Proposals a Person has been Asked To Review ==
+
+It's good to be able to find out which proposals you have been asked to
+review.
+
+    >>> login('[e-mail]')
+    >>> from lp.code.enums import BranchMergeProposalStatus
+    >>> fixit_proposal.requestReview()
+    >>> logout()
+
+    >>> proposals = webservice.named_get('/~target', 'getRequestedReviews'
+    ...     ).jsonBody()
+    >>> for proposal in proposals['entries']:
+    ...     print_proposal(proposal)
+    http://.../~source/fooix/fix-it/+merge/2 - Needs review
+

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2010-01-15 01:37:34 +0000
+++ lib/lp/registry/interfaces/person.py	2010-02-04 16:54:18 +0000
@@ -72,7 +72,8 @@
 from canonical.launchpad.interfaces.account import AccountStatus, IAccount
 from canonical.launchpad.interfaces.emailaddress import IEmailAddress
 from lp.app.interfaces.headings import IRootContext
-from lp.code.interfaces.hasbranches import IHasBranches, IHasMergeProposals
+from lp.code.interfaces.hasbranches import (
+    IHasBranches, IHasMergeProposals, IHasRequestedReviews)
 from lp.registry.interfaces.irc import IIrcID
 from lp.registry.interfaces.jabber import IJabberID
 from lp.services.worlddata.interfaces.language import ILanguage
@@ -476,7 +477,8 @@
 
 class IPersonPublic(IHasBranches, IHasSpecifications, IHasMentoringOffers,
                     IHasMergeProposals, IHasLogo, IHasMugshot, IHasIcon,
-                    IHasLocation, IObjectWithLocation, IPrivacy):
+                    IHasLocation, IHasRequestedReviews, IObjectWithLocation,
+                    IPrivacy):
     """Public attributes for a Person."""
 
     id = Int(title=_('ID'), required=True, readonly=True)

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-01-20 22:09:26 +0000
+++ lib/lp/registry/model/person.py	2010-02-04 16:54:18 +0000
@@ -81,7 +81,8 @@
 from lp.soyuz.interfaces.archivepermission import (
     IArchivePermissionSet)
 from canonical.launchpad.interfaces.authtoken import LoginTokenType
-from lp.code.model.hasbranches import HasBranchesMixin, HasMergeProposalsMixin
+from lp.code.model.hasbranches import (
+    HasBranchesMixin, HasMergeProposalsMixin, HasRequestedReviewsMixin)
 from lp.bugs.interfaces.bugtask import (
     BugTaskSearchParams, IBugTaskSet)
 from lp.bugs.interfaces.bugtarget import IBugTarget
@@ -212,7 +213,7 @@
 
 class Person(
     SQLBase, HasBugsBase, HasSpecificationsMixin, HasTranslationImportsMixin,
-    HasBranchesMixin, HasMergeProposalsMixin):
+    HasBranchesMixin, HasMergeProposalsMixin, HasRequestedReviewsMixin):
     """A Person."""
 
     implements(IPerson, IHasIcon, IHasLogo, IHasMugshot)