launchpad-reviewers team mailing list archive

[Ines Almeida <mp+445444@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:update-queued-webhook-table-permissions into launchpad:master.

Commit message:
Update 'queued' database permissions for webhook and webhookjob table

Add unit test to verify the change

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/445444

We recently deployed the change that triggers webhooks on bugs creation/change.
Publishing a package with which bugs were fixed within the changelog, changes the status of those bugs to 'FIX RELEASED' which in turn will try to trigger webhooks if they are set up.
This was causing problems because the `queued` user didn'thave permissions to access the `webhook` and `webhookjob` tables.

This MP adds those permissions to the needed user.

The added unit test failed before the database permissions update, and passes after the change is applied.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:update-queued-webhook-table-permissions into launchpad:master.

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 4cb79a4..c48a956 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -1678,6 +1678,8 @@ public.teammembership                   = SELECT
 public.teamparticipation                = SELECT, INSERT
 public.validpersoncache                 = SELECT
 public.validpersonorteamcache           = SELECT
+public.webhook                          = SELECT
+public.webhookjob                       = SELECT, INSERT
 public.xref                             = SELECT, INSERT
 type=user
 
diff --git a/lib/lp/soyuz/tests/test_packagecopyjob.py b/lib/lp/soyuz/tests/test_packagecopyjob.py
index 645683b..f4d2e53 100644
--- a/lib/lp/soyuz/tests/test_packagecopyjob.py
+++ b/lib/lp/soyuz/tests/test_packagecopyjob.py
@@ -18,6 +18,7 @@ from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
+from lp.bugs.interfaces.bugtarget import BUG_WEBHOOKS_FEATURE_FLAG
 from lp.bugs.interfaces.bugtask import BugTaskStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
@@ -1660,6 +1661,88 @@ class PlainPackageCopyJobTests(TestCaseWithFactory, LocalTestHelper):
         self.assertEqual(BugTaskStatus.FIXRELEASED, bugtask281.status)
         self.assertEqual(BugTaskStatus.NEW, bugtask280.status)
 
+    def test_copying_closes_bugs_with_webhooks(self):
+        # Copying a package into a primary archive closes any bugs mentioned
+        # in its recent changelog, including triggering their webhooks.
+
+        # Set bug webhooks feature flag on for the purpose of this test
+        self.useFixture(
+            FeatureFixture(
+                {
+                    BUG_WEBHOOKS_FEATURE_FLAG: "on",
+                }
+            )
+        )
+
+        target_archive = self.factory.makeArchive(
+            self.distroseries.distribution, purpose=ArchivePurpose.PRIMARY
+        )
+        source_archive = self.factory.makeArchive(
+            self.distroseries.distribution
+        )
+        bug = self.factory.makeBug()
+        webhook = self.factory.makeWebhook(
+            target=bug.default_bugtask.target,
+            event_types=["bug:comment:0.1", "bug:0.1"],
+        )
+
+        # Publish a package in the source archive and give it a changelog
+        # entry that closes a bug.
+        source_pub = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=self.distroseries,
+            sourcepackagename="libc",
+            version="2.8-2",
+            status=PackagePublishingStatus.PUBLISHED,
+            archive=source_archive,
+        )
+        spr = removeSecurityProxy(source_pub).sourcepackagerelease
+        changelog = dedent(
+            """\
+            libc (2.8-2) unstable; urgency=low
+
+              * closes: %s
+
+             -- Foo Bar <foo@xxxxxxxxxxx>  Tue, 01 Jan 1970 01:50:41 +0000
+
+            libc (2.8-1) unstable; urgency=low
+
+              * Initial release.
+
+             -- Foo Bar <foo@xxxxxxxxxxx>  Tue, 01 Jan 1970 01:50:41 +0000
+            """
+            % bug.id
+        ).encode("UTF-8")
+        spr.changelog = self.factory.makeLibraryFileAlias(content=changelog)
+        spr.changelog_entry = "dummy"
+        self.layer.txn.commit()  # Librarian.
+
+        # Now put the same named package in the target archive at the
+        # oldest version in the changelog.
+        self.publisher.getPubSource(
+            distroseries=self.distroseries,
+            sourcename="libc",
+            version="2.8-1",
+            status=PackagePublishingStatus.PUBLISHED,
+            archive=target_archive,
+        )
+
+        sp = self.distroseries.getSourcePackage(spr.sourcepackagename)
+        bugtask = self.factory.makeBugTask(target=sp, bug=bug, publish=False)
+
+        # Run the copy job.
+        requester = self.factory.makePerson()
+        with person_logged_in(target_archive.owner):
+            target_archive.newComponentUploader(requester, "main")
+        job = self.createCopyJobForSPPH(
+            source_pub, source_archive, target_archive, requester=requester
+        )
+        self.runJob(job)
+        self.assertEqual(JobStatus.COMPLETED, job.status)
+
+        # The bug is fixed, and a webhook delivery is scheduled.
+        self.assertEqual(BugTaskStatus.FIXRELEASED, bugtask.status)
+        self.assertEqual(1, webhook.deliveries.count())
+
     def test_copying_unembargoes_files(self):
         # The unembargo flag causes the job to unrestrict files.
         self.distroseries.status = SeriesStatus.CURRENT