launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/sharing-job-fflag-protection into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/sharing-job-fflag-protection/+merge/109072

== Implementation ==

Guard sharing job creation with "disclosure.enhanced_sharing.writable" feature flag so that jobs don't run on production till qa issues are sorted out.

== Tests ==

Update tests to add feature flag.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/bug.py
  lib/lp/bugs/model/bugtask.py
  lib/lp/bugs/model/tests/test_bugtask.py
  lib/lp/bugs/tests/test_bugvisibility.py
  lib/lp/registry/model/teammembership.py
  lib/lp/registry/tests/test_teammembership.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/sharing-job-fflag-protection/+merge/109072
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/sharing-job-fflag-protection into lp:launchpad.

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-06-06 20:36:12 +0000
+++ lib/lp/bugs/model/bug.py	2012-06-07 06:13:24 +0000
@@ -1809,10 +1809,12 @@
         self._reconcileAccess()
         self.updateHeat()
 
-        # As a result of the transition, some subscribers may no longer have
-        # access to the bug. We need to run a job to remove any such
-        # subscriptions.
-        getUtility(IRemoveBugSubscriptionsJobSource).create([self], who)
+        flag = 'disclosure.enhanced_sharing.writable'
+        if bool(getFeatureFlag(flag)):
+            # As a result of the transition, some subscribers may no longer
+            # have access to the bug. We need to run a job to remove any such
+            # subscriptions.
+            getUtility(IRemoveBugSubscriptionsJobSource).create([self], who)
 
         return True
 

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2012-06-05 02:03:44 +0000
+++ lib/lp/bugs/model/bugtask.py	2012-06-07 06:13:24 +0000
@@ -143,6 +143,7 @@
     SQLBase,
     sqlvalues,
     )
+from lp.services.features import getFeatureFlag
 from lp.services.helpers import shortlist
 from lp.services.propertycache import get_property_cache
 from lp.services.searchbuilder import any
@@ -1186,10 +1187,13 @@
             self.maybeConfirm()
         # END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
 
-        # As a result of the transition, some subscribers may no longer have
-        # access to the parent bug. We need to run a job to remove any such
-        # subscriptions.
-        getUtility(IRemoveBugSubscriptionsJobSource).create([self.bug], user)
+        flag = 'disclosure.enhanced_sharing.writable'
+        if bool(getFeatureFlag(flag)):
+            # As a result of the transition, some subscribers may no longer
+            # have access to the parent bug. We need to run a job to remove any
+            # such subscriptions.
+            getUtility(IRemoveBugSubscriptionsJobSource).create(
+                [self.bug], user)
 
     def updateTargetNameCache(self, newtarget=None):
         """See `IBugTask`."""

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2012-06-05 06:43:41 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2012-06-07 06:13:24 +0000
@@ -3342,6 +3342,7 @@
 
     def setUp(self):
         self.useFixture(FeatureFixture({
+            'disclosure.enhanced_sharing.writable': 'true',
             'jobs.celery.enabled_classes':
                 'RemoveBugSubscriptionsJob',
             'disclosure.access_mirror_triggers.removed': 'true',

=== modified file 'lib/lp/bugs/tests/test_bugvisibility.py'
--- lib/lp/bugs/tests/test_bugvisibility.py	2012-05-22 12:05:51 +0000
+++ lib/lp/bugs/tests/test_bugvisibility.py	2012-06-07 06:13:24 +0000
@@ -17,6 +17,7 @@
 
 
 LEGACY_VISIBILITY_FLAG = {
+    u"disclosure.enhanced_sharing.writable": "true",
     u"disclosure.legacy_subscription_visibility.enabled": u"true"}
 TRIGGERS_REMOVED_FLAG = {
     u"disclosure.access_mirror_triggers.removed": u"true"}

=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py	2012-05-22 12:05:51 +0000
+++ lib/lp/registry/model/teammembership.py	2012-06-07 06:13:24 +0000
@@ -65,6 +65,7 @@
     SQLBase,
     sqlvalues,
     )
+from lp.services.features import getFeatureFlag
 from lp.services.mail.helpers import (
     get_contact_email_addresses,
     get_email_template,
@@ -387,11 +388,13 @@
             _fillTeamParticipation(self.person, self.team)
         elif old_status in ACTIVE_STATES:
             _cleanTeamParticipation(self.person, self.team)
-            # A person has left the team so they may no longer have access to
-            # some artifacts shared with the team. We need to run a job to
-            # remove any subscriptions to such artifacts.
-            getUtility(IRemoveGranteeSubscriptionsJobSource).create(
-                None, self.person, user)
+            flag = 'disclosure.enhanced_sharing.writable'
+            if bool(getFeatureFlag(flag)):
+                # A person has left the team so they may no longer have access
+                # to some artifacts shared with the team. We need to run a job
+                # to remove any subscriptions to such artifacts.
+                getUtility(IRemoveGranteeSubscriptionsJobSource).create(
+                    None, self.person, user)
 
         else:
             # Changed from an inactive state to another inactive one, so no

=== modified file 'lib/lp/registry/tests/test_teammembership.py'
--- lib/lp/registry/tests/test_teammembership.py	2012-05-22 12:05:51 +0000
+++ lib/lp/registry/tests/test_teammembership.py	2012-06-07 06:13:24 +0000
@@ -504,7 +504,7 @@
         The number of db queries should be constant not O(depth).
         """
         self.assertStatementCount(
-            9,
+            7,
             self.team5.setMembershipData, self.no_priv,
             TeamMembershipStatus.DEACTIVATED, self.team5.teamowner)
 
@@ -998,6 +998,7 @@
 
     def setUp(self):
         self.useFixture(FeatureFixture({
+            'disclosure.enhanced_sharing.writable': 'true',
             'jobs.celery.enabled_classes':
                 'RemoveGranteeSubscriptionsJob',
         }))