launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/specification-policy into lp:launchpad.

Commit message:
Specification sharing policy creates information types.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1061621 in Launchpad itself: "Setting specification policy to "proprietary" does not add "proprietary" to sharing table"
  https://bugs.launchpad.net/launchpad/+bug/1061621

For more details, see:
https://code.launchpad.net/~abentley/launchpad/specification-policy/+merge/128998

= 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
-- 
https://code.launchpad.net/~abentley/launchpad/specification-policy/+merge/128998
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/specification-policy into lp:launchpad.

=== 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 17:03:22 +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 17:03:22 +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 17:03:22 +0000
@@ -731,7 +731,12 @@
         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.

=== 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 17:03:22 +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 17:03:22 +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/testing/factory.py'
--- lib/lp/testing/factory.py	2012-10-08 10:07:11 +0000
+++ lib/lp/testing/factory.py	2012-10-10 17:03:22 +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):