From 390e2a12e52000b75302e5d67388a832976119e4 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 7 Nov 2022 15:24:35 -0500 Subject: [PATCH] webhooks: Use get_short_sha to get shortened sha. This unifies the length of the shortened SHA our integrations generate, and ensures that they are long enough for projects of various sizes with a chosen value defined in get_short_sha. Fixes #23475 Signed-off-by: Zixuan James Li --- zerver/webhooks/bitbucket2/tests.py | 2 +- zerver/webhooks/bitbucket2/view.py | 3 ++- zerver/webhooks/circleci/tests.py | 8 ++++---- zerver/webhooks/circleci/view.py | 7 ++++--- zerver/webhooks/github/tests.py | 6 +++--- zerver/webhooks/github/view.py | 5 +++-- zerver/webhooks/semaphore/tests.py | 8 ++++---- zerver/webhooks/semaphore/view.py | 7 ++++--- zerver/webhooks/solano/tests.py | 6 +++--- zerver/webhooks/solano/view.py | 3 ++- 10 files changed, 30 insertions(+), 25 deletions(-) diff --git a/zerver/webhooks/bitbucket2/tests.py b/zerver/webhooks/bitbucket2/tests.py index 9d1eb51ef8..839b7b4c8a 100644 --- a/zerver/webhooks/bitbucket2/tests.py +++ b/zerver/webhooks/bitbucket2/tests.py @@ -88,7 +88,7 @@ class Bitbucket2HookTests(WebhookTestCase): self.check_webhook("commit_comment_created", TOPIC, expected_message) def test_bitbucket2_on_commit_status_changed_event(self) -> None: - expected_message = "[System mybuildtool](https://my-build-tool.com/builds/MY-PROJECT/BUILD-777) changed status of [9fec847](https://bitbucket.org/kolaszek/repository-name/commits/9fec847784abb10b2fa567ee63b85bd238955d0e) to SUCCESSFUL." + expected_message = "[System mybuildtool](https://my-build-tool.com/builds/MY-PROJECT/BUILD-777) changed status of [9fec847784a](https://bitbucket.org/kolaszek/repository-name/commits/9fec847784abb10b2fa567ee63b85bd238955d0e) to SUCCESSFUL." self.check_webhook("commit_status_changed", TOPIC, expected_message) def test_bitbucket2_on_issue_created_event(self) -> None: diff --git a/zerver/webhooks/bitbucket2/view.py b/zerver/webhooks/bitbucket2/view.py index 83f8737564..7483ed6575 100644 --- a/zerver/webhooks/bitbucket2/view.py +++ b/zerver/webhooks/bitbucket2/view.py @@ -25,6 +25,7 @@ from zerver.lib.webhooks.git import ( get_push_commits_event_message, get_push_tag_event_message, get_remove_branch_event_message, + get_short_sha, ) from zerver.models import UserProfile @@ -293,7 +294,7 @@ def get_commit_status_changed_body(payload: WildValue, include_title: bool) -> s commit_info = "[{short_commit_id}]({repo_url}/commits/{commit_id})".format( repo_url=get_repository_url(payload["repository"]), - short_commit_id=commit_id[:7], + short_commit_id=get_short_sha(commit_id), commit_id=commit_id, ) diff --git a/zerver/webhooks/circleci/tests.py b/zerver/webhooks/circleci/tests.py index 87e8320c47..6cf8212dcd 100644 --- a/zerver/webhooks/circleci/tests.py +++ b/zerver/webhooks/circleci/tests.py @@ -10,7 +10,7 @@ class CircleCiHookTests(WebhookTestCase): expected_topic = "circleci-test" expected_message = """ Build [#5](https://circleci.com/bb/Hypro999/circleci-test/5) of `build`/`workflow` on branch `unstable` has failed. -- **Commits (3):** [6b5361c166](https://bitbucket.org/Hypro999/circleci-test/commits/6b5361c1661581d975e84b68904ae9bfba75d5e5) ... [eaa88f9eac](https://bitbucket.org/Hypro999/circleci-test/commits/eaa88f9eac0fad86c46a8fe35462fe2c904d84b1) +- **Commits (3):** [6b5361c1661](https://bitbucket.org/Hypro999/circleci-test/commits/6b5361c1661581d975e84b68904ae9bfba75d5e5) ... [eaa88f9eac0](https://bitbucket.org/Hypro999/circleci-test/commits/eaa88f9eac0fad86c46a8fe35462fe2c904d84b1) - **Pull request:** https://bitbucket.org/Hypro999/circleci-test/pull-requests/1 - **Author:** Hemanth V. Alluri """.strip() @@ -22,7 +22,7 @@ Build [#5](https://circleci.com/bb/Hypro999/circleci-test/5) of `build`/`workflo expected_topic = "zulip" expected_message = """ Build [#1429](https://circleci.com/gh/Hypro999/zulip/1429) of `bionic-backend-frontend`/`Ubuntu 18.04 Bionic (Python 3.6, backend+frontend)` on branch `circleci` has failed. -- **Commits (2):** [73900eeb69 ... 5326f9ea40](https://github.com/Hypro999/zulip/compare/73900eeb69adbf0b83dc487e8eda90661b524eff...5326f9ea4084a01cc2bf1a461b9ad819b4ffdd14) +- **Commits (2):** [73900eeb69a ... 5326f9ea408](https://github.com/Hypro999/zulip/compare/73900eeb69adbf0b83dc487e8eda90661b524eff...5326f9ea4084a01cc2bf1a461b9ad819b4ffdd14) - **Author:** Hemanth V. Alluri (Hypro999) - **Committer:** Hemanth V. Alluri (Hypro999) @@ -35,7 +35,7 @@ Build [#1429](https://circleci.com/gh/Hypro999/zulip/1429) of `bionic-backend-fr expected_topic = "zulip" expected_message = """ Build [#1431](https://circleci.com/gh/Hypro999/zulip/1431) of `bionic-production-build`/`Production` on branch `circleci` has succeeded. -- **Commits (2):** [73900eeb69 ... 5326f9ea40](https://github.com/Hypro999/zulip/compare/73900eeb69adbf0b83dc487e8eda90661b524eff...5326f9ea4084a01cc2bf1a461b9ad819b4ffdd14) +- **Commits (2):** [73900eeb69a ... 5326f9ea408](https://github.com/Hypro999/zulip/compare/73900eeb69adbf0b83dc487e8eda90661b524eff...5326f9ea4084a01cc2bf1a461b9ad819b4ffdd14) - **Authors:** Gintoki Sakata (ShiroYasha999), Hemanth V. Alluri (Hypro999) - **Committers:** Hemanth V. Alluri (Hypro999), Sadaharu @@ -50,7 +50,7 @@ Build [#1431](https://circleci.com/gh/Hypro999/zulip/1431) of `bionic-production expected_topic = "zulip" expected_message = """ Build [#1420](https://circleci.com/gh/Hypro999/zulip/1420) of `bionic-production-install`/`Production` on branch `circleci` was canceled. -- **Commit:** [b0d6197fb4](https://github.com/Hypro999/zulip/commit/b0d6197fb4cacaf917adca77f77354882ee80621) +- **Commit:** [b0d6197fb4c](https://github.com/Hypro999/zulip/commit/b0d6197fb4cacaf917adca77f77354882ee80621) - **Author:** Hemanth V. Alluri (Hypro999) - **Committer:** Hemanth V. Alluri (Hypro999) diff --git a/zerver/webhooks/circleci/view.py b/zerver/webhooks/circleci/view.py index 146e70016e..e82121047b 100644 --- a/zerver/webhooks/circleci/view.py +++ b/zerver/webhooks/circleci/view.py @@ -5,6 +5,7 @@ from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success from zerver.lib.validator import WildValue, check_int, check_none_or, check_string, to_wild_value from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.git import get_short_sha from zerver.models import UserProfile outcome_to_formatted_status_map = { @@ -48,15 +49,15 @@ def get_commit_range_info(payload: WildValue) -> str: num_commits = len(commits) if num_commits == 1: - commit_id = commits[0]["commit"].tame(check_string)[:10] + commit_id = get_short_sha(commits[0]["commit"].tame(check_string)) commit_url = commits[0]["commit_url"].tame(check_string) return f"- **Commit:** [{commit_id}]({commit_url})" vcs_provider = payload["user"]["vcs_type"].tame(check_string) # Same as payload["why"]? first_commit_id = commits[0]["commit"].tame(check_string) - shortened_first_commit_id = first_commit_id[:10] + shortened_first_commit_id = get_short_sha(first_commit_id) last_commit_id = commits[-1]["commit"].tame(check_string) - shortened_last_commit_id = last_commit_id[:10] + shortened_last_commit_id = get_short_sha(last_commit_id) if vcs_provider == "github": # Then use GitHub's commit range feature to form the appropriate url. vcs_url = payload["vcs_url"].tame(check_string) diff --git a/zerver/webhooks/github/tests.py b/zerver/webhooks/github/tests.py index fcb5683642..679bbe09ba 100644 --- a/zerver/webhooks/github/tests.py +++ b/zerver/webhooks/github/tests.py @@ -232,11 +232,11 @@ class GitHubWebhookTest(WebhookTestCase): self.check_webhook("page_build__errored", TOPIC_REPO, expected_message) def test_status_msg(self) -> None: - expected_message = "[9049f12](https://github.com/baxterthehacker/public-repo/commit/9049f1265b7d61be4a8904a9a27120d2064dab3b) changed its status to success." + expected_message = "[9049f1265b7](https://github.com/baxterthehacker/public-repo/commit/9049f1265b7d61be4a8904a9a27120d2064dab3b) changed its status to success." self.check_webhook("status", TOPIC_REPO, expected_message) def test_status_with_target_url_msg(self) -> None: - expected_message = "[9049f12](https://github.com/baxterthehacker/public-repo/commit/9049f1265b7d61be4a8904a9a27120d2064dab3b) changed its status to [success](https://example.com/build/status)." + expected_message = "[9049f1265b7](https://github.com/baxterthehacker/public-repo/commit/9049f1265b7d61be4a8904a9a27120d2064dab3b) changed its status to [success](https://example.com/build/status)." self.check_webhook("status__with_target_url", TOPIC_REPO, expected_message) def test_pull_request_review_msg(self) -> None: @@ -346,7 +346,7 @@ class GitHubWebhookTest(WebhookTestCase): def test_check_run(self) -> None: expected_topic = "hello-world / checks" expected_message = """ -Check [randscape](http://github.com/github/hello-world/runs/4) completed (success). ([d6fde92](http://github.com/github/hello-world/commit/d6fde92930d4715a2b49857d24b940956b26d2d3)) +Check [randscape](http://github.com/github/hello-world/runs/4) completed (success). ([d6fde92930d](http://github.com/github/hello-world/commit/d6fde92930d4715a2b49857d24b940956b26d2d3)) """.strip() self.check_webhook("check_run__completed", expected_topic, expected_message) diff --git a/zerver/webhooks/github/view.py b/zerver/webhooks/github/view.py index ed790cd941..f7efdc92eb 100644 --- a/zerver/webhooks/github/view.py +++ b/zerver/webhooks/github/view.py @@ -32,6 +32,7 @@ from zerver.lib.webhooks.git import ( get_push_commits_event_message, get_push_tag_event_message, get_release_event_message, + get_short_sha, ) from zerver.models import UserProfile @@ -419,7 +420,7 @@ def get_status_body(helper: Helper) -> str: else: status = payload["state"].tame(check_string) return "[{}]({}) changed its status to {}.".format( - payload["sha"].tame(check_string)[:7], # TODO + get_short_sha(payload["sha"].tame(check_string)), payload["commit"]["html_url"].tame(check_string), status, ) @@ -566,7 +567,7 @@ Check [{name}]({html_url}) {status} ({conclusion}). ([{short_hash}]({commit_url} "name": payload["check_run"]["name"].tame(check_string), "html_url": payload["check_run"]["html_url"].tame(check_string), "status": payload["check_run"]["status"].tame(check_string), - "short_hash": payload["check_run"]["head_sha"].tame(check_string)[:7], + "short_hash": get_short_sha(payload["check_run"]["head_sha"].tame(check_string)), "commit_url": "{}/commit/{}".format( payload["repository"]["html_url"].tame(check_string), payload["check_run"]["head_sha"].tame(check_string), diff --git a/zerver/webhooks/semaphore/tests.py b/zerver/webhooks/semaphore/tests.py index 13fcbf12aa..8609610784 100644 --- a/zerver/webhooks/semaphore/tests.py +++ b/zerver/webhooks/semaphore/tests.py @@ -19,7 +19,7 @@ class SemaphoreHookTests(WebhookTestCase): expected_topic = "knighthood/master" # repo/branch expected_message = """ [Build 314](https://semaphoreci.com/donquixote/knighthood/branches/master/builds/314) passed: -* **Commit**: [a490b8d: Create user account for Rocinante](https://github.com/donquixote/knighthood/commit/a490b8d508ebbdab1d77a5c2aefa35ceb2d62daf) +* **Commit**: [a490b8d508e: Create user account for Rocinante](https://github.com/donquixote/knighthood/commit/a490b8d508ebbdab1d77a5c2aefa35ceb2d62daf) * **Author**: don@lamancha.com """.strip() self.check_webhook( @@ -33,7 +33,7 @@ class SemaphoreHookTests(WebhookTestCase): expected_topic = "knighthood/master" expected_message = """ [Deploy 17](https://semaphoreci.com/donquixote/knighthood/servers/lamancha-271/deploys/17) of [build 314](https://semaphoreci.com/donquixote/knighthood/branches/master/builds/314) passed: -* **Commit**: [a490b8d: Create user account for Rocinante](https://github.com/donquixote/knighthood/commit/a490b8d508ebbdab1d77a5c2aefa35ceb2d62daf) +* **Commit**: [a490b8d508e: Create user account for Rocinante](https://github.com/donquixote/knighthood/commit/a490b8d508ebbdab1d77a5c2aefa35ceb2d62daf) * **Author**: don@lamancha.com * **Server**: lamancha-271 """.strip() @@ -50,7 +50,7 @@ class SemaphoreHookTests(WebhookTestCase): expected_topic = "notifications/rw/webhook_impl" # repo/branch expected_message = """ [Notifications](https://semaphore.semaphoreci.com/workflows/acabe58e-4bcc-4d39-be06-e98d71917703) pipeline **stopped**: -* **Commit**: [(2d9f5fc)](https://github.com/renderedtext/notifications/commit/2d9f5fcec1ca7c68fa7bd44dd58ec4ff65814563) Implement webhooks for SemaphoreCI +* **Commit**: [(2d9f5fcec1c)](https://github.com/renderedtext/notifications/commit/2d9f5fcec1ca7c68fa7bd44dd58ec4ff65814563) Implement webhooks for SemaphoreCI * **Branch**: rw/webhook_impl * **Author**: [radwo](https://github.com/radwo) """.strip() @@ -62,7 +62,7 @@ class SemaphoreHookTests(WebhookTestCase): expected_topic = "notifications/rw/webhook_impl" # repo/branch expected_message = """ [Notifications](https://semaphore.semaphoreci.com/workflows/acabe58e-4bcc-4d39-be06-e98d71917703) pipeline **stopped**: -* **Commit**: (2d9f5fc) Implement webhooks for SemaphoreCI +* **Commit**: (2d9f5fcec1c) Implement webhooks for SemaphoreCI * **Branch**: rw/webhook_impl * **Author**: radwo """.strip() diff --git a/zerver/webhooks/semaphore/view.py b/zerver/webhooks/semaphore/view.py index 565fa7a2c9..cc0d759717 100644 --- a/zerver/webhooks/semaphore/view.py +++ b/zerver/webhooks/semaphore/view.py @@ -9,6 +9,7 @@ from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success from zerver.lib.validator import WildValue, check_int, check_string, to_wild_value from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.git import get_short_sha from zerver.models import UserProfile # Semaphore Classic templates @@ -129,7 +130,7 @@ def semaphore_classic(payload: WildValue) -> Tuple[str, str, str, str]: build_number=build_number, build_url=build_url, status=result, - commit_hash=commit_id[:7], + commit_hash=get_short_sha(commit_id), commit_message=message, commit_url=commit_url, email=author_email, @@ -147,7 +148,7 @@ def semaphore_classic(payload: WildValue) -> Tuple[str, str, str, str]: build_number=build_number, build_url=build_url, status=result, - commit_hash=commit_id[:7], + commit_hash=get_short_sha(commit_id), commit_message=message, commit_url=commit_url, email=author_email, @@ -181,7 +182,7 @@ def semaphore_2(payload: WildValue) -> Tuple[str, str, Optional[str], str]: context.update( branch_name=branch_name, commit_id=commit_id, - commit_hash=commit_id[:7], + commit_hash=get_short_sha(commit_id), commit_message=summary_line(payload["revision"]["commit_message"].tame(check_string)), commit_url=GITHUB_URL_TEMPLATES["commit"].format( repo_url=repo_url, commit_id=commit_id diff --git a/zerver/webhooks/solano/tests.py b/zerver/webhooks/solano/tests.py index d39a634ca6..ffa2850686 100644 --- a/zerver/webhooks/solano/tests.py +++ b/zerver/webhooks/solano/tests.py @@ -14,7 +14,7 @@ class SolanoHookTests(WebhookTestCase): expected_message = """ Build update (see [build log](https://ci.solanolabs.com:443/reports/3316175)): * **Author**: solano-ci[bot]@users.noreply.github.com -* **Commit**: [5f43840](github.com/fazerlicourice7/solano/commit/5f438401eb7cc7268cbc28438bfa70bb99f48a03) +* **Commit**: [5f438401eb7](github.com/fazerlicourice7/solano/commit/5f438401eb7cc7268cbc28438bfa70bb99f48a03) * **Status**: failed :thumbs_down: """.strip() @@ -33,7 +33,7 @@ Build update (see [build log](https://ci.solanolabs.com:443/reports/3316175)): expected_message = """ Build update (see [build log](https://ci.solanolabs.com:443/reports/3316723)): * **Author**: Unknown -* **Commit**: [5d0b92e](bitbucket.org/fazerlicourice7/test/commits/5d0b92e26448a9e91db794bfed4b8c3556eabc4e) +* **Commit**: [5d0b92e2644](bitbucket.org/fazerlicourice7/test/commits/5d0b92e26448a9e91db794bfed4b8c3556eabc4e) * **Status**: failed :thumbs_down: """.strip() @@ -52,7 +52,7 @@ Build update (see [build log](https://ci.solanolabs.com:443/reports/3316723)): expected_message = """ Build update (see [build log](https://ci.solanolabs.com:443/reports/3317799)): * **Author**: solano-ci[bot]@users.noreply.github.com -* **Commit**: [191d34f](github.com/anirudhjain75/scipy/commit/191d34f9da8ff7279b051cd68e44223253e18408) +* **Commit**: [191d34f9da8](github.com/anirudhjain75/scipy/commit/191d34f9da8ff7279b051cd68e44223253e18408) * **Status**: running :arrows_counterclockwise: """.strip() diff --git a/zerver/webhooks/solano/view.py b/zerver/webhooks/solano/view.py index eb521208c8..f2aa1853dd 100644 --- a/zerver/webhooks/solano/view.py +++ b/zerver/webhooks/solano/view.py @@ -7,6 +7,7 @@ from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success from zerver.lib.validator import WildValue, check_string, to_wild_value from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.webhooks.git import get_short_sha from zerver.models import UserProfile MESSAGE_TEMPLATE = """ @@ -63,7 +64,7 @@ def api_solano_webhook( body = MESSAGE_TEMPLATE.format( author=author, build_log_url=build_log, - commit_id=commit_id[:7], + commit_id=get_short_sha(commit_id), commit_url=commit_url, status=status, emoji=emoji,