Merge lp:~abentley/launchpad/person-product-merges into lp:launchpad

[Aaron Bentley]

Support +merges on PersonProduct.

= Summary =
Fix bug #739921: The link "see all merge proposals" on person/product/+activereviews 404s Remove

== Proposed fix ==
Provide an adapter to convert PersonProduct into an IBranchCollection, and implement IHasMergeProposals on PersonProduct.

== Pre-implementation notes ==
None

== Implementation details ==
Implemented displayname on PersonProduct to support the view classes.

Various drive-by lint fixes.

== Tests ==
bin/test -t test_PersonProduct_implements_hasmergeproposals -t person_product

== Demo and Q/A ==
Create a merge proposal on a Product. Go to lpn/people/+me/PRODUCT/+merges. It should be displayed.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/model/tests/test_hasmergeproposals.py
  lib/lp/code/configure.zcml
  lib/lp/registry/interfaces/personproduct.py
  lib/lp/code/adapters/branchcollection.py
  lib/lp/code/browser/tests/test_branchmergeproposallisting.py
  lib/lp/code/adapters/tests/test_branchcollection.py
  lib/lp/registry/model/personproduct.py

[Benji York (benji)]

This branch looks good. A couple of small points:

It took me a minute to figure out the intent of test_person_product. A
docstring or comment would probably help.

The HasMergeProposalsMixin import in
lib/lp/registry/model/personproduct.py doesn't have to be a mult-line
import.

review: Approve (code)

=== modified file 'lib/lp/code/adapters/branchcollection.py'
--- lib/lp/code/adapters/branchcollection.py	2009-09-02 10:36:33 +0000
+++ lib/lp/code/adapters/branchcollection.py	2011-05-09 19:41:38 +0000
@@ -35,6 +35,13 @@
     return getUtility(IAllBranches).ownedBy(person)
 
 
+def branch_collection_for_person_product(person_product):
+    """Adapt a PersonProduct to a branch collection."""
+    collection = getUtility(IAllBranches).ownedBy(person_product.person)
+    collection = collection.inProduct(person_product.product)
+    return collection
+
+
 def branch_collection_for_distribution(distribution):
     """Adapt a distribution to a branch collection."""
     return getUtility(IAllBranches).inDistribution(distribution)

=== added file 'lib/lp/code/adapters/tests/test_branchcollection.py'
--- lib/lp/code/adapters/tests/test_branchcollection.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/adapters/tests/test_branchcollection.py	2011-05-09 19:41:38 +0000
@@ -0,0 +1,37 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Functional tests for BranchCollection adapters."""
+
+__metaclass__ = type
+
+
+from lp.code.interfaces.branchcollection import IBranchCollection
+from lp.testing import TestCaseWithFactory
+from lp.registry.model.personproduct import PersonProduct
+from canonical.testing.layers import DatabaseFunctionalLayer
+
+
+class TestPersonProduct(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_person_product(self):
+        """A PersonProduct can be adapted to a collection.
+
+        The collection will only find branches matching both the person and
+        the product.
+        """
+        product = self.factory.makeProduct()
+        person = self.factory.makePerson()
+        person_product = PersonProduct(person, product)
+        random_branch = self.factory.makeBranch()
+        product_branch = self.factory.makeProductBranch(product=product)
+        person_branch = self.factory.makeBranch(owner=person)
+        person_product_branch = self.factory.makeProductBranch(
+            owner=person, product=product)
+        branches = IBranchCollection(person_product).getBranches()
+        self.assertNotIn(random_branch, branches)
+        self.assertNotIn(product_branch, branches)
+        self.assertNotIn(person_branch, branches)
+        self.assertIn(person_product_branch, branches)

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2010-12-02 16:13:51 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2011-05-09 19:41:38 +0000
@@ -23,8 +23,10 @@
     BranchMergeProposalStatus,
     CodeReviewVote,
     )
+from lp.registry.model.personproduct import PersonProduct
 from lp.testing import (
     ANONYMOUS,
+    BrowserTestCase,
     login,
     login_person,
     TestCaseWithFactory,
@@ -179,6 +181,18 @@
         self.assertEqual(4, comment_count)
 
 
+class TestMerges(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_person_product(self):
+        """The merges view should be enabled for PersonProduct."""
+        personproduct = PersonProduct(
+            self.factory.makePerson(), self.factory.makeProduct())
+        self.getViewBrowser(personproduct, '+merges',
+                rootsite='code')
+
+
 class ActiveReviewGroupsTest(TestCaseWithFactory):
     """Tests for groupings used in for active reviews."""
 
@@ -276,9 +290,9 @@
         # If the proposal is in needs review, the sort_key will be the
         # date_review_requested.
         bmp = self.factory.makeBranchMergeProposal(
-            date_created=datetime(2009,6,1,tzinfo=pytz.UTC))
+            date_created=datetime(2009, 6, 1, tzinfo=pytz.UTC))
         login_person(bmp.registrant)
-        request_date = datetime(2009,7,1,tzinfo=pytz.UTC)
+        request_date = datetime(2009, 7, 1, tzinfo=pytz.UTC)
         bmp.requestReview(request_date)
         item = BranchMergeProposalListingItem(bmp, None, None)
         self.assertEqual(request_date, item.sort_key)
@@ -287,13 +301,13 @@
         # If the proposal is approved, the sort_key will default to the
         # date_review_requested.
         bmp = self.factory.makeBranchMergeProposal(
-            date_created=datetime(2009,6,1,tzinfo=pytz.UTC))
+            date_created=datetime(2009, 6, 1, tzinfo=pytz.UTC))
         login_person(bmp.target_branch.owner)
-        request_date = datetime(2009,7,1,tzinfo=pytz.UTC)
+        request_date = datetime(2009, 7, 1, tzinfo=pytz.UTC)
         bmp.requestReview(request_date)
         bmp.approveBranch(
             bmp.target_branch.owner, 'rev-id',
-            datetime(2009,8,1,tzinfo=pytz.UTC))
+            datetime(2009, 8, 1, tzinfo=pytz.UTC))
         item = BranchMergeProposalListingItem(bmp, None, None)
         self.assertEqual(request_date, item.sort_key)
 
@@ -301,9 +315,9 @@
         # If the proposal is approved and the review has been bypassed, the
         # date_reviewed is used.
         bmp = self.factory.makeBranchMergeProposal(
-            date_created=datetime(2009,6,1,tzinfo=pytz.UTC))
+            date_created=datetime(2009, 6, 1, tzinfo=pytz.UTC))
         login_person(bmp.target_branch.owner)
-        review_date = datetime(2009,8,1,tzinfo=pytz.UTC)
+        review_date = datetime(2009, 8, 1, tzinfo=pytz.UTC)
         bmp.approveBranch(
             bmp.target_branch.owner, 'rev-id', review_date)
         item = BranchMergeProposalListingItem(bmp, None, None)
@@ -312,7 +326,7 @@
     def test_sort_key_wip(self):
         # If the proposal is a work in progress, the date_created is used.
         bmp = self.factory.makeBranchMergeProposal(
-            date_created=datetime(2009,6,1,tzinfo=pytz.UTC))
+            date_created=datetime(2009, 6, 1, tzinfo=pytz.UTC))
         login_person(bmp.target_branch.owner)
         item = BranchMergeProposalListingItem(bmp, None, None)
         self.assertEqual(bmp.date_created, item.sort_key)
@@ -328,13 +342,13 @@
         product = self.factory.makeProduct()
         bmp1 = self.factory.makeBranchMergeProposal(product=product)
         login_person(bmp1.source_branch.owner)
-        bmp1.requestReview(datetime(2009,6,1,tzinfo=pytz.UTC))
+        bmp1.requestReview(datetime(2009, 6, 1, tzinfo=pytz.UTC))
         bmp2 = self.factory.makeBranchMergeProposal(product=product)
         login_person(bmp2.source_branch.owner)
-        bmp2.requestReview(datetime(2009,3,1,tzinfo=pytz.UTC))
+        bmp2.requestReview(datetime(2009, 3, 1, tzinfo=pytz.UTC))
         bmp3 = self.factory.makeBranchMergeProposal(product=product)
         login_person(bmp3.source_branch.owner)
-        bmp3.requestReview(datetime(2009,1,1,tzinfo=pytz.UTC))
+        bmp3.requestReview(datetime(2009, 1, 1, tzinfo=pytz.UTC))
         login(ANONYMOUS)
         view = create_initialized_view(
             product, name='+activereviews', rootsite='code')

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2011-05-04 04:10:58 +0000
+++ lib/lp/code/configure.zcml	2011-05-09 19:41:38 +0000
@@ -122,6 +122,10 @@
       provides="lp.code.interfaces.branchcollection.IBranchCollection"
       factory="lp.code.adapters.branchcollection.branch_collection_for_product"/>
   <adapter
+      for="lp.registry.interfaces.personproduct.IPersonProduct"
+      provides="lp.code.interfaces.branchcollection.IBranchCollection"
+      factory="lp.code.adapters.branchcollection.branch_collection_for_person_product"/>
+  <adapter
       for="lp.registry.interfaces.projectgroup.IProjectGroup"
       provides="lp.code.interfaces.branchcollection.IBranchCollection"
       factory="lp.code.adapters.branchcollection.branch_collection_for_project"/>

=== modified file 'lib/lp/code/model/tests/test_hasmergeproposals.py'
--- lib/lp/code/model/tests/test_hasmergeproposals.py	2010-10-04 19:50:45 +0000
+++ lib/lp/code/model/tests/test_hasmergeproposals.py	2011-05-09 19:41:38 +0000
@@ -7,8 +7,10 @@
 
 import unittest
 
+from canonical.launchpad.webapp.testing import verifyObject
 from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.code.interfaces.hasbranches import IHasMergeProposals
+from lp.registry.model.personproduct import PersonProduct
 from lp.testing import TestCaseWithFactory
 
 
@@ -32,7 +34,12 @@
         project = self.factory.makeProject()
         self.assertProvides(project, IHasMergeProposals)
 
+    def test_PersonProduct_implements_hasmergeproposals(self):
+        # PersonProducts should implement IHasMergeProposals.
+        product = self.factory.makeProduct()
+        person_product = PersonProduct(product.owner, product)
+        verifyObject(IHasMergeProposals, person_product)
+
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
-

=== modified file 'lib/lp/registry/interfaces/personproduct.py'
--- lib/lp/registry/interfaces/personproduct.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/interfaces/personproduct.py	2011-05-09 19:41:38 +0000
@@ -13,22 +13,27 @@
 
 from lazr.restful.fields import Reference
 from zope.interface import Interface
+from zope.schema import (
+    TextLine,
+)
 
+from lp.code.interfaces.hasbranches import IHasMergeProposals
 from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.product import IProduct
 
 
-class IPersonProduct(Interface):
+class IPersonProduct(IHasMergeProposals):
     """A person's view on a product."""
 
     person = Reference(IPerson)
 
     product = Reference(IProduct)
 
+    displayname = TextLine()
+
 
 class IPersonProductFactory(Interface):
     """Creates `IPersonProduct`s."""
 
     def create(person, product):
         """Create and return an `IPersonProduct`."""
-

=== modified file 'lib/lp/registry/model/personproduct.py'
--- lib/lp/registry/model/personproduct.py	2010-08-20 20:31:18 +0000
+++ lib/lp/registry/model/personproduct.py	2011-05-09 19:41:38 +0000
@@ -13,13 +13,16 @@
     implements,
     )
 
+from lp.code.model.hasbranches import (
+    HasMergeProposalsMixin,
+    )
 from lp.registry.interfaces.personproduct import (
     IPersonProduct,
     IPersonProductFactory,
     )
 
 
-class PersonProduct:
+class PersonProduct(HasMergeProposalsMixin):
 
     implements(IPersonProduct)
 
@@ -32,3 +35,8 @@
     @staticmethod
     def create(person, product):
         return PersonProduct(person, product)
+
+    @property
+    def displayname(self):
+        return '%s in %s' % (
+            self.person.displayname, self.product.displayname)