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

[Tim Penhey]

Make the active reviews page work for branches.

[Tim Penhey (thumper)]

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

[Paul Hummer (rockstar)]

review: Approve (code ui*)

[Michael Nelson (michael.nelson)]

Thanks for the demo!

review: Approve (ui)

=== modified file 'lib/lp/code/browser/branchmergeproposallisting.py'
--- lib/lp/code/browser/branchmergeproposallisting.py	2009-12-01 20:41:59 +0000
+++ lib/lp/code/browser/branchmergeproposallisting.py	2010-01-17 06:03:15 +0000
@@ -7,6 +7,7 @@
 
 __all__ = [
     'ActiveReviewsView',
+    'BranchActiveReviewsView',
     'BranchMergeProposalListingItem',
     'BranchMergeProposalListingView',
     'PersonActiveReviewsView',
@@ -17,7 +18,6 @@
 
 from zope.component import getUtility
 from zope.interface import implements, Interface
-from zope.publisher.interfaces import NotFound
 from zope.schema import Choice
 
 from lazr.delegates import delegates
@@ -27,11 +27,11 @@
 from canonical.config import config
 from canonical.launchpad import _
 from canonical.launchpad.webapp import custom_widget, LaunchpadFormView
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.batching import TableBatchNavigator
 from canonical.widgets import LaunchpadDropdownWidget
 
 from lp.code.enums import BranchMergeProposalStatus, CodeReviewVote
-from lp.code.interfaces.branch import IBranch
 from lp.code.interfaces.branchcollection import (
     IAllBranches, IBranchCollection)
 from lp.code.interfaces.branchmergeproposal import (
@@ -324,10 +324,6 @@
         return self.user
 
     def initialize(self):
-        # Branches, despite implementing IHasMergeProposals, does not
-        # have an active reviews page.
-        if IBranch.providedBy(self.context):
-            raise NotFound(self.context, '+activereviews')
         # Work out the review groups
         self.review_groups = {}
         self.getter = getUtility(IBranchMergeProposalGetter)
@@ -401,6 +397,16 @@
         return "%s has no active code reviews." % self.context.displayname
 
 
+class BranchActiveReviewsView(ActiveReviewsView):
+    """Branch merge proposals for a branch that are needing review."""
+
+    def getProposals(self):
+        """See `ActiveReviewsView`."""
+        candidates = self.context.landing_candidates
+        return [proposal for proposal in candidates
+                if check_permission('launchpad.View', proposal)]
+
+
 class PersonActiveReviewsView(ActiveReviewsView):
     """Branch merge proposals for the person that are needing review."""
 

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2010-01-14 01:48:19 +0000
+++ lib/lp/code/browser/configure.zcml	2010-01-17 06:03:15 +0000
@@ -561,6 +561,13 @@
         template="../templates/inline-form-only-buttons.pt"
         permission="launchpad.AnyPerson"/>
     <browser:page
+        for="lp.code.interfaces.branch.IBranch"
+        class="lp.code.browser.branchmergeproposallisting.BranchActiveReviewsView"
+        permission="zope.Public"
+        name="+activereviews"
+        facet="branches"
+        template="../templates/active-reviews.pt"/>
+    <browser:page
         for="lp.code.interfaces.branch.IBranchBatchNavigator"
         name="+branch-listing"
         template="../templates/branch-listing.pt"

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2009-12-02 00:11:26 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2010-01-17 06:03:15 +0000
@@ -323,19 +323,5 @@
             [item.context for item in view.review_groups[view.OTHER]])
 
 
-class NoActiveReviewsForBranchTest(TestCaseWithFactory):
-    """A branch should not have +activereviews."""
-
-    layer = DatabaseFunctionalLayer
-
-    def test_no_active_reviews(self):
-        # 404 is more apt than an oops.
-        branch = self.factory.makeProductBranch()
-        self.assertRaises(
-            NotFound,
-            create_initialized_view,
-            branch, name='+activereviews')
-
-
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt'
--- lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt	2009-12-18 21:31:12 +0000
+++ lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt	2010-01-17 06:03:15 +0000
@@ -131,6 +131,22 @@
     lp://dev/~albert/fooix/review ... Albert ...            None
 
 
+Listings from a branch page
+===========================
+
+When looking at a branch that is a target of merge proposals, the user is
+shown a link and a count.
+
+    >>> browser.open('http://code.launchpad.dev/fooix')
+    >>> browser.getLink('lp://dev/fooix').click()
+    >>> print_tag_with_id(browser.contents, 'landing-candidates')
+    3 branches proposed for merging into this one.
+
+    >>> browser.getLink('3 branches').click()
+    >>> print browser.title
+    Active code reviews for lp://dev/fooix ...
+
+
 Line counts
 ===========
 

=== modified file 'lib/lp/code/templates/active-reviews.pt'
--- lib/lp/code/templates/active-reviews.pt	2009-08-17 22:31:09 +0000
+++ lib/lp/code/templates/active-reviews.pt	2010-01-17 06:03:15 +0000
@@ -77,6 +77,12 @@
      No reviews
   </em>
 
+  <div class="related">
+    <ul>
+      <li><a href="+merges">See all merge proposals</a></li>
+    </ul>
+  </div>
+
 </div>
 
 </body>

=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
--- lib/lp/code/templates/branch-pending-merges.pt	2009-11-14 09:39:05 +0000
+++ lib/lp/code/templates/branch-pending-merges.pt	2010-01-17 06:03:15 +0000
@@ -16,7 +16,7 @@
       <div id="landing-candidates"
            tal:condition="view/landing_candidates">
         <img src="/@@/merge-proposal-icon" />
-        <a href="+merges" tal:content="structure view/landing_candidate_count_text">
+        <a href="+activereviews" tal:content="structure view/landing_candidate_count_text">
           1 branch
         </a>
         proposed for merging into this one.