Skip to content

Commit cf48de0

Browse files
committed
fix: support team pull request reviewers
1 parent 1add5fe commit cf48de0

7 files changed

Lines changed: 54 additions & 11 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1155,7 +1155,7 @@ The following sets of tools are available:
11551155
- `owner`: Repository owner (string, required)
11561156
- `pullNumber`: Pull request number to update (number, required)
11571157
- `repo`: Repository name (string, required)
1158-
- `reviewers`: GitHub usernames to request reviews from (string[], optional)
1158+
- `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional)
11591159
- `state`: New state (string, optional)
11601160
- `title`: New title (string, optional)
11611161

pkg/github/__toolsnaps__/request_pull_request_reviewers.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"type": "string"
2222
},
2323
"reviewers": {
24-
"description": "GitHub usernames to request reviews from",
24+
"description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from",
2525
"items": {
2626
"type": "string"
2727
},
@@ -37,4 +37,4 @@
3737
"type": "object"
3838
},
3939
"name": "request_pull_request_reviewers"
40-
}
40+
}

pkg/github/__toolsnaps__/update_pull_request.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
"type": "string"
3535
},
3636
"reviewers": {
37-
"description": "GitHub usernames to request reviews from",
37+
"description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from",
3838
"items": {
3939
"type": "string"
4040
},
@@ -61,4 +61,4 @@
6161
"type": "object"
6262
},
6363
"name": "update_pull_request"
64-
}
64+
}

pkg/github/granular_tools_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,10 @@ func TestGranularUpdatePullRequestState(t *testing.T) {
635635

636636
func TestGranularRequestPullRequestReviewers(t *testing.T) {
637637
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
638-
PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)}),
638+
PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
639+
"reviewers": []any{"user1"},
640+
"team_reviewers": []any{"team1"},
641+
}).andThen(mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)})),
639642
}))
640643
deps := BaseDeps{Client: client}
641644
serverTool := GranularRequestPullRequestReviewers(translations.NullTranslationHelper)
@@ -645,7 +648,7 @@ func TestGranularRequestPullRequestReviewers(t *testing.T) {
645648
"owner": "owner",
646649
"repo": "repo",
647650
"pullNumber": float64(1),
648-
"reviewers": []string{"user1", "user2"},
651+
"reviewers": []string{"user1", "owner/team1"},
649652
})
650653
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
651654
require.NoError(t, err)

pkg/github/pullrequests.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
752752
},
753753
"reviewers": {
754754
Type: "array",
755-
Description: "GitHub usernames to request reviews from",
755+
Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from",
756756
Items: &jsonschema.Schema{
757757
Type: "string",
758758
},
@@ -944,8 +944,10 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo
944944
return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil
945945
}
946946

947+
userReviewers, teamReviewers := splitPullRequestReviewers(reviewers)
947948
reviewersRequest := github.ReviewersRequest{
948-
Reviewers: reviewers,
949+
Reviewers: userReviewers,
950+
TeamReviewers: teamReviewers,
949951
}
950952

951953
_, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, reviewersRequest)

pkg/github/pullrequests_granular.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i
297297
"pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)},
298298
"reviewers": {
299299
Type: "array",
300-
Description: "GitHub usernames to request reviews from",
300+
Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from",
301301
Items: &jsonschema.Schema{Type: "string"},
302302
},
303303
},
@@ -325,13 +325,17 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i
325325
if len(reviewers) == 0 {
326326
return utils.NewToolResultError("missing required parameter: reviewers"), nil, nil
327327
}
328+
userReviewers, teamReviewers := splitPullRequestReviewers(reviewers)
328329

329330
client, err := deps.GetClient(ctx)
330331
if err != nil {
331332
return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil
332333
}
333334

334-
pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{Reviewers: reviewers})
335+
pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{
336+
Reviewers: userReviewers,
337+
TeamReviewers: teamReviewers,
338+
})
335339
if err != nil {
336340
return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to request reviewers", resp, err), nil, nil
337341
}
@@ -351,6 +355,22 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i
351355
return st
352356
}
353357

358+
func splitPullRequestReviewers(reviewers []string) ([]string, []string) {
359+
userReviewers := make([]string, 0, len(reviewers))
360+
teamReviewers := make([]string, 0)
361+
362+
for _, reviewer := range reviewers {
363+
org, team, ok := strings.Cut(reviewer, "/")
364+
if ok && org != "" && team != "" && !strings.Contains(team, "/") {
365+
teamReviewers = append(teamReviewers, team)
366+
continue
367+
}
368+
userReviewers = append(userReviewers, reviewer)
369+
}
370+
371+
return userReviewers, teamReviewers
372+
}
373+
354374
// GranularCreatePullRequestReview creates a tool to create a PR review.
355375
func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool {
356376
st := NewTool(

pkg/github/pullrequests_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,24 @@ func Test_UpdatePullRequest(t *testing.T) {
257257
expectError: false,
258258
expectedPR: mockPRWithReviewers,
259259
},
260+
{
261+
name: "successful PR update with user and team reviewers",
262+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
263+
PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{
264+
"reviewers": []any{"reviewer1"},
265+
"team_reviewers": []any{"platform"},
266+
}).andThen(mockResponse(t, http.StatusOK, mockPRWithReviewers)),
267+
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPRWithReviewers),
268+
}),
269+
requestArgs: map[string]any{
270+
"owner": "owner",
271+
"repo": "repo",
272+
"pullNumber": float64(42),
273+
"reviewers": []any{"reviewer1", "owner/platform"},
274+
},
275+
expectError: false,
276+
expectedPR: mockPRWithReviewers,
277+
},
260278
{
261279
name: "successful PR update (title only)",
262280
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{

0 commit comments

Comments
 (0)