Merge lp:~stevenk/launchpad/rbsj-generalise into lp:launchpad

[Steve Kowalik]

Rename RemoveBugSubscriptionsJob to RemoveArtifactSubscriptionsJob in preparation for it also dealing with branches as well as bugs.

RemoveBugSubscriptionsJob has existed for a little while to remove subscriptions from bugs that the person can not see, due to things like the bug changing information type, or the person being dropped from a team. This is excellent, but it should be extended to also work with branches.

To that end, I have renamed it and all related pieces to RemoveArtifactSubscriptionsJob. Just to make the reviewers life easier, this branch only deals with the rename, extending the job to work with branches will be the focus of another branch.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-06-19 04:38:35 +0000
+++ lib/lp/bugs/model/bug.py	2012-07-02 23:43:24 +0000
@@ -178,7 +178,9 @@
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.interfaces.series import SeriesStatus
-from lp.registry.interfaces.sharingjob import IRemoveBugSubscriptionsJobSource
+from lp.registry.interfaces.sharingjob import (
+    IRemoveArtifactSubscriptionsJobSource,
+    )
 from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.model.accesspolicy import reconcile_access_for_artifact
 from lp.registry.model.person import (
@@ -1831,7 +1833,8 @@
             # 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(who, [self])
+            getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+                who, [self])
 
         return True
 

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2012-06-14 07:43:06 +0000
+++ lib/lp/bugs/model/bugtask.py	2012-07-02 23:43:24 +0000
@@ -119,7 +119,9 @@
 from lp.registry.interfaces.productseries import IProductSeries
 from lp.registry.interfaces.projectgroup import IProjectGroup
 from lp.registry.interfaces.role import IPersonRoles
-from lp.registry.interfaces.sharingjob import IRemoveBugSubscriptionsJobSource
+from lp.registry.interfaces.sharingjob import (
+    IRemoveArtifactSubscriptionsJobSource,
+    )
 from lp.registry.interfaces.sourcepackage import ISourcePackage
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.registry.model.pillar import pillar_sort_key
@@ -1192,7 +1194,7 @@
             # 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(
+            getUtility(IRemoveArtifactSubscriptionsJobSource).create(
                 user, [self.bug], pillar=target_before_change)
 
     def updateTargetNameCache(self, newtarget=None):

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2012-06-19 02:14:21 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2012-07-02 23:43:24 +0000
@@ -3334,15 +3334,15 @@
 
 
 class TestTransitionsRemovesSubscribersJob(TestCaseWithFactory):
-    """Test that various bug transitions invoke RemoveBugSubscribers job."""
+    """Test that various bug transitions invoke RemoveArtifactSubscribers
+    job."""
 
     layer = CeleryJobLayer
 
     def setUp(self):
         self.useFixture(FeatureFixture({
             'disclosure.unsubscribe_jobs.enabled': 'true',
-            'jobs.celery.enabled_classes':
-                'RemoveBugSubscriptionsJob',
+            'jobs.celery.enabled_classes': 'RemoveArtifactSubscriptionsJob',
         }))
         self.useFixture(disable_trigger_fixture())
         super(TestTransitionsRemovesSubscribersJob, self).setUp()

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-06-15 00:42:38 +0000
+++ lib/lp/registry/configure.zcml	2012-07-02 23:43:24 +0000
@@ -2016,17 +2016,17 @@
     </securedutility>
 
     <!-- Sharing jobs -->
-    <class class=".model.sharingjob.RemoveBugSubscriptionsJob">
-      <allow interface=".interfaces.sharingjob.IRemoveBugSubscriptionsJob"/>
+    <class class=".model.sharingjob.RemoveArtifactSubscriptionsJob">
+      <allow interface=".interfaces.sharingjob.IRemoveArtifactSubscriptionsJob"/>
       <allow attributes="
          context
          log_name"/>
     </class>
 
     <securedutility
-        component=".model.sharingjob.RemoveBugSubscriptionsJob"
-        provides=".interfaces.sharingjob.IRemoveBugSubscriptionsJobSource">
-      <allow interface=".interfaces.sharingjob.IRemoveBugSubscriptionsJobSource"/>
+        component=".model.sharingjob.RemoveArtifactSubscriptionsJob"
+        provides=".interfaces.sharingjob.IRemoveArtifactSubscriptionsJobSource">
+      <allow interface=".interfaces.sharingjob.IRemoveArtifactSubscriptionsJobSource"/>
     </securedutility>
 
 </configure>

=== modified file 'lib/lp/registry/interfaces/sharingjob.py'
--- lib/lp/registry/interfaces/sharingjob.py	2012-06-15 01:47:33 +0000
+++ lib/lp/registry/interfaces/sharingjob.py	2012-07-02 23:43:24 +0000
@@ -6,8 +6,8 @@
 __metaclass__ = type
 
 __all__ = [
-    'IRemoveBugSubscriptionsJob',
-    'IRemoveBugSubscriptionsJobSource',
+    'IRemoveArtifactSubscriptionsJob',
+    'IRemoveArtifactSubscriptionsJobSource',
     'ISharingJob',
     'ISharingJobSource',
     ]
@@ -66,7 +66,7 @@
         """The person who initiated the job."""
 
 
-class IRemoveBugSubscriptionsJob(ISharingJob):
+class IRemoveArtifactSubscriptionsJob(ISharingJob):
     """Job to remove subscriptions to artifacts for which access is revoked.
 
     Invalid subscriptions for a specific bug are removed.
@@ -80,13 +80,14 @@
         """Create a new ISharingJob."""
 
 
-class IRemoveBugSubscriptionsJobSource(ISharingJobSource):
-    """An interface for acquiring IRemoveBugSubscriptionsJobs."""
+class IRemoveArtifactSubscriptionsJobSource(ISharingJobSource):
+    """An interface for acquiring IRemoveArtifactSubscriptionsJobs."""
 
     def create(requestor, bugs=None, grantee=None, pillar=None,
                information_types=None):
-        """Create a new job to remove subscriptions for the specified bugs.
+        """Create a new job to remove subscriptions for the specified
+        artifacts.
 
-        Subscriptions for users who no longer have access to the bugs are
-        removed.
+        Subscriptions for users who no longer have access to the artifacts
+        are removed.
         """

=== modified file 'lib/lp/registry/model/sharingjob.py'
--- lib/lp/registry/model/sharingjob.py	2012-06-28 01:14:33 +0000
+++ lib/lp/registry/model/sharingjob.py	2012-07-02 23:43:24 +0000
@@ -8,7 +8,7 @@
 
 
 __all__ = [
-    'RemoveBugSubscriptionsJob',
+    'RemoveArtifactSubscriptionsJob',
     ]
 
 import contextlib
@@ -49,8 +49,8 @@
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.sharingjob import (
-    IRemoveBugSubscriptionsJob,
-    IRemoveBugSubscriptionsJobSource,
+    IRemoveArtifactSubscriptionsJob,
+    IRemoveArtifactSubscriptionsJobSource,
     ISharingJob,
     ISharingJobSource,
     )
@@ -82,17 +82,18 @@
         grant (either direct or indirect via team membership).
         """)
 
-    REMOVE_BUG_SUBSCRIPTIONS = DBItem(1, """
-        Remove subscriptions for users who can no longer access bugs.
+    REMOVE_ARTIFACT_SUBSCRIPTIONS = DBItem(1, """
+        Remove subscriptions for users who can no longer access artifacts.
 
-        This job removes subscriptions to a bug when access is
-        no longer possible because the subscriber no longer has an access
-        grant (either direct or indirect via team membership).
+        This job removes subscriptions to an artifact (such as a bug or
+        branch) when access is no longer possible because the subscriber
+        no longer has an access grant (either direct or indirect via team
+        membership).
         """)
 
 
 class SharingJob(StormBase):
-    """Base class for jobs related to branch merge proposals."""
+    """Base class for jobs related to sharing."""
 
     implements(ISharingJob)
 
@@ -238,21 +239,21 @@
         return vars
 
 
-class RemoveBugSubscriptionsJob(SharingJobDerived):
-    """See `IRemoveBugSubscriptionsJob`."""
-
-    implements(IRemoveBugSubscriptionsJob)
-    classProvides(IRemoveBugSubscriptionsJobSource)
-    class_job_type = SharingJobType.REMOVE_BUG_SUBSCRIPTIONS
-
-    config = config.IRemoveBugSubscriptionsJobSource
+class RemoveArtifactSubscriptionsJob(SharingJobDerived):
+    """See `IRemoveArtifactSubscriptionsJob`."""
+
+    implements(IRemoveArtifactSubscriptionsJob)
+    classProvides(IRemoveArtifactSubscriptionsJobSource)
+    class_job_type = SharingJobType.REMOVE_ARTIFACT_SUBSCRIPTIONS
+
+    config = config.IRemoveArtifactSubscriptionsJobSource
 
     @classmethod
-    def create(cls, requestor, bugs=None, grantee=None, pillar=None,
+    def create(cls, requestor, artifacts=None, grantee=None, pillar=None,
                information_types=None):
-        """See `IRemoveBugSubscriptionsJob`."""
+        """See `IRemoveArtifactSubscriptionsJob`."""
 
-        bug_ids = [bug.id for bug in bugs or []]
+        bug_ids = [bug.id for bug in artifacts or []]
         information_types = [
             info_type.value for info_type in information_types or []
         ]
@@ -261,7 +262,7 @@
             'information_types': information_types,
             'requestor.id': requestor.id
         }
-        return super(RemoveBugSubscriptionsJob, cls).create(
+        return super(RemoveArtifactSubscriptionsJob, cls).create(
             pillar, grantee, metadata)
 
     @property
@@ -314,8 +315,7 @@
                 '%s=%s' % (k, v) for (k, v) in sorted(info.items()) if v))
 
     def run(self):
-        """See `IRemoveBugSubscriptionsJob`."""
-
+        """See `IRemoveArtifactSubscriptionsJob`."""
         logger = logging.getLogger()
         logger.info(self.getOperationDescription())
 

=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py	2012-06-28 01:14:33 +0000
+++ lib/lp/registry/model/teammembership.py	2012-07-02 23:43:24 +0000
@@ -42,7 +42,9 @@
     IMembershipNotificationJobSource,
     )
 from lp.registry.interfaces.role import IPersonRoles
-from lp.registry.interfaces.sharingjob import IRemoveBugSubscriptionsJobSource
+from lp.registry.interfaces.sharingjob import (
+    IRemoveArtifactSubscriptionsJobSource,
+    )
 from lp.registry.interfaces.teammembership import (
     ACTIVE_STATES,
     CyclicalTeamMembershipError,
@@ -391,7 +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(IRemoveBugSubscriptionsJobSource).create(
+                getUtility(IRemoveArtifactSubscriptionsJobSource).create(
                     user, grantee=self.person)
         else:
             # Changed from an inactive state to another inactive one, so no

=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py	2012-06-28 01:14:33 +0000
+++ lib/lp/registry/services/sharingservice.py	2012-07-02 23:43:24 +0000
@@ -44,7 +44,9 @@
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.projectgroup import IProjectGroup
-from lp.registry.interfaces.sharingjob import IRemoveBugSubscriptionsJobSource
+from lp.registry.interfaces.sharingjob import (
+    IRemoveArtifactSubscriptionsJobSource,
+    )
 from lp.registry.interfaces.sharingservice import ISharingService
 from lp.registry.model.accesspolicy import (
     AccessArtifactGrant,
@@ -383,8 +385,8 @@
 
         # Create a job to remove subscriptions for artifacts the sharee can no
         # longer see.
-        getUtility(IRemoveBugSubscriptionsJobSource).create(
-            user, bugs=None, grantee=sharee, pillar=pillar,
+        getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+            user, artifacts=None, grantee=sharee, pillar=pillar,
             information_types=information_types)
 
     @available_with_permission('launchpad.Edit', 'pillar')
@@ -411,7 +413,7 @@
         # Create a job to remove subscriptions for artifacts the sharee can no
         # longer see.
         if bugs:
-            getUtility(IRemoveBugSubscriptionsJobSource).create(
+            getUtility(IRemoveArtifactSubscriptionsJobSource).create(
                 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-07-02 17:22:30 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py	2012-07-02 23:43:24 +0000
@@ -57,7 +57,7 @@
 WRITE_FLAG = {
     'disclosure.enhanced_sharing.writable': 'true',
     'disclosure.enhanced_sharing_details.enabled': 'true',
-    'jobs.celery.enabled_classes': 'RemoveBugSubscriptionsJob'}
+    'jobs.celery.enabled_classes': 'RemoveArtifactSubscriptionsJob'}
 DETAILS_FLAG = {'disclosure.enhanced_sharing_details.enabled': 'true'}
 
 

=== modified file 'lib/lp/registry/tests/test_sharingjob.py'
--- lib/lp/registry/tests/test_sharingjob.py	2012-06-19 04:38:35 +0000
+++ lib/lp/registry/tests/test_sharingjob.py	2012-07-02 23:43:24 +0000
@@ -12,6 +12,10 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.code.enums import (
+    BranchSubscriptionNotificationLevel,
+    CodeReviewNotificationLevel,
+    )
 from lp.registry.enums import InformationType
 from lp.registry.interfaces.accesspolicy import (
     IAccessArtifactGrantSource,
@@ -20,13 +24,13 @@
     )
 from lp.registry.interfaces.person import TeamSubscriptionPolicy
 from lp.registry.interfaces.sharingjob import (
-    IRemoveBugSubscriptionsJobSource,
+    IRemoveArtifactSubscriptionsJobSource,
     ISharingJob,
     ISharingJobSource,
     )
 from lp.registry.model.accesspolicy import reconcile_access_for_artifact
 from lp.registry.model.sharingjob import (
-    RemoveBugSubscriptionsJob,
+    RemoveArtifactSubscriptionsJob,
     SharingJob,
     SharingJobDerived,
     SharingJobType,
@@ -93,15 +97,15 @@
     def _makeJob(self):
         self.bug = self.factory.makeBug()
         self.requestor = self.factory.makePerson()
-        job = getUtility(IRemoveBugSubscriptionsJobSource).create(
-            self.requestor, bugs=[self.bug])
+        job = getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+            self.requestor, artifacts=[self.bug])
         return job
 
     def test_repr(self):
         job = self._makeJob()
         self.assertEqual(
-            '<REMOVE_BUG_SUBSCRIPTIONS job reconciling subscriptions for '
-            'bug_ids=[%d], requestor=%s>'
+            '<REMOVE_ARTIFACT_SUBSCRIPTIONS job reconciling subscriptions '
+            'for bug_ids=[%d], requestor=%s>'
             % (self.bug.id, self.requestor.name),
             repr(job))
 
@@ -126,27 +130,27 @@
         job_1 = self._makeJob()
         job_2 = self._makeJob()
         job_2.start()
-        jobs = list(RemoveBugSubscriptionsJob.iterReady())
+        jobs = list(RemoveArtifactSubscriptionsJob.iterReady())
         self.assertEqual(1, len(jobs))
         self.assertEqual(job_1, jobs[0])
 
     def test_log_name(self):
         # The log_name is the name of the implementing class.
         job = self._makeJob()
-        self.assertEqual('RemoveBugSubscriptionsJob', job.log_name)
+        self.assertEqual('RemoveArtifactSubscriptionsJob', job.log_name)
 
     def test_getOopsVars(self):
         # The pillar and grantee name are added to the oops vars.
         bug = self.factory.makeBug()
         requestor = self.factory.makePerson()
-        job = getUtility(IRemoveBugSubscriptionsJobSource).create(
-            requestor, bugs=[bug])
+        job = getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+            requestor, artifacts=[bug])
         oops_vars = job.getOopsVars()
         self.assertIs(True, len(oops_vars) >= 3)
         self.assertIn(
             ('sharing_job_type',
-            'Remove subscriptions for users who can no longer access bugs.'),
-            oops_vars)
+            'Remove subscriptions for users who can no longer access '
+            'artifacts.'), oops_vars)
 
 
 def disable_trigger_fixture():
@@ -208,44 +212,43 @@
         # removed.
 
         def create_job(distro, bug, grantee, owner):
-            job = getUtility(IRemoveBugSubscriptionsJobSource).create(
+            job = getUtility(IRemoveArtifactSubscriptionsJobSource).create(
                 owner, [bug])
             with person_logged_in(owner):
                 bug.transitionToInformationType(
                             InformationType.EMBARGOEDSECURITY, owner)
-            return job, IRemoveBugSubscriptionsJobSource.getName()
+            return job, IRemoveArtifactSubscriptionsJobSource.getName()
 
         self._assert_run_cronscript(create_job)
 
 
-class RemoveBugSubscriptionsJobTestCase(TestCaseWithFactory):
-    """Test case for the RemoveBugSubscriptionsJob class."""
+class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
+    """Test case for the RemoveArtifactSubscriptionsJob class."""
 
     layer = CeleryJobLayer
 
     def setUp(self):
         self.useFixture(FeatureFixture({
-            'jobs.celery.enabled_classes':
-                'RemoveBugSubscriptionsJob',
+            'jobs.celery.enabled_classes': 'RemoveArtifactSubscriptionsJob',
         }))
         self.useFixture(disable_trigger_fixture())
-        super(RemoveBugSubscriptionsJobTestCase, self).setUp()
+        super(RemoveArtifactSubscriptionsJobTestCase, self).setUp()
 
     def test_create(self):
-        # Create an instance of RemoveBugSubscriptionsJob.
+        # Create an instance of RemoveArtifactSubscriptionsJob.
         self.assertIs(
             True,
-            IRemoveBugSubscriptionsJobSource.providedBy(
-                RemoveBugSubscriptionsJob))
+            IRemoveArtifactSubscriptionsJobSource.providedBy(
+                RemoveArtifactSubscriptionsJob))
         self.assertEqual(
-            SharingJobType.REMOVE_BUG_SUBSCRIPTIONS,
-            RemoveBugSubscriptionsJob.class_job_type)
+            SharingJobType.REMOVE_ARTIFACT_SUBSCRIPTIONS,
+            RemoveArtifactSubscriptionsJob.class_job_type)
         requestor = self.factory.makePerson()
         bug = self.factory.makeBug()
-        job = getUtility(IRemoveBugSubscriptionsJobSource).create(
+        job = getUtility(IRemoveArtifactSubscriptionsJobSource).create(
             requestor, [bug])
         naked_job = removeSecurityProxy(job)
-        self.assertIsInstance(job, RemoveBugSubscriptionsJob)
+        self.assertIsInstance(job, RemoveArtifactSubscriptionsJob)
         self.assertEqual(requestor.id, naked_job.requestor_id)
         self.assertContentEqual([bug.id], naked_job.bug_ids)
 
@@ -254,7 +257,7 @@
         requestor = self.factory.makePerson()
         product = self.factory.makeProduct()
         bug = self.factory.makeBug(product=product)
-        job = getUtility(IRemoveBugSubscriptionsJobSource).create(
+        job = getUtility(IRemoveArtifactSubscriptionsJobSource).create(
             requestor, [bug], pillar=product)
         expected_emails = [
             format_address_for_person(person)
@@ -302,13 +305,14 @@
         self.assertIn(policy_team_grantee, subscribers)
         self.assertIn(policy_indirect_grantee, subscribers)
 
-        # Change bug bug attributes so that it can become inaccessible for
+        # Change bug attributes so that it can become inaccessible for
         # some users.
         change_callback(bug)
         reconcile_access_for_artifact(
             bug, bug.information_type, bug.affected_pillars)
 
-        getUtility(IRemoveBugSubscriptionsJobSource).create(owner, [bug])
+        getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+            owner, [bug])
         with block_on_job(self):
             transaction.commit()
 
@@ -375,7 +379,7 @@
             information_type=InformationType.EMBARGOEDSECURITY)
 
         # Now run the job, removing access to userdata artifacts.
-        getUtility(IRemoveBugSubscriptionsJobSource).create(
+        getUtility(IRemoveArtifactSubscriptionsJobSource).create(
             pillar.owner, pillar=pillar,
             information_types=[InformationType.USERDATA])
         with block_on_job(self):
@@ -399,7 +403,7 @@
             information_type=InformationType.USERDATA)
 
         bug.subscribe(admin, owner)
-        getUtility(IRemoveBugSubscriptionsJobSource).create(
+        getUtility(IRemoveArtifactSubscriptionsJobSource).create(
             owner, [bug], pillar=product)
         with block_on_job(self):
             transaction.commit()

=== modified file 'lib/lp/registry/tests/test_teammembership.py'
--- lib/lp/registry/tests/test_teammembership.py	2012-06-15 00:42:38 +0000
+++ lib/lp/registry/tests/test_teammembership.py	2012-07-02 23:43:24 +0000
@@ -999,7 +999,7 @@
     def setUp(self):
         self.useFixture(FeatureFixture({
             'disclosure.unsubscribe_jobs.enabled': 'true',
-            'jobs.celery.enabled_classes': 'RemoveBugSubscriptionsJob',
+            'jobs.celery.enabled_classes': 'RemoveArtifactSubscriptionsJob',
         }))
         super(TestTeamMembershipJobs, self).setUp()
 

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2012-06-15 00:42:38 +0000
+++ lib/lp/services/config/schema-lazr.conf	2012-07-02 23:43:24 +0000
@@ -1754,7 +1754,7 @@
     IPersonMergeJobSource,
     IPlainPackageCopyJobSource,
     IQuestionEmailJobSource,
-    IRemoveBugSubscriptionsJobSource,
+    IRemoveArtifactSubscriptionsJobSource,
     ISevenDayCommercialExpirationJobSource,
     IThirtyDayCommercialExpirationJobSource
 
@@ -1787,7 +1787,7 @@
 dbuser: answertracker
 crontab_group: MAIN
 
-[IRemoveBugSubscriptionsJobSource]
+[IRemoveArtifactSubscriptionsJobSource]
 # This section is used by cronscripts/process-job-source.py.
 module: lp.registry.interfaces.sharingjob
 dbuser: sharing-jobs