← Back to team overview

launchpad-reviewers team mailing list archive

lp:~wallyworld/launchpad/unshare-team-members-subscriptions2-1009358 into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/unshare-team-members-subscriptions2-1009358 into lp:launchpad with lp:~wallyworld/launchpad/bug-privacy-filter-storm-expressions as a prerequisite.

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/unshare-team-members-subscriptions2-1009358/+merge/110215

== Implementation ==

This branch stops using RemoveGranteeSubscriptionsJob and replaces it with an enhanced version of RemoveBugSubscriptionsJob. This ensures the correct functionality for all cases where subscriptions need to be removed for invisible bugs. A side effect is that for now, branch subscriptions aren't removed in some cases, but this functionality was only partially implemented anyway. The next step is to introduce a RemoveBranchSubscriptionsJob to handle branches.

It was discovered doing this work that a bug existed in the sharingservice deletePillarSharee method. This was fixed and a test enhancement done to test for the issue.

A subsequent branch will delete RemoveGranteeSubscriptionsJob.

== Tests ==

Add new tests for RemoveBugSubscriptionsJob:
- test_unsubscribe_pillar_artifacts_specific_info_types
- test_admins_retain_subscriptions

Update sharing service deletePillarSharee tests to ensure the identified defect is fixed.

== 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/bugtasksearch.py
  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
  lib/lp/registry/tests/test_sharingjob.py
  lib/lp/registry/tests/test_teammembership.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/unshare-team-members-subscriptions2-1009358/+merge/110215
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/unshare-team-members-subscriptions2-1009358 into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-06-14 02:33:22 +0000
+++ lib/lp/bugs/model/bug.py	2012-06-14 02:33:22 +0000
@@ -1811,7 +1811,7 @@
             # 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)
+            getUtility(IRemoveBugSubscriptionsJobSource).create(who, [self])
 
         return True
 

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2012-06-14 02:33:22 +0000
+++ lib/lp/bugs/model/bugtask.py	2012-06-14 02:33:22 +0000
@@ -1193,7 +1193,7 @@
             # have access to the parent bug. We need to run a job to remove any
             # such subscriptions.
             getUtility(IRemoveBugSubscriptionsJobSource).create(
-                [self.bug], user)
+                user, [self.bug])
 
     def updateTargetNameCache(self, newtarget=None):
         """See `IBugTask`."""

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2012-06-14 02:33:22 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2012-06-14 02:33:22 +0000
@@ -5,11 +5,13 @@
 
 __all__ = [
     'get_bug_privacy_filter',
+    'get_bug_privacy_filter_terms',
     'orderby_expression',
     'search_bugs',
     ]
 
 from lazr.enum import BaseItem
+from lazr.restful.utils import safe_hasattr
 from sqlobject.sqlbuilder import SQLConstant
 from storm.expr import (
     Alias,
@@ -1378,7 +1380,7 @@
     :return: A SQL filter, a decorator to cache visibility in a resultset that
         returns BugTask objects.
     """
-    bug_filter_terms = _get_bug_privacy_filter_terms(user)
+    bug_filter_terms = get_bug_privacy_filter_terms(user)
     if not bug_filter_terms:
         return True, _nocache_bug_decorator
     if len(bug_filter_terms) == 1:
@@ -1388,16 +1390,19 @@
     return expr, _make_cache_user_can_view_bug(user)
 
 
-def _get_bug_privacy_filter_terms(user):
+def get_bug_privacy_filter_terms(user_or_reference):
     public_bug_filter = (
         BugTaskFlat.information_type.is_in(PUBLIC_INFORMATION_TYPES))
 
-    if user is None:
+    if user_or_reference is None:
         return [public_bug_filter]
 
-    admin_team = getUtility(ILaunchpadCelebrities).admin
-    if removeSecurityProxy(user).inTeam(admin_team):
-        return []
+    user_key = user_or_reference
+    if safe_hasattr(user_or_reference, 'id'):
+        admin_team = getUtility(ILaunchpadCelebrities).admin
+        if removeSecurityProxy(user_or_reference).inTeam(admin_team):
+            return []
+        user_key = user_or_reference.id
 
     artifact_grant_query = Coalesce(
             ArrayIntersects(SQL('BugTaskFlat.access_grants'),
@@ -1405,7 +1410,7 @@
                 ArrayAgg(TeamParticipation.teamID),
                 tables=TeamParticipation,
                 where=(TeamParticipation.personID ==
-                       user.id)
+                       user_key)
             )), False)
 
     policy_grant_query = Coalesce(
@@ -1418,7 +1423,7 @@
                             AccessPolicyGrant.grantee_id)),
                 where=(
                     TeamParticipation.personID ==
-                    user.id)
+                    user_key)
             )), False)
 
     return [public_bug_filter, artifact_grant_query, policy_grant_query]

=== modified file 'lib/lp/registry/interfaces/sharingjob.py'
--- lib/lp/registry/interfaces/sharingjob.py	2012-05-22 12:05:51 +0000
+++ lib/lp/registry/interfaces/sharingjob.py	2012-06-14 02:33:22 +0000
@@ -92,7 +92,7 @@
 class IRemoveBugSubscriptionsJobSource(ISharingJobSource):
     """An interface for acquiring IRemoveBugSubscriptionsJobs."""
 
-    def create(pillar, bugs, requestor):
+    def create(pillar, requestor, bugs=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-14 02:33:22 +0000
+++ lib/lp/registry/model/sharingjob.py	2012-06-14 02:33:22 +0000
@@ -24,13 +24,14 @@
 import simplejson
 from sqlobject import SQLObjectNotFound
 from storm.expr import (
+    Alias,
     And,
-    Coalesce,
     In,
-    Join,
     Not,
+    Or,
     Select,
     SQL,
+    With,
     )
 from storm.locals import (
     Int,
@@ -48,7 +49,10 @@
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugsubscription import BugSubscription
 from lp.bugs.model.bugtaskflat import BugTaskFlat
-from lp.bugs.model.bugtasksearch import get_bug_privacy_filter
+from lp.bugs.model.bugtasksearch import (
+    get_bug_privacy_filter,
+    get_bug_privacy_filter_terms,
+    )
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.registry.enums import InformationType
 from lp.registry.interfaces.person import IPersonSet
@@ -61,7 +65,6 @@
     ISharingJob,
     ISharingJobSource,
     )
-from lp.registry.model.accesspolicy import AccessPolicyGrant
 from lp.registry.model.distribution import Distribution
 from lp.registry.model.person import Person
 from lp.registry.model.product import Product
@@ -70,10 +73,6 @@
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.lpstorm import IStore
 from lp.services.database.stormbase import StormBase
-from lp.services.database.stormexpr import (
-    ArrayAgg,
-    ArrayIntersects,
-    )
 from lp.services.job.model.job import (
     EnumeratedSubclass,
     Job,
@@ -404,14 +403,18 @@
     config = config.IRemoveBugSubscriptionsJobSource
 
     @classmethod
-    def create(cls, bugs, requestor):
+    def create(cls, requestor, bugs=None, information_types=None):
         """See `IRemoveBugSubscriptionsJob`."""
 
         bug_ids = [
-            bug.id for bug in bugs
+            bug.id for bug in bugs or []
+        ]
+        information_types = [
+            info_type.value for info_type in information_types or []
         ]
         metadata = {
             'bug_ids': bug_ids,
+            'information_types': information_types,
             'requestor.id': requestor.id
         }
         return super(RemoveBugSubscriptionsJob, cls).create(
@@ -433,6 +436,12 @@
     def bugs(self):
         return getUtility(IBugSet).getByNumbers(self.bug_ids)
 
+    @property
+    def information_types(self):
+        return [
+            enumerated_type_registry[InformationType.name].items[value]
+            for value in self.metadata['information_types']]
+
     def getErrorRecipients(self):
         # If something goes wrong we want to let the requestor know as well
         # as the pillar maintainer (if there is a pillar).
@@ -455,35 +464,33 @@
 
         # Find all bug subscriptions for which the subscriber cannot see the
         # bug.
-        constraints = [
-            BugTaskFlat.bug_id.is_in(self.bug_ids),
-            Not(Coalesce(
-                ArrayIntersects(SQL('BugTaskFlat.access_grants'),
-                Select(
-                    ArrayAgg(TeamParticipation.teamID),
-                    tables=TeamParticipation,
-                    where=(TeamParticipation.personID ==
-                           BugSubscription.person_id)
-                )), False)),
-            Not(Coalesce(
-                ArrayIntersects(SQL('BugTaskFlat.access_policies'),
-                Select(
-                    ArrayAgg(AccessPolicyGrant.policy_id),
-                    tables=(AccessPolicyGrant,
-                            Join(TeamParticipation,
-                                TeamParticipation.teamID ==
-                                AccessPolicyGrant.grantee_id)),
-                    where=(
-                        TeamParticipation.personID ==
-                        BugSubscription.person_id)
-                )), False))
-        ]
-        subscriptions = IStore(BugSubscription).find(
+        filter_terms = get_bug_privacy_filter_terms(BugSubscription.person_id)
+        invisible_bug_expr = Not(Or(*filter_terms))
+
+        if self.information_types:
+            bug_filter = BugTaskFlat.information_type.is_in(
+                self.information_types)
+        elif self.bug_ids:
+            bug_filter = BugTaskFlat.bug_id.is_in(self.bug_ids)
+        else:
+            bug_filter = True
+
+        # Admins can see all bugs so we need to retain any subscriptions they
+        # have even if there is no explicit grant.
+        admins = With("admins", Select(
+            Alias(TeamParticipation.personID, 'admin_id'),
+            where=And(
+                TeamParticipation.teamID == Person.id,
+                Person.name == 'admins')))
+        subscriptions = IStore(BugSubscription).with_(admins).find(
             BugSubscription,
             In(BugSubscription.bug_id,
                 Select(
                     BugTaskFlat.bug_id,
-                    where=And(*constraints)))
+                    where=And(bug_filter, invisible_bug_expr))),
+            Not(BugSubscription.person_id.is_in(
+                Select(SQL("admin_id"), tables="admins")
+            ))
         )
         for sub in subscriptions:
             sub.bug.unsubscribe(

=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py	2012-06-14 02:33:22 +0000
+++ lib/lp/registry/model/teammembership.py	2012-06-14 02:33:22 +0000
@@ -43,7 +43,7 @@
     )
 from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.sharingjob import (
-    IRemoveGranteeSubscriptionsJobSource,
+    IRemoveBugSubscriptionsJobSource,
     )
 from lp.registry.interfaces.teammembership import (
     ACTIVE_STATES,
@@ -393,9 +393,7 @@
                 # 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)
-
+                getUtility(IRemoveBugSubscriptionsJobSource).create(user)
         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 02:33:22 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-06-14 02:33:22 +0000
@@ -40,7 +40,7 @@
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.registry.interfaces.sharingjob import (
-    IRemoveGranteeSubscriptionsJobSource,
+    IRemoveBugSubscriptionsJobSource,
     )
 from lp.registry.interfaces.sharingservice import ISharingService
 from lp.registry.model.person import Person
@@ -328,12 +328,12 @@
         if len(to_delete) > 0:
             accessartifact_grant_source = getUtility(
                 IAccessArtifactGrantSource)
-            accessartifact_grant_source.revokeByArtifact(to_delete)
+            accessartifact_grant_source.revokeByArtifact(to_delete, [sharee])
 
         # Create a job to remove subscriptions for artifacts the sharee can no
         # longer see.
-        getUtility(IRemoveGranteeSubscriptionsJobSource).create(
-            pillar, sharee, user, information_types=information_types)
+        getUtility(IRemoveBugSubscriptionsJobSource).create(
+            user, bugs=None, information_types=information_types)
 
     @available_with_permission('launchpad.Edit', 'pillar')
     def revokeAccessGrants(self, pillar, sharee, user, branches=None,
@@ -358,8 +358,10 @@
 
         # Create a job to remove subscriptions for artifacts the sharee can no
         # longer see.
-        getUtility(IRemoveGranteeSubscriptionsJobSource).create(
-            pillar, sharee, user, bugs=bugs, branches=branches)
+        if bugs:
+            getUtility(IRemoveBugSubscriptionsJobSource).create(user, bugs)
+        # XXX 2012-06-13 wallyworld bug=1012448
+        # Remove branch subscriptions when information type fully implemented.
 
     def ensureAccessGrants(self, sharee, user, branches=None, bugs=None,
                            ignore_permissions=False):

=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py	2012-06-14 02:33:22 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-06-14 02:33:22 +0000
@@ -26,10 +26,12 @@
     )
 from lp.registry.interfaces.accesspolicy import (
     IAccessArtifactGrantSource,
+    IAccessArtifactSource,
     IAccessPolicyGrantFlatSource,
     IAccessPolicyGrantSource,
     IAccessPolicySource,
     )
+from lp.registry.interfaces.person import TeamSubscriptionPolicy
 from lp.registry.services.sharingservice import SharingService
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.tests import block_on_job
@@ -55,7 +57,7 @@
 WRITE_FLAG = {
     'disclosure.enhanced_sharing.writable': 'true',
     'disclosure.enhanced_sharing_details.enabled': 'true',
-    'jobs.celery.enabled_classes': 'RemoveGranteeSubscriptionsJob'}
+    'jobs.celery.enabled_classes': 'RemoveBugSubscriptionsJob'}
 DETAILS_FLAG = {'disclosure.enhanced_sharing_details.enabled': 'true'}
 
 
@@ -541,12 +543,15 @@
         # Make some artifact grants for our sharee.
         artifact = self.factory.makeAccessArtifact()
         self.factory.makeAccessArtifactGrant(artifact, grantee)
+        # Make some access policy grants for another sharee.
+        another = self.factory.makePerson()
+        self.factory.makeAccessPolicyGrant(access_policies[0], another)
+        # Make some artifact grants for our yet another sharee.
+        yet_another = self.factory.makePerson()
+        self.factory.makeAccessArtifactGrant(artifact, yet_another)
         for access_policy in access_policies:
             self.factory.makeAccessPolicyArtifact(
                 artifact=artifact, policy=access_policy)
-        # Make some access policy grants for another sharee.
-        another = self.factory.makePerson()
-        self.factory.makeAccessPolicyGrant(access_policies[0], another)
         # Delete data for a specific information type.
         with FeatureFixture(WRITE_FLAG):
             self.service.deletePillarSharee(
@@ -563,10 +568,16 @@
             expected_data = [
                 (grantee, {policy: SharingPermission.ALL}, [])
                 for policy in expected_policies]
-        # Add the expected data for the other sharee.
+        # Add the expected data for the other sharees.
         another_person_data = (
             another, {access_policies[0]: SharingPermission.ALL}, [])
         expected_data.append(another_person_data)
+        policy_permissions = dict([(
+            policy, SharingPermission.SOME) for policy in access_policies])
+        yet_another_person_data = (
+            yet_another, policy_permissions,
+            [InformationType.USERDATA, InformationType.EMBARGOEDSECURITY])
+        expected_data.append(yet_another_person_data)
         self.assertContentEqual(
             expected_data, self.service.getPillarSharees(pillar))
 
@@ -768,15 +779,102 @@
             information_type=InformationType.USERDATA)
         self._assert_revokeAccessGrants(distro, [bug], None)
 
-    def test_revokeAccessGrantsBranches(self):
+    # 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:
+            artifacts.extend(bugs)
+        if branches:
+            artifacts.extend(branches)
+        policy = self.factory.makeAccessPolicy(pillar=pillar)
+
+        person_grantee = self.factory.makePerson()
+        team_owner = self.factory.makePerson()
+        team_grantee = self.factory.makeTeam(
+            owner=team_owner,
+            subscription_policy=TeamSubscriptionPolicy.RESTRICTED,
+            members=[person_grantee])
+
+        # Subscribe the team and person grantees to the artifacts.
+        for person in [team_grantee, person_grantee]:
+            for bug in bugs or []:
+                bug.subscribe(person, pillar.owner)
+                # XXX 2012-06-12 wallyworld bug=1002596
+                # No need to revoke AAG with triggers removed.
+                if person == person_grantee:
+                    accessartifact_source = getUtility(IAccessArtifactSource)
+                    getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+                        accessartifact_source.find([bug]), [person_grantee])
+            for branch in branches or []:
+                branch.subscribe(person,
+                    BranchSubscriptionNotificationLevel.NOEMAIL, None,
+                    CodeReviewNotificationLevel.NOEMAIL, pillar.owner)
+
+        # Check that grantees have expected access grants and subscriptions.
+        for person in [team_grantee, person_grantee]:
+            visible_bugs, visible_branches = self.service.getVisibleArtifacts(
+                person, branches, bugs)
+            self.assertContentEqual(bugs or [], visible_bugs)
+            self.assertContentEqual(branches or [], visible_branches)
+        for person in [team_grantee, person_grantee]:
+            for bug in bugs or []:
+                self.assertIn(person, bug.getDirectSubscribers())
+
+        with FeatureFixture(WRITE_FLAG):
+            self.service.revokeAccessGrants(
+                pillar, team_grantee, pillar.owner,
+                bugs=bugs, branches=branches)
+        with block_on_job(self):
+            transaction.commit()
+
+        # The grantees now have no access to anything.
+        apgfs = getUtility(IAccessPolicyGrantFlatSource)
+        permission_info = apgfs.findGranteePermissionsByPolicy(
+            [policy], [team_grantee, person_grantee])
+        self.assertEqual(0, permission_info.count())
+
+        # Check that the grantee's subscriptions have been removed.
+        # Branches will be done once they have the information_type attribute.
+        for person in [team_grantee, person_grantee]:
+            for bug in bugs or []:
+                self.assertNotIn(person, bug.getDirectSubscribers())
+            visible_bugs, visible_branches = self.service.getVisibleArtifacts(
+                person, branches, bugs)
+            self.assertContentEqual([], visible_bugs)
+            self.assertContentEqual([], visible_branches)
+
+    def test_revokeTeamAccessGrantsBugs(self):
         # Users with launchpad.Edit can delete all access for a sharee.
         owner = self.factory.makePerson()
-        product = self.factory.makeProduct(owner=owner)
+        distro = self.factory.makeDistribution(owner=owner)
         login_person(owner)
-        branch = self.factory.makeBranch(
-            product=product, owner=owner,
+        bug = self.factory.makeBug(
+            distribution=distro, owner=owner,
             information_type=InformationType.USERDATA)
-        self._assert_revokeAccessGrants(product, None, [branch])
+        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

=== modified file 'lib/lp/registry/tests/test_sharingjob.py'
--- lib/lp/registry/tests/test_sharingjob.py	2012-06-14 02:33:22 +0000
+++ lib/lp/registry/tests/test_sharingjob.py	2012-06-14 02:33:22 +0000
@@ -482,7 +482,7 @@
 
         def create_job(distro, bug, grantee, owner):
             job = getUtility(IRemoveBugSubscriptionsJobSource).create(
-                [bug], owner)
+                owner, [bug])
             with person_logged_in(owner):
                 bug.transitionToInformationType(
                             InformationType.EMBARGOEDSECURITY, owner)
@@ -516,7 +516,7 @@
         requestor = self.factory.makePerson()
         bug = self.factory.makeBug()
         job = getUtility(IRemoveBugSubscriptionsJobSource).create(
-            [bug], requestor)
+            requestor, [bug])
         naked_job = removeSecurityProxy(job)
         self.assertIsInstance(job, RemoveBugSubscriptionsJob)
         self.assertEqual(requestor.id, naked_job.requestor_id)
@@ -528,7 +528,7 @@
         product = self.factory.makeProduct()
         bug = self.factory.makeBug(product=product)
         job = getUtility(IRemoveBugSubscriptionsJobSource).create(
-            [bug], requestor)
+            requestor, [bug])
         expected_emails = [
             format_address_for_person(person)
             for person in (product.owner, requestor)]
@@ -581,7 +581,7 @@
         reconcile_access_for_artifact(
             bug, bug.information_type, bug.affected_pillars)
 
-        getUtility(IRemoveBugSubscriptionsJobSource).create([bug], owner)
+        getUtility(IRemoveBugSubscriptionsJobSource).create(owner, [bug])
         with block_on_job(self):
             transaction.commit()
 
@@ -612,3 +612,69 @@
             removeSecurityProxy(bug).default_bugtask.product = another_product
 
         self._assert_bug_change_unsubscribes(change_target)
+
+    def _make_subscribed_bug(self, grantee, product=None, distribution=None,
+                             information_type=InformationType.USERDATA):
+        owner = self.factory.makePerson()
+        bug = self.factory.makeBug(
+            owner=owner, product=product, distribution=distribution,
+            information_type=information_type)
+        with person_logged_in(owner):
+            bug.subscribe(grantee, owner)
+        # Subscribing grantee to bug creates an access grant so we need to
+        # revoke that for our test.
+        accessartifact_source = getUtility(IAccessArtifactSource)
+        accessartifact_grant_source = getUtility(IAccessArtifactGrantSource)
+        accessartifact_grant_source.revokeByArtifact(
+            accessartifact_source.find([bug]), [grantee])
+
+        return bug, owner
+
+    def test_unsubscribe_pillar_artifacts_specific_info_types(self):
+        # Only remove subscriptions for bugs of the specified info type.
+
+        person_grantee = self.factory.makePerson(name='grantee')
+
+        owner = self.factory.makePerson(name='pillarowner')
+        pillar = self.factory.makeProduct(owner=owner)
+
+        # Make bugs the person_grantee is subscribed to.
+        bug1, ignored = self._make_subscribed_bug(
+            person_grantee, product=pillar,
+            information_type=InformationType.USERDATA)
+
+        bug2, ignored = self._make_subscribed_bug(
+            person_grantee, product=pillar,
+            information_type=InformationType.EMBARGOEDSECURITY)
+
+        # Now run the job, removing access to userdata artifacts.
+        getUtility(IRemoveBugSubscriptionsJobSource).create(
+            pillar.owner, information_types=[InformationType.USERDATA])
+        with block_on_job(self):
+            transaction.commit()
+
+        self.assertNotIn(
+            person_grantee, removeSecurityProxy(bug1).getDirectSubscribers())
+        self.assertIn(
+            person_grantee, removeSecurityProxy(bug2).getDirectSubscribers())
+
+    def test_admins_retain_subscriptions(self):
+        # Admins subscriptions are retained even if they don't have explicit
+        # access.
+        product = self.factory.makeProduct()
+        owner = self.factory.makePerson()
+        admin = getUtility(ILaunchpadCelebrities).admin.teamowner
+
+        login_person(owner)
+        bug = self.factory.makeBug(
+            owner=owner, product=product,
+            information_type=InformationType.USERDATA)
+
+        bug.subscribe(admin, owner)
+        getUtility(IRemoveBugSubscriptionsJobSource).create(owner, [bug])
+        with block_on_job(self):
+            transaction.commit()
+
+        # Check the result. admin should still be subscribed.
+        subscribers = removeSecurityProxy(bug).getDirectSubscribers()
+        self.assertIn(admin, subscribers)

=== modified file 'lib/lp/registry/tests/test_teammembership.py'
--- lib/lp/registry/tests/test_teammembership.py	2012-06-14 02:33:22 +0000
+++ lib/lp/registry/tests/test_teammembership.py	2012-06-14 02:33:22 +0000
@@ -999,8 +999,7 @@
     def setUp(self):
         self.useFixture(FeatureFixture({
             'disclosure.unsubscribe_jobs.enabled': 'true',
-            'jobs.celery.enabled_classes':
-                'RemoveGranteeSubscriptionsJob',
+            'jobs.celery.enabled_classes': 'RemoveBugSubscriptionsJob',
         }))
         super(TestTeamMembershipJobs, self).setUp()
 


Follow ups