launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/embargoed-sharing-policy-ui-1055617 into lp:launchpad.

Commit message:
If branch sharing policy is embargoed, do not allow it to be changed via the ui.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1055617 in Launchpad itself: "'Embargoed, can be proprietary' for branch sharing policy should be visible to select in UI"
  https://bugs.launchpad.net/launchpad/+bug/1055617

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/embargoed-sharing-policy-ui-1055617/+merge/126585

== Implementation ==

Tweak the sharing service getBranchSharingPolicies() method to only return embargoed if that's what the current policy is.

== Tests ==

Update test_getBranchSharingPolicies_product_with_embargoed

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/services/sharingservice.py
  lib/lp/registry/services/tests/test_sharingservice.py

-- 
https://code.launchpad.net/~wallyworld/launchpad/embargoed-sharing-policy-ui-1055617/+merge/126585
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/embargoed-sharing-policy-ui-1055617 into lp:launchpad.

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-09-26 22:51:02 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-09-27 01:26:23 +0000
@@ -426,14 +426,19 @@
         """See `ISharingService`."""
         # Only Products have branch sharing policies. Distributions just
         # default to Public.
-        allowed_policies = [BranchSharingPolicy.PUBLIC]
-        # Commercial projects also allow proprietary branches.
-        if (IProduct.providedBy(pillar)
-            and pillar.has_current_commercial_subscription):
-            allowed_policies.extend([
-                BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
-                BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
-                BranchSharingPolicy.PROPRIETARY])
+        # If the branch sharing policy is EMBARGOED_OR_PROPRIETARY, then we
+        # do not allow any other policies.
+        allowed_policies = []
+        if (pillar.branch_sharing_policy !=
+                BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY):
+            allowed_policies = [BranchSharingPolicy.PUBLIC]
+            # Commercial projects also allow proprietary branches.
+            if (IProduct.providedBy(pillar)
+                and pillar.has_current_commercial_subscription):
+                allowed_policies.extend([
+                    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)

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-09-26 22:51:02 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-09-27 01:26:23 +0000
@@ -204,17 +204,13 @@
             [BranchSharingPolicy.PUBLIC, BranchSharingPolicy.FORBIDDEN])
 
     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.
+        # If the current sharing policy is embargoed, that is all that is
+        # allowed.
         product = self.factory.makeProduct(
             branch_sharing_policy=BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY)
         self._assert_getBranchSharingPolicies(
             product,
-            [BranchSharingPolicy.PUBLIC,
-             BranchSharingPolicy.PUBLIC_OR_PROPRIETARY,
-             BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
-             BranchSharingPolicy.PROPRIETARY,
-             BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY])
+            [BranchSharingPolicy.EMBARGOED_OR_PROPRIETARY])
 
     def test_getBranchSharingPolicies_distro(self):
         distro = self.factory.makeDistribution()