mirror of
https://github.com/zulip/zulip.git
synced 2025-11-05 06:23:38 +00:00
webhooks/github: Handle empty 'requested_reviewers' key.
We recently received a bug report that implied that for certain payloads, the `requested_reviewers` key was empty whereas a singular `requested_reviewer` key containing one reviewer's information was present in its stead. Naturally, this raised some not so pretty IndexError exceptions. After some investigation and generating a few similar payloads, I discovered that in every case both the `requested_reviewers` and the `requested_reviewer` keys were correctly populated, so I had to manually edit the payload to reproduce the error on my end. My guess is that this anomaly goes back to when GitHub's reviewer request feature was new and didn't support requesting multiple reviewers, and that the singular `requested_reviewer` key could possibly just be there for backwards compatibility or might just be mere oversight. Either way, the solution here is to look for the plural `requested_reviewers` key, and if that is empty, fall back to the singular `requested_reviewer` key.
This commit is contained in:
@@ -284,6 +284,13 @@ class GithubWebhookTest(WebhookTestCase):
|
||||
expected_message,
|
||||
HTTP_X_GITHUB_EVENT='pull_request')
|
||||
|
||||
def test_pull_request_review_requested_singular_key_msg(self) -> None:
|
||||
expected_message = u"**eeshangarg** requested [rishig](https://github.com/rishig) for a review on [PR #6](https://github.com/eeshangarg/Scheduler/pull/6)."
|
||||
self.send_and_test_stream_message('pull_request_review_requested_singular_key',
|
||||
'Scheduler / PR #6 Mention how awesome this project is in ...',
|
||||
expected_message,
|
||||
HTTP_X_GITHUB_EVENT='pull_request')
|
||||
|
||||
def test_pull_request_review_requested_multiple_reviwers_msg(self) -> None:
|
||||
expected_message = u"**eeshangarg** requested [showell](https://github.com/showell), and [timabbott](https://github.com/timabbott) for a review on [PR #1](https://github.com/eeshangarg/Scheduler/pull/1)."
|
||||
self.send_and_test_stream_message('pull_request_review_requested_multiple_reviewers',
|
||||
|
||||
Reference in New Issue
Block a user