Merge lp:~james-w/launchpad/fix-getRequestedReviews into lp:launchpad

[James Westby]

getRequestedReviews no longer returns the merge proposals owned by the person in question.

[James Westby (james-w)]

Hi,

This fixes a bug with my getRequestedReviews implementation, which
was basically that I fail at reading docstrings.

This switches it to use the method that matches what getRequestedReviews
says that it will do, and modifies the test so that it should set up
an environment that will distinguish the two.

It is however untested.

Thanks,

James

[Aaron Bentley (abentley)]

> It is however untested.

The test fails:

abentley@lumpy:~/launchpad/fix-getRequestedReviews$ bin/test -t xx-branchmergeproposal.txt
Running canonical.testing.layers.PageTestLayer tests:
  Set up canonical.testing.layers.BaseLayer in 0.002 seconds.
  Set up canonical.testing.layers.DatabaseLayer in 0.348 seconds.
  Set up canonical.testing.layers.LibrarianLayer in 7.496 seconds.
  Set up canonical.testing.layers.MemcachedLayer in 0.115 seconds.
  Set up canonical.testing.layers.LaunchpadLayer in 0.000 seconds.
  Set up canonical.testing.layers.FunctionalLayer in 4.550 seconds.
  Set up canonical.testing.layers.GoogleServiceLayer in 2.321 seconds.
  Set up canonical.testing.layers.LaunchpadFunctionalLayer in 0.000 seconds.
  Set up canonical.testing.layers.PageTestLayer in 0.001 seconds.

Failure in test lib/lp/code/tests/../stories/webservice/xx-branchmergeproposal.txt
Failed doctest test for xx-branchmergeproposal.txt
  File "lib/lp/code/tests/../stories/webservice/xx-branchmergeproposal.txt", line 0

----------------------------------------------------------------------
File "lib/lp/code/tests/../stories/webservice/xx-branchmergeproposal.txt", line 474, in xx-branchmergeproposal.txt
Failed example:
    proposals = webservice.named_get('/~target', 'getRequestedReviews'
        ).jsonBody()
Exception raised:
    Traceback (most recent call last):
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/zope.testing-3.8.1-py2.5.egg/zope/testing/doctest.py", line 1361, in __run
        compileflags, 1) in test.globs
      File "<doctest xx-branchmergeproposal.txt[line 474, example 119]>", line 1, in <module>
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/lazr.restful-0.9.17-py2.5.egg/lazr/restful/testing/webservice.py", line 272, in named_get
        api_version=api_version)
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/lazr.restful-0.9.17-py2.5.egg/lazr/restful/testing/webservice.py", line 232, in get
        api_version=api_version)
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/lazr.restful-0.9.17-py2.5.egg/lazr/restful/testing/webservice.py", line 221, in __call__
        self.addHeadersTo(full_url, full_headers) # a hook
      File "/home/abentley/launchpad/fix-getRequestedReviews/lib/canonical/launchpad/testing/pages.py", line 139, in addHeadersTo
        self.consumer, self.access_token, http_url = full_url,
      File "/home/abentley/launchpad/fix-getRequestedReviews/lib/contrib/oauth.py", line 216, in from_consumer_and_token
        'oauth_consumer_key': oauth_consumer.key,
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/storm-0.15danilo_storm_launchpad_r342-py2.5-linux-i686.egg/storm/properties.py", line 60, in __get__
        return obj_info.variables[column].get()
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/storm-0.15danilo_storm_launchpad_r342-py2.5-linux-i686.egg/storm/store.py", line 879, in _resolve_lazy_value
        self.flush()
      File "/home/abentley/launchpad/fix-getRequestedReviews/eggs/storm-0.15danilo_storm_launchpad_r342-py2.5-linux-i686.egg/storm/store.py", line 486, in flush
        self._flush_one(obj_info...

review: Needs Fixing

[James Westby (james-w)]

Hi,

The test now passes.

Thanks,

James

[ Code changes < 5 minutes, waiting for test set up + tear down ~ 8 minutes,
  getting all the pieces in place to run the testsuite after not having done
  so for a few weeks ~ 80 minutes ]

[Graham Binns (gmb)]

This code looks good now the test passes.

review: Approve (code)

=== modified file 'lib/lp/code/model/hasbranches.py'
--- lib/lp/code/model/hasbranches.py	2010-02-08 14:37:50 +0000
+++ lib/lp/code/model/hasbranches.py	2010-03-02 01:06:25 +0000
@@ -59,5 +59,5 @@
 
         visible_branches = getUtility(IAllBranches).visibleByUser(
             visible_by_user)
-        proposals = visible_branches.getMergeProposalsForPerson(self, status)
+        proposals = visible_branches.getMergeProposalsForReviewer(self, status)
         return proposals

=== modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt'
--- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2010-02-26 00:30:09 +0000
+++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt	2010-03-02 01:06:25 +0000
@@ -464,12 +464,36 @@
 
     >>> login('[e-mail]')
     >>> from lp.code.enums import BranchMergeProposalStatus
-    >>> fixit_proposal.requestReview()
+
+First we create a review owned by someone else and requested of 'target'
+which is the one we want the method to return.
+
+    >>> source_branch = factory.makeBranch(owner=branch_owner,
+    ...     product=blob, name="foo")
+    >>> target_branch = factory.makeBranch(owner=target_owner,
+    ...     product=blob, name="bar")
+    >>> proposal = factory.makeBranchMergeProposal(
+    ...     target_branch=target_branch,
+    ...     product=blob, set_state=BranchMergeProposalStatus.NEEDS_REVIEW,
+    ...     registrant=branch_owner, source_branch=source_branch)
+    >>> proposal.nominateReviewer(target_owner, branch_owner)
+    <CodeReviewVoteReference at ...>
+
+And then we propose a merge the other way, so that the owner is target,
+but they have not been asked to review, meaning that the method shouldn't
+return this review.
+
+    >>> proposal = factory.makeBranchMergeProposal(
+    ...     target_branch=source_branch,
+    ...     product=blob, set_state=BranchMergeProposalStatus.NEEDS_REVIEW,
+    ...     registrant=target_owner, source_branch=target_branch)
+    >>> proposal.nominateReviewer(branch_owner, target_owner)
+    <CodeReviewVoteReference at ...>
     >>> 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
+    http://.../~source/blob/foo/+merge/4 - Needs review