launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:bugfix-private-mp-webhook-trigger into launchpad:master.

Commit message:
Fixing webhook trigger for private git repositories

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1888396 in Launchpad itself: "The webhook of team's private Git repo for Merge proposal doesn't work"
  https://bugs.launchpad.net/launchpad/+bug/1888396

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/387781
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:bugfix-private-mp-webhook-trigger into launchpad:master.

diff --git a/lib/lp/code/interfaces/gitref.py b/lib/lp/code/interfaces/gitref.py
index ef51d99..e361f9e 100644
--- a/lib/lp/code/interfaces/gitref.py
+++ b/lib/lp/code/interfaces/gitref.py
@@ -140,6 +140,10 @@ class IGitRefView(IHasMergeProposals, IHasRecipes, IPrivacy, IInformationType):
     target = Attribute(
         "The target of the repository containing this reference.")
 
+    stacked_on = Attribute(
+        "Attribute to make GitRef compatible with bzr Branch model. "
+        "This is always None, and cannot be changed.")
+
     namespace = Attribute(
         "The namespace of the repository containing this reference, as an "
         "`IGitNamespace`.")
diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py
index 5ebcf04..84b2d3c 100644
--- a/lib/lp/code/model/branchmergeproposal.py
+++ b/lib/lp/code/model/branchmergeproposal.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database class for branch merge proposals."""
@@ -903,14 +903,11 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
         the reviewer if the branch is private and the reviewer is an open
         team.
         """
-        if self.source_branch is None:
-            # This only applies to Bazaar, which has stacked branches.
-            return
-        source = self.source_branch
+        source = self.merge_source
         if (not source.visibleByUser(reviewer) and
             self._acceptable_to_give_visibility(source, reviewer)):
             self._subscribeUserToStackedBranch(source, reviewer)
-        target = self.target_branch
+        target = self.merge_target
         if (not target.visibleByUser(reviewer) and
             self._acceptable_to_give_visibility(source, reviewer)):
             self._subscribeUserToStackedBranch(target, reviewer)
diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
index f7dc142..b3bed20 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -150,6 +150,11 @@ class GitRefMixin:
         return self.repository.target
 
     @property
+    def stacked_on(self):
+        """See `IGitRef`."""
+        return None
+
+    @property
     def namespace(self):
         """See `IGitRef`."""
         return self.repository.namespace
diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
index 8936574..29784e9 100644
--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for BranchMergeProposals."""
@@ -91,6 +91,7 @@ from lp.services.webapp import canonical_url
 from lp.services.webhooks.testing import LogsScheduledWebhooks
 from lp.services.xref.interfaces import IXRefSet
 from lp.testing import (
+    admin_logged_in,
     ExpectedException,
     launchpadlib_for,
     login,
@@ -1102,6 +1103,45 @@ class TestMergeProposalWebhooks(WithScenarios, TestCaseWithFactory):
                     (hook, "merge-proposal:0.1",
                      MatchesDict(expected_redacted_payload))]))
 
+    def test_create_private_repo_triggers_webhooks(self):
+        # When a merge proposal is created, any relevant webhooks are
+        # triggered even if the repository is proprietary.
+        logger = self.useFixture(FakeLogger())
+        source = self.makeBranch()
+        target = self.makeBranch(same_target_as=source)
+
+        with admin_logged_in():
+            # Make source and target private.
+            self.factory.makeAccessPolicy(
+                source.target if self.git else source.product)
+            source.transitionToInformationType(
+                InformationType.PROPRIETARY, source.owner, False)
+            target.transitionToInformationType(
+                InformationType.PROPRIETARY, target.owner, False)
+
+            # Create the web hook and the proposal.
+            registrant = self.factory.makePerson()
+            hook = self.factory.makeWebhook(
+                target=self.getWebhookTarget(target),
+                event_types=["merge-proposal:0.1"])
+            proposal = source.addLandingTarget(
+                registrant, target, needs_review=True)
+            target_owner = target.owner
+
+        login_person(target_owner)
+        delivery = hook.deliveries.one()
+        expected_payload = {
+            "merge_proposal": Equals(self.getURL(proposal)),
+            "action": Equals("created"),
+            "new": MatchesDict(self.getExpectedPayload(proposal)),
+            }
+        expected_redacted_payload = dict(
+            expected_payload,
+            new=MatchesDict(
+                self.getExpectedPayload(proposal, redact=True)))
+        self.assertCorrectDelivery(expected_payload, hook, delivery)
+        self.assertCorrectLogging(expected_redacted_payload, hook, logger)
+
     def test_create_triggers_webhooks(self):
         # When a merge proposal is created, any relevant webhooks are
         # triggered.