launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-webhooks-permissions into lp:launchpad.

Commit message:
Fix permissions to allow dispatching of snap webhooks.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1535826 in Launchpad itself: "Add webhook support for snaps"
  https://bugs.launchpad.net/launchpad/+bug/1535826

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-webhooks-permissions/+merge/287851

Fix permissions to allow dispatching of snap webhooks.  All webhook-capable models now have tests to ensure that webhook job repr() works.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-webhooks-permissions into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2016-01-19 17:41:11 +0000
+++ database/schema/security.cfg	2016-03-02 21:26:51 +0000
@@ -2503,6 +2503,7 @@
 public.job                              = SELECT, UPDATE
 public.person                           = SELECT
 public.product                          = SELECT
+public.snap                             = SELECT
 public.sourcepackagename                = SELECT
 public.webhook                          = SELECT
 public.webhookjob                       = SELECT, UPDATE

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2015-10-15 16:01:50 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2016-03-02 21:26:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BranchMergeProposals."""
@@ -73,6 +73,7 @@
 from lp.registry.enums import TeamMembershipPolicy
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import IProductSet
+from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
 from lp.services.webapp import canonical_url
@@ -87,6 +88,7 @@
     WebServiceTestCase,
     ws_object,
     )
+from lp.testing.dbuser import dbuser
 from lp.testing.factory import LaunchpadObjectFactory
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
@@ -969,11 +971,16 @@
             }
         return {k: Equals(v) for k, v in payload.items()}
 
-    def assertCorrectDelivery(self, expected_payload, delivery):
+    def assertCorrectDelivery(self, expected_payload, hook, delivery):
         self.assertThat(
             delivery, MatchesStructure(
                 event_type=Equals("merge-proposal:0.1"),
                 payload=MatchesDict(expected_payload)))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            self.assertEqual(
+                "<WebhookDeliveryJob for webhook %d on %r>" % (
+                    hook.id, hook.target),
+                repr(delivery))
 
     def test_create_triggers_webhooks(self):
         # When a merge proposal is created, any relevant webhooks are
@@ -993,7 +1000,7 @@
             "action": Equals("created"),
             "new": MatchesDict(self.getExpectedPayload(proposal)),
             }
-        self.assertCorrectDelivery(expected_payload, delivery)
+        self.assertCorrectDelivery(expected_payload, hook, delivery)
 
     def test_modify_triggers_webhooks(self):
         # When an existing merge proposal is modified, any relevant webhooks
@@ -1019,7 +1026,7 @@
         expected_payload["new"] = MatchesDict(
             self.getExpectedPayload(proposal))
         delivery = hook.deliveries.one()
-        self.assertCorrectDelivery(expected_payload, delivery)
+        self.assertCorrectDelivery(expected_payload, hook, delivery)
 
     def test_delete_triggers_webhooks(self):
         # When an existing merge proposal is deleted, any relevant webhooks
@@ -1040,7 +1047,7 @@
             }
         proposal.deleteProposal()
         delivery = hook.deliveries.one()
-        self.assertCorrectDelivery(expected_payload, delivery)
+        self.assertCorrectDelivery(expected_payload, hook, delivery)
 
 
 class TestMergeProposalWebhooksBzr(

=== modified file 'lib/lp/code/model/tests/test_gitjob.py'
--- lib/lp/code/model/tests/test_gitjob.py	2016-02-05 16:51:12 +0000
+++ lib/lp/code/model/tests/test_gitjob.py	2016-03-02 21:26:51 +0000
@@ -39,6 +39,7 @@
     GitRefScanJob,
     ReclaimGitRepositorySpaceJob,
     )
+from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.runner import JobRunner
@@ -217,6 +218,11 @@
                             'old': None,
                             'new': {'commit_sha1': sha1('refs/tags/2.0')}},
                     })})))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            self.assertEqual(
+                "<WebhookDeliveryJob for webhook %d on %r>" % (
+                    hook.id, hook.target),
+                repr(delivery))
 
     def test_merge_detection_triggers_webhooks(self):
         self.useFixture(FeatureFixture(
@@ -258,6 +264,11 @@
                         {'queue_status': Equals('Work in progress')}),
                     'new': ContainsDict({'queue_status': Equals('Merged')}),
                     })))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            self.assertEqual(
+                "<WebhookDeliveryJob for webhook %d on %r>" % (
+                    hook.id, hook.target),
+                repr(delivery))
 
     def test_composeWebhookPayload(self):
         repository = self.factory.makeGitRepository()

=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py	2015-10-01 14:45:57 +0000
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py	2016-03-02 21:26:51 +0000
@@ -1,6 +1,6 @@
 #!/usr/bin/python
 #
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import datetime
@@ -774,6 +774,11 @@
                     "old": Equals({"revision_id": old_revid}),
                     "new": Equals({"revision_id": new_revid}),
                     })))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            self.assertEqual(
+                "<WebhookDeliveryJob for webhook %d on %r>" % (
+                    hook.id, hook.target),
+                repr(delivery))
 
 
 class TestRevisionProperty(BzrSyncTestCase):

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2016-02-19 14:18:22 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2016-03-02 21:26:51 +0000
@@ -52,6 +52,7 @@
     person_logged_in,
     TestCaseWithFactory,
     )
+from lp.testing.dbuser import dbuser
 from lp.testing.layers import (
     LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,
@@ -238,10 +239,16 @@
                 canonical_url(self.build.snap, force_local_path=True)),
             "status": Equals("Successfully built"),
             }
+        delivery = hook.deliveries.one()
         self.assertThat(
-            hook.deliveries.one(), MatchesStructure(
+            delivery, MatchesStructure(
                 event_type=Equals("snap:build:0.1"),
                 payload=MatchesDict(expected_payload)))
+        with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+            self.assertEqual(
+                "<WebhookDeliveryJob for webhook %d on %r>" % (
+                    hook.id, hook.target),
+                repr(delivery))
 
     def test_notify_fullybuilt(self):
         # notify does not send mail when a SnapBuild completes normally.