Merge lp:~abentley/launchpad/specification-policy into lp:launchpad

[Aaron Bentley]

Specification sharing policy creates information types.

= Summary =
Fix bug #1061621: Setting specification policy to "proprietary" does not add "proprietary" to sharing table

== Proposed fix ==
Update pruneUnusedPolicies to consider specification sharing policy.

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of private projects.

== Implementation details ==
There were already shared tests for policies. I included Specifications sharing policies in those tests and fixed the failures. This required adding SpecificationSharingPolicy.FORBIDDEN to SPECIFICATION_POLICY_ALLOWED_TYPES

Various tests assumed that policies would not be created, and were adjusted to assume they would.

== Tests ==
bin/test -t ProductSpecificationSharingPolicyTestCase -t test_specification -t test_sharingservice

== Demo and Q/A ==
Create a product with an open source license, then change the license to other/proprietary (so that the branch, bug, specification policies are PUBLIC).

View +sharing. "Proprietary" will not be listed under "Who it's shared with".

Change the specification policy to proprietary. "Proprietary" will be listed under "Who it's shared with".

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/services/tests/test_sharingservice.py
  lib/lp/testing/factory.py
  lib/lp/blueprints/model/specification.py
  lib/lp/registry/model/product.py
  lib/lp/blueprints/browser/tests/test_specification.py
  lib/lp/registry/tests/test_product.py

[Richard Harding (rharding)]

Thanks for the update Aaron. I think I follow along.

The comment in #49 implies there's some filtering after the fact around bug/branch policies. I just bring it up to verify there's nothing there to be added for the additional specification policy code.

Edit: never mind, but can you update the comment to also note specification types to reflect the update.

review: Approve

=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py	2012-10-08 10:07:11 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py	2012-10-10 19:20:29 +0000
@@ -460,9 +460,6 @@
             if sharing_policy is not None:
                 self.factory.makeCommercialSubscription(product)
                 product.setSpecificationSharingPolicy(sharing_policy)
-            policy = self.factory.makeAccessPolicy(product, information_type)
-            self.factory.makeAccessPolicyGrant(
-                policy, grantee=self.user, grantor=self.user)
             browser = self.getViewBrowser(product, view_name='+addspec')
             control = browser.getControl(information_type.title)
             if not control.selected:
@@ -619,7 +616,6 @@
         self.assertEqual(spec.information_type, InformationType.EMBARGOED)
 
 
-
 class TestNewSpecificationDefaultInformationTypeProduct(
     BrowserTestCase, BaseNewSpecificationInformationTypeDefaultMixin):
 

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2012-10-03 13:05:31 +0000
+++ lib/lp/blueprints/model/specification.py	2012-10-10 19:20:29 +0000
@@ -143,6 +143,7 @@
     SpecificationSharingPolicy.PROPRIETARY: [InformationType.PROPRIETARY],
     SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY:
         [InformationType.PROPRIETARY, InformationType.EMBARGOED],
+    SpecificationSharingPolicy.FORBIDDEN: [],
     }
 
 SPECIFICATION_POLICY_DEFAULT_TYPES = {

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-10-09 13:25:28 +0000
+++ lib/lp/registry/model/product.py	2012-10-10 19:20:29 +0000
@@ -731,10 +731,15 @@
         allowed_branch_types = set(
             BRANCH_POLICY_ALLOWED_TYPES.get(
                 self.branch_sharing_policy, FREE_INFORMATION_TYPES))
+        allowed_specification_types = set(
+            SPECIFICATION_POLICY_ALLOWED_TYPES.get(
+                self.specification_sharing_policy, [InformationType.PUBLIC])
+        )
         allowed_types = allowed_bug_types.union(allowed_branch_types)
+        allowed_types = allowed_types.union(allowed_specification_types)
         # Fetch all APs, and after filtering out ones that are forbidden
-        # by the bug and branch policies, the APs that have no APAs are
-        # unused and can be deleted.
+        # by the bug, branch, and specification policies, the APs that have no
+        # APAs are unused and can be deleted.
         ap_source = getUtility(IAccessPolicySource)
         access_policies = set(ap_source.findByPillar([self]))
         apa_source = getUtility(IAccessPolicyArtifactSource)

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-10-10 02:45:36 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-10-10 19:20:29 +0000
@@ -892,7 +892,8 @@
             artifacts.extend(branches)
         if specifications:
             artifacts.extend(specifications)
-        policy = self.factory.makeAccessPolicy(pillar=pillar)
+        policy = self.factory.makeAccessPolicy(pillar=pillar,
+                                               check_existing=True)
         # Grant access to a grantee and another person.
         grantee = self.factory.makePerson()
         someone = self.factory.makePerson()
@@ -981,8 +982,6 @@
         product = self.factory.makeProduct(
             owner=owner, specification_sharing_policy=(
                 SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY))
-        self.factory.makeAccessPolicy(
-            pillar=product, type=InformationType.EMBARGOED)
         login_person(owner)
         specification = self.factory.makeSpecification(
             product=product, owner=owner,
@@ -998,7 +997,8 @@
             artifacts.extend(branches)
         if specifications:
             artifacts.extend(specifications)
-        policy = self.factory.makeAccessPolicy(pillar=pillar)
+        policy = self.factory.makeAccessPolicy(pillar=pillar,
+                                               check_existing=True)
 
         person_grantee = self.factory.makePerson()
         team_owner = self.factory.makePerson()
@@ -1089,7 +1089,6 @@
         product = self.factory.makeProduct(
             owner=owner, specification_sharing_policy=(
                 SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY))
-        self.factory.makeAccessPolicy(product, InformationType.EMBARGOED)
         login_person(owner)
         specification = self.factory.makeSpecification(
             product=product, owner=owner,

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-10-10 12:18:43 +0000
+++ lib/lp/registry/tests/test_product.py	2012-10-10 19:20:29 +0000
@@ -1212,7 +1212,7 @@
             [policy.type for policy in
              getUtility(IAccessPolicySource).findByPillar([self.product])])
         self.setSharingPolicy(
-            BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY,
+            self.enum.EMBARGOED_OR_PROPRIETARY,
             self.commercial_admin)
         self.assertEqual(
             [InformationType.PRIVATESECURITY, InformationType.USERDATA,
@@ -1229,6 +1229,30 @@
                 self.product.owner))
 
 
+class ProductSpecificationSharingPolicyTestCase(
+    ProductBranchSharingPolicyTestCase):
+    """Test Product.specification_sharing_policy."""
+
+    layer = DatabaseFunctionalLayer
+
+    enum = SpecificationSharingPolicy
+    public_policy = SpecificationSharingPolicy.PUBLIC
+    commercial_policies = (
+        SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY,
+        SpecificationSharingPolicy.PROPRIETARY_OR_PUBLIC,
+        SpecificationSharingPolicy.PROPRIETARY,
+        SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY,
+        )
+
+    def setSharingPolicy(self, policy, user):
+        with person_logged_in(user):
+            result = self.product.setSpecificationSharingPolicy(policy)
+        return result
+
+    def getSharingPolicy(self):
+        return self.product.specification_sharing_policy
+
+
 class ProductSnapshotTestCase(TestCaseWithFactory):
     """Test product snapshots.
 

=== modified file 'lib/lp/registry/tests/test_sharingjob.py'
--- lib/lp/registry/tests/test_sharingjob.py	2012-09-24 14:08:00 +0000
+++ lib/lp/registry/tests/test_sharingjob.py	2012-10-10 19:20:29 +0000
@@ -286,8 +286,6 @@
         product = self.factory.makeProduct(
             specification_sharing_policy=(
                 SpecificationSharingPolicy.EMBARGOED_OR_PROPRIETARY))
-        self.factory.makeAccessPolicy(product, InformationType.PROPRIETARY)
-        self.factory.makeAccessPolicy(product, InformationType.EMBARGOED)
         owner = self.factory.makePerson()
         [policy] = getUtility(IAccessPolicySource).find(
             [(product, InformationType.USERDATA)])

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-10-08 10:07:11 +0000
+++ lib/lp/testing/factory.py	2012-10-10 19:20:29 +0000
@@ -4217,10 +4217,16 @@
             package_version=package_version, requester=requester)
 
     def makeAccessPolicy(self, pillar=None,
-                         type=InformationType.PROPRIETARY):
+                         type=InformationType.PROPRIETARY,
+                         check_existing=False):
         if pillar is None:
             pillar = self.makeProduct()
-        policies = getUtility(IAccessPolicySource).create([(pillar, type)])
+        policy_source = getUtility(IAccessPolicySource)
+        if check_existing:
+            policy = policy_source.find([(pillar, type)]).one()
+            if policy is not None:
+                return policy
+        policies = policy_source.create([(pillar, type)])
         return policies[0]
 
     def makeAccessArtifact(self, concrete=None):