launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:webhook-for-team-owned-private-repo into launchpad:master.

Commit message:
Webhook triggers for team owned private repo

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/403381

Checking in for the Unit test that demonstrates the problem of the webhook not triggering for the same setup we had for our user that reported this problem: user is member of a team and the team owns the private target repo/branch. 

Users's MP: https://code.launchpad.net/~cyruslien/oem-dev-tools/+git/lp-fish-tools/+merge/403239

Webhook that did not trigger for user: https://code.launchpad.net/~oem-solutions-group/oem-dev-tools/+git/lp-fish-tools/+webhook/13325
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:webhook-for-team-owned-private-repo into launchpad:master.

diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py
index c9a98cd..8c7140d 100644
--- a/lib/lp/code/model/branch.py
+++ b/lib/lp/code/model/branch.py
@@ -1539,6 +1539,9 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
             checked_branches = []
         if self.information_type in PUBLIC_INFORMATION_TYPES:
             can_access = True
+        elif self.information_type == InformationType.PROPRIETARY and user.is_team:
+            if self.owner.inTeam(user):
+                can_access = True
         elif user is None:
             can_access = False
         elif user.id in self._known_viewers:
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index d0ac773..3e8ec83 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -905,6 +905,9 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
         """See `IGitRepository`."""
         if self.information_type in PUBLIC_INFORMATION_TYPES:
             return True
+        elif self.information_type == InformationType.PROPRIETARY and user.is_team:
+            if self.owner.inTeam(user):
+                return True
         elif user is None:
             return False
         elif user.id in self._known_viewers:
diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
index 2f6fa07..63e91ad 100644
--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
@@ -1124,6 +1124,39 @@ class TestMergeProposalWebhooks(WithVCSScenarios, TestCaseWithFactory):
         self.assertCorrectDelivery(expected_payload, hook, delivery)
         self.assertCorrectLogging(expected_redacted_payload, hook, logger)
 
+    def test_create_private_repo_team_owned_triggers_webhooks(self):
+        # the user is not the owner of the target but a member of the team
+        # that owns the target
+        logger = self.useFixture(FakeLogger())
+        person = self.factory.makePerson(name='person')
+        team = self.factory.makeTeam(members=[person], name='team')
+        source = self.makeBranch(information_type=InformationType.PROPRIETARY,
+                                 owner=person)
+        target = self.makeBranch(
+            same_target_as=source,
+            information_type=InformationType.PROPRIETARY,
+            owner=team)
+
+        # Create the web hook and the proposal.
+        login_person(person)
+        hook = self.factory.makeWebhook(
+            target=self.getWebhookTarget(target),
+            event_types=["merge-proposal:0.1"])
+        proposal = source.addLandingTarget(
+            person, target, needs_review=True)
+        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.