diff --git a/backend/druks/contrib/software_factory/subscribers.py b/backend/druks/contrib/software_factory/subscribers.py index 2ed927cb..da453a18 100644 --- a/backend/druks/contrib/software_factory/subscribers.py +++ b/backend/druks/contrib/software_factory/subscribers.py @@ -53,7 +53,7 @@ async def policy_push_reprofiles_the_repo(*, repo: str, paths: list, **_: object await Profile.dispatch(project_repo, refresh_only=True) -@subscribe("pr.review_submitted") +@subscribe("pr.review_submitted", payload__author_can_write=True) async def pr_review_answers_the_gate(*, repo: str, pr_number: int, payload: dict) -> None: item = await WorkItem.get_for_pr(repo=repo, pr_number=pr_number) if item: diff --git a/backend/druks/core/webhooks/github.py b/backend/druks/core/webhooks/github.py index 0ddb95a8..71e1b2b7 100644 --- a/backend/druks/core/webhooks/github.py +++ b/backend/druks/core/webhooks/github.py @@ -66,6 +66,7 @@ async def on_pull_request_review_submitted(self) -> Response: "branch": pull_request["head"]["ref"], "action": action, "reviewer": sender["login"], + "author_can_write": review["author_association"] in _WRITERS, "body": review["body"] or "", # body is nullable on an approve }, ) diff --git a/backend/tests/software_factory/test_lane_reactions.py b/backend/tests/software_factory/test_lane_reactions.py index 3d436b8d..165dd63c 100644 --- a/backend/tests/software_factory/test_lane_reactions.py +++ b/backend/tests/software_factory/test_lane_reactions.py @@ -113,7 +113,13 @@ async def _push(self, status): assert pushed == [TicketStatus.IN_PROGRESS, TicketStatus.IN_REVIEW] -async def test_pr_review_answers_through_the_review_gate(druks_db, monkeypatch): +# On a public repo any GitHub account can approve; only a writer's review answers. +@pytest.mark.parametrize( + ("action", "author_can_write"), [("request_changes", True), ("approve", False)] +) +async def test_pr_review_answers_through_the_review_gate( + druks_db, monkeypatch, action, author_can_write +): item = await make_test_work_item( repo="acme/widget", title="t", source="linear", ticket_key="ACME-9" ) @@ -146,22 +152,15 @@ async def answer(subject, **reply): pr_number=item.pr_number, payload={ "branch": item.branch, - "action": "request_changes", + "action": action, "reviewer": "alice", + "author_can_write": author_can_write, "body": "Please split the migration.", }, ) - assert answers == [ - ( - item, - { - "action": "request_changes", - "reviewer": "alice", - "body": "Please split the migration.", - }, - ) - ] + reply = {"action": action, "reviewer": "alice", "body": "Please split the migration."} + assert answers == ([(item, reply)] if author_can_write else []) async def test_pr_open_reaches_the_work_item(druks_db):