launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29983
[Merge] ~ines-almeida/launchpad:add-bug-webhooks/update-webhook-model into launchpad:master
Ines Almeida has proposed merging ~ines-almeida/launchpad:add-bug-webhooks/update-webhook-model into launchpad:master.
Commit message:
Add new fields to webhook model
Update and add new unit tests to go with the change
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/442543
This MP follows up this DB patch: https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/442541
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:add-bug-webhooks/update-webhook-model into launchpad:master.
diff --git a/lib/lp/services/webhooks/model.py b/lib/lp/services/webhooks/model.py
index 114d953..bafd604 100644
--- a/lib/lp/services/webhooks/model.py
+++ b/lib/lp/services/webhooks/model.py
@@ -19,7 +19,7 @@ import psutil
import transaction
from lazr.delegates import delegate_to
from lazr.enum import DBEnumeratedType, DBItem
-from storm.expr import Desc
+from storm.expr import And, Desc
from storm.properties import JSON, Bool, DateTime, Int, Unicode
from storm.references import Reference
from storm.store import Store
@@ -92,6 +92,17 @@ class Webhook(StormBase):
charm_recipe_id = Int(name="charm_recipe")
charm_recipe = Reference(charm_recipe_id, "CharmRecipe.id")
+ project_id = Int(name="project")
+ project = Reference(project_id, "Product.id")
+
+ distribution_id = Int(name="distribution")
+ distribution = Reference(distribution_id, "Distribution.id")
+
+ source_package_name_id = Int(name="source_package_name")
+ source_package_name = Reference(
+ source_package_name_id, "SourcePackageName.id"
+ )
+
registrant_id = Int(name="registrant", allow_none=False)
registrant = Reference(registrant_id, "Person.id")
date_created = DateTime(tzinfo=timezone.utc, allow_none=False)
@@ -117,6 +128,15 @@ class Webhook(StormBase):
return self.oci_recipe
elif self.charm_recipe is not None:
return self.charm_recipe
+ elif self.project is not None:
+ return self.project
+ elif self.distribution is not None:
+ if self.source_package_name is not None:
+ return self.distribution.getSourcePackage(
+ self.source_package_name
+ )
+ else:
+ return self.distribution
else:
raise AssertionError("No target.")
@@ -178,6 +198,11 @@ class WebhookSet:
from lp.code.interfaces.branch import IBranch
from lp.code.interfaces.gitrepository import IGitRepository
from lp.oci.interfaces.ocirecipe import IOCIRecipe
+ from lp.registry.interfaces.distribution import IDistribution
+ from lp.registry.interfaces.distributionsourcepackage import (
+ IDistributionSourcePackage,
+ )
+ from lp.registry.interfaces.product import IProduct
from lp.snappy.interfaces.snap import ISnap
from lp.soyuz.interfaces.livefs import ILiveFS
@@ -194,6 +219,14 @@ class WebhookSet:
hook.oci_recipe = target
elif ICharmRecipe.providedBy(target):
hook.charm_recipe = target
+ elif IProduct.providedBy(target):
+ hook.project = target
+ elif IDistribution.providedBy(target):
+ hook.distribution = target
+ elif IDistributionSourcePackage.providedBy(target):
+ hook.distribution = target.distribution
+ hook.source_package_name = target.sourcepackagename
+
else:
raise AssertionError("Unsupported target: %r" % (target,))
hook.registrant = registrant
@@ -220,6 +253,11 @@ class WebhookSet:
from lp.code.interfaces.branch import IBranch
from lp.code.interfaces.gitrepository import IGitRepository
from lp.oci.interfaces.ocirecipe import IOCIRecipe
+ from lp.registry.interfaces.distribution import IDistribution
+ from lp.registry.interfaces.distributionsourcepackage import (
+ IDistributionSourcePackage,
+ )
+ from lp.registry.interfaces.product import IProduct
from lp.snappy.interfaces.snap import ISnap
from lp.soyuz.interfaces.livefs import ILiveFS
@@ -235,6 +273,18 @@ class WebhookSet:
target_filter = Webhook.oci_recipe == target
elif ICharmRecipe.providedBy(target):
target_filter = Webhook.charm_recipe == target
+ elif IProduct.providedBy(target):
+ target_filter = Webhook.project == target
+ elif IDistribution.providedBy(target):
+ target_filter = And(
+ Webhook.distribution == target,
+ Webhook.source_package_name == None,
+ )
+ elif IDistributionSourcePackage.providedBy(target):
+ target_filter = And(
+ Webhook.distribution == target.distribution,
+ Webhook.source_package_name == target.sourcepackagename,
+ )
else:
raise AssertionError("Unsupported target: %r" % (target,))
return (
diff --git a/lib/lp/services/webhooks/tests/test_model.py b/lib/lp/services/webhooks/tests/test_model.py
index 85d2c40..6a128d6 100644
--- a/lib/lp/services/webhooks/tests/test_model.py
+++ b/lib/lp/services/webhooks/tests/test_model.py
@@ -77,10 +77,13 @@ class TestWebhookPermissions(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
+ def get_target_owner(self, target):
+ return target.owner
+
def test_target_owner_can_view(self):
target = self.factory.makeGitRepository()
webhook = self.factory.makeWebhook(target=target)
- with person_logged_in(target.owner):
+ with person_logged_in(self.get_target_owner(target)):
self.assertTrue(check_permission("launchpad.View", webhook))
def test_random_cannot_view(self):
@@ -140,9 +143,12 @@ class TestWebhookSetBase:
layer = DatabaseFunctionalLayer
+ def get_target_owner(self, target):
+ return target.owner
+
def test_new(self):
target = self.makeTarget()
- login_person(target.owner)
+ login_person(self.get_target_owner(target))
person = self.factory.makePerson()
hook = getUtility(IWebhookSet).new(
target,
@@ -178,7 +184,7 @@ class TestWebhookSetBase:
self.factory.makeWebhook(
target, "http://path/%s/%d"; % (name, i)
)
- with person_logged_in(target1.owner):
+ with person_logged_in(self.get_target_owner(target1)):
self.assertContentEqual(
[
"http://path/one/0";,
@@ -190,7 +196,7 @@ class TestWebhookSetBase:
for hook in getUtility(IWebhookSet).findByTarget(target1)
],
)
- with person_logged_in(target2.owner):
+ with person_logged_in(self.get_target_owner(target2)):
self.assertContentEqual(
[
"http://path/two/0";,
@@ -205,7 +211,7 @@ class TestWebhookSetBase:
def test_delete(self):
target = self.makeTarget()
- login_person(target.owner)
+ login_person(self.get_target_owner(target))
hooks = []
for i in range(3):
hook = self.factory.makeWebhook(target, "http://path/to/%d"; % i)
@@ -237,7 +243,9 @@ class TestWebhookSetBase:
def test__checkVisibility_public_artifact(self):
target = self.makeTarget()
- self.assertTrue(WebhookSet._checkVisibility(target, target.owner))
+ self.assertTrue(
+ WebhookSet._checkVisibility(target, self.get_target_owner(target))
+ )
def test_trigger(self):
owner = self.factory.makePerson()
@@ -520,3 +528,70 @@ class TestWebhookSetCharmRecipe(TestWebhookSetBase, TestCaseWithFactory):
return self.factory.makeCharmRecipe(
registrant=owner, owner=owner, **kwargs
)
+
+
+class TestWebhookSetBugBase(TestWebhookSetBase):
+ def test__checkVisibility_private_artifact(self):
+ owner = self.factory.makePerson()
+ target = self.makeTarget(
+ owner=owner, information_type=InformationType.PROPRIETARY
+ )
+ self.assertTrue(WebhookSet._checkVisibility(target, owner))
+
+ def test_webhook_trigger(self):
+ owner = self.factory.makePerson()
+ target = self.makeTarget(
+ owner=owner,
+ information_type=InformationType.PROPRIETARY,
+ )
+ hook = self.factory.makeWebhook(
+ target=target, event_types=[self.event_type]
+ )
+
+ with admin_logged_in():
+ self.assertThat(list(hook.deliveries), HasLength(0))
+
+ getUtility(IWebhookSet).trigger(
+ target, self.event_type, {"some": "payload"}
+ )
+ with admin_logged_in():
+ self.assertThat(list(hook.deliveries), HasLength(1))
+ delivery = hook.deliveries.one()
+ self.assertEqual(delivery.payload, {"some": "payload"})
+
+
+class TestWebhookSetProductBugUpdate(
+ TestWebhookSetBugBase, TestCaseWithFactory
+):
+
+ event_type = "bug:0.1"
+
+ def makeTarget(self, **kwargs):
+ return self.factory.makeProduct(**kwargs)
+
+
+class TestWebhookSetDistributionBugUpdate(
+ TestWebhookSetBugBase, TestCaseWithFactory
+):
+
+ event_type = "bug:0.1"
+
+ def makeTarget(self, **kwargs):
+ return self.factory.makeDistribution(**kwargs)
+
+
+class TestWebhookSetDistributionSourcePackageBugUpdate(
+ TestWebhookSetBugBase, TestCaseWithFactory
+):
+
+ event_type = "bug:0.1"
+
+ def get_target_owner(self, target):
+ return target.distribution.owner
+
+ def makeTarget(self, **kwargs):
+ with admin_logged_in():
+ distribution = self.factory.makeDistribution(**kwargs)
+ return self.factory.makeDistributionSourcePackage(
+ distribution=distribution
+ )
