Normalize github pull requests event message.

Add detailed info (description, source and target branch, assignee) to message. Change subject to 'repo_name / PR #id title'. Modify some test fixtures for better coverage.
2025-11-12 01:47:41 +00:00 · 2016-10-11 18:58:04 +02:00
parent ce9ac30fac
commit 92bfadcca4
4 changed files with 77 additions and 17 deletions
--- a/zerver/fixtures/github/github_v1_pull_request_opened.json
+++ b/zerver/fixtures/github/github_v1_pull_request_opened.json
@@ -6,7 +6,24 @@
      "body": "Omitted something I think?",
      "merge_commit_sha": null,
      "number": 7,
-      "assignee": null,
+      "assignee": {
+      "following_url": "https://api.github.com/users/lfaraone/following{/other_user}",
+      "gists_url": "https://api.github.com/users/lfaraone/gists{/gist_id}",
+      "organizations_url": "https://api.github.com/users/lfaraone/orgs",
+      "url": "https://api.github.com/users/lfaraone",
+      "events_url": "https://api.github.com/users/lfaraone/events{/privacy}",
+      "html_url": "https://github.com/lfaraone",
+      "subscriptions_url": "https://api.github.com/users/lfaraone/subscriptions",
+      "avatar_url": "https://0.gravatar.com/avatar/43918f842a892d0deb9ef1d8dcc9fe49?d=https%3A%2F%2Fidenticons.github.com%2F38b70bc4e5dd6c315c3acbe51c86820c.png",
+      "repos_url": "https://api.github.com/users/lfaraone/repos",
+      "received_events_url": "https://api.github.com/users/lfaraone/received_events",
+      "gravatar_id": "43918f842a892d0deb9ef1d8dcc9fe49",
+      "starred_url": "https://api.github.com/users/lfaraone/starred{/owner}{/repo}",
+      "login": "lfaraone",
+      "type": "User",
+      "id": 73410,
+      "followers_url": "https://api.github.com/users/lfaraone/followers"
+    },
      "mergeable": null,
      "closed_at": null,
      "additions": 1,
--- a/zerver/fixtures/github/github_v2_pull_request_opened.json
+++ b/zerver/fixtures/github/github_v2_pull_request_opened.json
@@ -6,7 +6,24 @@
      "body": "Omitted something I think?",
      "merge_commit_sha": null,
      "number": 7,
-      "assignee": null,
+      "assignee": {
+      "following_url": "https://api.github.com/users/lfaraone/following{/other_user}",
+      "gists_url": "https://api.github.com/users/lfaraone/gists{/gist_id}",
+      "organizations_url": "https://api.github.com/users/lfaraone/orgs",
+      "url": "https://api.github.com/users/lfaraone",
+      "events_url": "https://api.github.com/users/lfaraone/events{/privacy}",
+      "html_url": "https://github.com/lfaraone",
+      "subscriptions_url": "https://api.github.com/users/lfaraone/subscriptions",
+      "avatar_url": "https://0.gravatar.com/avatar/43918f842a892d0deb9ef1d8dcc9fe49?d=https%3A%2F%2Fidenticons.github.com%2F38b70bc4e5dd6c315c3acbe51c86820c.png",
+      "repos_url": "https://api.github.com/users/lfaraone/repos",
+      "received_events_url": "https://api.github.com/users/lfaraone/received_events",
+      "gravatar_id": "43918f842a892d0deb9ef1d8dcc9fe49",
+      "starred_url": "https://api.github.com/users/lfaraone/starred{/owner}{/repo}",
+      "login": "lfaraone",
+      "type": "User",
+      "id": 73410,
+      "followers_url": "https://api.github.com/users/lfaraone/followers"
+    },
      "mergeable": null,
      "closed_at": null,
      "additions": 1,
--- a/zerver/tests/webhooks/test_github.py
+++ b/zerver/tests/webhooks/test_github.py
@@ -106,20 +106,20 @@ class GithubV1HookTests(WebhookTestCase):
    def test_pull_request_opened(self):
        # type: () -> None
        self.basic_test('pull_request_opened', 'commits',
-                        "zulip-test: pull request 7: Counting is hard.",
-                        "lfaraone opened [pull request 7](https://github.com/zbenjamin/zulip-test/pull/7)\n\n~~~ quote\nOmitted something I think?\n~~~")
+                        "zulip-test / PR #7 Counting is hard.",
+                        "lfaraone opened [PR](https://github.com/zbenjamin/zulip-test/pull/7)(assigned to lfaraone)\nfrom `patch-2` to `master`\n\n~~~ quote\nOmitted something I think?\n~~~")

    def test_pull_request_closed(self):
        # type: () -> None
        self.basic_test('pull_request_closed', 'commits',
-                        "zulip-test: pull request 7: Counting is hard.",
-                        "zbenjamin closed [pull request 7](https://github.com/zbenjamin/zulip-test/pull/7)")
+                        "zulip-test / PR #7 Counting is hard.",
+                        "zbenjamin closed [PR](https://github.com/zbenjamin/zulip-test/pull/7)")

    def test_pull_request_synchronize(self):
        # type: () -> None
        self.basic_test('pull_request_synchronize', 'commits',
-                        "zulip-test: pull request 13: Even more cowbell.",
-                        "zbenjamin synchronized [pull request 13](https://github.com/zbenjamin/zulip-test/pull/13)")
+                        "zulip-test / PR #13 Even more cowbell.",
+                        "zbenjamin synchronized [PR](https://github.com/zbenjamin/zulip-test/pull/13)")

    def test_pull_request_comment(self):
        # type: () -> None
@@ -247,20 +247,21 @@ class GithubV2HookTests(WebhookTestCase):
    def test_pull_request_opened(self):
        # type: () -> None
        self.basic_test('pull_request_opened', 'commits',
-                        "zulip-test: pull request 7: Counting is hard.",
-                        "lfaraone opened [pull request 7](https://github.com/zbenjamin/zulip-test/pull/7)\n\n~~~ quote\nOmitted something I think?\n~~~")
+                        "zulip-test / PR #7 Counting is hard.",
+                        "lfaraone opened [PR](https://github.com/zbenjamin/zulip-test/pull/7)(assigned to lfaraone)\nfrom `patch-2` to `master`\n\n~~~ quote\nOmitted something I think?\n~~~")

    def test_pull_request_closed(self):
        # type: () -> None
        self.basic_test('pull_request_closed', 'commits',
-                        "zulip-test: pull request 7: Counting is hard.",
-                        "zbenjamin closed [pull request 7](https://github.com/zbenjamin/zulip-test/pull/7)")
+                        "zulip-test / PR #7 Counting is hard.",
+                        "zbenjamin closed [PR](https://github.com/zbenjamin/zulip-test/pull/7)")

    def test_pull_request_synchronize(self):
        # type: () -> None
        self.basic_test('pull_request_synchronize', 'commits',
-                        "zulip-test: pull request 13: Even more cowbell.",
-                        "zbenjamin synchronized [pull request 13](https://github.com/zbenjamin/zulip-test/pull/13)")
+                        "zulip-test / PR #13 Even more cowbell.",
+
+                        "zbenjamin synchronized [PR](https://github.com/zbenjamin/zulip-test/pull/13)")

    def test_pull_request_comment(self):
        # type: () -> None
--- a/zerver/views/webhooks/github.py
+++ b/zerver/views/webhooks/github.py
@@ -7,7 +7,8 @@ from zerver.decorator import authenticated_api_view, REQ, has_request_variables,
 from zerver.views.messages import send_message_backend
 from zerver.lib.webhooks.git import get_push_commits_event_message,\
    SUBJECT_WITH_BRANCH_TEMPLATE, get_force_push_commits_event_message, \
-    get_remove_branch_event_message
+    get_remove_branch_event_message, get_pull_request_event_message,\
+    SUBJECT_WITH_PR_INFO_TEMPLATE
 import logging
 import re
 import ujson
@@ -28,6 +29,25 @@ def is_test_repository(repository):
 class UnknownEventType(Exception):
    pass

+def github_pull_request_content(payload):
+    # type: (Mapping[text_type, Any]) -> text_type
+    pull_request = payload['pull_request']
+    action = 'synchronized' if payload['action'] == 'synchronize' else payload['action']
+    if action in ('opened', 'edited'):
+        return get_pull_request_event_message(
+            payload['sender']['login'],
+            action,
+            pull_request['html_url'],
+            pull_request['head']['ref'],
+            pull_request['base']['ref'],
+            pull_request['body'],
+            pull_request.get('assignee', {}).get('login')
+        )
+    return get_pull_request_event_message(
+            payload['sender']['login'],
+            action,
+            pull_request['html_url'],
+        )

 def github_generic_subject(noun, topic_focus, blob):
    # type: (text_type, text_type, Mapping[text_type, Any]) -> text_type
@@ -80,8 +100,13 @@ def api_github_v2(user_profile, event, payload, branches, default_stream,
    # Event Handlers
    if event == 'pull_request':
        pull_req = payload['pull_request']
-        subject = github_generic_subject('pull request', topic_focus, pull_req)
-        content = github_generic_content('pull request', payload, pull_req)
+        subject = SUBJECT_WITH_PR_INFO_TEMPLATE.format(
+            repo=repository['name'],
+            type='PR',
+            id=pull_req['number'],
+            title=pull_req['title']
+        )
+        content = github_pull_request_content(payload)
    elif event == 'issues':
        # in v1, we assume that this stream exists since it is
        # deprecated and the few realms that use it already have the