launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/bug-sharing-policy-weirdness-1055250 into lp:launchpad.

Commit message:
The sharing service getBugSharingPolicy includes the pillar's current policy even if it is not nominally allowed.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1055250 in Launchpad itself: "Bug sharing policy weirdness on +sharing page"
  https://bugs.launchpad.net/launchpad/+bug/1055250

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/bug-sharing-policy-weirdness-1055250/+merge/125916

== Implementation ==

The UI was not updating correctly because the allowed bug sharing policies fetched from the sharing service did not include the current sharing policy. The current policy was no longer allowed but still needs to be supplied to the UI if it is the current one.

Branches worked correctly. Bugs needed to be updated to match.

== Tests ==

Add some more tests to test_sharingservice. The branch functionality was not tested so I added a test for that as well as one for the new bug functionality.

== Lint ==

Linting changed files:
  lib/lp/registry/services/sharingservice.py
  lib/lp/registry/services/tests/test_sharingservice.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/bug-sharing-policy-weirdness-1055250/+merge/125916
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/bug-sharing-policy-weirdness-1055250 into lp:launchpad.

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-09-19 13:22:42 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-09-24 03:48:22 +0000
@@ -386,6 +386,9 @@
                 BugSharingPolicy.PUBLIC_OR_PROPRIETARY,
                 BugSharingPolicy.PROPRIETARY_OR_PUBLIC,
                 BugSharingPolicy.PROPRIETARY])
+        if (pillar.bug_sharing_policy and
+            not pillar.bug_sharing_policy in allowed_policies):
+            allowed_policies.append(pillar.bug_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-09-19 13:22:42 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-09-24 03:48:22 +0000
@@ -190,6 +190,17 @@
              BranchSharingPolicy.PROPRIETARY_OR_PUBLIC,
              BranchSharingPolicy.PROPRIETARY])
 
+    def test_getBranchSharingPolicies_disallowed_policy(self):
+        # getBranchSharingPolicies includes a pillar's current policy even if
+        # it is nominally not allowed.
+        product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product, expired=True)
+        removeSecurityProxy(product).branch_sharing_policy = (
+            BranchSharingPolicy.FORBIDDEN)
+        self._assert_getBranchSharingPolicies(
+            product,
+            [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.
@@ -276,6 +287,16 @@
              BugSharingPolicy.PROPRIETARY_OR_PUBLIC,
              BugSharingPolicy.PROPRIETARY])
 
+    def test_getBugSharingPolicies_disallowed_policy(self):
+        # getBugSharingPolicies includes a pillar's current policy even if it
+        # is nominally not allowed.
+        product = self.factory.makeProduct()
+        self.factory.makeCommercialSubscription(product, expired=True)
+        removeSecurityProxy(product).bug_sharing_policy = (
+            BugSharingPolicy.FORBIDDEN)
+        self._assert_getBugSharingPolicies(
+            product, [BugSharingPolicy.PUBLIC, BugSharingPolicy.FORBIDDEN])
+
     def test_getBugSharingPolicies_distro(self):
         distro = self.factory.makeDistribution()
         self._assert_getBugSharingPolicies(distro, [BugSharingPolicy.PUBLIC])