launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/improved-bugremovesubscriptions-job into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1009358 in Launchpad itself: "Unsharing information from a team doesn't remove the members' bug subscriptions"
  https://bugs.launchpad.net/launchpad/+bug/1009358

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/improved-bugremovesubscriptions-job/+merge/110447

== Implementation ==

Use the grantee whose access is revoked (if one is specified) to fiyter the bug subscriptions for removal.

== Tests ==

Existing tests are sufficient, no new functionality added.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/interfaces/sharingjob.py
  lib/lp/registry/model/sharingjob.py
  lib/lp/registry/model/teammembership.py
  lib/lp/registry/services/sharingservice.py
  lib/lp/registry/services/tests/test_sharingservice.py

-- 
https://code.launchpad.net/~wallyworld/launchpad/improved-bugremovesubscriptions-job/+merge/110447
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/improved-bugremovesubscriptions-job into lp:launchpad.

=== modified file 'lib/lp/registry/interfaces/sharingjob.py'
--- lib/lp/registry/interfaces/sharingjob.py	2012-06-14 03:10:47 +0000
+++ lib/lp/registry/interfaces/sharingjob.py	2012-06-15 02:31:19 +0000
@@ -83,7 +83,8 @@
 class IRemoveBugSubscriptionsJobSource(ISharingJobSource):
     """An interface for acquiring IRemoveBugSubscriptionsJobs."""
 
-    def create(pillar, requestor, bugs=None, information_types=None):
+    def create(requestor, bugs=None, grantee=None, pillar=None,
+               information_types=None):
         """Create a new job to remove subscriptions for the specified bugs.
 
         Subscriptions for users who no longer have access to the bugs are

=== modified file 'lib/lp/registry/model/sharingjob.py'
--- lib/lp/registry/model/sharingjob.py	2012-06-15 00:42:38 +0000
+++ lib/lp/registry/model/sharingjob.py	2012-06-15 02:31:19 +0000
@@ -58,6 +58,7 @@
 from lp.registry.model.distribution import Distribution
 from lp.registry.model.person import Person
 from lp.registry.model.product import Product
+from lp.registry.model.teammembership import TeamParticipation
 from lp.services.config import config
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.lpstorm import IStore
@@ -331,10 +332,19 @@
                 filters.append(
                     BugTaskFlat.distribution == self.distro)
 
+        subscriptions_filter = [
+            In(BugSubscription.bug_id,
+                Select(BugTaskFlat.bug_id, where=And(*filters)))]
+        if self.grantee:
+            subscriptions_filter.append(
+                In(BugSubscription.person_id,
+                    Select(
+                        TeamParticipation.personID,
+                        where=TeamParticipation.team == self.grantee))
+            )
         subscriptions = IStore(BugSubscription).find(
             BugSubscription,
-            In(BugSubscription.bug_id,
-                Select(BugTaskFlat.bug_id, where=And(*filters)))
+            *subscriptions_filter
         )
         for sub in subscriptions:
             sub.bug.unsubscribe(

=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py	2012-06-14 07:43:06 +0000
+++ lib/lp/registry/model/teammembership.py	2012-06-15 02:31:19 +0000
@@ -394,7 +394,7 @@
                 # to some artifacts shared with the team. We need to run a job
                 # to remove any subscriptions to such artifacts.
                 getUtility(IRemoveBugSubscriptionsJobSource).create(
-                    user, grantee=self.team)
+                    user, grantee=self.person)
         else:
             # Changed from an inactive state to another inactive one, so no
             # need to fill/clean the TeamParticipation table.

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-06-14 07:43:06 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-06-15 02:31:19 +0000
@@ -333,7 +333,7 @@
         # Create a job to remove subscriptions for artifacts the sharee can no
         # longer see.
         getUtility(IRemoveBugSubscriptionsJobSource).create(
-            user, bugs=None, pillar=pillar,
+            user, bugs=None, grantee=sharee, pillar=pillar,
             information_types=information_types)
 
     @available_with_permission('launchpad.Edit', 'pillar')
@@ -361,7 +361,7 @@
         # longer see.
         if bugs:
             getUtility(IRemoveBugSubscriptionsJobSource).create(
-                user, bugs, pillar=pillar)
+                user, bugs, grantee=sharee, pillar=pillar)
         # XXX 2012-06-13 wallyworld bug=1012448
         # Remove branch subscriptions when information type fully implemented.
 

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-06-14 01:50:05 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-06-15 02:31:19 +0000
@@ -779,18 +779,6 @@
             information_type=InformationType.USERDATA)
         self._assert_revokeAccessGrants(distro, [bug], None)
 
-    # XXX 2012-06-13 wallyworld bug=1012448
-    # Remove branch subscriptions when information type fully implemented.
-#    def test_revokeAccessGrantsBranches(self):
-#        # Users with launchpad.Edit can delete all access for a sharee.
-#        owner = self.factory.makePerson()
-#        product = self.factory.makeProduct(owner=owner)
-#        login_person(owner)
-#        branch = self.factory.makeBranch(
-#            product=product, owner=owner,
-#            information_type=InformationType.USERDATA)
-#        self._assert_revokeAccessGrants(product, None, [branch])
-
     def _assert_revokeTeamAccessGrants(self, pillar, bugs, branches):
         artifacts = []
         if bugs:
@@ -864,18 +852,6 @@
             information_type=InformationType.USERDATA)
         self._assert_revokeTeamAccessGrants(distro, [bug], None)
 
-    # XXX 2012-06-13 wallyworld bug=1012448
-    # Remove branch subscriptions when information type fully implemented.
-#    def test_revokeAccessGrantsBranches(self):
-#        # Users with launchpad.Edit can delete all access for a sharee.
-#        owner = self.factory.makePerson()
-#        product = self.factory.makeProduct(owner=owner)
-#        login_person(owner)
-#        branch = self.factory.makeBranch(
-#            product=product, owner=owner,
-#            information_type=InformationType.USERDATA)
-#        self._assert_revokeTeamAccessGrants(distro, [bug], None)
-
     def _assert_revokeAccessGrantsUnauthorized(self):
         # revokeAccessGrants raises an Unauthorized exception if the user
         # is not permitted to do so.