launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11009
[Merge] lp:~wallyworld/launchpad/embargoed-information-type-1036187 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/embargoed-information-type-1036187 into lp:launchpad with lp:~wallyworld/launchpad/edit-sharing-policies2-1036437 as a prerequisite.
Requested reviews:
Curtis Hovey (sinzui)
Related bugs:
Bug #1036187 in Launchpad itself: "Add the Embargoed Information Type"
https://bugs.launchpad.net/launchpad/+bug/1036187
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/embargoed-information-type-1036187/+merge/119837
== Implementation ==
Add a new InformationType enum - EMBARGOED.
Add a new product branch sharing policy - EMBARGOED_OR_PROPRIETARY
Update the branch namespace used for products to implement the business rules: branches for these products start out embargoed by default but may become proprietary. The policy choice isn't available in the +sharing ui unless the product has been set to this new policy via the API.
TODO NEXT: a db patch to update those places where we do
"... information_type IN (3, 4, 5)..."
to account for the new private type "6".
== Tests ==
Update the relevant unit tests for the changed code.
Add tests to:
- TestBranchEditViewInformationTypes
- TestProductNamespacePrivacyWithInformationType
- TestBranchGetAllowedInformationTypes
- TestSharingService
I also added an additional test to TestBranchEditViewInformationTypes to increase the coverage for existing code: test_branch_for_project_with_proprietary.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/code/browser/branch.py
lib/lp/code/browser/tests/test_branch.py
lib/lp/code/model/branchnamespace.py
lib/lp/code/model/tests/test_branch.py
lib/lp/code/model/tests/test_branchnamespace.py
lib/lp/registry/enums.py
lib/lp/registry/model/product.py
lib/lp/registry/services/sharingservice.py
lib/lp/registry/services/tests/test_sharingservice.py
lib/lp/registry/tests/test_product.py
--
https://code.launchpad.net/~wallyworld/launchpad/embargoed-information-type-1036187/+merge/119837
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2012-08-07 02:31:56 +0000
+++ lib/lp/code/browser/branch.py 2012-08-16 06:27:21 +0000
@@ -737,6 +737,7 @@
InformationType.PUBLIC,
InformationType.USERDATA,
InformationType.PROPRIETARY,
+ InformationType.EMBARGOED,
)
# We only show Private Security and Public Security
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2012-08-09 01:59:54 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2012-08-16 06:27:21 +0000
@@ -35,9 +35,15 @@
BranchType,
BranchVisibilityRule,
)
-from lp.registry.enums import InformationType
+from lp.registry.enums import (
+ BranchSharingPolicy,
+ InformationType,
+ )
from lp.registry.interfaces.accesspolicy import IAccessPolicySource
-from lp.registry.interfaces.person import PersonVisibility
+from lp.registry.interfaces.person import (
+ IPersonSet,
+ PersonVisibility,
+ )
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
from lp.services.helpers import truncate_text
@@ -66,6 +72,7 @@
setupBrowser,
setupBrowserForUser,
)
+from lp.testing.sampledata import COMMERCIAL_ADMIN_EMAIL
from lp.testing.views import create_initialized_view
@@ -1073,6 +1080,37 @@
InformationType.PRIVATESECURITY, InformationType.USERDATA],
branch)
+ def test_branch_for_project_with_embargoed_and_proprietary(self):
+ # Branches for commercial projects which have a policy of embargoed or
+ # proprietary allow only embargoed and proprietary types.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ self.factory.makeCommercialSubscription(product=product)
+ comadmin = getUtility(IPersonSet).getByEmail(COMMERCIAL_ADMIN_EMAIL)
+ product.setBranchSharingPolicy(
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY, comadmin)
+ with person_logged_in(owner):
+ branch = self.factory.makeBranch(
+ product=product, owner=owner,
+ information_type=InformationType.PROPRIETARY)
+ self.assertShownTypes(
+ [InformationType.EMBARGOED, InformationType.PROPRIETARY], branch)
+
+ def test_branch_for_project_with_proprietary(self):
+ # Branches for commercial projects which have a policy of proprietary
+ # allow only the proprietary type.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ self.factory.makeCommercialSubscription(product=product)
+ comadmin = getUtility(IPersonSet).getByEmail(COMMERCIAL_ADMIN_EMAIL)
+ product.setBranchSharingPolicy(
+ BranchSharingPolicy.PROPRIETARY, comadmin)
+ with person_logged_in(owner):
+ branch = self.factory.makeBranch(
+ product=product, owner=owner,
+ information_type=InformationType.PROPRIETARY)
+ self.assertShownTypes([InformationType.PROPRIETARY], branch)
+
class TestBranchUpgradeView(TestCaseWithFactory):
=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py 2012-08-14 19:37:38 +0000
+++ lib/lp/code/model/branchnamespace.py 2012-08-16 06:27:21 +0000
@@ -91,6 +91,8 @@
BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: InformationType.items,
BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.items,
BranchSharingPolicy.PROPRIETARY: [InformationType.PROPRIETARY],
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY:
+ [InformationType.PROPRIETARY, InformationType.EMBARGOED],
}
POLICY_DEFAULT_TYPES = {
@@ -98,6 +100,7 @@
BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: InformationType.PUBLIC,
BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.PROPRIETARY,
BranchSharingPolicy.PROPRIETARY: InformationType.PROPRIETARY,
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY: InformationType.EMBARGOED,
}
POLICY_REQUIRED_GRANTS = {
@@ -105,6 +108,7 @@
BranchSharingPolicy.PUBLIC_OR_PROPRIETARY: None,
BranchSharingPolicy.PROPRIETARY_OR_PUBLIC: InformationType.PROPRIETARY,
BranchSharingPolicy.PROPRIETARY: InformationType.PROPRIETARY,
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY: InformationType.PROPRIETARY,
}
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2012-08-13 21:33:47 +0000
+++ lib/lp/code/model/tests/test_branch.py 2012-08-16 06:27:21 +0000
@@ -2429,6 +2429,9 @@
self.assertNotIn(
InformationType.PROPRIETARY,
branch.getAllowedInformationTypes(branch.owner))
+ self.assertNotIn(
+ InformationType.EMBARGOED,
+ branch.getAllowedInformationTypes(branch.owner))
def test_admin_sees_namespace_types(self):
# An admin sees all the types, since they occasionally need to
=== modified file 'lib/lp/code/model/tests/test_branchnamespace.py'
--- lib/lp/code/model/tests/test_branchnamespace.py 2012-08-14 19:37:38 +0000
+++ lib/lp/code/model/tests/test_branchnamespace.py 2012-08-16 06:27:21 +0000
@@ -529,6 +529,26 @@
InformationType.PROPRIETARY,
namespace.getDefaultInformationType())
+ def test_embargoed_or_proprietary_anyone(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY)
+ self.assertContentEqual([], namespace.getAllowedInformationTypes())
+ self.assertIs(None, namespace.getDefaultInformationType())
+
+ def test_embargoed_or_proprietary_grantee(self):
+ namespace = self.makeProductNamespace(
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY)
+ with admin_logged_in():
+ getUtility(IService, 'sharing').sharePillarInformation(
+ namespace.product, namespace.owner, namespace.product.owner,
+ {InformationType.PROPRIETARY: SharingPermission.ALL})
+ self.assertContentEqual(
+ [InformationType.PROPRIETARY, InformationType.EMBARGOED],
+ namespace.getAllowedInformationTypes())
+ self.assertEqual(
+ InformationType.EMBARGOED,
+ namespace.getDefaultInformationType())
+
class TestPackageNamespace(TestCaseWithFactory, NamespaceMixin):
"""Tests for `PackageNamespace`."""
=== modified file 'lib/lp/registry/enums.py'
--- lib/lp/registry/enums.py 2012-08-13 23:33:02 +0000
+++ lib/lp/registry/enums.py 2012-08-16 06:27:21 +0000
@@ -66,6 +66,12 @@
Only shared with users permitted to see proprietary information.
""")
+ EMBARGOED = DBItem(6, """
+ Embargoed
+
+ Only shared with users permitted to see embargoed information.
+ """)
+
PUBLIC_INFORMATION_TYPES = (
InformationType.PUBLIC, InformationType.PUBLICSECURITY)
@@ -73,7 +79,7 @@
PRIVATE_INFORMATION_TYPES = (
InformationType.PRIVATESECURITY, InformationType.USERDATA,
- InformationType.PROPRIETARY)
+ InformationType.PROPRIETARY, InformationType.EMBARGOED)
SECURITY_INFORMATION_TYPES = (
@@ -135,6 +141,14 @@
project's proprietary information can create new branches.
""")
+ EMBARGOED_OR_PROPRIETARY = DBItem(5, """
+ Embargoed, can be proprietary
+
+ New branches are embargoed, but can be made proprietary later. Only
+ people who can see the project's proprietary information can create
+ new branches.
+ """)
+
class BugSharingPolicy(DBEnumeratedType):
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2012-08-10 06:39:58 +0000
+++ lib/lp/registry/model/product.py 2012-08-16 06:27:21 +0000
@@ -92,7 +92,6 @@
from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
from lp.bugs.interfaces.bugtaskfilter import OrderedBugTask
-from lp.bugs.model.bug import BugSet
from lp.bugs.model.bugtarget import (
BugTargetBase,
OfficialBugTagTargetMixin,
@@ -573,7 +572,11 @@
raise CommercialSubscribersOnly(
"A current commercial subscription is required to use "
"proprietary branches.")
- self._ensurePolicies([InformationType.PROPRIETARY])
+ required_policies = [InformationType.PROPRIETARY]
+ if (branch_sharing_policy ==
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY):
+ required_policies.append(InformationType.EMBARGOED)
+ self._ensurePolicies(required_policies)
self.branch_sharing_policy = branch_sharing_policy
def setBugSharingPolicy(self, bug_sharing_policy, user):
=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py 2012-08-16 06:27:21 +0000
+++ lib/lp/registry/services/sharingservice.py 2012-08-16 06:27:21 +0000
@@ -285,6 +285,9 @@
BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
BranchSharingPolicy.PROPRIETARY])
+ if (pillar.branch_sharing_policy and
+ not pillar.branch_sharing_policy in allowed_policies):
+ allowed_policies.append(pillar.branch_sharing_policy)
return self._makeEnumData(allowed_policies)
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2012-08-16 06:27:21 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-08-16 06:27:21 +0000
@@ -182,6 +182,20 @@
BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
BranchSharingPolicy.PROPRIETARY])
+ def test_getBranchSharingPolicies_product_with_embargoed(self):
+ # The sharing policies will contain the product's sharing policy even
+ # if it is not in the nominally allowed policy list.
+ product = self.factory.makeProduct(
+ branch_sharing_policy=BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY)
+ self.factory.makeCommercialSubscription(product)
+ self._assert_getBranchSharingPolicies(
+ product,
+ [BranchSharingPolicy.PUBLIC,
+ BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
+ BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
+ BranchSharingPolicy.PROPRIETARY,
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY])
+
def test_getBranchSharingPolicies_distro(self):
distro = self.factory.makeDistribution()
self._assert_getBranchSharingPolicies(distro, [])
=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py 2012-08-14 23:27:07 +0000
+++ lib/lp/registry/tests/test_product.py 2012-08-16 06:27:21 +0000
@@ -765,6 +765,7 @@
BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
BranchSharingPolicy.PROPRIETARY,
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY,
)
def setSharingPolicy(self, policy, user):
@@ -773,6 +774,30 @@
def getSharingPolicy(self):
return self.product.branch_sharing_policy
+ def test_setting_embargoed_creates_access_policy(self):
+ # Setting a policy that allows Embargoed 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(
+ BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY,
+ self.commercial_admin)
+ self.assertEqual(
+ [InformationType.PRIVATESECURITY, InformationType.USERDATA,
+ InformationType.PROPRIETARY, InformationType.EMBARGOED],
+ [policy.type for policy in
+ getUtility(IAccessPolicySource).findByPillar([self.product])])
+ self.assertTrue(
+ getUtility(IService, 'sharing').checkPillarAccess(
+ self.product, InformationType.PROPRIETARY, self.product.owner))
+ self.assertTrue(
+ getUtility(IService, 'sharing').checkPillarAccess(
+ self.product, InformationType.EMBARGOED, self.product.owner))
+
class ProductSnapshotTestCase(TestCaseWithFactory):
"""Test product snapshots.
Follow ups
