launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10820
[Merge] lp:~wgrant/launchpad/sharing-policy-transitions into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/sharing-policy-transitions into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/sharing-policy-transitions/+merge/119087
This branch adds custom mutators to Product.bug_sharing_policy and Product.branch_sharing_policy and exposes them both over the API, writeable by commercial admins.
The mutators forbid setting a non-public policy on a project without a commercial subscription. They also restrict the method to only be usable by commercial admins, although this will be relaxed to include the owners soon. Hopefully we'll eventually be able to move it to launchpad.Edit, when all this is self-service and commercial admins can go away. The code is a bit duplicated at the moment, but the two will diverge once we actually start doing things with them.
Another change is that the mutators create a Proprietary access policy when a Proprietary sharing policy is selected. This used to be done immediately on commercial subscription activation, but that was unnecessarily early so I've dropped it.
I also removed the minimal ~registry-only UI for branch_sharing_policy from Product:+admin, as it'll be on +sharing soon instead.
--
https://code.launchpad.net/~wgrant/launchpad/sharing-policy-transitions/+merge/119087
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/sharing-policy-transitions into lp:launchpad.
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2012-07-30 19:57:10 +0000
+++ lib/lp/registry/browser/product.py 2012-08-10 07:36:19 +0000
@@ -1574,9 +1574,6 @@
"private_bugs",
]
- custom_widget(
- 'branch_sharing_policy', LaunchpadRadioWidgetWithDescription)
-
@property
def page_title(self):
"""The HTML page title."""
@@ -1594,8 +1591,6 @@
if not admin:
self.field_names.remove('owner')
self.field_names.remove('autoupdate')
- if getFeatureFlag('disclosure.branch_sharing_policy.show_to_admin'):
- self.field_names.append('branch_sharing_policy')
super(ProductAdminView, self).setUpFields()
self.form_fields = self._createAliasesField() + self.form_fields
if admin:
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2012-08-08 19:14:42 +0000
+++ lib/lp/registry/configure.zcml 2012-08-10 07:36:19 +0000
@@ -1322,7 +1322,7 @@
project_reviewed
license_approved"
set_schema="lp.registry.interfaces.product.IProductModerateRestricted"
- set_attributes="active branch_sharing_policy"/>
+ set_attributes="active"/>
<!-- IHasAliases -->
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2012-08-01 06:10:33 +0000
+++ lib/lp/registry/interfaces/product.py 2012-08-10 07:36:19 +0000
@@ -641,7 +641,7 @@
branch_sharing_policy = exported(Choice(
title=_('Branch sharing policy'),
description=_("Sharing policy for this project's branches."),
- required=False, readonly=False, vocabulary=BranchSharingPolicy),
+ required=False, readonly=True, vocabulary=BranchSharingPolicy),
as_of='devel')
bug_sharing_policy = exported(Choice(
title=_('Bug sharing policy'),
@@ -793,6 +793,29 @@
def setPrivateBugs(private_bugs, user):
"""Mutator for private_bugs that checks entitlement."""
+ @mutator_for(bug_sharing_policy)
+ @call_with(user=REQUEST_USER)
+ @operation_parameters(bug_sharing_policy=copy_field(bug_sharing_policy))
+ @export_write_operation()
+ @operation_for_version("devel")
+ def setBugSharingPolicy(bug_sharing_policy, user):
+ """Mutator for bug_sharing_policy.
+
+ Checks authorization and entitlement.
+ """
+
+ @mutator_for(branch_sharing_policy)
+ @call_with(user=REQUEST_USER)
+ @operation_parameters(
+ branch_sharing_policy=copy_field(branch_sharing_policy))
+ @export_write_operation()
+ @operation_for_version("devel")
+ def setBranchSharingPolicy(branch_sharing_policy, user):
+ """Mutator for branch_sharing_policy.
+
+ Checks authorization and entitlement.
+ """
+
def getAllowedBugInformationTypes():
"""Get the information types that a bug in this project can have.
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-08-08 05:36:44 +0000
+++ lib/lp/registry/model/product.py 2012-08-10 07:36:19 +0000
@@ -562,6 +562,34 @@
self.checkPrivateBugsTransitionAllowed(private_bugs, user)
self.private_bugs = private_bugs
+ def setBranchSharingPolicy(self, branch_sharing_policy, user):
+ """See `IProductPublic`."""
+ if not user or not IPersonRoles(user).in_commercial_admin:
+ raise Unauthorized(
+ "Only commercial admins can configure sharing policies right "
+ "now.")
+ if branch_sharing_policy != BranchSharingPolicy.PUBLIC:
+ if not self.has_current_commercial_subscription:
+ raise CommercialSubscribersOnly(
+ "A current commercial subscription is required to use "
+ "proprietary branches.")
+ self._ensurePolicies([InformationType.PROPRIETARY])
+ self.branch_sharing_policy = branch_sharing_policy
+
+ def setBugSharingPolicy(self, bug_sharing_policy, user):
+ """See `IProductPublic`."""
+ if not user or not IPersonRoles(user).in_commercial_admin:
+ raise Unauthorized(
+ "Only commercial admins can configure sharing policies right "
+ "now.")
+ if bug_sharing_policy != BugSharingPolicy.PUBLIC:
+ if not self.has_current_commercial_subscription:
+ raise CommercialSubscribersOnly(
+ "A current commercial subscription is required to use "
+ "proprietary bugs.")
+ self._ensurePolicies([InformationType.PROPRIETARY])
+ self.bug_sharing_policy = bug_sharing_policy
+
def getAllowedBugInformationTypes(self):
"""See `IProduct.`"""
types = set(InformationType.items)
@@ -663,10 +691,6 @@
self.commercial_subscription.registrant = registrant
self.commercial_subscription.purchaser = purchaser
- # The product now has a commercial subscription, so we need to ensure
- # it has a Proprietary access policy.
- self._ensurePolicies([InformationType.PROPRIETARY])
-
@property
def qualifies_for_free_hosting(self):
"""See `IProduct`."""
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2012-08-01 06:10:33 +0000
+++ lib/lp/registry/tests/test_product.py 2012-08-10 07:36:19 +0000
@@ -25,9 +25,14 @@
ILaunchpadUsage,
IServiceUsage,
)
+from lp.app.interfaces.services import IService
from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+ BranchSharingPolicy,
+ BugSharingPolicy,
+ InformationType,
+ )
from lp.registry.errors import (
CommercialSubscribersOnly,
OpenTeamLinkageError,
@@ -53,6 +58,7 @@
)
from lp.registry.model.productlicense import ProductLicense
from lp.testing import (
+ admin_logged_in,
celebrity_logged_in,
login,
login_person,
@@ -374,33 +380,6 @@
grantees = set([grant.grantee for grant in grants])
self.assertEqual(expected_grantess, grantees)
- def test_redeemSubscription_creates_proprietary_policy(self):
- # Creating a commercial subscription for a product also creates a
- # Proprietary policy.
- product = self.factory.makeProduct()
- product.redeemSubscriptionVoucher(
- 'hello', product.owner, product.owner, 1)
-
- ap = getUtility(IAccessPolicySource).findByPillar((product,))
- expected = [
- InformationType.USERDATA, InformationType.PRIVATESECURITY,
- InformationType.PROPRIETARY]
- self.assertContentEqual(expected, [policy.type for policy in ap])
-
- def test_redeemSubscription_grants_maintainer_access(self):
- # Creating a commercial subscription for a product creates an access
- # grant on the Proprietary policy for the maintainer.
- product = self.factory.makeProduct()
- product.redeemSubscriptionVoucher(
- 'hello', product.owner, product.owner, 1)
-
- policies = getUtility(IAccessPolicySource).find(
- [(product, InformationType.PROPRIETARY)])
- grants = getUtility(IAccessPolicyGrantSource).findByPolicy(policies)
- expected_grantess = set([product.owner])
- grantees = set([grant.grantee for grant in grants])
- self.assertEqual(expected_grantess, grantees)
-
def test_getAllowedBugInformationTypes(self):
# All projects currently support just the non-proprietary
# information types.
@@ -674,6 +653,129 @@
self.assertEqual(lp_janitor, cs.purchaser)
+class BaseSharingPolicyTests:
+ """Common tests for product sharing policies."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setSharingPolicy(self, policy, user):
+ raise NotImplementedError
+
+ def getSharingPolicy(self):
+ raise NotImplementedError
+
+ def setUp(self):
+ super(BaseSharingPolicyTests, self).setUp()
+ self.product = self.factory.makeProduct()
+ self.commercial_admin = self.factory.makePerson()
+ with admin_logged_in():
+ commercials = getUtility(ILaunchpadCelebrities).commercial_admin
+ commercials.addMember(self.commercial_admin, commercials)
+
+ def test_commercial_admin_can_set_policy(self):
+ # Commercial admins can set sharing policies.
+ self.setSharingPolicy(self.public_policy, self.commercial_admin)
+ self.assertEqual(self.public_policy, self.getSharingPolicy())
+
+ def test_random_cannot_set_policy(self):
+ # An unrelated user can't set sharing policies.
+ person = self.factory.makePerson()
+ self.assertRaises(
+ Unauthorized, self.setSharingPolicy, self.public_policy, person)
+
+ def test_owner_cannot_set_policy(self):
+ # The project owner can't yet set sharing policies. This will
+ # change once they're stable.
+ self.assertRaises(
+ Unauthorized, self.setSharingPolicy,
+ self.public_policy, self.product.owner)
+
+ def test_anonymous_cannot_set_policy(self):
+ # An anonymous user can't set sharing policies.
+ self.assertRaises(
+ Unauthorized, self.setSharingPolicy, self.public_policy, None)
+
+ def test_proprietary_forbidden_without_commercial_sub(self):
+ # No policy that allows Proprietary can be configured without a
+ # commercial subscription.
+ self.setSharingPolicy(self.public_policy, self.commercial_admin)
+ self.assertEqual(self.public_policy, self.getSharingPolicy())
+ for policy in self.commercial_policies:
+ self.assertRaises(
+ CommercialSubscribersOnly,
+ self.setSharingPolicy, policy, self.commercial_admin)
+
+ def test_proprietary_allowed_with_commercial_sub(self):
+ # All policies are valid when there's a current commercial
+ # subscription.
+ self.factory.makeCommercialSubscription(product=self.product)
+ for policy in self.enum.items:
+ self.setSharingPolicy(policy, self.commercial_admin)
+ self.assertEqual(policy, self.getSharingPolicy())
+
+ def test_setting_proprietary_creates_access_policy(self):
+ # Setting a policy that allows Proprietary creates a
+ # corresponding access policy and shares it with the the
+ # maintainer.
+ self.factory.makeCommercialSubscription(product=self.product)
+ self.assertEqual(
+ [InformationType.PRIVATESECURITY, InformationType.USERDATA],
+ [policy.type for policy in
+ getUtility(IAccessPolicySource).findByPillar([self.product])])
+ self.setSharingPolicy(
+ self.commercial_policies[0], self.commercial_admin)
+ self.assertEqual(
+ [InformationType.PRIVATESECURITY, InformationType.USERDATA,
+ InformationType.PROPRIETARY],
+ [policy.type for policy in
+ getUtility(IAccessPolicySource).findByPillar([self.product])])
+ self.assertTrue(
+ getUtility(IService, 'sharing').checkPillarAccess(
+ self.product, InformationType.PROPRIETARY, self.product.owner))
+
+
+class ProductBugSharingPolicyTestCase(BaseSharingPolicyTests,
+ TestCaseWithFactory):
+ """Test Product.bug_sharing_policy."""
+
+ layer = DatabaseFunctionalLayer
+
+ enum = BugSharingPolicy
+ public_policy = BugSharingPolicy.PUBLIC
+ commercial_policies = (
+ BugSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ BugSharingPolicy.PROPRIETARY_OR_PUBLIC,
+ BugSharingPolicy.PROPRIETARY,
+ )
+
+ def setSharingPolicy(self, policy, user):
+ return self.product.setBugSharingPolicy(policy, user)
+
+ def getSharingPolicy(self):
+ return self.product.bug_sharing_policy
+
+
+class ProductBranchSharingPolicyTestCase(BaseSharingPolicyTests,
+ TestCaseWithFactory):
+ """Test Product.branch_sharing_policy."""
+
+ layer = DatabaseFunctionalLayer
+
+ enum = BranchSharingPolicy
+ public_policy = BranchSharingPolicy.PUBLIC
+ commercial_policies = (
+ BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
+ BranchSharingPolicy.PROPRIETARY,
+ )
+
+ def setSharingPolicy(self, policy, user):
+ return self.product.setBranchSharingPolicy(policy, user)
+
+ def getSharingPolicy(self):
+ return self.product.branch_sharing_policy
+
+
class ProductSnapshotTestCase(TestCaseWithFactory):
"""Test product snapshots.
=== modified file 'lib/lp/registry/tests/test_product_webservice.py'
--- lib/lp/registry/tests/test_product_webservice.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_product_webservice.py 2012-08-10 07:36:19 +0000
@@ -3,11 +3,24 @@
__metaclass__ = type
+import json
+
+from lazr.uri import URI
+from testtools.matchers import MatchesStructure
from zope.security.proxy import removeSecurityProxy
+from lp.registry.enums import (
+ BranchSharingPolicy,
+ BugSharingPolicy,
+ )
+from lp.services.webapp.interfaces import OAuthPermission
+from lp.services.webapp.publisher import canonical_url
from lp.testing import TestCaseWithFactory
from lp.testing.layers import DatabaseFunctionalLayer
-from lp.testing.pages import LaunchpadWebServiceCaller
+from lp.testing.pages import (
+ LaunchpadWebServiceCaller,
+ webservice_for_person,
+ )
class TestProductAlias(TestCaseWithFactory):
@@ -26,3 +39,99 @@
self.assertEqual(
'http://api.launchpad.dev/beta/lemur',
response.getheader('location'))
+
+
+class TestProduct(TestCaseWithFactory):
+ """Webservice tests for products."""
+
+ layer = DatabaseFunctionalLayer
+
+ def patch(self, webservice, obj, **data):
+ return webservice.patch(
+ URI(canonical_url(obj)).path,
+ 'application/json', json.dumps(data),
+ api_version='devel')
+
+ def test_branch_sharing_policy_can_be_set(self):
+ # branch_sharing_policy can be set via the API.
+ product = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(product=product)
+ webservice = webservice_for_person(
+ self.factory.makeCommercialAdmin(),
+ permission=OAuthPermission.WRITE_PRIVATE)
+ response = self.patch(
+ webservice, product, branch_sharing_policy='Proprietary')
+ self.assertEqual(209, response.status)
+ self.assertEqual(
+ BranchSharingPolicy.PROPRIETARY, product.branch_sharing_policy)
+
+ def test_branch_sharing_policy_non_commercial(self):
+ # An API attempt to set a commercial-only branch_sharing_policy
+ # on a non-commercial project returns Forbidden.
+ product = self.factory.makeProduct()
+ webservice = webservice_for_person(
+ self.factory.makeCommercialAdmin(),
+ permission=OAuthPermission.WRITE_PRIVATE)
+ response = self.patch(
+ webservice, product, branch_sharing_policy='Proprietary')
+ self.assertThat(response, MatchesStructure.byEquality(
+ status=403,
+ body=('A current commercial subscription is required to use '
+ 'proprietary branches.')))
+ self.assertIs(None, product.branch_sharing_policy)
+
+ def test_branch_sharing_policy_random_user(self):
+ # Arbitrary users can't set branch_sharing_policy.
+ product = self.factory.makeProduct()
+ webservice = webservice_for_person(
+ self.factory.makePerson(),
+ permission=OAuthPermission.WRITE_PRIVATE)
+ response = self.patch(
+ webservice, product, branch_sharing_policy='Proprietary')
+ self.assertThat(response, MatchesStructure.byEquality(
+ status=401,
+ body=('Only commercial admins can configure sharing policies '
+ 'right now.')))
+ self.assertIs(None, product.branch_sharing_policy)
+
+ def test_bug_sharing_policy_can_be_set(self):
+ # bug_sharing_policy can be set via the API.
+ product = self.factory.makeProduct()
+ self.factory.makeCommercialSubscription(product=product)
+ webservice = webservice_for_person(
+ self.factory.makeCommercialAdmin(),
+ permission=OAuthPermission.WRITE_PRIVATE)
+ response = self.patch(
+ webservice, product, bug_sharing_policy='Proprietary')
+ self.assertEqual(209, response.status)
+ self.assertEqual(
+ BugSharingPolicy.PROPRIETARY, product.bug_sharing_policy)
+
+ def test_bug_sharing_policy_non_commercial(self):
+ # An API attempt to set a commercial-only bug_sharing_policy
+ # on a non-commercial project returns Forbidden.
+ product = self.factory.makeProduct()
+ webservice = webservice_for_person(
+ self.factory.makeCommercialAdmin(),
+ permission=OAuthPermission.WRITE_PRIVATE)
+ response = self.patch(
+ webservice, product, bug_sharing_policy='Proprietary')
+ self.assertThat(response, MatchesStructure.byEquality(
+ status=403,
+ body=('A current commercial subscription is required to use '
+ 'proprietary bugs.')))
+ self.assertIs(None, product.bug_sharing_policy)
+
+ def test_bug_sharing_policy_random_user(self):
+ # Arbitrary users can't set bug_sharing_policy.
+ product = self.factory.makeProduct()
+ webservice = webservice_for_person(
+ self.factory.makePerson(),
+ permission=OAuthPermission.WRITE_PRIVATE)
+ response = self.patch(
+ webservice, product, bug_sharing_policy='Proprietary')
+ self.assertThat(response, MatchesStructure.byEquality(
+ status=401,
+ body=('Only commercial admins can configure sharing policies '
+ 'right now.')))
+ self.assertIs(None, product.bug_sharing_policy)
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2012-08-07 02:31:39 +0000
+++ lib/lp/services/features/flags.py 2012-08-10 07:36:19 +0000
@@ -281,13 +281,6 @@
'',
'',
''),
- ('disclosure.branch_sharing_policy.show_to_admin',
- 'boolean',
- ('If true, the branch sharing policy field is shown on '
- 'Product:+admin, letting BranchVisibilityPolicy be overridden.'),
- '',
- '',
- ''),
('auditor.enabled',
'boolean',
'If true, send audit data to an auditor instance.',
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-08-09 04:25:18 +0000
+++ lib/lp/testing/factory.py 2012-08-10 07:36:19 +0000
@@ -532,17 +532,21 @@
@with_celebrity_logged_in('admin')
def makeAdministrator(self, name=None, email=None):
- user = self.makePerson(name=name, email=email)
- administrators = getUtility(ILaunchpadCelebrities).admin
- administrators.addMember(user, administrators.teamowner)
- return user
+ return self.makePerson(
+ name=name, email=email,
+ member_of=[getUtility(ILaunchpadCelebrities).admin])
@with_celebrity_logged_in('admin')
def makeRegistryExpert(self, name=None, email='expert@xxxxxxxxxxx'):
- user = self.makePerson(name=name, email=email)
- registry_team = getUtility(ILaunchpadCelebrities).registry_experts
- registry_team.addMember(user, registry_team.teamowner)
- return user
+ return self.makePerson(
+ name=name, email=email,
+ member_of=[getUtility(ILaunchpadCelebrities).registry_experts])
+
+ @with_celebrity_logged_in('admin')
+ def makeCommercialAdmin(self, name=None, email=None):
+ return self.makePerson(
+ name=name, email=email,
+ member_of=[getUtility(ILaunchpadCelebrities).commercial_admin])
def makeCopyArchiveLocation(self, distribution=None, owner=None,
name=None, enabled=True):
Follow ups
