launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/branch-sharing-policy-proprietary into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/branch-sharing-policy-proprietary/+merge/119318

Now that InformationTypeVocabulary doesn't exclude Proprietary whenever a global feature flag is set, we can start using Proprietary for branches. This branch fixes BranchNamespace's interpretation of BranchSharingPolicy to match the intended definitions: "Public" includes the non-Proprietary private types, and "Proprietary" is actually Proprietary rather than User Data.

I also changed the tests to use setBranchSharingPolicy rather than removeSecurityProxy, which necessitated adding a commercial subscription to make proprietary choices legal.
-- 
https://code.launchpad.net/~wgrant/launchpad/branch-sharing-policy-proprietary/+merge/119318
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/branch-sharing-policy-proprietary into lp:launchpad.

=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2012-07-26 07:02:49 +0000
+++ lib/lp/code/model/branchnamespace.py	2012-08-13 10:06:19 +0000
@@ -87,24 +87,24 @@
     PUBLIC_INFORMATION_TYPES + FREE_PRIVATE_INFORMATION_TYPES)
 
 POLICY_ALLOWED_TYPES = {
-    BranchSharingPolicy.PUBLIC: PUBLIC_INFORMATION_TYPES,
-    BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: FREE_INFORMATION_TYPES,
-    BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: FREE_INFORMATION_TYPES,
-    BranchSharingPolicy.PROPRIETARY: FREE_PRIVATE_INFORMATION_TYPES,
+    BranchSharingPolicy.PUBLIC: FREE_INFORMATION_TYPES,
+    BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: InformationType.items,
+    BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.items,
+    BranchSharingPolicy.PROPRIETARY: [InformationType.PROPRIETARY],
     }
 
 POLICY_DEFAULT_TYPES = {
     BranchSharingPolicy.PUBLIC: InformationType.PUBLIC,
     BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: InformationType.PUBLIC,
-    BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.USERDATA,
-    BranchSharingPolicy.PROPRIETARY: InformationType.USERDATA,
+    BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.PROPRIETARY,
+    BranchSharingPolicy.PROPRIETARY: InformationType.PROPRIETARY,
     }
 
 POLICY_REQUIRED_GRANTS = {
     BranchSharingPolicy.PUBLIC: None,
     BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: None,
-    BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.USERDATA,
-    BranchSharingPolicy.PROPRIETARY: InformationType.USERDATA,
+    BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.PROPRIETARY,
+    BranchSharingPolicy.PROPRIETARY: InformationType.PROPRIETARY,
     }
 
 

=== modified file 'lib/lp/code/model/tests/test_branchnamespace.py'
--- lib/lp/code/model/tests/test_branchnamespace.py	2012-07-26 07:02:49 +0000
+++ lib/lp/code/model/tests/test_branchnamespace.py	2012-08-13 10:06:19 +0000
@@ -50,6 +50,7 @@
     )
 from lp.registry.interfaces.distribution import NoSuchDistribution
 from lp.registry.interfaces.person import (
+    IPersonSet,
     NoSuchPerson,
     PersonVisibility,
     TeamSubscriptionPolicy,
@@ -62,6 +63,7 @@
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.sampledata import COMMERCIAL_ADMIN_EMAIL
 
 
 class NamespaceMixin:
@@ -466,8 +468,9 @@
         if person is None:
             person = self.factory.makePerson()
         product = self.factory.makeProduct()
-        removeSecurityProxy(product).branch_sharing_policy = (
-            sharing_policy)
+        self.factory.makeCommercialSubscription(product=product)
+        comadmin = getUtility(IPersonSet).getByEmail(COMMERCIAL_ADMIN_EMAIL)
+        product.setBranchSharingPolicy(sharing_policy, comadmin)
         namespace = ProductNamespace(person, product)
         return namespace
 
@@ -475,7 +478,7 @@
         namespace = self.makeProductNamespace(
             BranchSharingPolicy.PUBLIC)
         self.assertContentEqual(
-            PUBLIC_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
+            FREE_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
         self.assertEqual(
             InformationType.PUBLIC, namespace.getDefaultInformationType())
 
@@ -483,7 +486,7 @@
         namespace = self.makeProductNamespace(
             BranchSharingPolicy.PUBLIC_OR_PROPRIETARY)
         self.assertContentEqual(
-            FREE_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
+            InformationType.items, namespace.getAllowedInformationTypes())
         self.assertEqual(
             InformationType.PUBLIC, namespace.getDefaultInformationType())
 
@@ -499,11 +502,12 @@
         with admin_logged_in():
             getUtility(IService, 'sharing').sharePillarInformation(
                 namespace.product, namespace.owner, namespace.product.owner,
-                {InformationType.USERDATA: SharingPermission.ALL})
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
         self.assertContentEqual(
-            FREE_INFORMATION_TYPES, namespace.getAllowedInformationTypes())
+            InformationType.items, namespace.getAllowedInformationTypes())
         self.assertEqual(
-            InformationType.USERDATA, namespace.getDefaultInformationType())
+            InformationType.PROPRIETARY,
+            namespace.getDefaultInformationType())
 
     def test_proprietary_anyone(self):
         namespace = self.makeProductNamespace(
@@ -517,12 +521,13 @@
         with admin_logged_in():
             getUtility(IService, 'sharing').sharePillarInformation(
                 namespace.product, namespace.owner, namespace.product.owner,
-                {InformationType.USERDATA: SharingPermission.ALL})
+                {InformationType.PROPRIETARY: SharingPermission.ALL})
         self.assertContentEqual(
-            FREE_PRIVATE_INFORMATION_TYPES,
+            [InformationType.PROPRIETARY],
             namespace.getAllowedInformationTypes())
         self.assertEqual(
-            InformationType.USERDATA, namespace.getDefaultInformationType())
+            InformationType.PROPRIETARY,
+            namespace.getDefaultInformationType())
 
 
 class TestPackageNamespace(TestCaseWithFactory, NamespaceMixin):