According to GitHub's webhook docs, the scope of a membership
event can only be limited to 'teams', which holds true when a
new member is added to a team. However, we just found a payload
in our logs that indicates that when a user is removed from a
team, the scope of the membership is erroneously set to
'organization', not 'team'. This is most likely a bug on
GitHub's end because such behaviour is a direct violation of
their webhook API event specifications. We account for this
by restricting membership events to teams explicitly, at least
till GitHub's docs suggest otherwise.
We only support notifications for events where a check run has
completed. Notifications for when a check run has been queued or
is in progress are not very informative and may be too noisy.
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.
It was a painful amount of work to generate the actual payload.
Since the only difference was a small build URL, I manually
edited the payload and used that for testing.
This commit gets our GitHub webhook up to 100% test coverage.
This is a part of our efforts to get this webhook's coverage
up to 100%.
Note that apart from just testing an uncovered line of code, this
commit also fixes a minor bug in the code for messages about issue
comment deletion and editing.
All webhook fixtures in zerver/fixtures/<webhook_name> have now
been moved to dedicated webhook-specific directories under
zerver/webhooks/<webhook_name>/fixtures, where <webhook_name> is
the name of the webhook.
Eeshan Garg
Hemanth V. Alluri