Merge lp:~thumper/launchpad/review-clamining-fix into lp:launchpad

[Tim Penhey]

Fix the claiming team reviews from the review page, add padding for pending diff generation, fix wording on branch pending updates, make other related branch links refer to the code subdomain.

[Tim Penhey (thumper)]

Fixes four pretty trivial bugs.

[Jonathan Lange (jml)]

Code looks fine to me, and it's really, really refreshing to see a branch that cares about little things.

review: Approve

[Michael Nelson (michael.nelson)]

Padding and word-change are great, thanks!

review: Approve (ui)

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2009-12-18 14:51:37 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2010-01-13 22:28:16 +0000
@@ -1343,11 +1343,11 @@
             team = getUtility(IPersonSet).getByName(claim_review)
             if team is not None and self.user.inTeam(team):
                 # If the review type is None, then don't show the field.
+                self.reviewer = team.name
                 if self.initial_values['review_type'] == '':
                     self.form_fields = self.form_fields.omit('review_type')
                 else:
                     # Disable the review_type field
-                    self.reviewer = team.name
                     self.form_fields['review_type'].for_display = True
 
     @property
@@ -1382,7 +1382,7 @@
                 # Claim this vote reference, i.e. say that the individual
                 # self. user is doing this review ond behalf of the 'reviewer'
                 # team.
-                vote_ref.reviewer = self.user
+                vote_ref.claimReview(self.user)
 
         comment = self.context.createComment(
             self.user, subject=None, content=data['comment'],

=== added file 'lib/lp/code/stories/branches/xx-nearby-branches.txt'
--- lib/lp/code/stories/branches/xx-nearby-branches.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/stories/branches/xx-nearby-branches.txt	2010-01-13 22:28:16 +0000
@@ -0,0 +1,25 @@
+Nearby Branches
+===============
+
+At the bottom of the main branch page there are links to other related
+branches.
+
+    >>> login(ANONYMOUS)
+    >>> eric = factory.makePerson(name='eric')
+    >>> vikings = factory.makeTeam(owner=eric, name='vikings')
+    >>> fooix = factory.makeProduct(name='fooix')
+    >>> branch = factory.makeProductBranch(
+    ...     product=fooix, owner=vikings, registrant=eric)
+    >>> url = canonical_url(branch)
+    >>> logout()
+
+    >>> browser.open(url)
+
+Since the links point to the code subdomain, the links are relative to the root.
+
+    >>> div = find_tag_by_id(browser.contents, 'nearby-branches')
+    >>> for anchor in div.findAll('a'):
+    ...     print anchor['href'], anchor.string
+    /fooix                      Other Fooix branches
+    /~vikings                   Other branches owned by Vikings
+    /~eric/+registeredbranches  Other branches registered by Eric

=== added file 'lib/lp/code/stories/branches/xx-reviewing.txt'
--- lib/lp/code/stories/branches/xx-reviewing.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/stories/branches/xx-reviewing.txt	2010-01-13 22:28:16 +0000
@@ -0,0 +1,39 @@
+Reviewing a merge proposal
+==========================
+
+A frequent use case is when a team is asked to review, and a member of that
+team review the proposal on behalf of the team.
+
+    >>> login(ANONYMOUS)
+    >>> eric = factory.makePerson(
+    ...     name='eric', email='[e-mail]', password='test')
+    >>> vikings = factory.makeTeam(owner=eric, name='vikings')
+    >>> bmp = factory.makeBranchMergeProposal()
+    >>> login_person(bmp.registrant)
+    >>> ignored = bmp.nominateReviewer(vikings, bmp.registrant)
+    >>> url = canonical_url(bmp)
+    >>> logout()
+
+When the user looks at the merge proposal page, they see their team name in
+the reviewer block with the link '[Review]'.
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open(url)
+    >>> print_tag_with_id(browser.contents, 'code-review-votes')
+    Reviewer    Review Type    Date Requested   Status
+    Vikings                    ... ago          Pending   [Review]
+    Review via email:   [e-mail]
+
+    >>> browser.getLink('[Review]').click()
+
+The user can then add their comment.
+
+    >>> browser.getControl('Comment').value = 'Looks good'
+    >>> browser.getControl('Save Review').click()
+
+The team review is now claimed by the user.
+
+    >>> print_tag_with_id(browser.contents, 'code-review-votes')
+    Reviewer    Review Type    Date Requested   Status
+    Eric (community)           ... ago          Approve ... ago
+    Review via email:   [e-mail]

=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt	2010-01-05 19:45:58 +0000
+++ lib/lp/code/templates/branch-index.pt	2010-01-13 22:28:16 +0000
@@ -166,7 +166,7 @@
       <div id="branch-pending-writes" class="pending-update">
         <h3>Updating branch...</h3>
         <p>
-          Launchpad is processing new changes to this branch and will be
+          Launchpad is processing new changes to this branch which will be
           available in a few minutes. Reload to see the changes.
         </p>
       </div>
@@ -199,28 +199,28 @@
     </ul>
   </div>
 
-      <div class="related">
+      <div id="nearby-branches" class="related">
 
   <h2>Nearby</h2>
   <ul>
     <li tal:condition="context/product">
-      <a tal:attributes="href context/product/fmt:url">
+      <a tal:attributes="href context/product/fmt:url:code">
         Other <tal:product replace="context/product/displayname" /> branches
       </a>
     </li>
     <li tal:condition="context/sourcepackage">
-      <a tal:attributes="href context/sourcepackage/fmt:url">
+      <a tal:attributes="href context/sourcepackage/fmt:url:code">
         Other <tal:product replace="context/sourcepackage/displayname" /> branches
       </a>
     </li>
     <li>
-      <a tal:attributes="href context/owner/fmt:url">
+      <a tal:attributes="href context/owner/fmt:url:code">
         Other branches owned by <tal:person
         replace="context/owner/fmt:displayname">Owner</tal:person>
       </a>
     </li>
     <li tal:condition="not:view/owner_is_registrant">
-      <a tal:attributes="href string:${context/registrant/fmt:url}/+registeredbranches">
+      <a tal:attributes="href string:${context/registrant/fmt:url:code}/+registeredbranches">
         Other branches registered by <tal:person
         replace="context/registrant/fmt:displayname">Registrant</tal:person>
       </a>

=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt	2009-12-18 20:14:21 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt	2010-01-13 22:28:16 +0000
@@ -181,7 +181,7 @@
   </div>
 
   <div class="yui-g" tal:condition="view/pending_diff">
-    <div class="portlet pending-update" id="diff-pending-update">
+    <div class="pending-update" id="diff-pending-update">
       <h3>Updating diff...</h3>
       <p>
         An updated diff will be available in a few minutes.  Reload to see the