launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/infotype-mismatch-vocabulary into lp:launchpad with lp:~jcsackett/launchpad/productseries-branch-infotype-mismatch as a prerequisite.

Commit message:
Creates new vocabulary to ensure only appropriate branches are shown for +setbranch on a productseries

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1078011 in Launchpad itself: "Product series and series branch information types can mismatch"
  https://bugs.launchpad.net/launchpad/+bug/1078011

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/infotype-mismatch-vocabulary/+merge/134251

Summary
=======
In order to prevent awkward situations with product series that have a linked branch that doesn't agree on information type, the branch picker for +setbranch needs to use a vocabulary that only shows matching branches.

Preimp
======
Aaron Bentley, Orange Squad

Implementation
==============
A new vocab is created, subclassing the exacting product-restricting one. It uses a new method on BranchCollection, ofInformationType, to filter the collection to branches matching the product/productseries's information type.

Tests
=====
bin/test -vvct test_exclude_mismatch_infotype

QA
==
Find or create a product with branches that do not match its information type; in +setbranch for a series on that product, search for that branch in the picker. It shouldn't show up. Search for other branches--they should show up.

Lint
====
This branch is lint free.

LoC
===
Part of private projects
-- 
https://code.launchpad.net/~jcsackett/launchpad/infotype-mismatch-vocabulary/+merge/134251
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/infotype-mismatch-vocabulary into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py	2012-10-18 19:06:48 +0000
+++ lib/lp/code/interfaces/branchcollection.py	2012-11-14 17:32:23 +0000
@@ -163,6 +163,9 @@
     def isPrivate():
         """Restrict the collection to private branches."""
 
+    def ofInformationType(info_type):
+        """Restrict the collection to branches of 'info_type'."""
+
     def isExclusive():
         """Restrict the collection to branches owned by exclusive people."""
 

=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py	2012-10-18 19:06:48 +0000
+++ lib/lp/code/model/branchcollection.py	2012-11-14 17:32:23 +0000
@@ -614,6 +614,10 @@
         return self._filterBy([
             Branch.information_type.is_in(PRIVATE_INFORMATION_TYPES)])
 
+    def ofInformationType(self, info_type):
+        """See `IBranchCollection`."""
+        return self._filterBy([Branch.information_type == info_type])
+
     def isExclusive(self):
         """See `IBranchCollection`."""
         return self._filterBy(

=== modified file 'lib/lp/code/vocabularies/branch.py'
--- lib/lp/code/vocabularies/branch.py	2012-10-18 17:49:39 +0000
+++ lib/lp/code/vocabularies/branch.py	2012-11-14 17:32:23 +0000
@@ -113,6 +113,20 @@
         return getUtility(IAllBranches).inProduct(self.product).isExclusive()
 
 
+class RestrictedProductSeriesBranchVocabulary(
+    BranchRestrictedOnProductVocabulary):
+    """A vocabulary for searching for branches to link to a productseries.
+
+    In addition to the restrictions of searching for a product branch, this
+    must only return branches that share the productseries's information type.
+    """
+
+    def _getCollection(self):
+        branches = getUtility(IAllBranches)
+        info_type = self.product.information_type
+        return branches.inProduct(self.product).ofInformationType(info_type)
+
+
 class HostedBranchRestrictedOnOwnerVocabulary(BranchVocabularyBase):
     """A vocabulary for hosted branches owned by the current user.
 

=== modified file 'lib/lp/code/vocabularies/configure.zcml'
--- lib/lp/code/vocabularies/configure.zcml	2012-09-05 05:08:26 +0000
+++ lib/lp/code/vocabularies/configure.zcml	2012-11-14 17:32:23 +0000
@@ -55,4 +55,14 @@
     <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/>
   </class>
 
+  <securedutility
+    name="RestrictedProductSeriesBranchVocabulary"
+    component=".branch.RestrictedProductSeriesBranchVocabulary"
+    provides="zope.schema.interfaces.IVocabularyFactory">
+    <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
+  </securedutility>
+
+  <class class=".branch.RestrictedProductSeriesBranchVocabulary">
+    <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/>
+  </class>
 </configure>

=== modified file 'lib/lp/code/vocabularies/tests/test_branch_vocabularies.py'
--- lib/lp/code/vocabularies/tests/test_branch_vocabularies.py	2012-10-18 17:49:39 +0000
+++ lib/lp/code/vocabularies/tests/test_branch_vocabularies.py	2012-11-14 17:32:23 +0000
@@ -8,19 +8,29 @@
 from unittest import TestCase
 
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
+from lp.app.enums import InformationType
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.vocabularies.branch import (
     BranchRestrictedOnProductVocabulary,
     BranchVocabulary,
-    )
-from lp.registry.enums import TeamMembershipPolicy
+    RestrictedProductSeriesBranchVocabulary,
+    )
+from lp.registry.enums import (
+    BranchSharingPolicy,
+    TeamMembershipPolicy,
+    )
 from lp.registry.interfaces.product import IProductSet
 from lp.testing import (
     ANONYMOUS,
     login,
     logout,
     )
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 from lp.testing.factory import LaunchpadObjectFactory
 from lp.testing.layers import DatabaseFunctionalLayer
 
@@ -219,6 +229,31 @@
         self.assertEqual(['~scotty/widget/mountain'], branch_names)
 
 
+class TestRestrictedProductSeriesBranchVocabulary(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_exclude_mismatch_infotype(self):
+        # Branches with an infotype that doesn't match the product's are
+        # excluded.
+        person = self.factory.makePerson(name='dude')
+        product = self.factory.makeProduct(owner=person, name='fizz')
+        self.factory.makeBranch(owner=person, product=product, name='foo-foo')
+        self.factory.makeBranch(owner=person, product=product, name='foo-bar')
+        naked_product = removeSecurityProxy(product)
+        naked_product.information_type = InformationType.PROPRIETARY
+        naked_product.setBranchSharingPolicy(BranchSharingPolicy.PROPRIETARY)
+        self.factory.makeBranch(
+            owner=person, product=product, name='foo-baz',
+            information_type=InformationType.PROPRIETARY)
+        vocab = RestrictedProductSeriesBranchVocabulary(product)
+        with person_logged_in(person):
+            results = vocab.searchForTerms('foo')
+            branch_names = sorted([branch.token for branch in results])
+        expected = [u'~dude/fizz/foo-baz']
+        self.assertEqual(expected, branch_names)
+
+
 class TestRestrictedBranchVocabularyOnBranch(
     TestRestrictedBranchVocabularyOnProduct):
     """Test the BranchRestrictedOnProductVocabulary behaves as expected.

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2012-11-14 17:32:22 +0000
+++ lib/lp/registry/browser/productseries.py	2012-11-14 17:32:23 +0000
@@ -888,7 +888,7 @@
         branch = data.get('branch_location')
         if (branch.information_type !=
             self.context.product.information_type):
-            error_msg = ("A %s product series cannot have a %s series branch." 
+            error_msg = ("A %s product series cannot have a %s series branch."
                 % (self.context.product.information_type.title.lower(),
                    branch.information_type.title.lower()))
             self.setFieldError('branch_type', error_msg)
@@ -1007,7 +1007,7 @@
                 # Request an initial upload of translation files.
                 getUtility(IRosettaUploadJobSource).create(
                     self.context.branch, NULL_REVISION)
-           
+
             self.context.branch = branch
             self.request.response.addInfoNotification(
                 'Series code location updated.')

=== modified file 'lib/lp/registry/browser/tests/test_productseries_views.py'
--- lib/lp/registry/browser/tests/test_productseries_views.py	2012-11-14 17:32:22 +0000
+++ lib/lp/registry/browser/tests/test_productseries_views.py	2012-11-14 17:32:23 +0000
@@ -88,7 +88,7 @@
             product).information_type = InformationType.PROPRIETARY
         form = {
             'field.branch_type': 'link-lp-bzr',
-            'field.branch_location': branch_name, 
+            'field.branch_location': branch_name,
             'field.actions.update': 'Update',
             }
         with person_logged_in(product.owner):
@@ -96,7 +96,7 @@
                 productseries, '+setbranch', form=form)
         self.assertEqual(
             ['A proprietary product series cannot have a public series '
-             'branch.'],    
+             'branch.'],
             view.errors)
 
 

=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py	2012-10-18 14:05:41 +0000
+++ lib/lp/registry/interfaces/productseries.py	2012-11-14 17:32:23 +0000
@@ -238,7 +238,7 @@
     branch = exported(
         ReferenceChoice(
             title=_('Branch'),
-            vocabulary='BranchRestrictedOnProduct',
+            vocabulary='RestrictedProductSeriesBranchVocabulary',
             schema=IBranch,
             required=False,
             description=_("The Bazaar branch for this series.  Leave blank "