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
1=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
2--- lib/lp/code/browser/branchmergeproposal.py 2009-12-18 14:51:37 +0000
3+++ lib/lp/code/browser/branchmergeproposal.py 2010-01-13 22:28:16 +0000
4@@ -1343,11 +1343,11 @@
5 team = getUtility(IPersonSet).getByName(claim_review)
6 if team is not None and self.user.inTeam(team):
7 # If the review type is None, then don't show the field.
8+ self.reviewer = team.name
9 if self.initial_values['review_type'] == '':
10 self.form_fields = self.form_fields.omit('review_type')
11 else:
12 # Disable the review_type field
13- self.reviewer = team.name
14 self.form_fields['review_type'].for_display = True
15
16 @property
17@@ -1382,7 +1382,7 @@
18 # Claim this vote reference, i.e. say that the individual
19 # self. user is doing this review ond behalf of the 'reviewer'
20 # team.
21- vote_ref.reviewer = self.user
22+ vote_ref.claimReview(self.user)
23
24 comment = self.context.createComment(
25 self.user, subject=None, content=data['comment'],
26
27=== added file 'lib/lp/code/stories/branches/xx-nearby-branches.txt'
28--- lib/lp/code/stories/branches/xx-nearby-branches.txt 1970-01-01 00:00:00 +0000
29+++ lib/lp/code/stories/branches/xx-nearby-branches.txt 2010-01-13 22:28:16 +0000
30@@ -0,0 +1,25 @@
31+Nearby Branches
32+===============
33+
34+At the bottom of the main branch page there are links to other related
35+branches.
36+
37+ >>> login(ANONYMOUS)
38+ >>> eric = factory.makePerson(name='eric')
39+ >>> vikings = factory.makeTeam(owner=eric, name='vikings')
40+ >>> fooix = factory.makeProduct(name='fooix')
41+ >>> branch = factory.makeProductBranch(
42+ ... product=fooix, owner=vikings, registrant=eric)
43+ >>> url = canonical_url(branch)
44+ >>> logout()
45+
46+ >>> browser.open(url)
47+
48+Since the links point to the code subdomain, the links are relative to the root.
49+
50+ >>> div = find_tag_by_id(browser.contents, 'nearby-branches')
51+ >>> for anchor in div.findAll('a'):
52+ ... print anchor['href'], anchor.string
53+ /fooix Other Fooix branches
54+ /~vikings Other branches owned by Vikings
55+ /~eric/+registeredbranches Other branches registered by Eric
56
57=== added file 'lib/lp/code/stories/branches/xx-reviewing.txt'
58--- lib/lp/code/stories/branches/xx-reviewing.txt 1970-01-01 00:00:00 +0000
59+++ lib/lp/code/stories/branches/xx-reviewing.txt 2010-01-13 22:28:16 +0000
60@@ -0,0 +1,39 @@
61+Reviewing a merge proposal
62+==========================
63+
64+A frequent use case is when a team is asked to review, and a member of that
65+team review the proposal on behalf of the team.
66+
67+ >>> login(ANONYMOUS)
68+ >>> eric = factory.makePerson(
69+ ... name='eric', email='eric@example.com', password='test')
70+ >>> vikings = factory.makeTeam(owner=eric, name='vikings')
71+ >>> bmp = factory.makeBranchMergeProposal()
72+ >>> login_person(bmp.registrant)
73+ >>> ignored = bmp.nominateReviewer(vikings, bmp.registrant)
74+ >>> url = canonical_url(bmp)
75+ >>> logout()
76+
77+When the user looks at the merge proposal page, they see their team name in
78+the reviewer block with the link '[Review]'.
79+
80+ >>> browser = setupBrowser(auth='Basic eric@example.com:test')
81+ >>> browser.open(url)
82+ >>> print_tag_with_id(browser.contents, 'code-review-votes')
83+ Reviewer Review Type Date Requested Status
84+ Vikings ... ago Pending [Review]
85+ Review via email: mp...@code.launchpad.dev
86+
87+ >>> browser.getLink('[Review]').click()
88+
89+The user can then add their comment.
90+
91+ >>> browser.getControl('Comment').value = 'Looks good'
92+ >>> browser.getControl('Save Review').click()
93+
94+The team review is now claimed by the user.
95+
96+ >>> print_tag_with_id(browser.contents, 'code-review-votes')
97+ Reviewer Review Type Date Requested Status
98+ Eric (community) ... ago Approve ... ago
99+ Review via email: mp...@code.launchpad.dev
100
101=== modified file 'lib/lp/code/templates/branch-index.pt'
102--- lib/lp/code/templates/branch-index.pt 2010-01-05 19:45:58 +0000
103+++ lib/lp/code/templates/branch-index.pt 2010-01-13 22:28:16 +0000
104@@ -166,7 +166,7 @@
105 <div id="branch-pending-writes" class="pending-update">
106 <h3>Updating branch...</h3>
107 <p>
108- Launchpad is processing new changes to this branch and will be
109+ Launchpad is processing new changes to this branch which will be
110 available in a few minutes. Reload to see the changes.
111 </p>
112 </div>
113@@ -199,28 +199,28 @@
114 </ul>
115 </div>
116
117- <div class="related">
118+ <div id="nearby-branches" class="related">
119
120 <h2>Nearby</h2>
121 <ul>
122 <li tal:condition="context/product">
123- <a tal:attributes="href context/product/fmt:url">
124+ <a tal:attributes="href context/product/fmt:url:code">
125 Other <tal:product replace="context/product/displayname" /> branches
126 </a>
127 </li>
128 <li tal:condition="context/sourcepackage">
129- <a tal:attributes="href context/sourcepackage/fmt:url">
130+ <a tal:attributes="href context/sourcepackage/fmt:url:code">
131 Other <tal:product replace="context/sourcepackage/displayname" /> branches
132 </a>
133 </li>
134 <li>
135- <a tal:attributes="href context/owner/fmt:url">
136+ <a tal:attributes="href context/owner/fmt:url:code">
137 Other branches owned by <tal:person
138 replace="context/owner/fmt:displayname">Owner</tal:person>
139 </a>
140 </li>
141 <li tal:condition="not:view/owner_is_registrant">
142- <a tal:attributes="href string:${context/registrant/fmt:url}/+registeredbranches">
143+ <a tal:attributes="href string:${context/registrant/fmt:url:code}/+registeredbranches">
144 Other branches registered by <tal:person
145 replace="context/registrant/fmt:displayname">Registrant</tal:person>
146 </a>
147
148=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
149--- lib/lp/code/templates/branchmergeproposal-index.pt 2009-12-18 20:14:21 +0000
150+++ lib/lp/code/templates/branchmergeproposal-index.pt 2010-01-13 22:28:16 +0000
151@@ -181,7 +181,7 @@
152 </div>
153
154 <div class="yui-g" tal:condition="view/pending_diff">
155- <div class="portlet pending-update" id="diff-pending-update">
156+ <div class="pending-update" id="diff-pending-update">
157 <h3>Updating diff...</h3>
158 <p>
159 An updated diff will be available in a few minutes. Reload to see the