launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19479
[Merge] lp:~wgrant/launchpad/buglinks-user into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/buglinks-user into lp:launchpad.
Commit message:
(un)linkBug now take a user argument, and Question no longer overrides them.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #175545 in Launchpad itself: "BugLinkTargetMixin.linkBug() and unlinkBug() should take a user parameter rather than using LaunchBag"
https://bugs.launchpad.net/launchpad/+bug/175545
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/buglinks-user/+merge/272689
(un)linkBug now take a user argument, and Question no longer overrides them.
The user argument will be used to set XRef.creator.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/buglinks-user into lp:launchpad.
=== modified file 'lib/lp/answers/doc/question.txt'
--- lib/lp/answers/doc/question.txt 2015-09-25 03:02:28 +0000
+++ lib/lp/answers/doc/question.txt 2015-09-29 06:06:37 +0000
@@ -337,39 +337,6 @@
`answer-tracker-workflow.txt`.
-Bug linking
-===========
-
-Question implements the IBugLinkTarget interface which makes it possible
-to link bug report to question.
-
- >>> from lp.bugs.interfaces.buglink import IBugLinkTarget
- >>> verifyObject(IBugLinkTarget, firefox_question)
- True
-
-See ../../bugs/tests/buglinktarget.txt for the documentation and test of the
-IBugLinkTarget interface.
-
-When a bug is linked to a question, the question's owner is subscribed to the
-bug.
-
- >>> from lp.bugs.interfaces.bug import IBugSet
- >>> bug7 = getUtility(IBugSet).get(7)
- >>> bug7.isSubscribed(firefox_question.owner)
- False
- >>> firefox_question.linkBug(bug7)
- True
- >>> bug7.isSubscribed(firefox_question.owner)
- True
-
-When the link is removed, the owner is unsubscribed.
-
- >>> firefox_question.unlinkBug(bug7)
- True
- >>> bug7.isSubscribed(firefox_question.owner)
- False
-
-
Unsupported questions
=====================
=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py 2015-09-28 07:57:17 +0000
+++ lib/lp/answers/model/question.py 2015-09-29 06:06:37 +0000
@@ -661,20 +661,6 @@
return tktmsg
# IBugLinkTarget implementation
- def linkBug(self, bug, user=None):
- """See `IBugLinkTarget`."""
- # Subscribe the question's owner to the bug.
- bug.subscribe(self.owner, self.owner)
- return BugLinkTargetMixin.linkBug(self, bug, user=user)
-
- def unlinkBug(self, bug):
- """See `IBugLinkTarget`."""
- unlinked = BugLinkTargetMixin.unlinkBug(self, bug)
- if unlinked:
- # Additionally, unsubscribe the question's owner from the bug
- bug.unsubscribe(self.owner, self.owner)
- return unlinked
-
def createBugLink(self, bug):
"""See BugLinkTargetMixin."""
QuestionBug(question=self, bug=bug)
=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py 2015-01-06 06:50:20 +0000
+++ lib/lp/blueprints/interfaces/specification.py 2015-09-29 06:06:37 +0000
@@ -702,21 +702,23 @@
:param new_work_items: Work items to set.
"""
+ @call_with(user=REQUEST_USER)
@operation_parameters(
bug=Reference(schema=Interface)) # Really IBug
@export_write_operation()
@operation_for_version('devel')
- def linkBug(bug):
+ def linkBug(bug, user=None):
"""Link a bug to this specification.
:param bug: IBug to link.
"""
+ @call_with(user=REQUEST_USER)
@operation_parameters(
bug=Reference(schema=Interface)) # Really IBug
@export_write_operation()
@operation_for_version('devel')
- def unlinkBug(bug):
+ def unlinkBug(bug, user=None):
"""Unlink a bug to this specification.
:param bug: IBug to unlink.
=== modified file 'lib/lp/bugs/browser/buglinktarget.py'
--- lib/lp/bugs/browser/buglinktarget.py 2015-09-25 02:33:15 +0000
+++ lib/lp/bugs/browser/buglinktarget.py 2015-09-29 06:06:37 +0000
@@ -70,7 +70,7 @@
self.context, providing=providedBy(self.context))
bug = data['bug']
try:
- self.context.linkBug(bug)
+ self.context.linkBug(bug, user=self.user)
except Unauthorized:
# XXX flacoste 2006-08-23 bug=57470: This should use proper _().
self.setFieldError(
@@ -147,7 +147,7 @@
for bug in data['bugs']:
replacements = {'bugid': bug.id}
try:
- self.context.unlinkBug(bug)
+ self.context.unlinkBug(bug, user=self.user)
response.addNotification(
_('Removed link to bug #$bugid.', mapping=replacements))
except Unauthorized:
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2015-09-15 01:56:17 +0000
+++ lib/lp/bugs/browser/bugtask.py 2015-09-29 06:06:37 +0000
@@ -2373,7 +2373,7 @@
return
owner_is_subscribed = question.isSubscribed(self.context.bug.owner)
- question.unlinkBug(self.context.bug)
+ question.unlinkBug(self.context.bug, user=self.user)
# The question.owner was implicitly unsubscribed when the bug
# was unlinked. We resubscribe the owner if he was subscribed.
if owner_is_subscribed is True:
=== modified file 'lib/lp/bugs/interfaces/buglink.py'
--- lib/lp/bugs/interfaces/buglink.py 2015-09-25 10:15:37 +0000
+++ lib/lp/bugs/interfaces/buglink.py 2015-09-29 06:06:37 +0000
@@ -69,7 +69,7 @@
value_type=Reference(schema=IBug), readonly=True),
as_of="devel")
- def linkBug(bug):
+ def linkBug(bug, user=None):
"""Link the object with this bug.
If a new link is created by this method, an ObjectLinkedEvent is
@@ -78,7 +78,7 @@
:return: True if a new link was created, False if it already existed.
"""
- def unlinkBug(bug):
+ def unlinkBug(bug, user=None):
"""Remove any link between this object and the bug.
If a link is removed by this method, an ObjectUnlinkedEvent is
=== modified file 'lib/lp/bugs/model/buglinktarget.py'
--- lib/lp/bugs/model/buglinktarget.py 2015-09-28 07:57:17 +0000
+++ lib/lp/bugs/model/buglinktarget.py 2015-09-29 06:06:37 +0000
@@ -13,7 +13,6 @@
IObjectLinkedEvent,
IObjectUnlinkedEvent,
)
-from lp.services.webapp.authorization import check_permission
# XXX wgrant 2015-09-25: lazr.lifecycle.event.LifecyleEventBase is all
@@ -51,15 +50,9 @@
# IBugLinkTarget implementation
def linkBug(self, bug, user=None):
"""See IBugLinkTarget."""
- # XXX gmb 2007-12-11 bug=175545:
- # We shouldn't be calling check_permission here. The user's
- # permissions should have been checked before this method
- # was called. Also, we shouldn't be relying on the logged-in
- # user in this method; the method should accept a user
- # parameter.
- if not check_permission('launchpad.View', bug):
+ if not bug.userCanView(user):
raise Unauthorized(
- "cannot link to a private bug you don't have access to")
+ "Cannot link a private bug you don't have access to")
if bug in self.bugs:
return False
self.createBugLink(bug)
@@ -69,15 +62,9 @@
def unlinkBug(self, bug, user=None):
"""See IBugLinkTarget."""
- # XXX gmb 2007-12-11 bug=175545:
- # We shouldn't be calling check_permission here. The user's
- # permissions should have been checked before this method
- # was called. Also, we shouldn't be relying on the logged-in
- # user in this method; the method should accept a user
- # parameter.
- if not check_permission('launchpad.View', bug):
+ if not bug.userCanView(user):
raise Unauthorized(
- "cannot unlink a private bug you don't have access to")
+ "Cannot unlink a private bug you don't have access to")
if bug not in self.bugs:
return False
self.deleteBugLink(bug)
=== modified file 'lib/lp/bugs/scripts/checkwatches/tests/test_core.py'
--- lib/lp/bugs/scripts/checkwatches/tests/test_core.py 2012-01-20 15:42:44 +0000
+++ lib/lp/bugs/scripts/checkwatches/tests/test_core.py 2015-09-29 06:06:37 +0000
@@ -43,7 +43,6 @@
from lp.services.config import config
from lp.services.log.logger import BufferLogger
from lp.testing import (
- login,
TestCaseWithFactory,
ZopeTestInSubProcess,
)
@@ -288,13 +287,9 @@
bug_with_question = getUtility(IBugSet).get(10)
question = getUtility(IQuestionSet).get(1)
- # XXX gmb 2007-12-11 bug 175545:
- # We shouldn't have to login() here, but since
- # database.buglinktarget.BugLinkTargetMixin.linkBug()
- # doesn't accept a user parameter, instead depending on the
- # currently logged in user, we get an exception if we don't.
- login('test@xxxxxxxxxxxxx')
- question.linkBug(bug_with_question)
+ sample_person = getUtility(IPersonSet).getByEmail(
+ 'test@xxxxxxxxxxxxx')
+ question.linkBug(bug_with_question, sample_person)
# We subscribe launchpad_developers to the question since this
# indirectly subscribes foo.bar@xxxxxxxxxxxxx to it, too. We can
@@ -309,8 +304,6 @@
# For test_can_update_bug_with_questions we also need a bug
# watch and by extension a bug tracker.
- sample_person = getUtility(IPersonSet).getByEmail(
- 'test@xxxxxxxxxxxxx')
bugtracker = new_bugtracker(BugTrackerType.ROUNDUP)
self.bugtask_with_question = getUtility(IBugTaskSet).createTask(
bug_with_question, sample_person,
=== modified file 'lib/lp/bugs/tests/buglinktarget.txt'
--- lib/lp/bugs/tests/buglinktarget.txt 2015-09-25 10:15:37 +0000
+++ lib/lp/bugs/tests/buglinktarget.txt 2015-09-29 06:06:37 +0000
@@ -87,13 +87,16 @@
>>> private_bug = bugset.get(6)
>>> private_bug.setPrivate(True, factory.makePerson())
True
- >>> target.linkBug(private_bug)
+ >>> target.linkBug(private_bug, factory.makePerson())
Traceback (most recent call last):
...
Unauthorized...
+ >>> from lp.registry.interfaces.person import IPersonSet
+ >>> admin = getUtility(IPersonSet).getByEmail('admin@xxxxxxxxxxxxx')
>>> login('foo.bar@xxxxxxxxxxxxx')
- >>> private_link = target.linkBug(private_bug)
+ >>> target.linkBug(private_bug, admin)
+ True
== bugs ==
@@ -111,7 +114,7 @@
This method is only available to registered users:
>>> login(ANONYMOUS)
- >>> target.unlinkBug(bug2)
+ >>> target.unlinkBug(bug2, None)
Traceback (most recent call last):
...
Unauthorized...
@@ -126,7 +129,7 @@
... Interface, IObjectUnlinkedEvent,
... lambda object, event: unlinked_events.append(event))
- >>> target.unlinkBug(bug1)
+ >>> target.unlinkBug(bug1, factory.makePerson())
True
>>> unlinked_events[-2].object == bug1
True
@@ -158,7 +161,7 @@
Unauthorized...
>>> login('foo.bar@xxxxxxxxxxxxx')
- >>> target.unlinkBug(private_bug) == private_link
+ >>> target.unlinkBug(private_bug, admin)
True
== Cleanup ==
=== modified file 'lib/lp/coop/answersbugs/browser.py'
--- lib/lp/coop/answersbugs/browser.py 2012-01-01 02:58:52 +0000
+++ lib/lp/coop/answersbugs/browser.py 2015-09-29 06:06:37 +0000
@@ -68,7 +68,7 @@
params = CreateBugParams(
owner=self.user, title=data['title'], comment=data['description'])
bug = question.target.createBug(params)
- question.linkBug(bug)
+ question.linkBug(bug, user=self.user)
bug.subscribe(question.owner, self.user)
bug_added_event = ObjectModifiedEvent(
question, unmodifed_question, ['bugs'])
=== modified file 'lib/lp/coop/answersbugs/configure.zcml'
--- lib/lp/coop/answersbugs/configure.zcml 2015-09-25 10:04:56 +0000
+++ lib/lp/coop/answersbugs/configure.zcml 2015-09-29 06:06:37 +0000
@@ -12,12 +12,22 @@
<subscriber
for="lp.bugs.interfaces.bugtask.IBugTask
lazr.lifecycle.interfaces.IObjectModifiedEvent"
- handler=".notification.dispatch_linked_question_notifications"
- />
-
- <subscriber
- for="lp.answers.interfaces.question.IQuestion lp.bugs.interfaces.buglink.IObjectLinkedEvent"
- handler=".karma.question_bug_linked"
+ handler=".subscribers.dispatch_linked_question_notifications"
+ />
+
+ <subscriber
+ for="lp.answers.interfaces.question.IQuestion lp.bugs.interfaces.buglink.IObjectLinkedEvent"
+ handler=".subscribers.assign_question_bug_link_karma"
+ />
+
+ <subscriber
+ for="lp.answers.interfaces.question.IQuestion lp.bugs.interfaces.buglink.IObjectLinkedEvent"
+ handler=".subscribers.subscribe_owner_to_bug"
+ />
+
+ <subscriber
+ for="lp.answers.interfaces.question.IQuestion lp.bugs.interfaces.buglink.IObjectUnlinkedEvent"
+ handler=".subscribers.unsubscribe_owner_from_bug"
/>
<browser:page
=== removed file 'lib/lp/coop/answersbugs/karma.py'
--- lib/lp/coop/answersbugs/karma.py 2015-09-25 08:07:28 +0000
+++ lib/lp/coop/answersbugs/karma.py 1970-01-01 00:00:00 +0000
@@ -1,20 +0,0 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Karma for the Answer Tracker / Bugs extension."""
-
-__metaclass__ = type
-__all__ = []
-
-from lp.answers.karma import assignKarmaUsingQuestionContext
-from lp.bugs.interfaces.bug import IBug
-from lp.registry.interfaces.person import IPerson
-from lp.services.database.sqlbase import block_implicit_flushes
-
-
-@block_implicit_flushes
-def question_bug_linked(questionbug, event):
- """Assign karma to the user which added <questionbug>."""
- if IBug.providedBy(event.other_object):
- assignKarmaUsingQuestionContext(
- IPerson(event.user), event.object, 'questionlinkedtobug')
=== renamed file 'lib/lp/coop/answersbugs/notification.py' => 'lib/lp/coop/answersbugs/subscribers.py'
--- lib/lp/coop/answersbugs/notification.py 2015-07-10 15:31:28 +0000
+++ lib/lp/coop/answersbugs/subscribers.py 2015-09-29 06:06:37 +0000
@@ -8,12 +8,38 @@
from lazr.lifecycle.interfaces import IObjectModifiedEvent
+from lp.answers.karma import assignKarmaUsingQuestionContext
from lp.answers.notification import QuestionNotification
+from lp.bugs.interfaces.bug import IBug
from lp.bugs.interfaces.bugtask import IBugTask
+from lp.registry.interfaces.person import IPerson
+from lp.services.database.sqlbase import block_implicit_flushes
from lp.services.mail.helpers import get_email_template
from lp.services.webapp.publisher import canonical_url
+@block_implicit_flushes
+def assign_question_bug_link_karma(question, event):
+ """Assign karma to the user which added <questionbug>."""
+ if IBug.providedBy(event.other_object):
+ assignKarmaUsingQuestionContext(
+ IPerson(event.user), event.object, 'questionlinkedtobug')
+
+
+def subscribe_owner_to_bug(question, event):
+ """Subscribe a question's owner when it's linked to a bug."""
+ if IBug.providedBy(event.other_object):
+ if not event.other_object.isSubscribed(question.owner):
+ event.other_object.subscribe(question.owner, question.owner)
+
+
+def unsubscribe_owner_from_bug(question, event):
+ """Unsubscribe a question's owner when it's unlinked from a bug."""
+ if IBug.providedBy(event.other_object):
+ if event.other_object.isSubscribed(question.owner):
+ event.other_object.unsubscribe(question.owner, question.owner)
+
+
def dispatch_linked_question_notifications(bugtask, event):
"""Send notifications to linked question subscribers when the bugtask
status change.
=== modified file 'lib/lp/coop/answersbugs/tests/test_questionbug.py'
--- lib/lp/coop/answersbugs/tests/test_questionbug.py 2015-09-28 07:39:28 +0000
+++ lib/lp/coop/answersbugs/tests/test_questionbug.py 2015-09-29 06:06:37 +0000
@@ -43,3 +43,60 @@
self.assertContentEqual([bug1], question2.bugs)
self.assertContentEqual([question2], bug1.questions)
self.assertContentEqual([], bug2.questions)
+
+ def test_link_subscribes_creator_to_bug(self):
+ login_person(self.factory.makePerson())
+ bug = self.factory.makeBug()
+ question = self.factory.makeQuestion()
+ self.assertFalse(bug.isSubscribed(question.owner))
+
+ # Linking a bug to a question subscribes the question's creator
+ # to the bug.
+ question.linkBug(bug)
+ self.assertTrue(bug.isSubscribed(question.owner))
+
+ # If the creator manually unsubscribes, recreating the existing
+ # link does nothing.
+ bug.unsubscribe(question.owner, question.owner)
+ self.assertFalse(bug.isSubscribed(question.owner))
+ question.linkBug(bug)
+ self.assertFalse(bug.isSubscribed(question.owner))
+
+ def test_link_copes_with_existing_subscription(self):
+ # Linking a bug to a question doesn't complain if the creator is
+ # already subscriber.
+ login_person(self.factory.makePerson())
+ bug = self.factory.makeBug()
+ question = self.factory.makeQuestion()
+ bug.subscribe(question.owner, question.owner)
+ self.assertTrue(bug.isSubscribed(question.owner))
+ question.linkBug(bug)
+ self.assertTrue(bug.isSubscribed(question.owner))
+
+ def test_unlink_unsubscribes_creator_from_bug(self):
+ login_person(self.factory.makePerson())
+ bug = self.factory.makeBug()
+ question = self.factory.makeQuestion()
+ question.linkBug(bug)
+ self.assertTrue(bug.isSubscribed(question.owner))
+
+ # Unlinking the bug unsubscribes the question's creator.
+ question.unlinkBug(bug)
+ self.assertFalse(bug.isSubscribed(question.owner))
+
+ # Reunlinking an unlinked bug doesn't unsubscribe.
+ bug.subscribe(question.owner, question.owner)
+ question.unlinkBug(bug)
+ self.assertTrue(bug.isSubscribed(question.owner))
+
+ def test_unlink_copes_with_no_subscription(self):
+ # Unlinking a bug from a question doesn't complain if the
+ # creator isn't subscribed.
+ login_person(self.factory.makePerson())
+ bug = self.factory.makeBug()
+ question = self.factory.makeQuestion()
+ question.linkBug(bug)
+ bug.unsubscribe(question.owner, question.owner)
+ self.assertFalse(bug.isSubscribed(question.owner))
+ question.unlinkBug(bug)
+ self.assertFalse(bug.isSubscribed(question.owner))
Follow ups
