launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20680
[Merge] lp:~cjwatson/launchpad/bug-bmp-activity into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/bug-bmp-activity into lp:launchpad with lp:~cjwatson/launchpad/bmp-buglinktarget-git as a prerequisite.
Commit message:
Record merge proposal links in bug activity logs and karma.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1492926 in Launchpad itself: "Git merge proposals can't be linked to bugs"
https://bugs.launchpad.net/launchpad/+bug/1492926
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bug-bmp-activity/+merge/298367
Record merge proposal links in bug activity logs and karma.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bug-bmp-activity into lp:launchpad.
=== modified file 'lib/lp/bugs/adapters/bugchange.py'
--- lib/lp/bugs/adapters/bugchange.py 2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/adapters/bugchange.py 2016-06-25 09:48:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Implementations for bug changes."""
@@ -9,16 +9,10 @@
'ATTACHMENT_REMOVED',
'BRANCH_LINKED',
'BRANCH_UNLINKED',
+ 'BranchLinkedToBug',
+ 'BranchUnlinkedFromBug',
'BUG_WATCH_ADDED',
'BUG_WATCH_REMOVED',
- 'CHANGED_DUPLICATE_MARKER',
- 'CVE_LINKED',
- 'CVE_UNLINKED',
- 'MARKED_AS_DUPLICATE',
- 'REMOVED_DUPLICATE_MARKER',
- 'REMOVED_SUBSCRIBER',
- 'BranchLinkedToBug',
- 'BranchUnlinkedFromBug',
'BugAttachmentChange',
'BugConvertedToQuestion',
'BugDescriptionChange',
@@ -36,12 +30,22 @@
'BugTitleChange',
'BugWatchAdded',
'BugWatchRemoved',
+ 'CHANGED_DUPLICATE_MARKER',
+ 'CVE_LINKED',
+ 'CVE_UNLINKED',
'CveLinkedToBug',
'CveUnlinkedFromBug',
+ 'get_bug_change_class',
+ 'get_bug_changes',
+ 'MARKED_AS_DUPLICATE',
+ 'MERGE_PROPOSAL_LINKED',
+ 'MERGE_PROPOSAL_UNLINKED',
+ 'MergeProposalLinkedToBug',
+ 'MergeProposalUnlinkedFromBug',
+ 'REMOVED_DUPLICATE_MARKER',
+ 'REMOVED_SUBSCRIBER',
'SeriesNominated',
'UnsubscribedFromBug',
- 'get_bug_change_class',
- 'get_bug_changes',
]
from textwrap import dedent
@@ -73,6 +77,8 @@
CVE_LINKED = 'cve linked'
CVE_UNLINKED = 'cve unlinked'
MARKED_AS_DUPLICATE = 'marked as duplicate'
+MERGE_PROPOSAL_LINKED = 'merge proposal linked'
+MERGE_PROPOSAL_UNLINKED = 'merge proposal unlinked'
REMOVED_DUPLICATE_MARKER = 'removed duplicate marker'
REMOVED_SUBSCRIBER = 'removed subscriber'
@@ -395,6 +401,56 @@
return {'text': '** Branch unlinked: %s' % self.branch.bzr_identity}
+class MergeProposalLinkedToBug(BugChangeBase):
+ """A merge proposal got linked to the bug."""
+
+ def __init__(self, when, person, merge_proposal, bug):
+ super(MergeProposalLinkedToBug, self).__init__(when, person)
+ self.merge_proposal = merge_proposal
+ self.bug = bug
+
+ def getBugActivity(self):
+ """See `IBugChange`."""
+ if self.merge_proposal.private:
+ return None
+ return dict(
+ whatchanged=MERGE_PROPOSAL_LINKED,
+ newvalue=canonical_url(self.merge_proposal))
+
+ def getBugNotification(self):
+ """See `IBugChange`."""
+ if self.merge_proposal.private or self.bug.is_complete:
+ return None
+ return {
+ 'text': '** Merge proposal linked: %s' % (
+ canonical_url(self.merge_proposal))}
+
+
+class MergeProposalUnlinkedFromBug(BugChangeBase):
+ """A merge proposal got unlinked from the bug."""
+
+ def __init__(self, when, person, merge_proposal, bug):
+ super(MergeProposalUnlinkedFromBug, self).__init__(when, person)
+ self.merge_proposal = merge_proposal
+ self.bug = bug
+
+ def getBugActivity(self):
+ """See `IBugChange`."""
+ if self.merge_proposal.private:
+ return None
+ return dict(
+ whatchanged=MERGE_PROPOSAL_UNLINKED,
+ oldvalue=canonical_url(self.merge_proposal))
+
+ def getBugNotification(self):
+ """See `IBugChange`."""
+ if self.merge_proposal.private or self.bug.is_complete:
+ return None
+ return {
+ 'text': '** Merge proposal unlinked: %s' % (
+ canonical_url(self.merge_proposal))}
+
+
class BugDescriptionChange(AttributeChange):
"""Describes a change to a bug's description."""
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2016-06-25 09:48:33 +0000
+++ lib/lp/bugs/configure.zcml 2016-06-25 09:48:33 +0000
@@ -86,6 +86,9 @@
for="lp.bugs.interfaces.bug.IBug lp.bugs.interfaces.buglink.IObjectLinkedEvent"
handler="lp.bugs.subscribers.karma.branch_linked"/>
<subscriber
+ for="lp.bugs.interfaces.bug.IBug lp.bugs.interfaces.buglink.IObjectLinkedEvent"
+ handler="lp.bugs.subscribers.karma.merge_proposal_linked"/>
+ <subscriber
for="lp.bugs.interfaces.bugmessage.IBugMessage lazr.lifecycle.interfaces.IObjectCreatedEvent"
handler="lp.bugs.subscribers.bug.notify_bug_comment_added"/>
<subscriber
@@ -819,6 +822,12 @@
for="lp.bugs.interfaces.bug.IBug lp.bugs.interfaces.buglink.IObjectUnlinkedEvent"
handler="lp.bugs.subscribers.bugactivity.record_cve_unlinked_from_bug"/>
<subscriber
+ for="lp.bugs.interfaces.bug.IBug lp.bugs.interfaces.buglink.IObjectLinkedEvent"
+ handler="lp.bugs.subscribers.bugactivity.record_merge_proposal_linked_to_bug"/>
+ <subscriber
+ for="lp.bugs.interfaces.bug.IBug lp.bugs.interfaces.buglink.IObjectUnlinkedEvent"
+ handler="lp.bugs.subscribers.bugactivity.record_merge_proposal_unlinked_from_bug"/>
+ <subscriber
for="lp.bugs.interfaces.bugsubscription.IBugSubscription zope.lifecycleevent.interfaces.IObjectCreatedEvent"
handler="lp.bugs.subscribers.bugactivity.record_bugsubscription_added"/>
<subscriber
=== modified file 'lib/lp/bugs/doc/malone-karma.txt'
--- lib/lp/bugs/doc/malone-karma.txt 2015-10-08 09:46:08 +0000
+++ lib/lp/bugs/doc/malone-karma.txt 2016-06-25 09:48:33 +0000
@@ -59,6 +59,14 @@
Karma added: action=bugcverefadded, distribution=debian
True
+Link a merge proposal to a bug:
+
+ >>> dsp = factory.makeDistributionSourcePackage(distribution=debian)
+ >>> merge_proposal = factory.makeBranchMergeProposalForGit(target=dsp)
+ Karma added: action=branchmergeproposed, distribution=debian
+ >>> bug.linkMergeProposal(merge_proposal, getUtility(ILaunchBag).user)
+ Karma added: action=bugbranchcreated, distribution=debian
+
Add watch for external bug to the bug:
>>> from lp.bugs.interfaces.bugtracker import IBugTrackerSet
@@ -245,7 +253,7 @@
>>> link_change = getUtility(
... IKarmaActionSet).getByName('bugextrefadded')
>>> karma_helper.added_karma_actions.add(link_change)
- >>> karma_helper.added_karma_actions == set(bugs_karma_actions)
+ >>> karma_helper.added_karma_actions.issuperset(bugs_karma_actions)
True
Unregister the event listener to make sure we won't interfere in other tests.
=== modified file 'lib/lp/bugs/model/bugactivity.py'
--- lib/lp/bugs/model/bugactivity.py 2016-02-04 04:52:25 +0000
+++ lib/lp/bugs/model/bugactivity.py 2016-06-25 09:48:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -24,6 +24,8 @@
CVE_LINKED,
CVE_UNLINKED,
MARKED_AS_DUPLICATE,
+ MERGE_PROPOSAL_LINKED,
+ MERGE_PROPOSAL_UNLINKED,
REMOVED_DUPLICATE_MARKER,
REMOVED_SUBSCRIBER,
)
@@ -101,6 +103,8 @@
result = 'watches'
elif result in (CVE_LINKED, CVE_UNLINKED):
result = 'cves'
+ elif result in (MERGE_PROPOSAL_LINKED, MERGE_PROPOSAL_UNLINKED):
+ result = 'linked_merge_proposals'
elif str(result).startswith(REMOVED_SUBSCRIBER):
result = 'removed_subscriber'
elif result == 'summary':
=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
--- lib/lp/bugs/scripts/bugnotification.py 2016-02-04 04:52:25 +0000
+++ lib/lp/bugs/scripts/bugnotification.py 2016-06-25 09:48:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Functions related to sending bug notifications."""
@@ -65,7 +65,8 @@
key = activity.attribute
if activity.target is not None:
key = ':'.join((activity.target, key))
- if key in ('attachments', 'watches', 'cves', 'linked_branches'):
+ if key in ('attachments', 'watches', 'cves', 'linked_branches',
+ 'linked_merge_proposals'):
# We are intentionally leaving bug task bugwatches out of this
# list, so we use the key rather than the activity.attribute.
if activity.oldvalue is not None:
=== modified file 'lib/lp/bugs/subscribers/bugactivity.py'
--- lib/lp/bugs/subscribers/bugactivity.py 2015-09-25 08:50:34 +0000
+++ lib/lp/bugs/subscribers/bugactivity.py 2016-06-25 09:48:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -17,11 +17,14 @@
BugWatchRemoved,
CveLinkedToBug,
CveUnlinkedFromBug,
+ MergeProposalLinkedToBug,
+ MergeProposalUnlinkedFromBug,
)
from lp.bugs.enums import BugNotificationLevel
from lp.bugs.interfaces.bug import IBug
from lp.bugs.interfaces.bugactivity import IBugActivitySet
from lp.bugs.interfaces.cve import ICve
+from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
from lp.registry.enums import PersonVisibility
from lp.registry.interfaces.milestone import IMilestone
from lp.registry.interfaces.person import IPerson
@@ -128,6 +131,28 @@
@block_implicit_flushes
+def record_merge_proposal_linked_to_bug(bug, event):
+ """Record when a merge proposal is linked to a bug."""
+ if not IBranchMergeProposal.providedBy(event.other_object):
+ return
+ bug.addChange(
+ MergeProposalLinkedToBug(
+ when=None, person=IPerson(event.user),
+ merge_proposal=event.other_object, bug=event.object))
+
+
+@block_implicit_flushes
+def record_merge_proposal_unlinked_from_bug(bug, event):
+ """Record when a merge proposal is unlinked from a bug."""
+ if not IBranchMergeProposal.providedBy(event.other_object):
+ return
+ bug.addChange(
+ MergeProposalUnlinkedFromBug(
+ when=None, person=IPerson(event.user),
+ merge_proposal=event.other_object, bug=event.object))
+
+
+@block_implicit_flushes
def record_bugsubscription_added(bugsubscription_added, object_created_event):
subscribed_user = bugsubscription_added.person
if subscribed_user.visibility == PersonVisibility.PUBLIC:
=== modified file 'lib/lp/bugs/subscribers/karma.py'
--- lib/lp/bugs/subscribers/karma.py 2016-02-04 12:37:59 +0000
+++ lib/lp/bugs/subscribers/karma.py 2016-06-25 09:48:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Assign karma for bugs domain activity."""
@@ -128,3 +128,13 @@
return
event.other_object.target.assignKarma(
IPerson(event.user), 'bugbranchcreated')
+
+
+@block_implicit_flushes
+def merge_proposal_linked(bug, event):
+ """Assign karma to the user who linked the bug to the merge proposal."""
+ from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
+ if not IBranchMergeProposal.providedBy(event.other_object):
+ return
+ event.other_object.target.assignKarma(
+ IPerson(event.user), 'bugbranchcreated')
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2016-01-26 15:47:37 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2016-06-25 09:48:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for recording changes done to a bug."""
@@ -555,6 +555,115 @@
self.bug.unlinkBranch(branch, self.user)
self.assertRecordedChange()
+ def test_link_merge_proposal(self):
+ # Linking a merge proposal to a bug adds both to the activity log
+ # and sends an email notification.
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ self.bug.linkMergeProposal(bmp, self.user)
+
+ # This checks the activity's attribute and target attributes.
+ activity = self.bug.activity[-1]
+ self.assertEqual(activity.attribute, 'linked_merge_proposals')
+ self.assertEqual(activity.target, None)
+
+ added_activity = {
+ 'person': self.user,
+ 'whatchanged': 'merge proposal linked',
+ 'newvalue': canonical_url(bmp),
+ }
+ added_notification = {
+ 'text': "** Merge proposal linked: %s" % canonical_url(bmp),
+ 'person': self.user,
+ }
+ self.assertRecordedChange(
+ expected_activity=added_activity,
+ expected_notification=added_notification)
+
+ def test_link_merge_proposal_to_complete_bug(self):
+ # Linking a merge proposal to a bug that is "complete" (see
+ # IBug.is_complete) adds to the activity log but does *not* send an
+ # email notification.
+ for bug_task in self.bug.bugtasks:
+ bug_task.transitionToStatus(
+ BugTaskStatus.FIXRELEASED, user=self.user)
+ self.assertTrue(self.bug.is_complete)
+ self.saveOldChanges()
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ self.bug.linkMergeProposal(bmp, self.user)
+ expected_activity = {
+ 'person': self.user,
+ 'whatchanged': 'merge proposal linked',
+ 'newvalue': canonical_url(bmp),
+ }
+ self.assertRecordedChange(
+ expected_activity=expected_activity)
+
+ def test_link_private_merge_proposal(self):
+ # Linking a *private* merge proposal to a bug adds *nothing* to the
+ # activity log and does *not* send an email notification.
+ [git_ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ bmp = self.factory.makeBranchMergeProposalForGit(source_ref=git_ref)
+ self.bug.linkMergeProposal(bmp, self.user)
+ self.assertRecordedChange()
+
+ def test_unlink_merge_proposal(self):
+ # Unlinking a merge proposal from a bug adds both to the activity
+ # log and sends an email notification.
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ self.bug.linkMergeProposal(bmp, self.user)
+ self.saveOldChanges()
+ self.bug.unlinkMergeProposal(bmp, self.user)
+
+ # This checks the activity's attribute and target attributes.
+ activity = self.bug.activity[-1]
+ self.assertEqual(activity.attribute, 'linked_merge_proposals')
+ self.assertEqual(activity.target, None)
+
+ added_activity = {
+ 'person': self.user,
+ 'whatchanged': 'merge proposal unlinked',
+ 'oldvalue': canonical_url(bmp),
+ }
+ added_notification = {
+ 'text': "** Merge proposal unlinked: %s" % canonical_url(bmp),
+ 'person': self.user,
+ }
+ self.assertRecordedChange(
+ expected_activity=added_activity,
+ expected_notification=added_notification)
+
+ def test_unlink_merge_proposal_from_complete_bug(self):
+ # Unlinking a merge proposal from a bug that is "complete" (see
+ # IBug.is_complete) adds to the activity log but does *not* send an
+ # email notification.
+ for bug_task in self.bug.bugtasks:
+ bug_task.transitionToStatus(
+ BugTaskStatus.FIXRELEASED, user=self.user)
+ self.assertTrue(self.bug.is_complete)
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ self.bug.linkMergeProposal(bmp, self.user)
+ self.saveOldChanges()
+ self.bug.unlinkMergeProposal(bmp, self.user)
+ expected_activity = {
+ 'person': self.user,
+ 'whatchanged': 'merge proposal unlinked',
+ 'oldvalue': canonical_url(bmp),
+ }
+ self.assertRecordedChange(
+ expected_activity=expected_activity)
+
+ def test_unlink_private_merge_proposal(self):
+ # Unlinking a *private* merge proposal from a bug adds *nothing* to
+ # the activity log and does *not* send an email notification.
+ [git_ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ bmp = self.factory.makeBranchMergeProposalForGit(source_ref=git_ref)
+ self.bug.linkMergeProposal(bmp, self.user)
+ self.saveOldChanges()
+ self.bug.unlinkMergeProposal(bmp, self.user)
+ self.assertRecordedChange()
+
def test_change_information_type(self):
# Changing the information type of a bug adds items to the activity
# log and notifications.
Follow ups
