diff --git a/zerver/webhooks/bitbucket3/tests.py b/zerver/webhooks/bitbucket3/tests.py index 6c12782a15..13560f9901 100644 --- a/zerver/webhooks/bitbucket3/tests.py +++ b/zerver/webhooks/bitbucket3/tests.py @@ -32,7 +32,7 @@ class Bitbucket3HookTests(WebhookTestCase): self.send_and_test_stream_message("repo_forked", self.EXPECTED_TOPIC, expected_message) def test_bitbucket3_repo_modified(self) -> None: - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) changed the name of the **sandbox** repo from **sandbox** to **sandbox v2**""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) changed the name of the **sandbox** repo from **sandbox** to **sandbox v2**.""" expected_topic = "sandbox v2" self.send_and_test_stream_message("repo_modified", expected_topic, expected_message) @@ -115,7 +115,7 @@ class Bitbucket3HookTests(WebhookTestCase): def test_pr_opened_with_two_reviewers(self) -> None: expected_topic = "sandbox / PR #5 Add Notes Feature" - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) opened [PR #5](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/5)\nfrom `master` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) and [sougo](http://139.59.64.214:7990/users/sougo) for review)""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) opened [PR #5](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/5) from `master` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) and [sougo](http://139.59.64.214:7990/users/sougo) for review).""" self.send_and_test_stream_message("pull_request_opened_with_two_reviewers", expected_topic, expected_message) @@ -124,28 +124,28 @@ class Bitbucket3HookTests(WebhookTestCase): expected_topic = "sandbox / PR #5 Add Notes Feature" expected_topic = "custom_topic" self.url = self.build_webhook_url(topic='custom_topic') - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) opened [PR #5 Add Notes Feature](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/5)\nfrom `master` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) and [sougo](http://139.59.64.214:7990/users/sougo) for review)""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) opened [PR #5 Add Notes Feature](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/5) from `master` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) and [sougo](http://139.59.64.214:7990/users/sougo) for review).""" self.send_and_test_stream_message("pull_request_opened_with_two_reviewers", expected_topic, expected_message) def test_pr_opened_with_mulitple_reviewers(self) -> None: expected_topic = "sandbox / PR #6 sample_file: Add sample_file.txt." - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) opened [PR #6](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/6)\nfrom `master` to `master` (assigned to [sougo](http://139.59.64.214:7990/users/sougo), [zura](http://139.59.64.214:7990/users/zura) and [shimura](http://139.59.64.214:7990/users/shimura) for review)\n\n~~~ quote\nAdd a simple text file for further testing purposes.\n~~~""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) opened [PR #6](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/6) from `master` to `master` (assigned to [sougo](http://139.59.64.214:7990/users/sougo), [zura](http://139.59.64.214:7990/users/zura) and [shimura](http://139.59.64.214:7990/users/shimura) for review):\n\n~~~ quote\nAdd a simple text file for further testing purposes.\n~~~""" self.send_and_test_stream_message("pull_request_opened_with_multiple_reviewers", expected_topic, expected_message) def test_pr_modified(self) -> None: expected_topic = "sandbox / PR #1 Branch1" - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) modified [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1)\nfrom `branch1` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) for review)\n\n~~~ quote\n* Add file2.txt\n* Add file3.txt\nBoth of these files would be important additions to the project!\n~~~""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) modified [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) from `branch1` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) for review):\n\n~~~ quote\n* Add file2.txt\n* Add file3.txt\nBoth of these files would be important additions to the project!\n~~~""" self.send_and_test_stream_message("pull_request_modified", expected_topic, expected_message) def test_pr_modified_with_include_title(self) -> None: expected_topic = "custom_topic" - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) modified [PR #1 Branch1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1)\nfrom `branch1` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) for review)\n\n~~~ quote\n* Add file2.txt\n* Add file3.txt\nBoth of these files would be important additions to the project!\n~~~""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) modified [PR #1 Branch1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) from `branch1` to `master` (assigned to [shimura](http://139.59.64.214:7990/users/shimura) for review):\n\n~~~ quote\n* Add file2.txt\n* Add file3.txt\nBoth of these files would be important additions to the project!\n~~~""" self.url = self.build_webhook_url(topic='custom_topic') self.send_and_test_stream_message("pull_request_modified", expected_topic, @@ -197,28 +197,28 @@ class Bitbucket3HookTests(WebhookTestCase): def test_pr_marked_as_needs_review(self) -> None: expected_topic = "sandbox / PR #6 sample_file: Add sample_file.txt." - expected_message = """[zura](http://139.59.64.214:7990/users/zura) marked [PR #6](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/6) as \"needs work\"""" + expected_message = """[zura](http://139.59.64.214:7990/users/zura) marked [PR #6](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/6) as \"needs work\".""" self.send_and_test_stream_message("pull_request_needs_work", expected_topic, expected_message) def test_pr_marked_as_needs_review_and_include_title(self) -> None: expected_topic = "custom_topic" - expected_message = """[zura](http://139.59.64.214:7990/users/zura) marked [PR #6 sample_file: Add sample_file.txt.](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/6) as \"needs work\"""" + expected_message = """[zura](http://139.59.64.214:7990/users/zura) marked [PR #6 sample_file: Add sample_file.txt.](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/6) as \"needs work\".""" self.url = self.build_webhook_url(topic='custom_topic') self.send_and_test_stream_message("pull_request_needs_work", expected_topic, expected_message) def test_pull_request_reviewer_added(self) -> None: - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) reassigned [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) to [shimura](http://139.59.64.214:7990/users/shimura)""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) reassigned [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) to [shimura](http://139.59.64.214:7990/users/shimura).""" expected_topic = "sandbox / PR #1 Branch1" self.send_and_test_stream_message("pull_request_add_reviewer", expected_topic, expected_message) def test_pull_request_reviewer_added_and_include_title(self) -> None: - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) reassigned [PR #1 Branch1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) to [shimura](http://139.59.64.214:7990/users/shimura)""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) reassigned [PR #1 Branch1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) to [shimura](http://139.59.64.214:7990/users/shimura).""" expected_topic = "custom_topic" self.url = self.build_webhook_url(topic='custom_topic') self.send_and_test_stream_message("pull_request_add_reviewer", @@ -226,21 +226,21 @@ class Bitbucket3HookTests(WebhookTestCase): expected_message) def test_pull_request_reviewers_added(self) -> None: - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) reassigned [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) to [shimura](http://139.59.64.214:7990/users/shimura) and [sougo](http://139.59.64.214:7990/users/sougo)""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) reassigned [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1) to [shimura](http://139.59.64.214:7990/users/shimura) and [sougo](http://139.59.64.214:7990/users/sougo).""" expected_topic = "sandbox / PR #1 Branch1" self.send_and_test_stream_message("pull_request_add_two_reviewers", expected_topic, expected_message) def test_pull_request_remove_all_reviewers(self) -> None: - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) removed all reviewers from [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1)""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) removed all reviewers from [PR #1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1).""" expected_topic = "sandbox / PR #1 Branch1" self.send_and_test_stream_message("pull_request_remove_reviewer", expected_topic, expected_message) def test_pull_request_remove_all_reviewers_with_title(self) -> None: - expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) removed all reviewers from [PR #1 Branch1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1)""" + expected_message = """[hypro999](http://139.59.64.214:7990/users/hypro999) removed all reviewers from [PR #1 Branch1](http://139.59.64.214:7990/projects/SBOX/repos/sandbox/pull-requests/1).""" expected_topic = "sandbox / PR #1 Branch1" expected_topic = "custom_topic" self.url = self.build_webhook_url(topic='custom_topic') diff --git a/zerver/webhooks/bitbucket3/view.py b/zerver/webhooks/bitbucket3/view.py index 71347bfffe..44b8e64b0d 100644 --- a/zerver/webhooks/bitbucket3/view.py +++ b/zerver/webhooks/bitbucket3/view.py @@ -1,5 +1,6 @@ from inspect import signature from functools import partial +import string from typing import Any, Dict, Optional, List, Callable from django.http import HttpRequest, HttpResponse @@ -19,23 +20,26 @@ from zerver.webhooks.bitbucket2.view import BITBUCKET_TOPIC_TEMPLATE, \ BITBUCKET_FORK_BODY, BITBUCKET_REPO_UPDATED_CHANGED BRANCH_UPDATED_MESSAGE_TEMPLATE = "{user_name} pushed to branch {branch_name}. Head is now {head}." -PULL_REQUEST_MARKED_AS_NEEDS_WORK_TEMPLATE = """{user_name} marked [PR #{number}]({url}) \ -as \"needs work\"""" -PULL_REQUEST_MARKED_AS_NEEDS_WORK_TEMPLATE_WITH_TITLE = """{user_name} marked \ -[PR #{number} {title}]({url}) as \"needs work\"""" -PULL_REQUEST_REASSIGNED_TEMPLATE = """{user_name} reassigned [PR #{number}]({url}) \ -to {assignees}""" -PULL_REQUEST_REASSIGNED_TEMPLATE_WITH_TITLE = """{user_name} reassigned [PR #{number} \ -{title}]({url}) to {assignees}""" -PULL_REQUEST_REASSIGNED_TO_NONE_TEMPLATE = """{user_name} removed all reviewers from [PR \ -#{number}]({url})""" -PULL_REQUEST_REASSIGNED_TO_NONE_TEMPLATE_WITH_TITLE = """{user_name} removed all reviewers \ -from [PR #{number} {title}]({url})""" -PULL_REQUEST_OPENED_OR_MODIFIED_TEMPLATE_WITH_REVIEWERS = """{user_name} {action} [PR #{number}]\ -({url})\nfrom `{source}` to `{destination}` (assigned to {assignees} for review)""" -PULL_REQUEST_OPENED_OR_MODIFIED_TEMPLATE_WITH_REVIEWERS_WITH_TITLE = """{user_name} {action} \ -[PR #{number} {title}]({url})\nfrom `{source}` to `{destination}` (assigned to {assignees} for \ -review)""" +PULL_REQUEST_MARKED_AS_NEEDS_WORK_TEMPLATE = "{user_name} marked [PR #{number}]({url}) as \"needs work\"." +PULL_REQUEST_MARKED_AS_NEEDS_WORK_TEMPLATE_WITH_TITLE = """ +{user_name} marked [PR #{number} {title}]({url}) as \"needs work\". +""".strip() +PULL_REQUEST_REASSIGNED_TEMPLATE = "{user_name} reassigned [PR #{number}]({url}) to {assignees}." +PULL_REQUEST_REASSIGNED_TEMPLATE_WITH_TITLE = """ +{user_name} reassigned [PR #{number} {title}]({url}) to {assignees}. +""".strip() +PULL_REQUEST_REASSIGNED_TO_NONE_TEMPLATE = "{user_name} removed all reviewers from [PR #{number}]({url})." +PULL_REQUEST_REASSIGNED_TO_NONE_TEMPLATE_WITH_TITLE = """ +{user_name} removed all reviewers from [PR #{number} {title}]({url}) +""".strip() +PULL_REQUEST_OPENED_OR_MODIFIED_TEMPLATE_WITH_REVIEWERS = """ +{user_name} {action} [PR #{number}]({url}) from `{source}` to \ +`{destination}` (assigned to {assignees} for review) +""".strip() +PULL_REQUEST_OPENED_OR_MODIFIED_TEMPLATE_WITH_REVIEWERS_WITH_TITLE = """ +{user_name} {action} [PR #{number} {title}]({url}) from `{source}` to \ +`{destination}` (assigned to {assignees} for review) +""".strip() def get_user_name(payload: Dict[str, Any]) -> str: user_name = "[{name}]({url})".format(name=payload["actor"]["name"], @@ -73,13 +77,16 @@ def repo_forked_handler(payload: Dict[str, Any]) -> List[Dict[str, str]]: def repo_modified_handler(payload: Dict[str, Any]) -> List[Dict[str, str]]: subject_new = BITBUCKET_TOPIC_TEMPLATE.format(repository_name=payload["new"]["name"]) + new_name = payload['new']['name'] body = BITBUCKET_REPO_UPDATED_CHANGED.format( actor=get_user_name(payload), change="name", repo_name=payload["old"]["name"], old=payload["old"]["name"], - new=payload["new"]["name"] + new=new_name ) # As of writing this, the only change we'd be notified about is a name change. + punctuation = '.' if new_name[-1] not in string.punctuation else '' + body = "{}{}".format(body, punctuation) return [{"subject": subject_new, "body": body}] def repo_push_branch_data(payload: Dict[str, Any], change: Dict[str, Any]) -> Dict[str, str]: @@ -195,6 +202,8 @@ def get_pr_opened_or_modified_body(payload: Dict[str, Any], action: str, ) else: body = PULL_REQUEST_OPENED_OR_MODIFIED_TEMPLATE_WITH_REVIEWERS.format(**parameters) + punctuation = ':' if description else '.' + body = "{}{}".format(body, punctuation) if description: body += '\n' + CONTENT_MESSAGE_TEMPLATE.format(message=description) return body @@ -235,12 +244,15 @@ def get_pr_reassigned_body(payload: Dict[str, Any], include_title: Optional[bool number=pr["id"], url=pr["links"]["self"][0]["href"] ) - return PULL_REQUEST_REASSIGNED_TO_NONE_TEMPLATE_WITH_TITLE.format( + punctuation = '.' if pr['title'][-1] not in string.punctuation else '' + message = PULL_REQUEST_REASSIGNED_TO_NONE_TEMPLATE_WITH_TITLE.format( user_name=get_user_name(payload), number=pr["id"], url=pr["links"]["self"][0]["href"], title=pr["title"] ) + message = "{}{}".format(message, punctuation) + return message if not include_title: return PULL_REQUEST_REASSIGNED_TEMPLATE.format( user_name=get_user_name(payload),