launchpad-reviewers team mailing list archive

[Vaishnavi Asawale <mp+494979@xxxxxxxxxxxxxxxxxx>]

Vaishnavi Asawale has proposed merging ~vaishnavi-asawale/launchpad:binary-package-upload-webhooks into launchpad:master.

Commit message:
Add webhooks for binary package uploads

In this MP:
1) Add subscriber and handler functions to trigger webhooks when the
binary package upload status changes to APPROVED/UNAPPROVED/REJECTED.
2)) Add tests to ensure webhooks are triggered/ not triggered based on whether
the package upload status changes and the feature flag to allow webhooks 
to be configured on archives is set.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~vaishnavi-asawale/launchpad/+git/launchpad/+merge/494979
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~vaishnavi-asawale/launchpad:binary-package-upload-webhooks into launchpad:master.

diff --git a/lib/lp/services/webhooks/interfaces.py b/lib/lp/services/webhooks/interfaces.py
index e922075..e3dca6a 100644
--- a/lib/lp/services/webhooks/interfaces.py
+++ b/lib/lp/services/webhooks/interfaces.py
@@ -74,6 +74,13 @@ WEBHOOK_EVENT_TYPES = {
     "upload rejected",
     "archive:source-package-upload:0.1::unapproved": "Source package"
     "upload unapproved",
+    "archive:binary-package-upload:0.1": "Binary package upload",
+    "archive:binary-package-upload:0.1::accepted": "Binary package"
+    " upload accepted",
+    "archive:binary-package-upload:0.1::rejected": "Binary package"
+    " upload rejected",
+    "archive:binary-package-upload:0.1::unapproved": "Binary package"
+    " upload unapproved",
     "archive:binary-build:0.1": "Binary build",
     "archive:binary-build:0.1::fullybuilt": "Binary build fully built",
     "archive:binary-build:0.1::failedtobuild": "Binary build failed to build",
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index e5d8392..a5f74bb 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -237,6 +237,10 @@ class Archive(StormBase, WebhookTargetMixin):
             "archive:source-package-upload:0.1::accepted",
             "archive:source-package-upload:0.1::rejected",
             "archive:source-package-upload:0.1::unapproved",
+            "archive:binary-package-upload:0.1",
+            "archive:binary-package-upload:0.1::accepted",
+            "archive:binary-package-upload:0.1::rejected",
+            "archive:binary-package-upload:0.1::unapproved",
             "archive:binary-build:0.1",
             "archive:binary-build:0.1::fullybuilt",
             "archive:binary-build:0.1::failedtobuild",
diff --git a/lib/lp/soyuz/subscribers/archive.py b/lib/lp/soyuz/subscribers/archive.py
index a4202de..2c652c7 100644
--- a/lib/lp/soyuz/subscribers/archive.py
+++ b/lib/lp/soyuz/subscribers/archive.py
@@ -34,6 +34,20 @@ def _trigger_source_package_status_change_webhook(upload, event_type):
         getUtility(IWebhookSet).trigger(upload.archive, event_type, payload)
 
 
+def _trigger_binary_package_status_change_webhook(upload, event_type):
+    if getFeatureFlag(ARCHIVE_WEBHOOKS_FEATURE_FLAG):
+        payload = {
+            "package_upload": canonical_url(upload, force_local_path=True),
+            "action": "status-changed",
+            "status": upload.status.name,
+            "archive": canonical_url(upload.archive, force_local_path=True),
+            "source_package_name": str(
+                upload.builds[0].build.source_package_release.sourcepackagename
+            ),
+        }
+        getUtility(IWebhookSet).trigger(upload.archive, event_type, payload)
+
+
 def _trigger_build_status_change_webhook(build, event_type):
     if getFeatureFlag(ARCHIVE_WEBHOOKS_FEATURE_FLAG):
         payload = {
@@ -77,6 +91,23 @@ def package_status_change_webhook(upload, event):
                 f"{upload.status.name.lower()}",
             )
 
+    # For binary packages
+    if upload.builds:
+        if (
+            event.edited_fields
+            and "status" in event.edited_fields
+            and (
+                upload.status == PackageUploadStatus.ACCEPTED
+                or upload.status == PackageUploadStatus.REJECTED
+                or upload.status == PackageUploadStatus.UNAPPROVED
+            )
+        ):
+            _trigger_binary_package_status_change_webhook(
+                upload,
+                f"archive:binary-package-upload:0.1::"
+                f"{upload.status.name.lower()}",
+            )
+
 
 def build_status_change_webhook(build, event):
     """Webhook for binary builds"""
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index 3f8551f..c0327f0 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -7511,6 +7511,158 @@ class TestSourcePackageUploadWebhooks(TestCaseWithFactory):
         self.assertEqual(hook.deliveries.count(), 0)
 
 
+class TestBinaryackageUploadWebhooks(TestCaseWithFactory):
+    """Tests that binary package upload webhooks trigger correctly."""
+
+    layer = LaunchpadZopelessLayer
+
+    def test_binary_package_upload_status_change_triggers_webhook(self):
+        self.useFixture(
+            FeatureFixture(
+                {
+                    ARCHIVE_WEBHOOKS_FEATURE_FLAG: "on",
+                }
+            )
+        )
+        archive = self.factory.makeArchive(name="test-archive")
+        hook = self.factory.makeWebhook(
+            target=archive,
+            delivery_url="http://localhost/test-webhook";,
+            event_types=["archive:binary-package-upload:0.1"],
+        )
+
+        upload = self.factory.makePackageUpload(
+            archive=archive,
+            status=PackageUploadStatus.NEW,
+        )
+
+        upload.addBuild(
+            self.factory.makeBinaryPackageBuild(
+                archive=archive, status=BuildStatus.FULLYBUILT
+            )
+        )
+
+        upload_unwrapped = removeSecurityProxy(upload)
+        upload_unwrapped.setAccepted()
+
+        job = hook.deliveries.one()
+        self.assertEqual(job.event_type, "archive:binary-package-upload:0.1")
+
+        payload = job.payload
+        self.assertEqual(
+            payload["package_upload"],
+            canonical_url(upload, force_local_path=True),
+        )
+        self.assertEqual(payload["action"], "status-changed")
+        self.assertEqual(payload["status"], "ACCEPTED")
+        self.assertEqual(
+            payload["archive"],
+            canonical_url(archive, force_local_path=True),
+        )
+        self.assertIsNotNone(
+            upload.builds[0].build.source_package_release.sourcepackagename
+        )
+
+    def test_binary_package_upload_no_status_change_does_not_trigger_webhook(
+        self,
+    ):
+        self.useFixture(
+            FeatureFixture(
+                {
+                    ARCHIVE_WEBHOOKS_FEATURE_FLAG: "on",
+                }
+            )
+        )
+        archive = self.factory.makeArchive(name="test-archive")
+        hook = self.factory.makeWebhook(
+            target=archive,
+            delivery_url="http://localhost/test-webhook";,
+            event_types=["archive:binary-package-upload:0.1"],
+        )
+
+        upload = self.factory.makePackageUpload(
+            archive=archive,
+            status=PackageUploadStatus.NEW,
+        )
+
+        upload.addBuild(
+            self.factory.makeBinaryPackageBuild(
+                archive=archive, status=BuildStatus.FULLYBUILT
+            )
+        )
+
+        self.assertEqual(hook.deliveries.count(), 0)
+
+    def test_only_webhook_for_chosen_subscope_is_triggered(self):
+        """If the webhook is configured only for the subscope 'accepted',
+        then rejection should not trigger a webhook, but acceptance should."""
+        self.useFixture(
+            FeatureFixture(
+                {
+                    ARCHIVE_WEBHOOKS_FEATURE_FLAG: "on",
+                }
+            )
+        )
+        archive = self.factory.makeArchive(name="test-archive")
+        hook = self.factory.makeWebhook(
+            target=archive,
+            delivery_url="http://localhost/test-webhook";,
+            event_types=["archive:binary-package-upload:0.1::accepted"],
+        )
+
+        upload = self.factory.makePackageUpload(
+            archive=archive,
+            status=PackageUploadStatus.NEW,
+        )
+
+        upload.addBuild(
+            self.factory.makeBinaryPackageBuild(
+                archive=archive, status=BuildStatus.FULLYBUILT
+            )
+        )
+
+        upload_unwrapped = removeSecurityProxy(upload)
+        upload_unwrapped.setRejected()
+
+        self.assertEqual(hook.deliveries.count(), 0)
+
+        upload_unwrapped.setAccepted()
+
+        job = hook.deliveries.one()
+        self.assertEqual(job.event_type, "archive:binary-package-upload:0.1")
+
+    def test_no_webhook_triggered_when_feature_flag_is_not_on(self):
+        self.useFixture(
+            FeatureFixture(
+                {
+                    ARCHIVE_WEBHOOKS_FEATURE_FLAG: "",
+                }
+            )
+        )
+        archive = self.factory.makeArchive(name="test-archive")
+        hook = self.factory.makeWebhook(
+            target=archive,
+            delivery_url="http://localhost/test-webhook";,
+            event_types=["archive:binary-package-upload:0.1"],
+        )
+
+        upload = self.factory.makePackageUpload(
+            archive=archive,
+            status=PackageUploadStatus.NEW,
+        )
+
+        upload.addBuild(
+            self.factory.makeBinaryPackageBuild(
+                archive=archive, status=BuildStatus.FULLYBUILT
+            )
+        )
+
+        upload_unwrapped = removeSecurityProxy(upload)
+        upload_unwrapped.setAccepted()
+
+        self.assertEqual(hook.deliveries.count(), 0)
+
+
 class TestBinaryBuildFinishWebhooks(TestCaseWithFactory):
     """Tests that webhooks trigger correctly on binary package
     status change."""