launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29992
[Merge] ~ines-almeida/launchpad:add-bug-webhooks/hook-webhooks-up-to-events into launchpad:master
Ines Almeida has proposed merging ~ines-almeida/launchpad:add-bug-webhooks/hook-webhooks-up-to-events into launchpad:master with ~ines-almeida/launchpad:add-bug-webhooks/add-interfaces as a prerequisite.
Commit message:
Subscribe webhooks to creation/modification events
The new bug webhooks are triggered by bug creation/modification and bugtask creation/modification.
The new bug comment webhook is only triggered by comment creation.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/442744
With this change, webhooks can now be fully setup and triggered.
This is based of these 2 other proposals:
- https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/442544
- https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/442543
To test it locally (or later on dogfood):
- Set the feature flag by going into /+feature-rules and adding `webhooks.new.enabled default 0 on`
- Go to a project (or distribution, or distribution source package). You should now see the button 'Manage Webhooks'
- Add a new webhook
- Create a bug and a bug comment for that project
- [Locally] See the logs for a new webhook trigger
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:add-bug-webhooks/hook-webhooks-up-to-events into launchpad:master.
diff --git a/lib/lp/bugs/configure.zcml b/lib/lp/bugs/configure.zcml
index fe55acc..4beb6e0 100644
--- a/lib/lp/bugs/configure.zcml
+++ b/lib/lp/bugs/configure.zcml
@@ -92,6 +92,10 @@
for="lp.bugs.interfaces.bugmessage.IBugMessage lazr.lifecycle.interfaces.IObjectCreatedEvent"
handler="lp.bugs.subscribers.bug.notify_bug_comment_added"/>
<subscriber
+ for="lp.bugs.interfaces.bugmessage.IBugMessage
+ lazr.lifecycle.interfaces.IObjectCreatedEvent"
+ handler="lp.bugs.subscribers.webhooks.bug_comment_added"/>
+ <subscriber
for="lp.bugs.interfaces.bugmessage.IBugMessage lazr.lifecycle.interfaces.IObjectCreatedEvent"
handler="lp.bugs.subscribers.karma.bug_comment_added"/>
<subscriber
@@ -115,6 +119,22 @@
<subscriber
for="lp.bugs.interfaces.bug.IBug lazr.lifecycle.interfaces.IObjectModifiedEvent"
handler="lp.bugs.subscribers.bug.notify_bug_modified"/>
+ <subscriber
+ for="lp.bugs.interfaces.bug.IBug
+ lazr.lifecycle.interfaces.IObjectCreatedEvent"
+ handler="lp.bugs.subscribers.webhooks.bug_created"/>
+ <subscriber
+ for="lp.bugs.interfaces.bug.IBug
+ lazr.lifecycle.interfaces.IObjectModifiedEvent"
+ handler="lp.bugs.subscribers.webhooks.bug_modified"/>
+ <subscriber
+ for="lp.bugs.interfaces.bug.IBugTask
+ lazr.lifecycle.interfaces.IObjectCreatedEvent"
+ handler="lp.bugs.subscribers.webhooks.bugtask_created"/>
+ <subscriber
+ for="lp.bugs.interfaces.bug.IBugTask
+ lazr.lifecycle.interfaces.IObjectModifiedEvent"
+ handler="lp.bugs.subscribers.webhooks.bugtask_modified"/>
<lp:securedutility
class="lp.systemhomes.MaloneApplication"
provides="lp.bugs.interfaces.malone.IMaloneApplication">
diff --git a/lib/lp/bugs/subscribers/webhooks.py b/lib/lp/bugs/subscribers/webhooks.py
new file mode 100644
index 0000000..9b3758a
--- /dev/null
+++ b/lib/lp/bugs/subscribers/webhooks.py
@@ -0,0 +1,122 @@
+# Copyright 2023 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Webhook subscribers for bugs and bugtasks."""
+
+from lazr.lifecycle.interfaces import IObjectCreatedEvent, IObjectModifiedEvent
+from zope.component import getUtility
+
+from lp.bugs.interfaces.bug import IBug
+from lp.bugs.interfaces.bugmessage import IBugMessage
+from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
+from lp.bugs.interfaces.bugtask import IBugTask
+from lp.bugs.subscribers.bugactivity import what_changed
+from lp.services.database.sqlbase import block_implicit_flushes
+from lp.services.features import getFeatureFlag
+from lp.services.webapp.publisher import canonical_url
+from lp.services.webhooks.interfaces import IWebhookSet, IWebhookTarget
+from lp.services.webhooks.payload import compose_webhook_payload
+
+
+@block_implicit_flushes
+def _trigger_bugtask_webhook(bugtask: IBugTask, action: str):
+ """Triggers 'bug' event for a specific bugtask"""
+ if not getFeatureFlag(BUG_WEBHOOKS_FEATURE_FLAG):
+ return
+
+ if IWebhookTarget.providedBy(bugtask.target):
+ payload = create_bugtask_payload(bugtask, action)
+ getUtility(IWebhookSet).trigger(bugtask.target, "bug:0.1", payload)
+
+
+@block_implicit_flushes
+def _trigger_bug_comment_webhook(bug_comment: IBugMessage, action: str):
+ """Triggers 'bug:comment' events for each bug target for that comment"""
+ if not getFeatureFlag(BUG_WEBHOOKS_FEATURE_FLAG):
+ return
+
+ bugtasks = bug_comment.bug.bugtasks
+
+ # We trigger one webhook for each coment's bugtask that has webhooks set up
+ for bugtask in bugtasks:
+ target = bugtask.target
+ if IWebhookTarget.providedBy(target):
+ payload = create_bug_comment_payload(bugtask, bug_comment, action)
+ getUtility(IWebhookSet).trigger(target, "bug:comment:0.1", payload)
+
+
+def create_bugtask_payload(bugtask, action):
+ payload = {
+ "target": canonical_url(bugtask.target, force_local_path=True),
+ "action": action,
+ "bug": canonical_url(bugtask.bug, force_local_path=True),
+ }
+ payload.update(
+ compose_webhook_payload(
+ IBug,
+ bugtask.bug,
+ ["title", "description", "owner"],
+ )
+ )
+ payload.update(
+ compose_webhook_payload(
+ IBugTask,
+ bugtask,
+ ["status", "importance", "assignee", "datecreated"],
+ )
+ )
+ return payload
+
+
+def create_bug_comment_payload(bugtask, bug_comment, action):
+ payload = {
+ "target": canonical_url(bugtask.target, force_local_path=True),
+ "action": action,
+ "bug": canonical_url(bug_comment.bug, force_local_path=True),
+ "bug_comment": canonical_url(bug_comment, force_local_path=True),
+ "content": bug_comment.text_contents,
+ }
+ payload.update(
+ compose_webhook_payload(IBugMessage, bug_comment, ["owner"])
+ )
+ return payload
+
+
+def bug_created(bug: IBug, event: IObjectCreatedEvent):
+ """Trigger a 'created' event when a new Bug is created
+
+ #NOTE When a user creates a new Bug, only the 'bug_created' is called
+ (not 'bugtask_created'). On the other hand, when a new project is added to
+ a bug as being affected by it, then only 'bugtask_created' is called.
+ """
+ for bugtask in bug.bugtasks:
+ _trigger_bugtask_webhook(bugtask, "created")
+
+
+def bug_modified(bug: IBug, event: IObjectCreatedEvent):
+ """Trigger a '<field>-updated' event when a bug is modified"""
+ changed_fields = what_changed(event)
+
+ for field in changed_fields:
+ action = "{}-updated".format(field)
+ for bugtask in bug.bugtasks:
+ _trigger_bugtask_webhook(bugtask, action)
+
+
+def bugtask_created(bugtask: IBugTask, event: IObjectCreatedEvent):
+ """Trigger a 'created' event when a BugTask is created in existing bug"""
+ _trigger_bugtask_webhook(bugtask, "created")
+
+
+def bugtask_modified(bugtask: IBugTask, event: IObjectModifiedEvent):
+ """Trigger a '<field>-updated' event when a BugTask is modified"""
+ changed_fields = what_changed(event)
+
+ for field in changed_fields:
+ action = "{}-updated".format(field)
+ _trigger_bugtask_webhook(bugtask, action)
+
+
+def bug_comment_added(comment, event: IObjectCreatedEvent):
+ """Trigger a 'created' event when a new comment is added to a Bug"""
+ _trigger_bug_comment_webhook(comment, "created")
diff --git a/lib/lp/bugs/tests/test_subscribers.py b/lib/lp/bugs/tests/test_subscribers.py
index 9a398e0..403432f 100644
--- a/lib/lp/bugs/tests/test_subscribers.py
+++ b/lib/lp/bugs/tests/test_subscribers.py
@@ -3,11 +3,18 @@
"""Tests for the BugActivity code."""
-from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.event import ObjectCreatedEvent, ObjectModifiedEvent
from lazr.lifecycle.snapshot import Snapshot
+from testtools.matchers import Equals, MatchesDict, MatchesStructure
+from zope.event import notify
from lp.bugs.interfaces.bug import IBug
+from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
+from lp.bugs.interfaces.bugtask import BugTaskStatus
from lp.bugs.subscribers.bugactivity import what_changed
+from lp.services.features.testing import FeatureFixture
+from lp.services.webapp.publisher import canonical_url
+from lp.services.webapp.snapshot import notify_modified
from lp.testing import TestCaseWithFactory, person_logged_in
from lp.testing.layers import DatabaseFunctionalLayer
@@ -44,3 +51,164 @@ class TestWhatChanged(TestCaseWithFactory):
expected_changes = {"private": ["False", "True"]}
changes = what_changed(event)
self.assertEqual(expected_changes, changes)
+
+
+class TestBugWebhooksTriggered(TestCaseWithFactory):
+ """Tests that bug and bug comment webhooks get triggered"""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super().setUp()
+ self.useFixture(
+ FeatureFixture(
+ {
+ BUG_WEBHOOKS_FEATURE_FLAG: "on",
+ }
+ )
+ )
+ self.target = self.factory.makeProduct()
+ self.owner = self.target.owner
+ self.bugtask = self.factory.makeBugTask(target=self.target)
+ self.hook = self.factory.makeWebhook(
+ target=self.target, event_types=["bug:comment:0.1", "bug:0.1"]
+ )
+
+ def _assert_last_webhook_delivery(self, hook, event, payload):
+ with person_logged_in(self.owner):
+ delivery = hook.deliveries.last()
+ expected_structure = MatchesStructure(
+ event_type=Equals(event),
+ payload=MatchesDict(payload),
+ )
+ self.assertThat(delivery, expected_structure)
+
+ def _build_comment_expected_payload(self, comment, content):
+ return {
+ "action": Equals("created"),
+ "bug_comment": Equals(
+ canonical_url(comment, force_local_path=True)
+ ),
+ "bug": Equals(
+ canonical_url(self.bugtask.bug, force_local_path=True)
+ ),
+ "target": Equals(
+ canonical_url(self.target, force_local_path=True)
+ ),
+ "content": Equals(content),
+ "owner": Equals("/~" + comment.owner.name),
+ }
+
+ def _build_bugtask_expected_payload(self, bugtask, action):
+ assignee = "/~" + bugtask.assignee.name if bugtask.assignee else None
+
+ return {
+ "action": Equals(action),
+ "bug": Equals(canonical_url(bugtask.bug, force_local_path=True)),
+ "target": Equals(
+ canonical_url(self.target, force_local_path=True)
+ ),
+ "title": Equals(bugtask.bug.title),
+ "description": Equals(bugtask.bug.description),
+ "owner": Equals("/~" + bugtask.bug.owner.name),
+ "status": Equals(bugtask.status.title),
+ "importance": Equals(bugtask.importance.title),
+ "assignee": Equals(assignee),
+ "datecreated": Equals(bugtask.datecreated.isoformat()),
+ }
+
+ def test_new_bug_comment_triggers_webhook(self):
+ """Adding a comment to a bug with a webhook, triggers webhook"""
+ comment = self.factory.makeBugComment(
+ bug=self.bugtask.bug, body="test comment"
+ )
+ expected_payload = self._build_comment_expected_payload(
+ comment, content="test comment"
+ )
+ self._assert_last_webhook_delivery(
+ self.hook, "bug:comment:0.1", expected_payload
+ )
+
+ def test_new_bug_triggers_webhook(self):
+ """Adding a bug to a target with a webhook, triggers webhook"""
+ bug = self.factory.makeBug(target=self.target)
+ expected_payload = self._build_bugtask_expected_payload(
+ bug.bugtasks[0], "created"
+ )
+ self._assert_last_webhook_delivery(
+ self.hook, "bug:0.1", expected_payload
+ )
+
+ def test_new_bugtask_triggers_webhook(self):
+ """Adding a new task targeted at our webhook target, will invoke the
+ bug 'created' event"""
+ bug = self.factory.makeBug()
+ with person_logged_in(self.owner):
+ new_task = bug.addTask(owner=self.owner, target=self.target)
+ # The ObjectCreatedEvent would be triggered in BugAlsoAffectsView
+ notify(ObjectCreatedEvent(new_task))
+
+ expected_payload = self._build_bugtask_expected_payload(
+ new_task, "created"
+ )
+ self._assert_last_webhook_delivery(
+ self.hook, "bug:0.1", expected_payload
+ )
+
+ def test_bug_modified_triggers_webhook(self):
+ """Modifying the bug fields, will trigger webhook"""
+ bug = self.bugtask.bug
+ with person_logged_in(self.owner), notify_modified(
+ bug, ["title"], user=self.owner
+ ):
+ bug.title = "new title"
+ expected_payload = self._build_bugtask_expected_payload(
+ self.bugtask, "title-updated"
+ )
+ self._assert_last_webhook_delivery(
+ self.hook, "bug:0.1", expected_payload
+ )
+
+ def test_bugtask_modified_triggers_webhook(self):
+ """Modifying the bug task fields, will trigger webhook"""
+ with person_logged_in(self.owner):
+ self.bugtask.bug.setStatus(
+ self.target, BugTaskStatus.FIXRELEASED, self.owner
+ )
+ expected_payload = self._build_bugtask_expected_payload(
+ self.bugtask, "status-updated"
+ )
+ self._assert_last_webhook_delivery(
+ self.hook, "bug:0.1", expected_payload
+ )
+
+ def test_webhook_subscription(self):
+ """Check that a webhook only subscribed to the 'bug:0.1' event, does
+ not get a 'bug:comment:0.1' event, and vice-versa"""
+ bug_hook = self.factory.makeWebhook(
+ target=self.target, event_types=["bug:0.1"]
+ )
+ comment_hook = self.factory.makeWebhook(
+ target=self.target, event_types=["bug:comment:0.1"]
+ )
+
+ bug = self.factory.makeBug(target=self.target)
+ comment = self.factory.makeBugComment(
+ bug=self.bugtask.bug, body="test comment"
+ )
+
+ b_exptd_payload = self._build_bugtask_expected_payload(
+ bug.bugtasks[0], "created"
+ )
+ c_exptd_payload = self._build_comment_expected_payload(
+ comment,
+ content="test comment",
+ )
+
+ self._assert_last_webhook_delivery(
+ bug_hook, "bug:0.1", b_exptd_payload
+ )
+
+ self._assert_last_webhook_delivery(
+ comment_hook, "bug:comment:0.1", c_exptd_payload
+ )
