launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08096
lp:~wallyworld/launchpad/bug-unsubscribe-pillar-infotype-change into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/bug-unsubscribe-pillar-infotype-change into lp:launchpad with lp:~wallyworld/launchpad/subscribe-grants-access-1000045 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1002836 in Launchpad itself: "Remove subscriptions for users who cannot see a bug after it changes"
https://bugs.launchpad.net/launchpad/+bug/1002836
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/bug-unsubscribe-pillar-infotype-change/+merge/106792
== Implementation ==
The core work in this mp is relatively small, but the ancillary changes to tests blow out the size a bit sadly.
This branch adds a new sharing job - RemoveBugSubscriptionsJob
The job is run when a bug's information type is changed, or when a bugtask is re-targetted. When either of these things happens, the bug may become inaccessible to some of its subscribers. These subscribers need to be unsubscribed. This is what the job does.
The bug transitionToInformationType() and bugtask transitionToTarget() methods have been updated to create a RemoveBugSubscriptionsJob. The job needs to be initialised with the user who initiated the action. The transitionToTarget() method did not have this passed in so the method needed to be extended, and hence a lot of little clean up needed to be done to the callsites, expecially doc tests and unit tests.
The job is only really useful once the legacy mirroring triggers are removed. This together with previous work to grant access to subscribers and revoke access from people who are unsubscribed forms the basis of the work allowing the triggers to be removed.
== Tests ==
Add tests for the job itself: RemoveBugSubscriptionsJobTestCase
Add tests to check that when bug transitionToInformationType() and bugtask transitionToTarget() are invoked the job is created and runs.
Update various tests (doc and unit) to account for the transitionToTarget() API change.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
database/schema/security.cfg
lib/lp/bugs/browser/bugtask.py
lib/lp/bugs/browser/tests/bugtask-edit-views.txt
lib/lp/bugs/browser/tests/test_bugtask.py
lib/lp/bugs/doc/bug-nomination.txt
lib/lp/bugs/doc/bug-set-status.txt
lib/lp/bugs/doc/bugmail-headers.txt
lib/lp/bugs/doc/bugtask-expiration.txt
lib/lp/bugs/doc/initial-bug-contacts.txt
lib/lp/bugs/doc/security-teams.txt
lib/lp/bugs/interfaces/bugtask.py
lib/lp/bugs/mail/commands.py
lib/lp/bugs/model/bug.py
lib/lp/bugs/model/bugtask.py
lib/lp/bugs/model/tests/test_bugtask.py
lib/lp/bugs/stories/standalone/xx-show-distrorelease-cve-report.txt
lib/lp/bugs/tests/test_bugchanges.py
lib/lp/bugs/tests/test_bugvisibility.py
lib/lp/registry/configure.zcml
lib/lp/registry/browser/tests/test_milestone.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
lib/lp/services/webapp/doc/canonical_url_examples.txt
Just some doc test lint
--
https://code.launchpad.net/~wallyworld/launchpad/bug-unsubscribe-pillar-infotype-change/+merge/106792
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/bug-unsubscribe-pillar-infotype-change into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2012-05-22 12:23:20 +0000
+++ database/schema/security.cfg 2012-05-22 12:23:21 +0000
@@ -1885,6 +1885,7 @@
public.accesspolicy = SELECT
public.accesspolicygrant = SELECT, UPDATE, DELETE
public.accesspolicygrantflat = SELECT
+public.account = SELECT
public.branch = SELECT
public.branchsubscription = SELECT, UPDATE, DELETE
public.bug = SELECT, UPDATE
@@ -1893,6 +1894,7 @@
public.bugsubscription = SELECT, UPDATE, DELETE
public.bugtask = SELECT
public.bugtaskflat = SELECT
+public.bugwatch = SELECT
public.distribution = SELECT
public.emailaddress = SELECT
public.job = SELECT, INSERT, UPDATE
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2012-05-18 05:57:41 +0000
+++ lib/lp/bugs/browser/bugtask.py 2012-05-22 12:23:21 +0000
@@ -1609,7 +1609,7 @@
# from the form.
if new_target is not missing and bugtask.target != new_target:
changed = True
- bugtask.transitionToTarget(new_target)
+ bugtask.transitionToTarget(new_target, self.user)
# Now that we've updated the bugtask we can add messages about
# milestone changes, if there were any.
=== modified file 'lib/lp/bugs/browser/tests/bugtask-edit-views.txt'
--- lib/lp/bugs/browser/tests/bugtask-edit-views.txt 2012-02-21 23:08:55 +0000
+++ lib/lp/bugs/browser/tests/bugtask-edit-views.txt 2012-05-22 12:23:21 +0000
@@ -70,7 +70,8 @@
... distroseries=
... ubuntu_thunderbird_task.target.distribution.currentseries,
... sourcepackagename='thunderbird')
- >>> ubuntu_thunderbird_task.transitionToTarget(ubuntu_thunderbird)
+ >>> ubuntu_thunderbird_task.transitionToTarget(
+ ... ubuntu_thunderbird, getUtility(ILaunchBag).user)
If we try to change the source package to package name that doesn't
exist in Launchpad. we'll get an error message.
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2012-05-10 22:48:06 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2012-05-22 12:23:21 +0000
@@ -746,7 +746,7 @@
bug = self.factory.makeBug(series=series)
self.assertEqual(2, len(bug.bugtasks))
new_prod = self.factory.makeProduct()
- bug.getBugTask(series.product).transitionToTarget(new_prod)
+ bug.getBugTask(series.product).transitionToTarget(new_prod, bug.owner)
view = create_initialized_view(bug, "+bugtasks-and-nominations-table")
subviews = view.getBugTaskAndNominationViews()
=== modified file 'lib/lp/bugs/doc/bug-nomination.txt'
--- lib/lp/bugs/doc/bug-nomination.txt 2012-04-10 14:01:17 +0000
+++ lib/lp/bugs/doc/bug-nomination.txt 2012-05-22 12:23:21 +0000
@@ -463,7 +463,8 @@
u'thunderbird (Ubuntu Grumpy)'
>>> thunderbird_grumpy.transitionToTarget(
- ... ubuntu.getSeries('grumpy').getSourcePackage('pmount'))
+ ... ubuntu.getSeries('grumpy').getSourcePackage('pmount'),
+ ... getUtility(ILaunchBag).user)
>>> tasks = sorted(
... bug_one.bugtasks, key=by_bugtargetdisplayname)
@@ -490,7 +491,8 @@
u'pmount (Ubuntu)'
>>> ubuntu_thunderbird = ubuntu.getSourcePackage('thunderbird')
- >>> pmount_ubuntu.transitionToTarget(ubuntu_thunderbird)
+ >>> pmount_ubuntu.transitionToTarget(
+ ... ubuntu_thunderbird, getUtility(ILaunchBag).user)
>>> tasks = sorted(
... bug_one.bugtasks, key=by_bugtargetdisplayname)
=== modified file 'lib/lp/bugs/doc/bug-set-status.txt'
--- lib/lp/bugs/doc/bug-set-status.txt 2011-08-01 05:25:59 +0000
+++ lib/lp/bugs/doc/bug-set-status.txt 2012-05-22 12:23:21 +0000
@@ -164,7 +164,11 @@
If the bug is targeted to a source package, that bugtask is of course
edited.
- >>> ubuntu_bugtask.transitionToTarget(ubuntu_firefox)
+ # Need to be privileged user to transition the target.
+ >>> from lp.services.webapp.interfaces import ILaunchBag
+ >>> login('foo.bar@xxxxxxxxxxxxx')
+ >>> ubuntu_bugtask.transitionToTarget(
+ ... ubuntu_firefox, getUtility(ILaunchBag).user)
>>> ubuntu_firefox_task = bug.setStatus(
... ubuntu_firefox, BugTaskStatus.INCOMPLETE, no_priv)
>>> ubuntu_firefox_task.target.displayname
=== modified file 'lib/lp/bugs/doc/bugmail-headers.txt'
--- lib/lp/bugs/doc/bugmail-headers.txt 2011-07-27 08:04:46 +0000
+++ lib/lp/bugs/doc/bugmail-headers.txt 2012-05-22 12:23:21 +0000
@@ -54,11 +54,14 @@
>>> login("foo.bar@xxxxxxxxxxxxx")
>>> from lp.registry.interfaces.distribution import IDistributionSet
+ >>> from lp.services.webapp.interfaces import ILaunchBag
>>> debian = getUtility(IDistributionSet)['debian']
>>> warty = getUtility(IDistributionSet)['ubuntu'].getSeries('warty')
>>> mozilla_firefox_packagename = debian_firefox_bugtask.sourcepackagename
- >>> debian_firefox_bugtask.transitionToTarget(debian)
- >>> warty_firefox_bugtask.transitionToTarget(warty)
+ >>> debian_firefox_bugtask.transitionToTarget(
+ ... debian, getUtility(ILaunchBag).user)
+ >>> warty_firefox_bugtask.transitionToTarget(
+ ... warty, getUtility(ILaunchBag).user)
>>> debian_firefox_bugtask.asEmailHeaderValue()
u'distribution=debian; sourcepackage=None; component=None;
=== modified file 'lib/lp/bugs/doc/bugtask-expiration.txt'
--- lib/lp/bugs/doc/bugtask-expiration.txt 2012-04-04 05:46:26 +0000
+++ lib/lp/bugs/doc/bugtask-expiration.txt 2012-05-22 12:23:21 +0000
@@ -156,7 +156,7 @@
>>> ubuntu_alsa = ubuntu.getSourcePackage('alsa-utils')
>>> another_assigned_bugtask = create_old_bug(
... 'assigned', 61, ubuntu, assignee=sample_person)
- >>> another_assigned_bugtask.transitionToTarget(ubuntu_alsa)
+ >>> another_assigned_bugtask.transitionToTarget(ubuntu_alsa, sample_person)
>>> ubuntu_evolution = ubuntu.getSourcePackage('evolution')
>>> invalid_bugtask = bugtaskset.createTask(
... another_assigned_bugtask.bug, sample_person, ubuntu_evolution,
=== modified file 'lib/lp/bugs/doc/initial-bug-contacts.txt'
--- lib/lp/bugs/doc/initial-bug-contacts.txt 2012-04-20 17:07:54 +0000
+++ lib/lp/bugs/doc/initial-bug-contacts.txt 2012-05-22 12:23:21 +0000
@@ -143,7 +143,7 @@
>>> old_state = Snapshot(
... bug_one_in_ubuntu_firefox, providing=IBugTask)
- >>> bug_one_in_ubuntu_firefox.transitionToTarget(ubuntu_pmount)
+ >>> bug_one_in_ubuntu_firefox.transitionToTarget(ubuntu_pmount, daf)
>>> source_package_changed = ObjectModifiedEvent(
... bug_one_in_ubuntu_firefox, old_state,
@@ -240,7 +240,7 @@
>>> old_state = Snapshot(
... bug_one_in_ubuntu_firefox, providing=IBugTask)
- >>> bug_one_in_ubuntu_firefox.transitionToTarget(ubuntu)
+ >>> bug_one_in_ubuntu_firefox.transitionToTarget(ubuntu, daf)
>>> source_package_changed = ObjectModifiedEvent(
... bug_one_in_ubuntu_firefox, old_state, ["sourcepackagename"])
@@ -273,7 +273,7 @@
>>> old_state = Snapshot(
... bug_one_in_ubuntu_firefox, providing=IBugTask)
- >>> bug_one_in_ubuntu_firefox.transitionToTarget(ubuntu_pmount)
+ >>> bug_one_in_ubuntu_firefox.transitionToTarget(ubuntu_pmount, daf)
>>> source_package_changed = ObjectModifiedEvent(
... bug_one_in_ubuntu_firefox, old_state, ["sourcepackagename"])
@@ -322,7 +322,7 @@
>>> old_state = Snapshot(bug_two_in_ubuntu, providing=IBugTask)
- >>> bug_two_in_ubuntu.transitionToTarget(mozilla_firefox)
+ >>> bug_two_in_ubuntu.transitionToTarget(mozilla_firefox, daf)
>>> product_changed = ObjectModifiedEvent(
... bug_two_in_ubuntu, old_state, ["id", "title", "product"])
=== modified file 'lib/lp/bugs/doc/security-teams.txt'
--- lib/lp/bugs/doc/security-teams.txt 2012-04-19 21:15:25 +0000
+++ lib/lp/bugs/doc/security-teams.txt 2012-05-22 12:23:21 +0000
@@ -228,7 +228,7 @@
>>> from lp.bugs.interfaces.bugtask import IBugTask
>>> old_state = Snapshot(bug_in_evolution, providing=IBugTask)
- >>> bug_in_evolution.transitionToTarget(thunderbird)
+ >>> bug_in_evolution.transitionToTarget(thunderbird, stub)
>>> bug_product_changed = ObjectModifiedEvent(
... bug_in_evolution, old_state, ["product"])
=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py 2012-05-16 02:09:30 +0000
+++ lib/lp/bugs/interfaces/bugtask.py 2012-05-22 12:23:21 +0000
@@ -830,10 +830,11 @@
"""
@mutator_for(target)
+ @call_with(user=REQUEST_USER)
@operation_parameters(
target=copy_field(target))
@export_write_operation()
- def transitionToTarget(target):
+ def transitionToTarget(target, user):
"""Convert the bug task to a different bug target."""
def updateTargetNameCache():
=== modified file 'lib/lp/bugs/mail/commands.py'
--- lib/lp/bugs/mail/commands.py 2012-05-04 00:03:07 +0000
+++ lib/lp/bugs/mail/commands.py 2012-05-22 12:23:21 +0000
@@ -616,7 +616,8 @@
if bugtask is not None:
bugtask_before_edit = Snapshot(
bugtask, providing=IBugTask)
- bugtask.transitionToTarget(bug_target)
+ bugtask.transitionToTarget(
+ bug_target, getUtility(ILaunchBag).user)
event = ObjectModifiedEvent(
bugtask, bugtask_before_edit, ['sourcepackagename'])
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2012-05-22 12:23:20 +0000
+++ lib/lp/bugs/model/bug.py 2012-05-22 12:23:21 +0000
@@ -4,7 +4,6 @@
# pylint: disable-msg=E0611,W0212
"""Launchpad bug-related database table classes."""
-from lp.app.interfaces.services import IService
__metaclass__ = type
@@ -89,6 +88,7 @@
UserCannotUnsubscribePerson,
)
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.interfaces.services import IService
from lp.app.validators import LaunchpadValidationError
from lp.bugs.adapters.bug import convert_to_information_type
from lp.bugs.adapters.bugchange import (
@@ -177,6 +177,8 @@
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.productseries import IProductSeries
from lp.registry.interfaces.role import IPersonRoles
+from lp.registry.interfaces.sharingjob import (
+ IRemoveBugSubscriptionsJobSource)
from lp.registry.interfaces.series import SeriesStatus
from lp.registry.interfaces.sourcepackage import ISourcePackage
from lp.registry.model.person import (
@@ -847,7 +849,9 @@
service = getUtility(IService, 'sharing')
bugs, ignored = service.getVisibleArtifacts(person, bugs=[self])
if not bugs:
- service.createAccessGrants(subscribed_by, person, bugs=[self])
+ service.createAccessGrants(
+ subscribed_by, person, bugs=[self],
+ ignore_permissions=True)
# In some cases, a subscription should be created without
# email notifications. suppress_notify determines if
@@ -1803,6 +1807,12 @@
self.information_type = information_type
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)
+
return True
def getRequiredSubscribers(self, information_type, who):
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2012-05-15 05:20:34 +0000
+++ lib/lp/bugs/model/bugtask.py 2012-05-22 12:23:21 +0000
@@ -41,7 +41,6 @@
import pytz
from sqlobject import (
ForeignKey,
- IntCol,
SQLObjectNotFound,
StringCol,
)
@@ -120,6 +119,7 @@
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.sourcepackage import ISourcePackage
from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
from lp.registry.model.pillar import pillar_sort_key
@@ -683,7 +683,7 @@
"""See `IBugTask`."""
return self.bug.isSubscribed(person)
- def _syncSourcePackages(self, new_spn):
+ def _syncSourcePackages(self, new_spn, user):
"""Synchronize changes to source packages with other distrotasks.
If one distroseriestask's source package is changed, all the
@@ -706,6 +706,7 @@
key['sourcepackagename'] = new_spn
bugtask.transitionToTarget(
bug_target_from_key(**key),
+ user,
_sync_sourcepackages=False)
def getContributorInfo(self, user, person):
@@ -1140,7 +1141,7 @@
validate_target(self.bug, target)
- def transitionToTarget(self, target, _sync_sourcepackages=True):
+ def transitionToTarget(self, target, user, _sync_sourcepackages=True):
"""See `IBugTask`.
If _sync_sourcepackages is True (the default) and the
@@ -1169,7 +1170,7 @@
# sourcepackagename. This keeps series tasks consistent.
if (_sync_sourcepackages and
new_key['sourcepackagename'] != self.sourcepackagename):
- self._syncSourcePackages(new_key['sourcepackagename'])
+ self._syncSourcePackages(new_key['sourcepackagename'], user)
for name, value in new_key.iteritems():
setattr(self, name, value)
@@ -1183,6 +1184,11 @@
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], user)
+
def updateTargetNameCache(self, newtarget=None):
"""See `IBugTask`."""
if newtarget is None:
=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py 2012-05-16 18:24:37 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py 2012-05-22 12:23:21 +0000
@@ -59,7 +59,6 @@
from lp.bugs.model.bugtasksearch import (
_build_status_clause,
_build_tag_search_clause,
- get_bug_privacy_filter,
)
from lp.bugs.scripts.bugtasktargetnamecaches import (
BugTaskTargetNameCacheUpdater)
@@ -72,6 +71,8 @@
from lp.registry.interfaces.accesspolicy import (
IAccessPolicyGrantSource,
IAccessPolicySource,
+ IAccessArtifactGrantSource,
+ IAccessArtifactSource,
)
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.distributionsourcepackage import (
@@ -93,6 +94,8 @@
flush_database_updates,
flush_database_caches,
)
+from lp.services.features.testing import FeatureFixture
+from lp.services.job.tests import block_on_job
from lp.services.log.logger import FakeLogger
from lp.services.searchbuilder import (
all,
@@ -124,8 +127,10 @@
)
from lp.testing.factory import LaunchpadObjectFactory
from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import DisableTriggerFixture
from lp.testing.layers import (
AppServerLayer,
+ CeleryJobLayer,
DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
)
@@ -715,7 +720,7 @@
bug_task, providing=providedBy(bug_task))
new_product = self.factory.makeProduct(owner=user)
- bug_task.transitionToTarget(new_product)
+ bug_task.transitionToTarget(new_product, user)
self.check_delta(bug_task_before_modification, bug_task,
target=dict(old=product, new=new_product))
@@ -2480,7 +2485,7 @@
sourcepackagename=source_package_name)
with person_logged_in(data.owner):
data.generic_task.transitionToTarget(
- data.distro.getSourcePackage(source_package_name))
+ data.distro.getSourcePackage(source_package_name), data.owner)
self.assertEqual(source_package_name,
data.series_task.sourcepackagename)
@@ -2852,7 +2857,7 @@
with person_logged_in(person):
bug_task.maybeConfirm()
self.assertEqual(BugTaskStatus.NEW, bug_task.status)
- bug_task.transitionToTarget(autoconfirm_product)
+ bug_task.transitionToTarget(autoconfirm_product, person)
self.assertEqual(BugTaskStatus.NEW, bug_task.status)
def test_transitionToTarget(self):
@@ -2871,7 +2876,7 @@
with person_logged_in(person):
bug_task.maybeConfirm()
self.assertEqual(BugTaskStatus.NEW, bug_task.status)
- bug_task.transitionToTarget(autoconfirm_product)
+ bug_task.transitionToTarget(autoconfirm_product, person)
self.assertEqual(BugTaskStatus.CONFIRMED, bug_task.status)
# END TEMPORARY BIT FOR BUGTASK AUTOCONFIRM FEATURE FLAG.
@@ -3121,7 +3126,7 @@
self.assertRaisesWithContent(
IllegalTarget,
"A fix for this bug has already been requested for %s"
- % p.displayname, task2.transitionToTarget, p)
+ % p.displayname, task2.transitionToTarget, p, task2.owner)
class TestTransitionToTarget(TestCaseWithFactory):
@@ -3136,7 +3141,7 @@
old_state = Snapshot(task, providing=providedBy(task))
with person_logged_in(task.owner):
task.bug.subscribe(p, p)
- task.transitionToTarget(new)
+ task.transitionToTarget(new, p)
notify(ObjectModifiedEvent(task, old_state, ["target"]))
return task
@@ -3169,7 +3174,8 @@
with person_logged_in(task.owner):
self.assertRaisesWithContent(
IllegalTarget, msg,
- task.transitionToTarget, self.factory.makeProduct())
+ task.transitionToTarget, self.factory.makeProduct(),
+ task.owner)
self.assertEqual(p, task.target)
def test_transition_to_same_is_noop(self):
@@ -3184,7 +3190,7 @@
task = self.factory.makeBugTask(target=product)
with person_logged_in(task.owner):
task.milestone = self.factory.makeMilestone(product=product)
- task.transitionToTarget(self.factory.makeProduct())
+ task.transitionToTarget(self.factory.makeProduct(), task.owner)
self.assertIs(None, task.milestone)
def test_milestone_preserved_if_transition_rejected(self):
@@ -3196,7 +3202,8 @@
product=product)
self.assertRaises(
IllegalTarget,
- task.transitionToTarget, self.factory.makeSourcePackage())
+ task.transitionToTarget, self.factory.makeSourcePackage(),
+ task.owner)
self.assertEqual(milestone, task.milestone)
def test_milestone_preserved_within_a_pillar(self):
@@ -3208,14 +3215,14 @@
with person_logged_in(task.owner):
task.milestone = milestone = self.factory.makeMilestone(
distribution=dsp.distribution)
- task.transitionToTarget(dsp)
+ task.transitionToTarget(dsp, task.owner)
self.assertEqual(milestone, task.milestone)
def test_targetnamecache_updated(self):
new_product = self.factory.makeProduct()
task = self.factory.makeBugTask()
with person_logged_in(task.owner):
- task.transitionToTarget(new_product)
+ task.transitionToTarget(new_product, task.owner)
self.assertEqual(
new_product.bugtargetdisplayname,
removeSecurityProxy(task).targetnamecache)
@@ -3238,12 +3245,100 @@
[ds, ds.distribution, other_distro])
sp = self.factory.makeSourcePackage(distroseries=ds, publish=True)
with person_logged_in(ds_task.owner):
- ds_task.transitionToTarget(sp)
+ ds_task.transitionToTarget(sp, ds_task.owner)
self.assertContentEqual(
(t.target for t in bug.bugtasks),
[sp, sp.distribution_sourcepackage, other_distro])
+def disable_trigger_fixture():
+ # XXX 2012-05-22 wallyworld bug=1002596
+ # No need to use this fixture when triggers are removed.
+ return DisableTriggerFixture(
+ {'bugsubscription':
+ 'bugsubscription_mirror_legacy_access_t',
+ 'bug': 'bug_mirror_legacy_access_t',
+ 'bugtask': 'bugtask_mirror_legacy_access_t',
+ })
+
+
+class TestTransitionsRemovesSubscribersJob(TestCaseWithFactory):
+ """Test that various bug transitions invoke RemoveBugSubscribers job."""
+
+ layer = CeleryJobLayer
+
+ def setUp(self):
+ self.useFixture(FeatureFixture({
+ 'jobs.celery.enabled_classes':
+ 'RemoveBugSubscriptionsJob',
+ 'disclosure.access_mirror_triggers.removed': 'true',
+ }))
+ self.useFixture(disable_trigger_fixture())
+ super(TestTransitionsRemovesSubscribersJob, self).setUp()
+
+ def _assert_bug_change_unsubscribes(self, change_callback):
+ # Subscribers are unsubscribed if the bug becomes invisible due to a
+ # task being retargetted.
+ product = self.factory.makeProduct()
+ owner = self.factory.makePerson()
+ [policy] = getUtility(IAccessPolicySource).find(
+ [(product, InformationType.USERDATA)])
+ # The policy grantees will lose access.
+ policy_grantee = self.factory.makePerson()
+
+ self.factory.makeAccessPolicyGrant(policy, policy_grantee, owner)
+ login_person(owner)
+ bug = self.factory.makeBug(
+ owner=owner, product=product,
+ information_type=InformationType.USERDATA)
+
+ # Change bug bug attributes so that it can become inaccessible for
+ # some users.
+ change_callback(bug)
+
+ # The artifact grantees will not lose access when the job is run.
+ artifact_grantee = self.factory.makePerson()
+
+ bug.subscribe(policy_grantee, owner)
+ bug.subscribe(artifact_grantee, owner)
+ # Subscribing policy_grantee has created and artifact grant so we
+ # need to revoke that to test the job.
+ getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+ getUtility(IAccessArtifactSource).find(
+ [bug]), [policy_grantee])
+
+ # policy grantees are subscribed because the job has not been run yet.
+ subscribers = removeSecurityProxy(bug).getDirectSubscribers()
+ self.assertIn(policy_grantee, subscribers)
+
+ with block_on_job(self):
+ transaction.commit()
+
+ # Check the result. Policy grantees will be unsubscribed.
+ subscribers = removeSecurityProxy(bug).getDirectSubscribers()
+ self.assertNotIn(policy_grantee, subscribers)
+ self.assertIn(artifact_grantee, subscribers)
+
+ def test_change_information_type(self):
+ # Changing the information type of a bug unsubscribes users who can no
+ # longer see the bug.
+ def change_information_type(bug):
+ bug.transitionToInformationType(
+ InformationType.EMBARGOEDSECURITY)
+
+ self._assert_bug_change_unsubscribes(change_information_type)
+
+ def test_change_target(self):
+ # Changing the target of a bug unsubscribes users who can no
+ # longer see the bug.
+ def change_target(bug):
+ another_product = self.factory.makeProduct()
+ removeSecurityProxy(bug).default_bugtask.transitionToTarget(
+ another_product)
+
+ self._assert_bug_change_unsubscribes(change_target)
+
+
class TestBugTargetKeys(TestCaseWithFactory):
"""Tests for bug_target_to_key and bug_target_from_key."""
@@ -3785,7 +3880,7 @@
thunderbird = getUtility(IProductSet).get(8)
upstream_task_id = upstream_task.id
- upstream_task.transitionToTarget(thunderbird)
+ upstream_task.transitionToTarget(thunderbird, bug_one.owner)
self.assertEqual(upstream_task.bugtargetdisplayname,
u'Mozilla Thunderbird')
=== modified file 'lib/lp/bugs/stories/standalone/xx-show-distrorelease-cve-report.txt'
--- lib/lp/bugs/stories/standalone/xx-show-distrorelease-cve-report.txt 2011-12-24 15:18:32 +0000
+++ lib/lp/bugs/stories/standalone/xx-show-distrorelease-cve-report.txt 2012-05-22 12:23:21 +0000
@@ -1,17 +1,17 @@
Let's look at all CVE issues in Debian and Debian Woody
- >>> browser.open('http://launchpad.dev/debian/woody/+cve')
- >>> print '\n'+browser.contents
- <BLANKLINE>
- ...
- ...CVEs related to bugs in Debian Woody...
- ...Open bugs...
- ...Bug #2: Blackhole Trash folder...
- ...CVE-1999-2345...
- ...mozilla-firefox (Debian Woody)...
- ...Resolved bugs...
- ...There are no CVEs related to bugs resolved in Debian Woody.
- ...
+ >>> browser.open('http://launchpad.dev/debian/woody/+cve')
+ >>> print '\n'+browser.contents
+ <BLANKLINE>
+ ...
+ ...CVEs related to bugs in Debian Woody...
+ ...Open bugs...
+ ...Bug #2: Blackhole Trash folder...
+ ...CVE-1999-2345...
+ ...mozilla-firefox (Debian Woody)...
+ ...Resolved bugs...
+ ...There are no CVEs related to bugs resolved in Debian Woody.
+ ...
Bugs listed on this report won't necessarily be attached to source
packages. Let's demonstrate this by temporarily clearing the
@@ -21,6 +21,7 @@
>>> from lp.testing import login, logout
>>> from lp.bugs.interfaces.bug import IBugSet
>>> from lp.registry.interfaces.distribution import IDistributionSet
+ >>> from lp.services.webapp.interfaces import ILaunchBag
>>> login("foo.bar@xxxxxxxxxxxxx")
@@ -30,7 +31,8 @@
>>> bug_two = getUtility(IBugSet).get(2)
>>> debian_woody_firefox_task = bug_two.getBugTask(
... debian_woody_firefox)
- >>> debian_woody_firefox_task.transitionToTarget(woody)
+ >>> debian_woody_firefox_task.transitionToTarget(
+ ... woody, getUtility(ILaunchBag).user)
>>> logout()
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2012-05-08 01:06:31 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2012-05-22 12:23:21 +0000
@@ -1049,7 +1049,7 @@
self.bug_task, providing=providedBy(self.bug_task))
new_target = self.factory.makeProduct(owner=self.user)
- self.bug_task.transitionToTarget(new_target)
+ self.bug_task.transitionToTarget(new_target, self.user)
notify(ObjectModifiedEvent(
self.bug_task, bug_task_before_modification,
['target', 'product'], user=self.user))
@@ -1092,7 +1092,7 @@
bug_task_before_modification = Snapshot(
bug_task, providing=providedBy(bug_task))
- bug_task.transitionToTarget(new_target)
+ bug_task.transitionToTarget(new_target, self.user)
notify(ObjectModifiedEvent(
bug_task, bug_task_before_modification,
['target', 'product'], user=self.user))
@@ -1173,7 +1173,7 @@
bug_task_before_modification = Snapshot(
source_package_bug_task,
providing=providedBy(source_package_bug_task))
- source_package_bug_task.transitionToTarget(new_target)
+ source_package_bug_task.transitionToTarget(new_target, self.user)
notify(ObjectModifiedEvent(
source_package_bug_task, bug_task_before_modification,
@@ -1211,9 +1211,11 @@
new_product = self.factory.makeProduct()
subscriber = self.factory.makePerson()
new_product.addBugSubscription(subscriber, subscriber)
+ owner = self.factory.makePerson()
bug = self.factory.makeBug(
- product=old_product, information_type=InformationType.USERDATA)
- bug.default_bugtask.transitionToTarget(new_product)
+ product=old_product, owner=owner,
+ information_type=InformationType.USERDATA)
+ bug.default_bugtask.transitionToTarget(new_product, owner)
self.assertNotIn(subscriber, bug.getDirectSubscribers())
self.assertNotIn(subscriber, bug.getIndirectSubscribers())
=== modified file 'lib/lp/bugs/tests/test_bugvisibility.py'
--- lib/lp/bugs/tests/test_bugvisibility.py 2012-05-22 12:23:20 +0000
+++ lib/lp/bugs/tests/test_bugvisibility.py 2012-05-22 12:23:21 +0000
@@ -91,6 +91,8 @@
@property
def disable_trigger_fixture(self):
+ # XXX 2012-05-22 wallyworld bug=1002596
+ # No need to use this fixture when triggers are removed.
return DisableTriggerFixture(
{'bugsubscription':
'bugsubscription_mirror_legacy_access_t',
=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py 2012-05-02 05:25:11 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py 2012-05-22 12:23:21 +0000
@@ -469,7 +469,7 @@
self.factory.makeSourcePackagePublishingHistory(
distroseries=self.ubuntu.currentseries,
sourcepackagename=distrosourcepackage.sourcepackagename)
- bug.bugtasks[0].transitionToTarget(distrosourcepackage)
+ bug.bugtasks[0].transitionToTarget(distrosourcepackage, self.owner)
bug.bugtasks[0].transitionToMilestone(
self.milestone, self.owner)
# This is necessary to test precaching of assignees.
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/configure.zcml 2012-05-22 12:23:21 +0000
@@ -1985,17 +1985,30 @@
</securedutility>
<!-- Sharing jobs -->
- <class class=".model.sharingjob.RemoveSubscriptionsJob">
- <allow interface=".interfaces.sharingjob.IRemoveSubscriptionsJob"/>
- <allow attributes="
- context
- log_name"/>
- </class>
-
- <securedutility
- component=".model.sharingjob.RemoveSubscriptionsJob"
- provides=".interfaces.sharingjob.IRemoveSubscriptionsJobSource">
- <allow interface=".interfaces.sharingjob.IRemoveSubscriptionsJobSource"/>
+ <class class=".model.sharingjob.RemoveGranteeSubscriptionsJob">
+ <allow interface=".interfaces.sharingjob.IRemoveGranteeSubscriptionsJob"/>
+ <allow attributes="
+ context
+ log_name"/>
+ </class>
+
+ <securedutility
+ component=".model.sharingjob.RemoveGranteeSubscriptionsJob"
+ provides=".interfaces.sharingjob.IRemoveGranteeSubscriptionsJobSource">
+ <allow interface=".interfaces.sharingjob.IRemoveGranteeSubscriptionsJobSource"/>
+ </securedutility>
+
+ <class class=".model.sharingjob.RemoveBugSubscriptionsJob">
+ <allow interface=".interfaces.sharingjob.IRemoveBugSubscriptionsJob"/>
+ <allow attributes="
+ context
+ log_name"/>
+ </class>
+
+ <securedutility
+ component=".model.sharingjob.RemoveBugSubscriptionsJob"
+ provides=".interfaces.sharingjob.IRemoveBugSubscriptionsJobSource">
+ <allow interface=".interfaces.sharingjob.IRemoveBugSubscriptionsJobSource"/>
</securedutility>
</configure>
=== modified file 'lib/lp/registry/interfaces/sharingjob.py'
--- lib/lp/registry/interfaces/sharingjob.py 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/interfaces/sharingjob.py 2012-05-22 12:23:21 +0000
@@ -6,8 +6,10 @@
__metaclass__ = type
__all__ = [
- 'IRemoveSubscriptionsJob',
- 'IRemoveSubscriptionsJobSource',
+ 'IRemoveBugSubscriptionsJob',
+ 'IRemoveBugSubscriptionsJobSource',
+ 'IRemoveGranteeSubscriptionsJob',
+ 'IRemoveGranteeSubscriptionsJobSource',
'ISharingJob',
'ISharingJobSource',
]
@@ -66,8 +68,18 @@
"""The person who initiated the job."""
-class IRemoveSubscriptionsJob(ISharingJob):
- """Job to remove subscriptions to artifacts for which access is revoked."""
+class IRemoveBugSubscriptionsJob(ISharingJob):
+ """Job to remove subscriptions to artifacts for which access is revoked.
+
+ Invalid subscriptions for a specific bug are removed.
+ """
+
+
+class IRemoveGranteeSubscriptionsJob(ISharingJob):
+ """Job to remove subscriptions to artifacts for which access is revoked.
+
+ Invalid subscription for a specific grantee are removed.
+ """
class ISharingJobSource(IJobSource):
@@ -77,8 +89,19 @@
"""Create a new ISharingJob."""
-class IRemoveSubscriptionsJobSource(ISharingJobSource):
- """An interface for acquiring IRemoveSubscriptionsJobs."""
+class IRemoveBugSubscriptionsJobSource(ISharingJobSource):
+ """An interface for acquiring IRemoveBugSubscriptionsJobs."""
+
+ def create(pillar, bugs, requestor):
+ """Create a new job to remove subscriptions for the specified bugs.
+
+ Subscriptions for users who no longer have access to the bugs are
+ removed.
+ """
+
+
+class IRemoveGranteeSubscriptionsJobSource(ISharingJobSource):
+ """An interface for acquiring IRemoveGranteeSubscriptionsJobs."""
def create(pillar, grantee, requestor, bugs=None, branches=None):
"""Create a new job to revoke access to the specified artifacts.
=== modified file 'lib/lp/registry/model/sharingjob.py'
--- lib/lp/registry/model/sharingjob.py 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/model/sharingjob.py 2012-05-22 12:23:21 +0000
@@ -8,7 +8,8 @@
__all__ = [
- 'RemoveSubscriptionsJob',
+ 'RemoveBugSubscriptionsJob',
+ 'RemoveGranteeSubscriptionsJob',
]
import contextlib
@@ -24,9 +25,12 @@
from sqlobject import SQLObjectNotFound
from storm.expr import (
And,
+ Coalesce,
In,
+ Join,
Not,
Select,
+ SQL,
)
from storm.locals import (
Int,
@@ -50,18 +54,26 @@
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.sharingjob import (
- IRemoveSubscriptionsJob,
- IRemoveSubscriptionsJobSource,
+ IRemoveBugSubscriptionsJob,
+ IRemoveBugSubscriptionsJobSource,
+ IRemoveGranteeSubscriptionsJob,
+ IRemoveGranteeSubscriptionsJobSource,
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
+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
from lp.services.database.stormbase import StormBase
+from lp.services.database.stormexpr import (
+ ArrayAgg,
+ ArrayIntersects,
+ )
from lp.services.job.model.job import (
EnumeratedSubclass,
Job,
@@ -76,7 +88,7 @@
class SharingJobType(DBEnumeratedType):
"""Values that ISharingJob.job_type can take."""
- REMOVE_SUBSCRIPTIONS = DBItem(0, """
+ REMOVE_GRANTEE_SUBSCRIPTIONS = DBItem(0, """
Remove subscriptions of artifacts which are inaccessible.
This job removes subscriptions to artifacts when access is
@@ -84,6 +96,14 @@
grant (either direct or indirect via team membership).
""")
+ REMOVE_BUG_SUBSCRIPTIONS = DBItem(1, """
+ Remove subscriptions for users who can no longer access bugs.
+
+ 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).
+ """)
+
class SharingJob(StormBase):
"""Base class for jobs related to branch merge proposals."""
@@ -161,10 +181,15 @@
self.context = job
def __repr__(self):
- return '<%(job_type)s job for %(grantee)s and %(pillar)s>' % {
- 'job_type': self.context.job_type.name,
- 'grantee': self.grantee.displayname,
- 'pillar': self.pillar_text,
+ if self.grantee:
+ return '<%(job_type)s job for %(grantee)s and %(pillar)s>' % {
+ 'job_type': self.context.job_type.name,
+ 'grantee': self.grantee.displayname,
+ 'pillar': self.pillar_text,
+ }
+ else:
+ return '<%(job_type)s job>' % {
+ 'job_type': self.context.job_type.name,
}
@property
@@ -223,8 +248,9 @@
vars = BaseRunnableJob.getOopsVars(self)
vars.extend([
('sharing_job_id', self.context.id),
- ('sharing_job_type', self.context.job_type.title),
- ('grantee', self.grantee.name)])
+ ('sharing_job_type', self.context.job_type.title)])
+ if self.grantee:
+ vars.append(('grantee', self.grantee.name))
if self.product:
vars.append(('product', self.product.name))
if self.distro:
@@ -232,19 +258,19 @@
return vars
-class RemoveSubscriptionsJob(SharingJobDerived):
- """See `IRemoveSubscriptionsJob`."""
+class RemoveGranteeSubscriptionsJob(SharingJobDerived):
+ """See `IRemoveGranteeSubscriptionsJob`."""
- implements(IRemoveSubscriptionsJob)
- classProvides(IRemoveSubscriptionsJobSource)
- class_job_type = SharingJobType.REMOVE_SUBSCRIPTIONS
+ implements(IRemoveGranteeSubscriptionsJob)
+ classProvides(IRemoveGranteeSubscriptionsJobSource)
+ class_job_type = SharingJobType.REMOVE_GRANTEE_SUBSCRIPTIONS
config = config.sharing_jobs
@classmethod
def create(cls, pillar, grantee, requestor, information_types=None,
bugs=None, branches=None):
- """See `IRemoveSubscriptionsJob`."""
+ """See `IRemoveGranteeSubscriptionsJob`."""
bug_ids = [
bug.id for bug in bugs or []
@@ -261,7 +287,7 @@
'information_types': information_types,
'requestor.id': requestor.id
}
- return super(RemoveSubscriptionsJob, cls).create(
+ return super(RemoveGranteeSubscriptionsJob, cls).create(
pillar, grantee, metadata)
@property
@@ -300,7 +326,7 @@
'for %s on %s' % (self.grantee.displayname, self.pillar_text))
def run(self):
- """See `IRemoveSubscriptionsJob`."""
+ """See `IRemoveGranteeSubscriptionsJob`."""
logger = logging.getLogger()
logger.info(self.getOperationDescription())
@@ -333,7 +359,7 @@
# Branches are not handled until information_type is supported.
# Do the bugs.
- privacy_filter = get_bug_privacy_filter(self.grantee, use_flat=True)
+ privacy_filter = get_bug_privacy_filter(self.grantee)
bug_filter = Not(In(
Bug.id,
Select(
@@ -353,3 +379,98 @@
for bug in subscribed_invisible_bugs:
bug.unsubscribe(
self.grantee, self.requestor, ignore_permissions=True)
+
+
+class RemoveBugSubscriptionsJob(SharingJobDerived):
+ """See `IRemoveBugSubscriptionsJob`."""
+
+ implements(IRemoveBugSubscriptionsJob)
+ classProvides(IRemoveBugSubscriptionsJobSource)
+ class_job_type = SharingJobType.REMOVE_BUG_SUBSCRIPTIONS
+
+ config = config.sharing_jobs
+
+ @classmethod
+ def create(cls, bugs, requestor):
+ """See `IRemoveBugSubscriptionsJob`."""
+
+ bug_ids = [
+ bug.id for bug in bugs
+ ]
+ metadata = {
+ 'bug_ids': bug_ids,
+ 'requestor.id': requestor.id
+ }
+ return super(RemoveBugSubscriptionsJob, cls).create(
+ None, None, metadata)
+
+ @property
+ def requestor_id(self):
+ return self.metadata['requestor.id']
+
+ @property
+ def requestor(self):
+ return getUtility(IPersonSet).get(self.requestor_id)
+
+ @property
+ def bug_ids(self):
+ return self.metadata['bug_ids']
+
+ @property
+ def bugs(self):
+ return getUtility(IBugSet).getByNumbers(self.bug_ids)
+
+ 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).
+ result = set()
+ result.add(format_address_for_person(self.requestor))
+ for bug in self.bugs:
+ for pillar in bug.affected_pillars:
+ if pillar.owner.preferredemail:
+ result.add(format_address_for_person(pillar.owner))
+ return list(result)
+
+ def getOperationDescription(self):
+ return 'removing subscriptions for bugs %s' % self.bug_ids
+
+ def run(self):
+ """See `IRemoveBugSubscriptionsJob`."""
+
+ logger = logging.getLogger()
+ logger.info(self.getOperationDescription())
+
+ # Unsubscribe grantee from the specified bugs.
+ 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(
+ BugSubscription,
+ In(BugSubscription.bug_id,
+ Select(
+ BugTaskFlat.bug_id,
+ where=And(*constraints)))
+ )
+ for sub in subscriptions:
+ sub.bug.unsubscribe(
+ sub.person, self.requestor, ignore_permissions=True)
=== modified file 'lib/lp/registry/model/teammembership.py'
--- lib/lp/registry/model/teammembership.py 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/model/teammembership.py 2012-05-22 12:23:21 +0000
@@ -42,7 +42,9 @@
IMembershipNotificationJobSource,
)
from lp.registry.interfaces.role import IPersonRoles
-from lp.registry.interfaces.sharingjob import IRemoveSubscriptionsJobSource
+from lp.registry.interfaces.sharingjob import (
+ IRemoveGranteeSubscriptionsJobSource,
+ )
from lp.registry.interfaces.teammembership import (
ACTIVE_STATES,
CyclicalTeamMembershipError,
@@ -388,7 +390,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(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
None, self.person, user)
else:
=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/services/sharingservice.py 2012-05-22 12:23:21 +0000
@@ -35,7 +35,9 @@
)
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.projectgroup import IProjectGroup
-from lp.registry.interfaces.sharingjob import IRemoveSubscriptionsJobSource
+from lp.registry.interfaces.sharingjob import (
+ IRemoveGranteeSubscriptionsJobSource,
+ )
from lp.registry.interfaces.sharingservice import ISharingService
from lp.registry.model.person import Person
from lp.services.features import getFeatureFlag
@@ -269,7 +271,8 @@
# For information types with permission 'nothing', we can simply
# call the deletePillarSharee method directly.
if len(info_types_for_nothing) > 0:
- self.deletePillarSharee(pillar, sharee, info_types_for_nothing)
+ self.deletePillarSharee(
+ pillar, user, sharee, info_types_for_nothing)
# Return sharee data to the caller.
ap_grant_flat = getUtility(IAccessPolicyGrantFlatSource)
@@ -319,7 +322,7 @@
# Create a job to remove subscriptions for artifacts the sharee can no
# longer see.
- getUtility(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, sharee, user, information_types=information_types)
@available_with_permission('launchpad.Edit', 'pillar')
@@ -345,10 +348,11 @@
# Create a job to remove subscriptions for artifacts the sharee can no
# longer see.
- getUtility(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, sharee, user, bugs=bugs, branches=branches)
- def createAccessGrants(self, user, sharee, branches=None, bugs=None):
+ def createAccessGrants(self, user, sharee, branches=None, bugs=None,
+ **kwargs):
"""See `ISharingService`."""
if not self.write_enabled:
@@ -359,11 +363,13 @@
artifacts.extend(branches)
if bugs:
artifacts.extend(bugs)
- # The user needs to have launchpad.Edit permission on all supplied
- # bugs and branches or else we raise an Unauthorized exception.
- for artifact in artifacts or []:
- if not check_permission('launchpad.Edit', artifact):
- raise Unauthorized
+ ignore_permissions = kwargs.get('ignore_permissions', False)
+ if not ignore_permissions:
+ # The user needs to have launchpad.Edit permission on all supplied
+ # bugs and branches or else we raise an Unauthorized exception.
+ for artifact in artifacts or []:
+ if not check_permission('launchpad.Edit', artifact):
+ raise Unauthorized
# Ensure there are access artifacts associated with the bugs and
# branches.
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-05-22 12:23:21 +0000
@@ -55,7 +55,7 @@
WRITE_FLAG = {
'disclosure.enhanced_sharing.writable': 'true',
'disclosure.enhanced_sharing_details.enabled': 'true',
- 'jobs.celery.enabled_classes': 'RemoveSubscriptionsJob'}
+ 'jobs.celery.enabled_classes': 'RemoveGranteeSubscriptionsJob'}
DETAILS_FLAG = {'disclosure.enhanced_sharing_details.enabled': 'true'}
@@ -651,6 +651,7 @@
expected_information_types = (
set(information_types).difference(types_to_delete))
# Check that grantee is unsubscribed.
+ login_person(product.owner)
for bug in bugs:
if bug.information_type in expected_information_types:
self.assertIn(grantee, bug.getDirectSubscribers())
@@ -696,7 +697,7 @@
for bug in bugs or []:
bug.subscribe(person, pillar.owner)
for branch in branches or []:
- branch.subscribe(grantee,
+ branch.subscribe(person,
BranchSubscriptionNotificationLevel.NOEMAIL, None,
CodeReviewNotificationLevel.NOEMAIL, pillar.owner)
@@ -720,7 +721,7 @@
# Check that the grantee's subscriptions have been removed.
# Branches will be done once they have the information_type attribute.
- for bug in bugs:
+ for bug in bugs or []:
self.assertNotIn(grantee, bug.getDirectSubscribers())
# Someone else still has access to the bugs and branches.
@@ -728,9 +729,9 @@
access_artifacts, [someone])
self.assertEqual(1, grants.count())
# Someone else still has subscriptions to the bugs and branches.
- for bug in bugs:
+ for bug in bugs or []:
self.assertIn(someone, bug.getDirectSubscribers())
- for branch in branches:
+ for branch in branches or []:
self.assertIn(someone, branch.subscribers)
def test_revokeAccessGrantsBugs(self):
@@ -909,11 +910,8 @@
for i, bug in enumerate(bugs):
grant_access(bug, i == 9)
- # For branches we also need to call makeAccessPolicyArtifact.
- [policy] = getUtility(IAccessPolicySource).find(
- [(product, InformationType.USERDATA)])
for i, branch in enumerate(branches):
- artifact = grant_access(branch, i == 9)
+ grant_access(branch, i == 9)
# XXX bug=1001042 wallyworld 2012-05-18
# for now we need to subscribe users to the branch in order
# for the underlying BranchCollection to allow access. This will
@@ -925,8 +923,6 @@
BranchSubscriptionDiffSize.NODIFF,
CodeReviewNotificationLevel.NOEMAIL,
owner)
- self.factory.makeAccessPolicyArtifact(
- artifact=artifact, policy=policy)
# Check the results.
shared_bugtasks, shared_branches = self.service.getSharedArtifacts(
=== modified file 'lib/lp/registry/tests/test_sharingjob.py'
--- lib/lp/registry/tests/test_sharingjob.py 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/tests/test_sharingjob.py 2012-05-22 12:23:21 +0000
@@ -18,15 +18,18 @@
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactSource,
IAccessArtifactGrantSource,
+ IAccessPolicySource,
)
from lp.registry.interfaces.person import TeamSubscriptionPolicy
from lp.registry.interfaces.sharingjob import (
- IRemoveSubscriptionsJobSource,
+ IRemoveBugSubscriptionsJobSource,
+ IRemoveGranteeSubscriptionsJobSource,
ISharingJob,
ISharingJobSource,
)
from lp.registry.model.sharingjob import (
- RemoveSubscriptionsJob,
+ RemoveBugSubscriptionsJob,
+ RemoveGranteeSubscriptionsJob,
SharingJob,
SharingJobDerived,
SharingJobType,
@@ -35,9 +38,11 @@
from lp.services.job.tests import block_on_job
from lp.services.mail.sendmail import format_address_for_person
from lp.testing import (
+ login_person,
person_logged_in,
TestCaseWithFactory,
)
+from lp.testing.fixture import DisableTriggerFixture
from lp.testing.layers import (
CeleryJobLayer,
DatabaseFunctionalLayer,
@@ -55,9 +60,10 @@
grantee = self.factory.makePerson()
metadata = ('some', 'arbitrary', 'metadata')
sharing_job = SharingJob(
- SharingJobType.REMOVE_SUBSCRIPTIONS, pillar, grantee, metadata)
+ SharingJobType.REMOVE_GRANTEE_SUBSCRIPTIONS,
+ pillar, grantee, metadata)
self.assertEqual(
- SharingJobType.REMOVE_SUBSCRIPTIONS, sharing_job.job_type)
+ SharingJobType.REMOVE_GRANTEE_SUBSCRIPTIONS, sharing_job.job_type)
self.assertEqual(pillar, sharing_job.product)
self.assertEqual(grantee, sharing_job.grantee)
expected_json_data = '["some", "arbitrary", "metadata"]'
@@ -73,7 +79,8 @@
pillar = self.factory.makeProduct()
grantee = self.factory.makePerson()
sharing_job = SharingJob(
- SharingJobType.REMOVE_SUBSCRIPTIONS, pillar, grantee, metadata)
+ SharingJobType.REMOVE_GRANTEE_SUBSCRIPTIONS,
+ pillar, grantee, metadata)
metadata['a_list'] = list(metadata['a_list'])
self.assertEqual(metadata, sharing_job.metadata)
@@ -87,14 +94,15 @@
pillar = self.factory.makeProduct(name=prod_name)
grantee = self.factory.makePerson(name=grantee_name)
requestor = self.factory.makePerson()
- job = getUtility(IRemoveSubscriptionsJobSource).create(
+ job = getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, grantee, requestor)
return job
def test_repr(self):
job = self._makeJob('prod', 'fred')
self.assertEqual(
- '<REMOVE_SUBSCRIPTIONS job for Fred and Prod>', repr(job))
+ '<REMOVE_GRANTEE_SUBSCRIPTIONS job for Fred and Prod>',
+ repr(job))
def test_create_success(self):
# Create an instance of SharingJobDerived that delegates to SharingJob.
@@ -117,71 +125,71 @@
job_1 = self._makeJob()
job_2 = self._makeJob()
job_2.start()
- jobs = list(RemoveSubscriptionsJob.iterReady())
+ jobs = list(RemoveGranteeSubscriptionsJob.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('RemoveSubscriptionsJob', job.log_name)
+ self.assertEqual('RemoveGranteeSubscriptionsJob', job.log_name)
def test_getOopsVars(self):
# The pillar and grantee name are added to the oops vars.
pillar = self.factory.makeDistribution()
grantee = self.factory.makePerson()
requestor = self.factory.makePerson()
- job = getUtility(IRemoveSubscriptionsJobSource).create(
+ job = getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, grantee, requestor)
oops_vars = job.getOopsVars()
self.assertIs(True, len(oops_vars) > 4)
self.assertIn(('distro', pillar.name), oops_vars)
self.assertIn(('grantee', grantee.name), oops_vars)
- def test_getErrorRecipients(self):
- # The pillar owner and job requestor are the error recipients.
- pillar = self.factory.makeDistribution()
- grantee = self.factory.makePerson()
- requestor = self.factory.makePerson()
- job = getUtility(IRemoveSubscriptionsJobSource).create(
- pillar, grantee, requestor)
- expected_emails = [
- format_address_for_person(person)
- for person in (pillar.owner, requestor)]
- self.assertContentEqual(
- expected_emails, job.getErrorRecipients())
-
-
-class RemoveSubscriptionsJobTestCase(TestCaseWithFactory):
- """Test case for the RemoveSubscriptionsJob class."""
+
+def disable_trigger_fixture():
+ # XXX 2012-05-22 wallyworld bug=1002596
+ # No need to use this fixture when triggers are removed.
+ return DisableTriggerFixture(
+ {'bugsubscription':
+ 'bugsubscription_mirror_legacy_access_t',
+ 'bug': 'bug_mirror_legacy_access_t',
+ 'bugtask': 'bugtask_mirror_legacy_access_t',
+ })
+
+
+class RemoveGranteeSubscriptionsJobTestCase(TestCaseWithFactory):
+ """Test case for the RemoveGranteeSubscriptionsJob class."""
layer = CeleryJobLayer
def setUp(self):
self.useFixture(FeatureFixture({
- 'jobs.celery.enabled_classes': 'RemoveSubscriptionsJob',
+ 'jobs.celery.enabled_classes':
+ 'RemoveGranteeSubscriptionsJob',
}))
- super(RemoveSubscriptionsJobTestCase, self).setUp()
+ super(RemoveGranteeSubscriptionsJobTestCase, self).setUp()
def test_create(self):
- # Create an instance of RemoveSubscriptionsJob that stores
+ # Create an instance of RemoveGranteeSubscriptionsJob that stores
# the information type and artifact information.
self.assertIs(
True,
- IRemoveSubscriptionsJobSource.providedBy(RemoveSubscriptionsJob))
+ IRemoveGranteeSubscriptionsJobSource.providedBy(
+ RemoveGranteeSubscriptionsJob))
self.assertEqual(
- SharingJobType.REMOVE_SUBSCRIPTIONS,
- RemoveSubscriptionsJob.class_job_type)
+ SharingJobType.REMOVE_GRANTEE_SUBSCRIPTIONS,
+ RemoveGranteeSubscriptionsJob.class_job_type)
pillar = self.factory.makeProduct()
grantee = self.factory.makePerson()
requestor = self.factory.makePerson()
bug = self.factory.makeBug(product=pillar)
branch = self.factory.makeBranch(product=pillar)
info_type = InformationType.USERDATA
- job = getUtility(IRemoveSubscriptionsJobSource).create(
+ job = getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, grantee, requestor, [info_type], [bug], [branch])
naked_job = removeSecurityProxy(job)
- self.assertIsInstance(job, RemoveSubscriptionsJob)
+ self.assertIsInstance(job, RemoveGranteeSubscriptionsJob)
self.assertEqual(pillar, job.pillar)
self.assertEqual(grantee, job.grantee)
self.assertEqual(requestor.id, naked_job.requestor_id)
@@ -189,15 +197,28 @@
self.assertContentEqual([bug.id], naked_job.bug_ids)
self.assertContentEqual([branch.unique_name], naked_job.branch_names)
+ def test_getErrorRecipients(self):
+ # The pillar owner and job requestor are the error recipients.
+ pillar = self.factory.makeDistribution()
+ grantee = self.factory.makePerson()
+ requestor = self.factory.makePerson()
+ job = getUtility(IRemoveGranteeSubscriptionsJobSource).create(
+ pillar, grantee, requestor)
+ expected_emails = [
+ format_address_for_person(person)
+ for person in (pillar.owner, requestor)]
+ self.assertContentEqual(
+ expected_emails, job.getErrorRecipients())
+
def test_create_no_pillar(self):
- # Create an instance of RemoveSubscriptionsJob that stores
+ # Create an instance of RemoveGranteeSubscriptionsJob that stores
# the information type and artifact information but with no pillar.
grantee = self.factory.makePerson()
requestor = self.factory.makePerson()
- job = getUtility(IRemoveSubscriptionsJobSource).create(
+ job = getUtility(IRemoveGranteeSubscriptionsJobSource).create(
None, grantee, requestor)
naked_job = removeSecurityProxy(job)
- self.assertIsInstance(job, RemoveSubscriptionsJob)
+ self.assertIsInstance(job, RemoveGranteeSubscriptionsJob)
self.assertEqual(None, job.pillar)
self.assertEqual(grantee, job.grantee)
self.assertEqual(requestor.id, naked_job.requestor_id)
@@ -220,7 +241,7 @@
grantee = self.factory.makePerson()
owner = self.factory.makePerson()
bug, ignored = self._make_subscribed_bug(grantee, distribution=pillar)
- getUtility(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, grantee, owner, bugs=[bug])
with block_on_job(self):
transaction.commit()
@@ -243,7 +264,7 @@
pillar = self.factory.makeProduct(owner=owner)
grantee = self.factory.makePerson()
branch = self._make_subscribed_branch(pillar, grantee)
- getUtility(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, grantee, owner, branches=[branch])
with block_on_job(self):
transaction.commit()
@@ -272,7 +293,7 @@
# Now run the job.
requestor = self.factory.makePerson()
- getUtility(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, grantee, requestor)
with block_on_job(self):
transaction.commit()
@@ -323,7 +344,7 @@
# Now run the job.
requestor = self.factory.makePerson()
- getUtility(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, person_grantee, requestor)
with block_on_job(self):
transaction.commit()
@@ -367,7 +388,7 @@
accessartifact_source.find([bug1, bug2]), [person_grantee])
# Now run the job, removing access to userdata artifacts.
- getUtility(IRemoveSubscriptionsJobSource).create(
+ getUtility(IRemoveGranteeSubscriptionsJobSource).create(
pillar, person_grantee, owner, [InformationType.USERDATA])
with block_on_job(self):
transaction.commit()
@@ -376,3 +397,125 @@
person_grantee, removeSecurityProxy(bug1).getDirectSubscribers())
self.assertIn(
person_grantee, removeSecurityProxy(bug2).getDirectSubscribers())
+
+
+class RemoveBugSubscriptionsJobTestCase(TestCaseWithFactory):
+ """Test case for the RemoveBugSubscriptionsJob class."""
+
+ layer = CeleryJobLayer
+
+ def setUp(self):
+ self.useFixture(FeatureFixture({
+ 'jobs.celery.enabled_classes':
+ 'RemoveBugSubscriptionsJob',
+ 'disclosure.access_mirror_triggers.removed': 'true',
+ }))
+ self.useFixture(disable_trigger_fixture())
+ super(RemoveBugSubscriptionsJobTestCase, self).setUp()
+
+ def test_create(self):
+ # Create an instance of RemoveBugSubscriptionsJob.
+ self.assertIs(
+ True,
+ IRemoveBugSubscriptionsJobSource.providedBy(
+ RemoveBugSubscriptionsJob))
+ self.assertEqual(
+ SharingJobType.REMOVE_BUG_SUBSCRIPTIONS,
+ RemoveBugSubscriptionsJob.class_job_type)
+ requestor = self.factory.makePerson()
+ bug = self.factory.makeBug()
+ job = getUtility(IRemoveBugSubscriptionsJobSource).create(
+ [bug], requestor)
+ naked_job = removeSecurityProxy(job)
+ self.assertIsInstance(job, RemoveBugSubscriptionsJob)
+ self.assertEqual(requestor.id, naked_job.requestor_id)
+ self.assertContentEqual([bug.id], naked_job.bug_ids)
+
+ def test_getErrorRecipients(self):
+ # The pillar owner and job requestor are the error recipients.
+ requestor = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ bug = self.factory.makeBug(product=product)
+ job = getUtility(IRemoveBugSubscriptionsJobSource).create(
+ [bug], requestor)
+ expected_emails = [
+ format_address_for_person(person)
+ for person in (product.owner, requestor)]
+ self.assertContentEqual(
+ expected_emails, job.getErrorRecipients())
+
+ def _assert_bug_change_unsubscribes(self, change_callback):
+ # Subscribers are unsubscribed if the bug becomes invisible due to a
+ # change in information_type.
+ product = self.factory.makeProduct()
+ owner = self.factory.makePerson()
+ [policy] = getUtility(IAccessPolicySource).find(
+ [(product, InformationType.USERDATA)])
+ # The policy grantees will lose access.
+ policy_indirect_grantee = self.factory.makePerson()
+ policy_team_grantee = self.factory.makeTeam(
+ subscription_policy=TeamSubscriptionPolicy.RESTRICTED,
+ members=[policy_indirect_grantee])
+
+ self.factory.makeAccessPolicyGrant(policy, policy_team_grantee, owner)
+ login_person(owner)
+ bug = self.factory.makeBug(
+ owner=owner, product=product,
+ information_type=InformationType.USERDATA)
+
+ # Change bug bug attributes so that it can become inaccessible for
+ # some users.
+ change_callback(bug)
+
+ # The artifact grantees will not lose access when the job is run.
+ artifact_indirect_grantee = self.factory.makePerson()
+ artifact_team_grantee = self.factory.makeTeam(
+ subscription_policy=TeamSubscriptionPolicy.RESTRICTED,
+ members=[artifact_indirect_grantee])
+
+ bug.subscribe(policy_team_grantee, owner)
+ bug.subscribe(policy_indirect_grantee, owner)
+ bug.subscribe(artifact_team_grantee, owner)
+ bug.subscribe(artifact_indirect_grantee, owner)
+ # Subscribing policy_team_grantee has created and artifact grant so we
+ # need to revoke that to test the job.
+ getUtility(IAccessArtifactGrantSource).revokeByArtifact(
+ getUtility(IAccessArtifactSource).find(
+ [bug]), [policy_team_grantee])
+
+ # policy grantees are subscribed because the job has not been run yet.
+ subscribers = removeSecurityProxy(bug).getDirectSubscribers()
+ self.assertIn(policy_team_grantee, subscribers)
+ self.assertIn(policy_indirect_grantee, subscribers)
+
+ getUtility(IRemoveBugSubscriptionsJobSource).create([bug], owner)
+ with block_on_job(self):
+ transaction.commit()
+
+ # Check the result. Policy grantees will be unsubscribed.
+ subscribers = removeSecurityProxy(bug).getDirectSubscribers()
+ self.assertNotIn(policy_team_grantee, subscribers)
+ self.assertNotIn(policy_indirect_grantee, subscribers)
+ self.assertIn(artifact_team_grantee, subscribers)
+ self.assertIn(artifact_indirect_grantee, subscribers)
+
+ def test_change_information_type(self):
+ # Changing the information type of a bug unsubscribes users who can no
+ # longer see the bug.
+ def change_information_type(bug):
+ # Set the info_type attribute directly since
+ # transitionToInformationType queues a job.
+ removeSecurityProxy(bug).information_type = (
+ InformationType.EMBARGOEDSECURITY)
+
+ self._assert_bug_change_unsubscribes(change_information_type)
+
+ def test_change_target(self):
+ # Changing the target of a bug unsubscribes users who can no
+ # longer see the bug.
+ def change_target(bug):
+ # Set the new target directly since transitionToTarget queues a job
+ another_product = self.factory.makeProduct()
+ removeSecurityProxy(bug).default_bugtask.product = another_product
+
+ self._assert_bug_change_unsubscribes(change_target)
=== modified file 'lib/lp/registry/tests/test_teammembership.py'
--- lib/lp/registry/tests/test_teammembership.py 2012-05-22 12:23:20 +0000
+++ lib/lp/registry/tests/test_teammembership.py 2012-05-22 12:23:21 +0000
@@ -504,7 +504,7 @@
The number of db queries should be constant not O(depth).
"""
self.assertStatementCount(
- 7,
+ 9,
self.team5.setMembershipData, self.no_priv,
TeamMembershipStatus.DEACTIVATED, self.team5.teamowner)
@@ -998,7 +998,8 @@
def setUp(self):
self.useFixture(FeatureFixture({
- 'jobs.celery.enabled_classes': 'RemoveSubscriptionsJob',
+ 'jobs.celery.enabled_classes':
+ 'RemoveGranteeSubscriptionsJob',
}))
super(TestTeamMembershipJobs, self).setUp()
=== modified file 'lib/lp/services/webapp/doc/canonical_url_examples.txt'
--- lib/lp/services/webapp/doc/canonical_url_examples.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/services/webapp/doc/canonical_url_examples.txt 2012-05-22 12:23:21 +0000
@@ -221,10 +221,11 @@
>>> login("foo.bar@xxxxxxxxxxxxx")
>>> temp_target = distro_task.target
- >>> distro_task.transitionToTarget(distro_task.target.distribution)
+ >>> distro_task.transitionToTarget(
+ ... distro_task.target.distribution, getUtility(ILaunchBag).user)
>>> canonical_url(distro_task)
u'http://bugs.launchpad.dev/debian/+bug/1'
- >>> distro_task.transitionToTarget(temp_target)
+ >>> distro_task.transitionToTarget(temp_target, getUtility(ILaunchBag).user)
An IBugTask on a distribution series source package.
@@ -236,10 +237,11 @@
>>> temp_target = distro_series_task.target
>>> distro_series_task.transitionToTarget(
- ... distro_series_task.target.distroseries)
+ ... distro_series_task.target.distroseries, getUtility(ILaunchBag).user)
>>> canonical_url(distro_series_task)
u'http://bugs.launchpad.dev/debian/sarge/+bug/3'
- >>> distro_series_task.transitionToTarget(temp_target)
+ >>> distro_series_task.transitionToTarget(
+ ... temp_target, getUtility(ILaunchBag).user)
A private bug, as an anonymous user! (We'll temporarily subscribe to the bug,
to ensure that at least one person has the perms to edit it while it's set
Follow ups
