palantir / policy-bot

A GitHub App that enforces approval policies on pull requests
Apache License 2.0
780 stars 108 forks source link

Feature request / Github API bug workaround - deduplicate team review requests before send #855

Closed neil176 closed 1 month ago

neil176 commented 1 month ago

Would it be possible to deduplicate team review requests prior to making the request to the Github API? We have observed that if some combination of policy clauses results in the same team being included twice, the Github API returns an error like this despite all configuration on the repo permissions being correct and the request succeeding when the list is deduped:

422 Reviews may only be requested from collaborators.
One or more of the teams you specified is not a collaborator of the something/something repository.

I'm happy to contribute a PR for this if it would be accepted.

Edit: Also want to note that this does not appear to be a problem for user review requests, only teams.

maschwenk commented 1 month ago

Reproing the request that PolicyBot makes with curl (on 1.36.0):

curl --location 'https://api.github.com/repos/orgName/repoName/pulls/7523/requested_reviewers' \
--header 'Content-Type: application/json' \
--header 'Authorization: Bearer <your PAT>' \
--data '{"team_reviewers":["team-A", "team-B", "team-A"]}'

which yields:

{
    "message": "Reviews may only be requested from collaborators. One or more of the teams you specified is not a collaborator of the latticehr/infrastructure repository.",
    "documentation_url": "https://docs.github.com/rest/pulls/review-requests#request-reviewers-for-a-pull-request",
    "status": "422"
}
bluekeyes commented 1 month ago

Good catch, we should be deduplicating these lists (for both users and teams) even if the GitHub API accepted duplicates.

If you'd like to submit a PR, that would be great or I could try to fix this in the next few days.

neil176 commented 1 month ago

Wow thank you @bluekeyes and @FraBle that was fast!