launchpad-reviewers team mailing list archive

[Guruprasad <mp+494750@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
> index 85a37d0..57f5f2b 100644
> --- a/lib/lp/services/features/flags.py
> +++ b/lib/lp/services/features/flags.py
> @@ -349,6 +349,14 @@ flag_info = sorted(
>              "",
>              "",
>          ),
> +        (
> +            "archive.webhooks.enabled",

Do we need a finer granularity of control here since this covers code and trigger scenarios executed in different units?

> +            "boolean",
> +            "If true, allow webhooks to be configured on archives",
> +            "enabled",
> +            "",
> +            "",
> +        ),
>      ]
>  )
>  
> diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
> index bffac3e..d57c39b 100644
> --- a/lib/lp/soyuz/tests/test_archive.py
> +++ b/lib/lp/soyuz/tests/test_archive.py
> @@ -7355,3 +7358,89 @@ class TestArchiveWebhooks(TestCaseWithFactory):
>              archive.delete(archive.owner)
>              transaction.commit()
>              self.assertRaises(LostObjectError, getattr, webhook, "event_types")
> +
> +
> +class TestSourcePackageUploadWebhooks(TestCaseWithFactory):

Can you add a test or two for verifying the behaviour with the feature rule `off`?

> +    """Tests that source package upload webhooks fire correctly."""
> +
> +    layer = LaunchpadZopelessLayer
> +
> +    def test_source_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:source-package-upload:0.1"],
> +        )
> +
> +        upload = self.factory.makePackageUpload(
> +            archive=archive,
> +            status=PackageUploadStatus.NEW,
> +        )
> +        upload.addSource(
> +            self.factory.makeSourcePackageRelease(
> +                sourcepackagename=self.factory.makeSourcePackageName(
> +                    name="mypkg"
> +                ),
> +                version="1.0.0",
> +            )
> +        )
> +
> +        upload_unwrapped = removeSecurityProxy(upload)
> +        upload_unwrapped.setAccepted()
> +
> +        job = hook.deliveries.one()
> +        self.assertEqual(job.event_type, "archive:source-package-upload:0.1")
> +
> +        payload = job.payload
> +        self.assertEqual(
> +            payload["package_upload"],
> +            canonical_url(upload, force_local_path=True),
> +        )
> +        self.assertEqual(payload["action"], "source-package-upload-accepted")
> +        self.assertEqual(payload["status"], "ACCEPTED")
> +        self.assertEqual(
> +            payload["archive"],
> +            canonical_url(archive, force_local_path=True),
> +        )
> +        self.assertEqual(payload["package_name"], "mypkg")
> +        self.assertEqual(payload["package_version"], "1.0.0")
> +
> +    def test_source_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:source-package-upload:0.1"],
> +        )
> +
> +        upload = self.factory.makePackageUpload(
> +            archive=archive,
> +            status=PackageUploadStatus.NEW,
> +        )
> +        upload.addSource(
> +            self.factory.makeSourcePackageRelease(
> +                sourcepackagename=self.factory.makeSourcePackageName(
> +                    name="mypkg"
> +                ),
> +                version="1.0.0",
> +            )
> +        )
> +
> +        self.assertEqual(hook.deliveries.count(), 0)


-- 
https://code.launchpad.net/~vaishnavi-asawale/launchpad/+git/launchpad/+merge/494750
Your team Launchpad code reviewers is requested to review the proposed merge of ~vaishnavi-asawale/launchpad:source-upload-webhooks into launchpad:master.