fix adding/removing team reviewers with gh pr edit#3701
Conversation
Collaborator
|
Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message. |
gh pr edit
samcoe
approved these changes
May 27, 2021
Contributor
samcoe
left a comment
There was a problem hiding this comment.
This looks great! Thanks for the contribution. The thorough explanation of the details of the bug and the added tests made this very easy to follow 👍
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3705
The issue:
According to the documentation of
gh pr edit, the--add-reviewerand--remove-reviewerflags accept team users with the following format:myorg/team-slug.If the PR doesn't have any team user specified in the "reviewers" section, the existing code works.
But if the PR already has a team user specified in the "reviewers" section, the
gh pr edit 1 --add-reviewer myorg/other-teamcommand fails with the error message:'' not found.Identifying the root cause:
This error is returned by the TeamsToIDs function in
api/queries_repo.go, because one element innamesis an empty string.Going further in debugging this, I found out that the empty string was generated by the Logins function in
api/queries_pr.go.The Logins function was using the RequestedReviewer.Login attribute for both normal users and team users. But teams don't have a Login field (see Team graphql reference). So, when parsing an existing PR with a team as reviewer, the Logins function returns a list with an empty string element.
Fix proposal:
This fix updates the graphql query to get the slug and the organization login in case of a team element, and it adds a check on the typename in Logins to build the login for a team concatenating the org login and the team slug. That way the team login can be compared to values in --add-reviewer, --remove-reviewer flags and in the metadata of the repo.
Testing:
I added a simple unit test for the Logins function. I also built the gh command with the changes and manually tried the
gh pr editcommand with different combinations of the--add-reviewerand--remove-reviewerflags.Thank you in advance for your inputs on this PR. Please let me know in case changes to the fix would be needed.