Launchpad itself

Merge lp://staging/~thumper/launchpad/branch-active-reviews into lp://staging/launchpad

  1. branch-active-reviews
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~thumper/launchpad/branch-active-reviews
Merge into: lp://staging/launchpad
Diff against target: 159 lines (+42/-21)
6 files modified
lib/lp/code/browser/branchmergeproposallisting.py (+12/-6)
lib/lp/code/browser/configure.zcml (+7/-0)
lib/lp/code/browser/tests/test_branchmergeproposallisting.py (+0/-14)
lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt (+16/-0)
lib/lp/code/templates/active-reviews.pt (+6/-0)
lib/lp/code/templates/branch-pending-merges.pt (+1/-1)
To merge this branch: bzr merge lp://staging/~thumper/launchpad/branch-active-reviews
Reviewer Review Type Date Requested Status
Michael Nelson (community) ui Approve
Paul Hummer (community) code ui* Approve
Review via email: mp+17433@code.staging.launchpad.net

Commit message

Make the active reviews page work for branches.

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

Makes active reviews work for branches, and links the "3 branches proposed for merging into this one." to go to the active reviews page.

xx-branchmergeproposal-listings
test_branchmergeproposallisting

Revision history for this message
Paul Hummer (rockstar) :
review: Approve (code ui*)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Thanks for the demo!

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/branchmergeproposallisting.py'
2--- lib/lp/code/browser/branchmergeproposallisting.py 2009-12-01 20:41:59 +0000
3+++ lib/lp/code/browser/branchmergeproposallisting.py 2010-01-17 06:03:15 +0000
4@@ -7,6 +7,7 @@
5
6 __all__ = [
7 'ActiveReviewsView',
8+ 'BranchActiveReviewsView',
9 'BranchMergeProposalListingItem',
10 'BranchMergeProposalListingView',
11 'PersonActiveReviewsView',
12@@ -17,7 +18,6 @@
13
14 from zope.component import getUtility
15 from zope.interface import implements, Interface
16-from zope.publisher.interfaces import NotFound
17 from zope.schema import Choice
18
19 from lazr.delegates import delegates
20@@ -27,11 +27,11 @@
21 from canonical.config import config
22 from canonical.launchpad import _
23 from canonical.launchpad.webapp import custom_widget, LaunchpadFormView
24+from canonical.launchpad.webapp.authorization import check_permission
25 from canonical.launchpad.webapp.batching import TableBatchNavigator
26 from canonical.widgets import LaunchpadDropdownWidget
27
28 from lp.code.enums import BranchMergeProposalStatus, CodeReviewVote
29-from lp.code.interfaces.branch import IBranch
30 from lp.code.interfaces.branchcollection import (
31 IAllBranches, IBranchCollection)
32 from lp.code.interfaces.branchmergeproposal import (
33@@ -324,10 +324,6 @@
34 return self.user
35
36 def initialize(self):
37- # Branches, despite implementing IHasMergeProposals, does not
38- # have an active reviews page.
39- if IBranch.providedBy(self.context):
40- raise NotFound(self.context, '+activereviews')
41 # Work out the review groups
42 self.review_groups = {}
43 self.getter = getUtility(IBranchMergeProposalGetter)
44@@ -401,6 +397,16 @@
45 return "%s has no active code reviews." % self.context.displayname
46
47
48+class BranchActiveReviewsView(ActiveReviewsView):
49+ """Branch merge proposals for a branch that are needing review."""
50+
51+ def getProposals(self):
52+ """See `ActiveReviewsView`."""
53+ candidates = self.context.landing_candidates
54+ return [proposal for proposal in candidates
55+ if check_permission('launchpad.View', proposal)]
56+
57+
58 class PersonActiveReviewsView(ActiveReviewsView):
59 """Branch merge proposals for the person that are needing review."""
60
61
62=== modified file 'lib/lp/code/browser/configure.zcml'
63--- lib/lp/code/browser/configure.zcml 2010-01-14 01:48:19 +0000
64+++ lib/lp/code/browser/configure.zcml 2010-01-17 06:03:15 +0000
65@@ -561,6 +561,13 @@
66 template="../templates/inline-form-only-buttons.pt"
67 permission="launchpad.AnyPerson"/>
68 <browser:page
69+ for="lp.code.interfaces.branch.IBranch"
70+ class="lp.code.browser.branchmergeproposallisting.BranchActiveReviewsView"
71+ permission="zope.Public"
72+ name="+activereviews"
73+ facet="branches"
74+ template="../templates/active-reviews.pt"/>
75+ <browser:page
76 for="lp.code.interfaces.branch.IBranchBatchNavigator"
77 name="+branch-listing"
78 template="../templates/branch-listing.pt"
79
80=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
81--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py 2009-12-02 00:11:26 +0000
82+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py 2010-01-17 06:03:15 +0000
83@@ -323,19 +323,5 @@
84 [item.context for item in view.review_groups[view.OTHER]])
85
86
87-class NoActiveReviewsForBranchTest(TestCaseWithFactory):
88- """A branch should not have +activereviews."""
89-
90- layer = DatabaseFunctionalLayer
91-
92- def test_no_active_reviews(self):
93- # 404 is more apt than an oops.
94- branch = self.factory.makeProductBranch()
95- self.assertRaises(
96- NotFound,
97- create_initialized_view,
98- branch, name='+activereviews')
99-
100-
101 def test_suite():
102 return TestLoader().loadTestsFromName(__name__)
103
104=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt'
105--- lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt 2009-12-18 21:31:12 +0000
106+++ lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt 2010-01-17 06:03:15 +0000
107@@ -131,6 +131,22 @@
108 lp://dev/~albert/fooix/review ... Albert ... None
109
110
111+Listings from a branch page
112+===========================
113+
114+When looking at a branch that is a target of merge proposals, the user is
115+shown a link and a count.
116+
117+ >>> browser.open('http://code.launchpad.dev/fooix')
118+ >>> browser.getLink('lp://dev/fooix').click()
119+ >>> print_tag_with_id(browser.contents, 'landing-candidates')
120+ 3 branches proposed for merging into this one.
121+
122+ >>> browser.getLink('3 branches').click()
123+ >>> print browser.title
124+ Active code reviews for lp://dev/fooix ...
125+
126+
127 Line counts
128 ===========
129
130
131=== modified file 'lib/lp/code/templates/active-reviews.pt'
132--- lib/lp/code/templates/active-reviews.pt 2009-08-17 22:31:09 +0000
133+++ lib/lp/code/templates/active-reviews.pt 2010-01-17 06:03:15 +0000
134@@ -77,6 +77,12 @@
135 No reviews
136 </em>
137
138+ <div class="related">
139+ <ul>
140+ <li><a href="+merges">See all merge proposals</a></li>
141+ </ul>
142+ </div>
143+
144 </div>
145
146 </body>
147
148=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
149--- lib/lp/code/templates/branch-pending-merges.pt 2009-11-14 09:39:05 +0000
150+++ lib/lp/code/templates/branch-pending-merges.pt 2010-01-17 06:03:15 +0000
151@@ -16,7 +16,7 @@
152 <div id="landing-candidates"
153 tal:condition="view/landing_candidates">
154 <img src="/@@/merge-proposal-icon" />
155- <a href="+merges" tal:content="structure view/landing_candidate_count_text">
156+ <a href="+activereviews" tal:content="structure view/landing_candidate_count_text">
157 1 branch
158 </a>
159 proposed for merging into this one.