diff --git a/README.md b/README.md index b387b61f1..487a1bddb 100644 --- a/README.md +++ b/README.md @@ -1155,7 +1155,7 @@ The following sets of tools are available: - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number to update (number, required) - `repo`: Repository name (string, required) - - `reviewers`: GitHub usernames to request reviews from (string[], optional) + - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional) - `state`: New state (string, optional) - `title`: New title (string, optional) diff --git a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap index 67b701447..7e6d33a27 100644 --- a/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap +++ b/pkg/github/__toolsnaps__/request_pull_request_reviewers.snap @@ -21,7 +21,7 @@ "type": "string" }, "reviewers": { - "description": "GitHub usernames to request reviews from", + "description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from", "items": { "type": "string" }, @@ -37,4 +37,4 @@ "type": "object" }, "name": "request_pull_request_reviewers" -} \ No newline at end of file +} diff --git a/pkg/github/__toolsnaps__/update_pull_request.snap b/pkg/github/__toolsnaps__/update_pull_request.snap index ef330188f..640df7970 100644 --- a/pkg/github/__toolsnaps__/update_pull_request.snap +++ b/pkg/github/__toolsnaps__/update_pull_request.snap @@ -34,7 +34,7 @@ "type": "string" }, "reviewers": { - "description": "GitHub usernames to request reviews from", + "description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from", "items": { "type": "string" }, @@ -61,4 +61,4 @@ "type": "object" }, "name": "update_pull_request" -} \ No newline at end of file +} diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index 59eb47822..91b6b044b 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -635,7 +635,10 @@ func TestGranularUpdatePullRequestState(t *testing.T) { func TestGranularRequestPullRequestReviewers(t *testing.T) { client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)}), + PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "reviewers": []any{"user1"}, + "team_reviewers": []any{"team1"}, + }).andThen(mockResponse(t, http.StatusOK, &gogithub.PullRequest{Number: gogithub.Ptr(1)})), })) deps := BaseDeps{Client: client} serverTool := GranularRequestPullRequestReviewers(translations.NullTranslationHelper) @@ -645,7 +648,7 @@ func TestGranularRequestPullRequestReviewers(t *testing.T) { "owner": "owner", "repo": "repo", "pullNumber": float64(1), - "reviewers": []string{"user1", "user2"}, + "reviewers": []string{"user1", "owner/team1"}, }) result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index c298d875a..cbf8a2ff3 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -752,7 +752,7 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo }, "reviewers": { Type: "array", - Description: "GitHub usernames to request reviews from", + Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from", Items: &jsonschema.Schema{ Type: "string", }, @@ -944,8 +944,10 @@ func UpdatePullRequest(t translations.TranslationHelperFunc) inventory.ServerToo return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } + userReviewers, teamReviewers := splitPullRequestReviewers(reviewers) reviewersRequest := github.ReviewersRequest{ - Reviewers: reviewers, + Reviewers: userReviewers, + TeamReviewers: teamReviewers, } _, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, reviewersRequest) diff --git a/pkg/github/pullrequests_granular.go b/pkg/github/pullrequests_granular.go index 30d7f78d6..6bc2b99f3 100644 --- a/pkg/github/pullrequests_granular.go +++ b/pkg/github/pullrequests_granular.go @@ -297,7 +297,7 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i "pullNumber": {Type: "number", Description: "The pull request number", Minimum: jsonschema.Ptr(1.0)}, "reviewers": { Type: "array", - Description: "GitHub usernames to request reviews from", + Description: "GitHub usernames or ORG/team-slug team reviewers to request reviews from", Items: &jsonschema.Schema{Type: "string"}, }, }, @@ -325,13 +325,17 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i if len(reviewers) == 0 { return utils.NewToolResultError("missing required parameter: reviewers"), nil, nil } + userReviewers, teamReviewers := splitPullRequestReviewers(reviewers) client, err := deps.GetClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{Reviewers: reviewers}) + pr, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, gogithub.ReviewersRequest{ + Reviewers: userReviewers, + TeamReviewers: teamReviewers, + }) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to request reviewers", resp, err), nil, nil } @@ -351,6 +355,22 @@ func GranularRequestPullRequestReviewers(t translations.TranslationHelperFunc) i return st } +func splitPullRequestReviewers(reviewers []string) ([]string, []string) { + userReviewers := make([]string, 0, len(reviewers)) + teamReviewers := make([]string, 0) + + for _, reviewer := range reviewers { + org, team, ok := strings.Cut(reviewer, "/") + if ok && org != "" && team != "" && !strings.Contains(team, "/") { + teamReviewers = append(teamReviewers, team) + continue + } + userReviewers = append(userReviewers, reviewer) + } + + return userReviewers, teamReviewers +} + // GranularCreatePullRequestReview creates a tool to create a PR review. func GranularCreatePullRequestReview(t translations.TranslationHelperFunc) inventory.ServerTool { st := NewTool( diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 097651b66..0faee23e2 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -257,6 +257,24 @@ func Test_UpdatePullRequest(t *testing.T) { expectError: false, expectedPR: mockPRWithReviewers, }, + { + name: "successful PR update with user and team reviewers", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber: expectRequestBody(t, map[string]any{ + "reviewers": []any{"reviewer1"}, + "team_reviewers": []any{"platform"}, + }).andThen(mockResponse(t, http.StatusOK, mockPRWithReviewers)), + GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPRWithReviewers), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "reviewers": []any{"reviewer1", "owner/platform"}, + }, + expectError: false, + expectedPR: mockPRWithReviewers, + }, { name: "successful PR update (title only)", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{