Launchpad itself

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

  1. review-clamining-fix
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Merged at revision: not available
Proposed branch: lp:~thumper/launchpad/review-clamining-fix
Merge into: lp:launchpad
Diff against target: 159 lines (+73/-9)
5 files modified
lib/lp/code/browser/branchmergeproposal.py (+2/-2)
lib/lp/code/stories/branches/xx-nearby-branches.txt (+25/-0)
lib/lp/code/stories/branches/xx-reviewing.txt (+39/-0)
lib/lp/code/templates/branch-index.pt (+6/-6)
lib/lp/code/templates/branchmergeproposal-index.pt (+1/-1)
To merge this branch: bzr merge lp:~thumper/launchpad/review-clamining-fix
Reviewer Review Type Date Requested Status
Michael Nelson (community) ui Approve
Jonathan Lange (community) Approve
Review via email: mp+17323@code.launchpad.net

Commit message

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.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Fixes four pretty trivial bugs.

Revision history for this message
Jonathan Lange (jml) wrote :

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

review: Approve
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Padding and word-change are great, thanks!

review: Approve (ui)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1343 team = getUtility(IPersonSet).getByName(claim_review)1343 team = getUtility(IPersonSet).getByName(claim_review)
1344 if team is not None and self.user.inTeam(team):1344 if team is not None and self.user.inTeam(team):
1345 # If the review type is None, then don't show the field.1345 # If the review type is None, then don't show the field.
1346 self.reviewer = team.name
1346 if self.initial_values['review_type'] == '':1347 if self.initial_values['review_type'] == '':
1347 self.form_fields = self.form_fields.omit('review_type')1348 self.form_fields = self.form_fields.omit('review_type')
1348 else:1349 else:
1349 # Disable the review_type field1350 # Disable the review_type field
1350 self.reviewer = team.name
1351 self.form_fields['review_type'].for_display = True1351 self.form_fields['review_type'].for_display = True
13521352
1353 @property1353 @property
@@ -1382,7 +1382,7 @@
1382 # Claim this vote reference, i.e. say that the individual1382 # Claim this vote reference, i.e. say that the individual
1383 # self. user is doing this review ond behalf of the 'reviewer'1383 # self. user is doing this review ond behalf of the 'reviewer'
1384 # team.1384 # team.
1385 vote_ref.reviewer = self.user1385 vote_ref.claimReview(self.user)
13861386
1387 comment = self.context.createComment(1387 comment = self.context.createComment(
1388 self.user, subject=None, content=data['comment'],1388 self.user, subject=None, content=data['comment'],
13891389
=== 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 @@
1Nearby Branches
2===============
3
4At the bottom of the main branch page there are links to other related
5branches.
6
7 >>> login(ANONYMOUS)
8 >>> eric = factory.makePerson(name='eric')
9 >>> vikings = factory.makeTeam(owner=eric, name='vikings')
10 >>> fooix = factory.makeProduct(name='fooix')
11 >>> branch = factory.makeProductBranch(
12 ... product=fooix, owner=vikings, registrant=eric)
13 >>> url = canonical_url(branch)
14 >>> logout()
15
16 >>> browser.open(url)
17
18Since the links point to the code subdomain, the links are relative to the root.
19
20 >>> div = find_tag_by_id(browser.contents, 'nearby-branches')
21 >>> for anchor in div.findAll('a'):
22 ... print anchor['href'], anchor.string
23 /fooix Other Fooix branches
24 /~vikings Other branches owned by Vikings
25 /~eric/+registeredbranches Other branches registered by Eric
026
=== 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 @@
1Reviewing a merge proposal
2==========================
3
4A frequent use case is when a team is asked to review, and a member of that
5team review the proposal on behalf of the team.
6
7 >>> login(ANONYMOUS)
8 >>> eric = factory.makePerson(
9 ... name='eric', email='eric@example.com', password='test')
10 >>> vikings = factory.makeTeam(owner=eric, name='vikings')
11 >>> bmp = factory.makeBranchMergeProposal()
12 >>> login_person(bmp.registrant)
13 >>> ignored = bmp.nominateReviewer(vikings, bmp.registrant)
14 >>> url = canonical_url(bmp)
15 >>> logout()
16
17When the user looks at the merge proposal page, they see their team name in
18the reviewer block with the link '[Review]'.
19
20 >>> browser = setupBrowser(auth='Basic eric@example.com:test')
21 >>> browser.open(url)
22 >>> print_tag_with_id(browser.contents, 'code-review-votes')
23 Reviewer Review Type Date Requested Status
24 Vikings ... ago Pending [Review]
25 Review via email: mp...@code.launchpad.dev
26
27 >>> browser.getLink('[Review]').click()
28
29The user can then add their comment.
30
31 >>> browser.getControl('Comment').value = 'Looks good'
32 >>> browser.getControl('Save Review').click()
33
34The team review is now claimed by the user.
35
36 >>> print_tag_with_id(browser.contents, 'code-review-votes')
37 Reviewer Review Type Date Requested Status
38 Eric (community) ... ago Approve ... ago
39 Review via email: mp...@code.launchpad.dev
040
=== 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 @@
166 <div id="branch-pending-writes" class="pending-update">166 <div id="branch-pending-writes" class="pending-update">
167 <h3>Updating branch...</h3>167 <h3>Updating branch...</h3>
168 <p>168 <p>
169 Launchpad is processing new changes to this branch and will be169 Launchpad is processing new changes to this branch which will be
170 available in a few minutes. Reload to see the changes.170 available in a few minutes. Reload to see the changes.
171 </p>171 </p>
172 </div>172 </div>
@@ -199,28 +199,28 @@
199 </ul>199 </ul>
200 </div>200 </div>
201201
202 <div class="related">202 <div id="nearby-branches" class="related">
203203
204 <h2>Nearby</h2>204 <h2>Nearby</h2>
205 <ul>205 <ul>
206 <li tal:condition="context/product">206 <li tal:condition="context/product">
207 <a tal:attributes="href context/product/fmt:url">207 <a tal:attributes="href context/product/fmt:url:code">
208 Other <tal:product replace="context/product/displayname" /> branches208 Other <tal:product replace="context/product/displayname" /> branches
209 </a>209 </a>
210 </li>210 </li>
211 <li tal:condition="context/sourcepackage">211 <li tal:condition="context/sourcepackage">
212 <a tal:attributes="href context/sourcepackage/fmt:url">212 <a tal:attributes="href context/sourcepackage/fmt:url:code">
213 Other <tal:product replace="context/sourcepackage/displayname" /> branches213 Other <tal:product replace="context/sourcepackage/displayname" /> branches
214 </a>214 </a>
215 </li>215 </li>
216 <li>216 <li>
217 <a tal:attributes="href context/owner/fmt:url">217 <a tal:attributes="href context/owner/fmt:url:code">
218 Other branches owned by <tal:person218 Other branches owned by <tal:person
219 replace="context/owner/fmt:displayname">Owner</tal:person>219 replace="context/owner/fmt:displayname">Owner</tal:person>
220 </a>220 </a>
221 </li>221 </li>
222 <li tal:condition="not:view/owner_is_registrant">222 <li tal:condition="not:view/owner_is_registrant">
223 <a tal:attributes="href string:${context/registrant/fmt:url}/+registeredbranches">223 <a tal:attributes="href string:${context/registrant/fmt:url:code}/+registeredbranches">
224 Other branches registered by <tal:person224 Other branches registered by <tal:person
225 replace="context/registrant/fmt:displayname">Registrant</tal:person>225 replace="context/registrant/fmt:displayname">Registrant</tal:person>
226 </a>226 </a>
227227
=== 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 @@
181 </div>181 </div>
182182
183 <div class="yui-g" tal:condition="view/pending_diff">183 <div class="yui-g" tal:condition="view/pending_diff">
184 <div class="portlet pending-update" id="diff-pending-update">184 <div class="pending-update" id="diff-pending-update">
185 <h3>Updating diff...</h3>185 <h3>Updating diff...</h3>
186 <p>186 <p>
187 An updated diff will be available in a few minutes. Reload to see the187 An updated diff will be available in a few minutes. Reload to see the