From d2589a5bd1228e1baebd7dad8d2126696605faa3 Mon Sep 17 00:00:00 2001 From: Satyam Bansal Date: Wed, 12 Jul 2023 01:48:48 +0530 Subject: [PATCH] integrations: Send GitHub pull request comment alerts to correct topic. Pull request comment alerts were previously sent to a topic for an issue, which resulted in two different topics for the same PR. Fixes: #26086. Co-authored-by: Lauryn Menard --- zerver/webhooks/github/tests.py | 3 +-- zerver/webhooks/github/view.py | 12 +++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/zerver/webhooks/github/tests.py b/zerver/webhooks/github/tests.py index 6299656641..9a20141367 100644 --- a/zerver/webhooks/github/tests.py +++ b/zerver/webhooks/github/tests.py @@ -146,9 +146,8 @@ class GitHubWebhookTest(WebhookTestCase): self.check_webhook("issue_comment", expected_topic, expected_message) def test_issue_comment_pull_request_comment_msg(self) -> None: - expected_topic = "public-repo / issue #1 Update the README with new informa..." expected_message = "sbansal1999 [commented](https://github.com/sbansal1999/public-repo/pull/1#issuecomment-1631445420) on [issue #1](https://github.com/sbansal1999/public-repo/pull/1):\n\n~~~ quote\nSome comment\n~~~" - self.check_webhook("issue_comment__pull_request_comment", expected_topic, expected_message) + self.check_webhook("issue_comment__pull_request_comment", TOPIC_PR, expected_message) def test_issue_msg(self) -> None: expected_message = "baxterthehacker opened [issue #2](https://github.com/baxterthehacker/public-repo/issues/2):\n\n~~~ quote\nIt looks like you accidentally spelled 'commit' with two 't's.\n~~~" diff --git a/zerver/webhooks/github/view.py b/zerver/webhooks/github/view.py index 60ae0cc194..b7d37af026 100644 --- a/zerver/webhooks/github/view.py +++ b/zerver/webhooks/github/view.py @@ -666,6 +666,15 @@ def is_merge_queue_push_event(payload: WildValue) -> bool: return payload["ref"].tame(check_string).startswith("refs/heads/gh-readonly-queue/") +def is_pull_request_comment_event(payload: WildValue) -> bool: + # When a comment is made on a PR, the event still has the header + # "issue_comment", but the payload has a "pull_request" key. + # This is just a workaround to get the correct topic. + if "pull_request" in payload["issue"]: + return True + return False + + def get_topic_based_on_type(payload: WildValue, event: str) -> str: if "pull_request" in event: return TOPIC_WITH_PR_OR_ISSUE_INFO_TEMPLATE.format( @@ -675,9 +684,10 @@ def get_topic_based_on_type(payload: WildValue, event: str) -> str: title=payload["pull_request"]["title"].tame(check_string), ) elif event.startswith("issue"): + type_for_topic = "PR" if is_pull_request_comment_event(payload) else "issue" return TOPIC_WITH_PR_OR_ISSUE_INFO_TEMPLATE.format( repo=get_repository_name(payload), - type="issue", + type=type_for_topic, id=payload["issue"]["number"].tame(check_int), title=payload["issue"]["title"].tame(check_string), )