launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09379
[Merge] lp:~stevenk/launchpad/rbsj-generalise into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/rbsj-generalise into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/rbsj-generalise/+merge/112684
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.
--
https://code.launchpad.net/~stevenk/launchpad/rbsj-generalise/+merge/112684
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/rbsj-generalise into lp:launchpad.
=== 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-06-29 02:25:25 +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-06-29 02:25:25 +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-06-29 02:25:25 +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-06-29 02:25:25 +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-06-29 02:25:25 +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-06-29 02:25:25 +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-06-29 02:25:25 +0000
@@ -42,7 +42,13 @@
IMembershipNotificationJobSource,
)
from lp.registry.interfaces.role import IPersonRoles
+<<<<<<< TREE
from lp.registry.interfaces.sharingjob import IRemoveBugSubscriptionsJobSource
+=======
+from lp.registry.interfaces.sharingjob import (
+ IRemoveArtifactSubscriptionsJobSource,
+ )
+>>>>>>> MERGE-SOURCE
from lp.registry.interfaces.teammembership import (
ACTIVE_STATES,
CyclicalTeamMembershipError,
@@ -391,7 +397,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-06-29 02:25:25 +0000
@@ -44,7 +44,13 @@
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.projectgroup import IProjectGroup
+<<<<<<< TREE
from lp.registry.interfaces.sharingjob import IRemoveBugSubscriptionsJobSource
+=======
+from lp.registry.interfaces.sharingjob import (
+ IRemoveArtifactSubscriptionsJobSource,
+ )
+>>>>>>> MERGE-SOURCE
from lp.registry.interfaces.sharingservice import ISharingService
from lp.registry.model.accesspolicy import (
AccessArtifactGrant,
@@ -383,7 +389,7 @@
# Create a job to remove subscriptions for artifacts the sharee can no
# longer see.
- getUtility(IRemoveBugSubscriptionsJobSource).create(
+ getUtility(IRemoveArtifactSubscriptionsJobSource).create(
user, bugs=None, grantee=sharee, pillar=pillar,
information_types=information_types)
@@ -411,7 +417,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-06-19 03:01:24 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-06-29 02:25:25 +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-06-29 02:25:25 +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-06-29 02:25:25 +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-06-29 02:25:25 +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
Follow ups