launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/allow-noop-claims into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #608778 in Launchpad itself: "If I click twice on Claim Review I get a ClaimReviewFailed OOPS."
  https://bugs.launchpad.net/launchpad/+bug/608778

For more details, see:
https://code.launchpad.net/~abentley/launchpad/allow-noop-claims/+merge/59795

= Summary =
Fix bug #608778: If I click twice on Claim Review I get a ClaimReviewFailed OOPS.

== Proposed fix ==
Do nothing if attempting to claim an already-claimed review.

== Pre-implementation notes ==
None

== Implementation details ==
As a drive-by, removed assignment to unused variables.

== Tests ==
bin/test -t test_repeat_claim test_codereviewvote

== Demo and Q/A ==
Create a merge proposal.  Open up two copies of the merge proposal page.  Click "Claim review" in the first one.  Click "Claim review" in the second one.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/model/codereviewvote.py
  lib/lp/code/model/tests/test_codereviewvote.py
-- 
https://code.launchpad.net/~abentley/launchpad/allow-noop-claims/+merge/59795
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/allow-noop-claims into lp:launchpad.

=== modified file 'lib/lp/code/model/codereviewvote.py'
--- lib/lp/code/model/codereviewvote.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/codereviewvote.py	2011-05-03 16:04:42 +0000
@@ -81,6 +81,8 @@
 
     def claimReview(self, claimant):
         """See `ICodeReviewVote`"""
+        if self.reviewer == claimant:
+            return
         self.validateClaimReview(claimant)
         self.reviewer = claimant
 

=== modified file 'lib/lp/code/model/tests/test_codereviewvote.py'
--- lib/lp/code/model/tests/test_codereviewvote.py	2010-12-02 16:13:51 +0000
+++ lib/lp/code/model/tests/test_codereviewvote.py	2011-05-03 16:04:42 +0000
@@ -121,6 +121,11 @@
         review.claimReview(self.claimant)
         self.assertEqual(self.claimant, review.reviewer)
 
+    def test_repeat_claim(self):
+        # Attempting to claim an already-claimed review works.
+        review = self.factory.makeCodeReviewVoteReference()
+        review.claimReview(review.reviewer)
+
 
 class TestCodeReviewVoteReferenceDelete(TestCaseWithFactory):
     """Tests for CodeReviewVoteReference.delete."""
@@ -233,8 +238,7 @@
         # pending review assigned to them.
         bmp, review = self.makeMergeProposalWithReview()
         reviewer = self.factory.makePerson(name='eric')
-        user_review = bmp.nominateReviewer(
-            reviewer=reviewer, registrant=bmp.registrant)
+        bmp.nominateReviewer(reviewer=reviewer, registrant=bmp.registrant)
         self.assertRaisesWithContent(
             UserHasExistingReview,
             'Eric (eric) has already been asked to review this',
@@ -258,7 +262,7 @@
         # review assigned to them.
         bmp, review = self.makeMergeProposalWithReview()
         reviewer_team = self.factory.makeTeam()
-        team_review = bmp.nominateReviewer(
+        bmp.nominateReviewer(
             reviewer=reviewer_team, registrant=bmp.registrant)
         review.reassignReview(reviewer_team)
         self.assertEqual(reviewer_team, review.reviewer)