Merge pull request #12567 from cli/kw/multi-select-with-search-and-ccr
`gh pr edit`: Add support for Copilot as reviewer with search capability, performance and accessibility improvements
This commit is contained in:
Kynan Ware
GitHub
commit
fa43288852
9 changed files with 965 additions and 85 deletions
|
|
@ -6,6 +6,7 @@ import (
|
|||
"fmt"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
|
|
@ -316,6 +317,9 @@ type RequestedReviewer struct {
|
|||
} `json:"organization"`
|
||||
}
|
||||
|
||||
const teamTypeName = "Team"
|
||||
const botTypeName = "Bot"
|
||||
|
||||
func (r RequestedReviewer) LoginOrSlug() string {
|
||||
if r.TypeName == teamTypeName {
|
||||
return fmt.Sprintf("%s/%s", r.Organization.Login, r.Slug)
|
||||
|
|
@ -323,7 +327,21 @@ func (r RequestedReviewer) LoginOrSlug() string {
|
|||
return r.Login
|
||||
}
|
||||
|
||||
const teamTypeName = "Team"
|
||||
// DisplayName returns a user-friendly name for the reviewer.
|
||||
// For Copilot bot, returns "Copilot (AI)". For teams, returns "org/slug".
|
||||
// For users, returns "login (Name)" if name is available, otherwise just login.
|
||||
func (r RequestedReviewer) DisplayName() string {
|
||||
if r.TypeName == teamTypeName {
|
||||
return fmt.Sprintf("%s/%s", r.Organization.Login, r.Slug)
|
||||
}
|
||||
if r.TypeName == botTypeName && r.Login == CopilotReviewerLogin {
|
||||
return "Copilot (AI)"
|
||||
}
|
||||
if r.Name != "" {
|
||||
return fmt.Sprintf("%s (%s)", r.Login, r.Name)
|
||||
}
|
||||
return r.Login
|
||||
}
|
||||
|
||||
func (r ReviewRequests) Logins() []string {
|
||||
logins := make([]string, len(r.Nodes))
|
||||
|
|
@ -333,6 +351,15 @@ func (r ReviewRequests) Logins() []string {
|
|||
return logins
|
||||
}
|
||||
|
||||
// DisplayNames returns user-friendly display names for all requested reviewers.
|
||||
func (r ReviewRequests) DisplayNames() []string {
|
||||
names := make([]string, len(r.Nodes))
|
||||
for i, r := range r.Nodes {
|
||||
names[i] = r.RequestedReviewer.DisplayName()
|
||||
}
|
||||
return names
|
||||
}
|
||||
|
||||
func (pr PullRequest) HeadLabel() string {
|
||||
if pr.IsCrossRepository {
|
||||
return fmt.Sprintf("%s:%s", pr.HeadRepositoryOwner.Login, pr.HeadRefName)
|
||||
|
|
@ -631,7 +658,32 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter
|
|||
return pr, nil
|
||||
}
|
||||
|
||||
// extractTeamSlugs extracts just the slug portion from team identifiers.
|
||||
// Team identifiers can be in "org/slug" format; this returns just the slug.
|
||||
func extractTeamSlugs(teams []string) []string {
|
||||
slugs := make([]string, 0, len(teams))
|
||||
for _, t := range teams {
|
||||
if t == "" {
|
||||
continue
|
||||
}
|
||||
s := strings.SplitN(t, "/", 2)
|
||||
slugs = append(slugs, s[len(s)-1])
|
||||
}
|
||||
return slugs
|
||||
}
|
||||
|
||||
// toGitHubV4Strings converts a string slice to a githubv4.String slice,
|
||||
// optionally appending a suffix to each element.
|
||||
func toGitHubV4Strings(strs []string, suffix string) []githubv4.String {
|
||||
result := make([]githubv4.String, len(strs))
|
||||
for i, s := range strs {
|
||||
result[i] = githubv4.String(s + suffix)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// AddPullRequestReviews adds the given user and team reviewers to a pull request using the REST API.
|
||||
// Team identifiers can be in "org/slug" format.
|
||||
func AddPullRequestReviews(client *Client, repo ghrepo.Interface, prNumber int, users, teams []string) error {
|
||||
if len(users) == 0 && len(teams) == 0 {
|
||||
return nil
|
||||
|
|
@ -641,9 +693,6 @@ func AddPullRequestReviews(client *Client, repo ghrepo.Interface, prNumber int,
|
|||
if users == nil {
|
||||
users = []string{}
|
||||
}
|
||||
if teams == nil {
|
||||
teams = []string{}
|
||||
}
|
||||
|
||||
path := fmt.Sprintf(
|
||||
"repos/%s/%s/pulls/%d/requested_reviewers",
|
||||
|
|
@ -656,7 +705,7 @@ func AddPullRequestReviews(client *Client, repo ghrepo.Interface, prNumber int,
|
|||
TeamReviewers []string `json:"team_reviewers"`
|
||||
}{
|
||||
Reviewers: users,
|
||||
TeamReviewers: teams,
|
||||
TeamReviewers: extractTeamSlugs(teams),
|
||||
}
|
||||
buf := &bytes.Buffer{}
|
||||
if err := json.NewEncoder(buf).Encode(body); err != nil {
|
||||
|
|
@ -667,6 +716,7 @@ func AddPullRequestReviews(client *Client, repo ghrepo.Interface, prNumber int,
|
|||
}
|
||||
|
||||
// RemovePullRequestReviews removes requested reviewers from a pull request using the REST API.
|
||||
// Team identifiers can be in "org/slug" format.
|
||||
func RemovePullRequestReviews(client *Client, repo ghrepo.Interface, prNumber int, users, teams []string) error {
|
||||
if len(users) == 0 && len(teams) == 0 {
|
||||
return nil
|
||||
|
|
@ -676,9 +726,6 @@ func RemovePullRequestReviews(client *Client, repo ghrepo.Interface, prNumber in
|
|||
if users == nil {
|
||||
users = []string{}
|
||||
}
|
||||
if teams == nil {
|
||||
teams = []string{}
|
||||
}
|
||||
|
||||
path := fmt.Sprintf(
|
||||
"repos/%s/%s/pulls/%d/requested_reviewers",
|
||||
|
|
@ -691,7 +738,7 @@ func RemovePullRequestReviews(client *Client, repo ghrepo.Interface, prNumber in
|
|||
TeamReviewers []string `json:"team_reviewers"`
|
||||
}{
|
||||
Reviewers: users,
|
||||
TeamReviewers: teams,
|
||||
TeamReviewers: extractTeamSlugs(teams),
|
||||
}
|
||||
buf := &bytes.Buffer{}
|
||||
if err := json.NewEncoder(buf).Encode(body); err != nil {
|
||||
|
|
@ -701,6 +748,55 @@ func RemovePullRequestReviews(client *Client, repo ghrepo.Interface, prNumber in
|
|||
return client.REST(repo.RepoHost(), "DELETE", path, buf, nil)
|
||||
}
|
||||
|
||||
// RequestReviewsByLogin sets requested reviewers on a pull request using the GraphQL mutation.
|
||||
// This mutation replaces existing reviewers with the provided set unless union is true.
|
||||
// Only available on github.com, not GHES.
|
||||
// Bot logins should include the [bot] suffix (e.g., "copilot-pull-request-reviewer[bot]").
|
||||
// Team slugs should be in the format "org/team-slug".
|
||||
// When union is false (replace mode), passing empty slices will remove all reviewers.
|
||||
func RequestReviewsByLogin(client *Client, repo ghrepo.Interface, prID string, userLogins, botLogins, teamSlugs []string, union bool) error {
|
||||
// In union mode (additive), nothing to do if all lists are empty.
|
||||
// In replace mode, we may still need to call the mutation to clear reviewers.
|
||||
if union && len(userLogins) == 0 && len(botLogins) == 0 && len(teamSlugs) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
var mutation struct {
|
||||
RequestReviewsByLogin struct {
|
||||
ClientMutationId string `graphql:"clientMutationId"`
|
||||
} `graphql:"requestReviewsByLogin(input: $input)"`
|
||||
}
|
||||
|
||||
type RequestReviewsByLoginInput struct {
|
||||
PullRequestID githubv4.ID `json:"pullRequestId"`
|
||||
UserLogins *[]githubv4.String `json:"userLogins,omitempty"`
|
||||
BotLogins *[]githubv4.String `json:"botLogins,omitempty"`
|
||||
TeamSlugs *[]githubv4.String `json:"teamSlugs,omitempty"`
|
||||
Union githubv4.Boolean `json:"union"`
|
||||
}
|
||||
|
||||
input := RequestReviewsByLoginInput{
|
||||
PullRequestID: githubv4.ID(prID),
|
||||
Union: githubv4.Boolean(union),
|
||||
}
|
||||
|
||||
userLoginValues := toGitHubV4Strings(userLogins, "")
|
||||
input.UserLogins = &userLoginValues
|
||||
|
||||
// Bot logins require the [bot] suffix for the mutation
|
||||
botLoginValues := toGitHubV4Strings(botLogins, "[bot]")
|
||||
input.BotLogins = &botLoginValues
|
||||
|
||||
teamSlugValues := toGitHubV4Strings(teamSlugs, "")
|
||||
input.TeamSlugs = &teamSlugValues
|
||||
|
||||
variables := map[string]interface{}{
|
||||
"input": input,
|
||||
}
|
||||
|
||||
return client.Mutate(repo.RepoHost(), "RequestReviewsByLogin", &mutation, variables)
|
||||
}
|
||||
|
||||
// SuggestedAssignableActors fetches up to 10 suggested actors for a specific assignable
|
||||
// (Issue or PullRequest) node ID. `assignableID` is the GraphQL node ID for the Issue/PR.
|
||||
// Returns the actors, the total count of available assignees in the repo, and an error.
|
||||
|
|
@ -793,6 +889,222 @@ func SuggestedAssignableActors(client *Client, repo ghrepo.Interface, assignable
|
|||
return actors, availableAssigneesCount, nil
|
||||
}
|
||||
|
||||
// ReviewerCandidate represents a potential reviewer for a pull request.
|
||||
// This can be a User, Bot, or Team.
|
||||
type ReviewerCandidate interface {
|
||||
DisplayName() string
|
||||
Login() string
|
||||
|
||||
sealedReviewerCandidate()
|
||||
}
|
||||
|
||||
// ReviewerUser is a user who can review a pull request.
|
||||
type ReviewerUser struct {
|
||||
AssignableUser
|
||||
}
|
||||
|
||||
func NewReviewerUser(login, name string) ReviewerUser {
|
||||
return ReviewerUser{
|
||||
AssignableUser: NewAssignableUser("", login, name),
|
||||
}
|
||||
}
|
||||
|
||||
func (r ReviewerUser) sealedReviewerCandidate() {}
|
||||
|
||||
// ReviewerBot is a bot who can review a pull request.
|
||||
type ReviewerBot struct {
|
||||
AssignableBot
|
||||
}
|
||||
|
||||
func NewReviewerBot(login string) ReviewerBot {
|
||||
return ReviewerBot{
|
||||
AssignableBot: NewAssignableBot("", login),
|
||||
}
|
||||
}
|
||||
|
||||
func (b ReviewerBot) DisplayName() string {
|
||||
if b.login == CopilotReviewerLogin {
|
||||
return fmt.Sprintf("%s (AI)", CopilotActorName)
|
||||
}
|
||||
return b.Login()
|
||||
}
|
||||
|
||||
func (r ReviewerBot) sealedReviewerCandidate() {}
|
||||
|
||||
// ReviewerTeam is a team that can review a pull request.
|
||||
type ReviewerTeam struct {
|
||||
org string
|
||||
teamSlug string
|
||||
}
|
||||
|
||||
// NewReviewerTeam creates a new ReviewerTeam.
|
||||
func NewReviewerTeam(orgName, teamSlug string) ReviewerTeam {
|
||||
return ReviewerTeam{org: orgName, teamSlug: teamSlug}
|
||||
}
|
||||
|
||||
func (r ReviewerTeam) DisplayName() string {
|
||||
return fmt.Sprintf("%s/%s", r.org, r.teamSlug)
|
||||
}
|
||||
|
||||
func (r ReviewerTeam) Login() string {
|
||||
return fmt.Sprintf("%s/%s", r.org, r.teamSlug)
|
||||
}
|
||||
|
||||
func (r ReviewerTeam) Slug() string {
|
||||
return r.teamSlug
|
||||
}
|
||||
|
||||
func (r ReviewerTeam) sealedReviewerCandidate() {}
|
||||
|
||||
// SuggestedReviewerActors fetches suggested reviewers for a pull request.
|
||||
// It combines results from three sources using a cascading quota system:
|
||||
// - suggestedReviewerActors - suggested based on PR activity (base quota: 5)
|
||||
// - repository collaborators - all collaborators (base quota: 5 + unfilled from suggestions)
|
||||
// - organization teams - all teams for org repos (base quota: 5 + unfilled from collaborators)
|
||||
//
|
||||
// This ensures we show up to 15 total candidates, with each source filling any
|
||||
// unfilled quota from the previous source. Results are deduplicated.
|
||||
// Returns the candidates, a MoreResults count, and an error.
|
||||
func SuggestedReviewerActors(client *Client, repo ghrepo.Interface, prID string, query string) ([]ReviewerCandidate, int, error) {
|
||||
// Fetch 10 from each source to allow cascading quota to fill from available results.
|
||||
// Use a single query that includes organization.teams - if the owner is not an org,
|
||||
// we'll get a "Could not resolve to an Organization" error which we handle gracefully.
|
||||
// We also fetch unfiltered total counts via aliases for the "X more" display.
|
||||
type responseData struct {
|
||||
Node struct {
|
||||
PullRequest struct {
|
||||
SuggestedActors struct {
|
||||
Nodes []struct {
|
||||
IsAuthor bool
|
||||
IsCommenter bool
|
||||
Reviewer struct {
|
||||
TypeName string `graphql:"__typename"`
|
||||
User struct {
|
||||
Login string
|
||||
Name string
|
||||
} `graphql:"... on User"`
|
||||
Bot struct {
|
||||
Login string
|
||||
} `graphql:"... on Bot"`
|
||||
}
|
||||
}
|
||||
} `graphql:"suggestedReviewerActors(first: 10, query: $query)"`
|
||||
} `graphql:"... on PullRequest"`
|
||||
} `graphql:"node(id: $id)"`
|
||||
Repository struct {
|
||||
Collaborators struct {
|
||||
Nodes []struct {
|
||||
Login string
|
||||
Name string
|
||||
}
|
||||
} `graphql:"collaborators(first: 10, query: $query)"`
|
||||
CollaboratorsTotalCount struct {
|
||||
TotalCount int
|
||||
} `graphql:"collaboratorsTotalCount: collaborators(first: 0)"`
|
||||
} `graphql:"repository(owner: $owner, name: $name)"`
|
||||
Organization struct {
|
||||
Teams struct {
|
||||
Nodes []struct {
|
||||
Slug string
|
||||
}
|
||||
} `graphql:"teams(first: 10, query: $query)"`
|
||||
TeamsTotalCount struct {
|
||||
TotalCount int
|
||||
} `graphql:"teamsTotalCount: teams(first: 0)"`
|
||||
} `graphql:"organization(login: $owner)"`
|
||||
}
|
||||
|
||||
variables := map[string]interface{}{
|
||||
"id": githubv4.ID(prID),
|
||||
"query": githubv4.String(query),
|
||||
"owner": githubv4.String(repo.RepoOwner()),
|
||||
"name": githubv4.String(repo.RepoName()),
|
||||
}
|
||||
|
||||
var result responseData
|
||||
err := client.Query(repo.RepoHost(), "SuggestedReviewerActors", &result, variables)
|
||||
// Handle the case where the owner is not an organization - the query still returns
|
||||
// partial data (repository, node), so we can continue processing.
|
||||
if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) {
|
||||
return nil, 0, err
|
||||
}
|
||||
|
||||
// Build candidates using cascading quota logic:
|
||||
// Each source has a base quota of 5, plus any unfilled quota from previous sources.
|
||||
// This ensures we show up to 15 total candidates, filling gaps when earlier sources have fewer.
|
||||
seen := make(map[string]bool)
|
||||
var candidates []ReviewerCandidate
|
||||
const baseQuota = 5
|
||||
|
||||
// Suggested reviewers (excluding author)
|
||||
suggestionsAdded := 0
|
||||
for _, n := range result.Node.PullRequest.SuggestedActors.Nodes {
|
||||
if suggestionsAdded >= baseQuota {
|
||||
break
|
||||
}
|
||||
if n.IsAuthor {
|
||||
continue
|
||||
}
|
||||
var candidate ReviewerCandidate
|
||||
var login string
|
||||
if n.Reviewer.TypeName == "User" && n.Reviewer.User.Login != "" {
|
||||
login = n.Reviewer.User.Login
|
||||
candidate = NewReviewerUser(login, n.Reviewer.User.Name)
|
||||
} else if n.Reviewer.TypeName == "Bot" && n.Reviewer.Bot.Login != "" {
|
||||
login = n.Reviewer.Bot.Login
|
||||
candidate = NewReviewerBot(login)
|
||||
} else {
|
||||
continue
|
||||
}
|
||||
if !seen[login] {
|
||||
seen[login] = true
|
||||
candidates = append(candidates, candidate)
|
||||
suggestionsAdded++
|
||||
}
|
||||
}
|
||||
|
||||
// Collaborators: quota = base + unfilled from suggestions
|
||||
collaboratorsQuota := baseQuota + (baseQuota - suggestionsAdded)
|
||||
collaboratorsAdded := 0
|
||||
for _, c := range result.Repository.Collaborators.Nodes {
|
||||
if collaboratorsAdded >= collaboratorsQuota {
|
||||
break
|
||||
}
|
||||
if c.Login == "" {
|
||||
continue
|
||||
}
|
||||
if !seen[c.Login] {
|
||||
seen[c.Login] = true
|
||||
candidates = append(candidates, NewReviewerUser(c.Login, c.Name))
|
||||
collaboratorsAdded++
|
||||
}
|
||||
}
|
||||
|
||||
// Teams: quota = base + unfilled from collaborators
|
||||
teamsQuota := baseQuota + (collaboratorsQuota - collaboratorsAdded)
|
||||
teamsAdded := 0
|
||||
ownerName := repo.RepoOwner()
|
||||
for _, t := range result.Organization.Teams.Nodes {
|
||||
if teamsAdded >= teamsQuota {
|
||||
break
|
||||
}
|
||||
if t.Slug == "" {
|
||||
continue
|
||||
}
|
||||
teamLogin := fmt.Sprintf("%s/%s", ownerName, t.Slug)
|
||||
if !seen[teamLogin] {
|
||||
seen[teamLogin] = true
|
||||
candidates = append(candidates, NewReviewerTeam(ownerName, t.Slug))
|
||||
teamsAdded++
|
||||
}
|
||||
}
|
||||
|
||||
// MoreResults uses unfiltered total counts (teams will be 0 for personal repos)
|
||||
moreResults := result.Repository.CollaboratorsTotalCount.TotalCount + result.Organization.TeamsTotalCount.TotalCount
|
||||
|
||||
return candidates, moreResults, nil
|
||||
}
|
||||
|
||||
func UpdatePullRequestBranch(client *Client, repo ghrepo.Interface, params githubv4.UpdatePullRequestBranchInput) error {
|
||||
var mutation struct {
|
||||
UpdatePullRequestBranch struct {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ package api
|
|||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
|
|
@ -136,3 +138,227 @@ func Test_Logins(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
// mockReviewerResponse generates a GraphQL response for SuggestedReviewerActors tests.
|
||||
// It creates suggestions (s1, s2...), collaborators (c1, c2...), and teams (team1, team2...).
|
||||
// totalCollabs and totalTeams set the unfiltered TotalCount fields (for "more results" calculation).
|
||||
func mockReviewerResponse(suggestions, collabs, teams, totalCollabs, totalTeams int) string {
|
||||
var suggestionNodes, collabNodes, teamNodes []string
|
||||
|
||||
for i := 1; i <= suggestions; i++ {
|
||||
suggestionNodes = append(suggestionNodes,
|
||||
fmt.Sprintf(`{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s%d", "name": "S%d"}}`, i, i))
|
||||
}
|
||||
for i := 1; i <= collabs; i++ {
|
||||
collabNodes = append(collabNodes,
|
||||
fmt.Sprintf(`{"login": "c%d", "name": "C%d"}`, i, i))
|
||||
}
|
||||
for i := 1; i <= teams; i++ {
|
||||
teamNodes = append(teamNodes,
|
||||
fmt.Sprintf(`{"slug": "team%d"}`, i))
|
||||
}
|
||||
|
||||
return fmt.Sprintf(`{
|
||||
"data": {
|
||||
"node": {"suggestedReviewerActors": {"nodes": [%s]}},
|
||||
"repository": {
|
||||
"collaborators": {"nodes": [%s]},
|
||||
"collaboratorsTotalCount": {"totalCount": %d}
|
||||
},
|
||||
"organization": {
|
||||
"teams": {"nodes": [%s]},
|
||||
"teamsTotalCount": {"totalCount": %d}
|
||||
}
|
||||
}
|
||||
}`, strings.Join(suggestionNodes, ","), strings.Join(collabNodes, ","), totalCollabs,
|
||||
strings.Join(teamNodes, ","), totalTeams)
|
||||
}
|
||||
|
||||
func TestSuggestedReviewerActors(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
httpStubs func(*httpmock.Registry)
|
||||
expectedCount int
|
||||
expectedLogins []string
|
||||
expectedMore int
|
||||
expectError bool
|
||||
}{
|
||||
{
|
||||
name: "all sources plentiful - 5 each from cascading quota",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(mockReviewerResponse(6, 6, 6, 20, 10)))
|
||||
},
|
||||
expectedCount: 15,
|
||||
expectedLogins: []string{"s1", "s2", "s3", "s4", "s5", "c1", "c2", "c3", "c4", "c5", "OWNER/team1", "OWNER/team2", "OWNER/team3", "OWNER/team4", "OWNER/team5"},
|
||||
expectedMore: 30,
|
||||
},
|
||||
{
|
||||
name: "few suggestions - collaborators fill gap",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(mockReviewerResponse(2, 10, 6, 50, 10)))
|
||||
},
|
||||
expectedCount: 15,
|
||||
expectedLogins: []string{"s1", "s2", "c1", "c2", "c3", "c4", "c5", "c6", "c7", "c8", "OWNER/team1", "OWNER/team2", "OWNER/team3", "OWNER/team4", "OWNER/team5"},
|
||||
expectedMore: 60,
|
||||
},
|
||||
{
|
||||
name: "few suggestions and collaborators - teams fill gap",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(mockReviewerResponse(2, 3, 10, 3, 10)))
|
||||
},
|
||||
expectedCount: 15,
|
||||
expectedLogins: []string{"s1", "s2", "c1", "c2", "c3", "OWNER/team1", "OWNER/team2", "OWNER/team3", "OWNER/team4", "OWNER/team5", "OWNER/team6", "OWNER/team7", "OWNER/team8", "OWNER/team9", "OWNER/team10"},
|
||||
expectedMore: 13,
|
||||
},
|
||||
{
|
||||
name: "no suggestions or collaborators - teams only",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(mockReviewerResponse(0, 0, 10, 0, 10)))
|
||||
},
|
||||
expectedCount: 10, // max 15, but only 10 teams available
|
||||
expectedLogins: []string{"OWNER/team1", "OWNER/team2", "OWNER/team3", "OWNER/team4", "OWNER/team5", "OWNER/team6", "OWNER/team7", "OWNER/team8", "OWNER/team9", "OWNER/team10"},
|
||||
expectedMore: 10,
|
||||
},
|
||||
{
|
||||
name: "author excluded from suggestions",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
// Custom response with author flag
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(`{
|
||||
"data": {
|
||||
"node": {"suggestedReviewerActors": {"nodes": [
|
||||
{"isAuthor": true, "reviewer": {"__typename": "User", "login": "author", "name": "Author"}},
|
||||
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s1", "name": "S1"}},
|
||||
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s2", "name": "S2"}}
|
||||
]}},
|
||||
"repository": {
|
||||
"collaborators": {"nodes": [{"login": "c1", "name": "C1"}]},
|
||||
"collaboratorsTotalCount": {"totalCount": 5}
|
||||
},
|
||||
"organization": {
|
||||
"teams": {"nodes": [{"slug": "team1"}]},
|
||||
"teamsTotalCount": {"totalCount": 3}
|
||||
}
|
||||
}
|
||||
}`))
|
||||
},
|
||||
expectedCount: 4,
|
||||
expectedLogins: []string{"s1", "s2", "c1", "OWNER/team1"},
|
||||
expectedMore: 8,
|
||||
},
|
||||
{
|
||||
name: "deduplication across sources",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
// Custom response with duplicate user
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(`{
|
||||
"data": {
|
||||
"node": {"suggestedReviewerActors": {"nodes": [
|
||||
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "shareduser", "name": "Shared"}}
|
||||
]}},
|
||||
"repository": {
|
||||
"collaborators": {"nodes": [
|
||||
{"login": "shareduser", "name": "Shared"},
|
||||
{"login": "c1", "name": "C1"}
|
||||
]},
|
||||
"collaboratorsTotalCount": {"totalCount": 10}
|
||||
},
|
||||
"organization": {
|
||||
"teams": {"nodes": [{"slug": "team1"}]},
|
||||
"teamsTotalCount": {"totalCount": 5}
|
||||
}
|
||||
}
|
||||
}`))
|
||||
},
|
||||
expectedCount: 3,
|
||||
expectedLogins: []string{"shareduser", "c1", "OWNER/team1"},
|
||||
expectedMore: 15,
|
||||
},
|
||||
{
|
||||
name: "personal repo - no organization teams",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(`{
|
||||
"data": {
|
||||
"node": {"suggestedReviewerActors": {"nodes": [
|
||||
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s1", "name": "S1"}}
|
||||
]}},
|
||||
"repository": {
|
||||
"collaborators": {"nodes": [{"login": "c1", "name": "C1"}]},
|
||||
"collaboratorsTotalCount": {"totalCount": 3}
|
||||
},
|
||||
"organization": null
|
||||
},
|
||||
"errors": [{"message": "Could not resolve to an Organization with the login of 'OWNER'."}]
|
||||
}`))
|
||||
},
|
||||
expectedCount: 2,
|
||||
expectedLogins: []string{"s1", "c1"},
|
||||
expectedMore: 3,
|
||||
},
|
||||
{
|
||||
name: "bot reviewer included",
|
||||
httpStubs: func(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query SuggestedReviewerActors\b`),
|
||||
httpmock.StringResponse(`{
|
||||
"data": {
|
||||
"node": {"suggestedReviewerActors": {"nodes": [
|
||||
{"isAuthor": false, "reviewer": {"__typename": "Bot", "login": "copilot-pull-request-reviewer"}},
|
||||
{"isAuthor": false, "reviewer": {"__typename": "User", "login": "s1", "name": "S1"}}
|
||||
]}},
|
||||
"repository": {
|
||||
"collaborators": {"nodes": []},
|
||||
"collaboratorsTotalCount": {"totalCount": 5}
|
||||
},
|
||||
"organization": {
|
||||
"teams": {"nodes": []},
|
||||
"teamsTotalCount": {"totalCount": 0}
|
||||
}
|
||||
}
|
||||
}`))
|
||||
},
|
||||
expectedCount: 2,
|
||||
expectedLogins: []string{"copilot-pull-request-reviewer", "s1"},
|
||||
expectedMore: 5,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
reg := &httpmock.Registry{}
|
||||
if tt.httpStubs != nil {
|
||||
tt.httpStubs(reg)
|
||||
}
|
||||
|
||||
client := newTestClient(reg)
|
||||
repo, _ := ghrepo.FromFullName("OWNER/REPO")
|
||||
|
||||
candidates, moreResults, err := SuggestedReviewerActors(client, repo, "PR_123", "")
|
||||
if tt.expectError {
|
||||
assert.Error(t, err)
|
||||
return
|
||||
}
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, tt.expectedCount, len(candidates), "candidate count mismatch")
|
||||
assert.Equal(t, tt.expectedMore, moreResults, "moreResults mismatch")
|
||||
|
||||
logins := make([]string, len(candidates))
|
||||
for i, c := range candidates {
|
||||
logins[i] = c.Login()
|
||||
}
|
||||
assert.Equal(t, tt.expectedLogins, logins)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1080,10 +1080,11 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error)
|
|||
return projects, nil
|
||||
}
|
||||
|
||||
// Expected login for Copilot when retrieved as an Actor
|
||||
// This is returned from assignable actors and issue/pr assigned actors.
|
||||
// We use this to check if the actor is Copilot.
|
||||
const CopilotActorLogin = "copilot-swe-agent"
|
||||
// Expected login for Copilot when retrieved as an assignee
|
||||
const CopilotAssigneeLogin = "copilot-swe-agent"
|
||||
|
||||
// Expected login for Copilot when retrieved as a Pull Request Reviewer.
|
||||
const CopilotReviewerLogin = "copilot-pull-request-reviewer"
|
||||
const CopilotActorName = "Copilot"
|
||||
|
||||
type AssignableActor interface {
|
||||
|
|
@ -1144,7 +1145,7 @@ func NewAssignableBot(id, login string) AssignableBot {
|
|||
}
|
||||
|
||||
func (b AssignableBot) DisplayName() string {
|
||||
if b.login == CopilotActorLogin {
|
||||
if b.login == CopilotAssigneeLogin {
|
||||
return fmt.Sprintf("%s (AI)", CopilotActorName)
|
||||
}
|
||||
return b.Login()
|
||||
|
|
|
|||
|
|
@ -94,12 +94,16 @@ var issueClosedByPullRequestsReferences = shortenQuery(`
|
|||
}
|
||||
`)
|
||||
|
||||
// prReviewRequests includes ...on Bot to support Copilot as a reviewer on github.com.
|
||||
// On GHES, Bot is not part of the RequestedReviewer union, but the fragment is
|
||||
// silently ignored (verified on GHES 3.19).
|
||||
var prReviewRequests = shortenQuery(`
|
||||
reviewRequests(first: 100) {
|
||||
nodes {
|
||||
requestedReviewer {
|
||||
__typename,
|
||||
...on User{login},
|
||||
...on User{login,name},
|
||||
...on Bot{login},
|
||||
...on Team{
|
||||
organization{login}
|
||||
name,
|
||||
|
|
|
|||
|
|
@ -69,13 +69,15 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
|
|||
- %[1]s@me%[1]s: assign or unassign yourself
|
||||
- %[1]s@copilot%[1]s: assign or unassign Copilot (not supported on GitHub Enterprise Server)
|
||||
|
||||
The %[1]s--add-reviewer%[1]s and %[1]s--remove-reviewer%[1]s flags do not support
|
||||
these special values.
|
||||
The %[1]s--add-reviewer%[1]s and %[1]s--remove-reviewer%[1]s flags support
|
||||
the following special value:
|
||||
- %[1]s@copilot%[1]s: request or remove review from Copilot (not supported on GitHub Enterprise Server)
|
||||
`, "`"),
|
||||
Example: heredoc.Doc(`
|
||||
$ gh pr edit 23 --title "I found a bug" --body "Nothing works"
|
||||
$ gh pr edit 23 --add-label "bug,help wanted" --remove-label "core"
|
||||
$ gh pr edit 23 --add-reviewer monalisa,hubot --remove-reviewer myorg/team-name
|
||||
$ gh pr edit 23 --add-reviewer "@copilot"
|
||||
$ gh pr edit 23 --add-assignee "@me" --remove-assignee monalisa,hubot
|
||||
$ gh pr edit 23 --add-assignee "@copilot"
|
||||
$ gh pr edit 23 --add-project "Roadmap" --remove-project v1,v2
|
||||
|
|
@ -188,8 +190,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
|
|||
cmd.Flags().StringVarP(&opts.Editable.Body.Value, "body", "b", "", "Set the new body.")
|
||||
cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)")
|
||||
cmd.Flags().StringVarP(&opts.Editable.Base.Value, "base", "B", "", "Change the base `branch` for this pull request")
|
||||
cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Add, "add-reviewer", nil, "Add reviewers by their `login`.")
|
||||
cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Remove, "remove-reviewer", nil, "Remove reviewers by their `login`.")
|
||||
cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Add, "add-reviewer", nil, "Add reviewers by their `login`. Use \"@copilot\" to request review from Copilot.")
|
||||
cmd.Flags().StringSliceVar(&opts.Editable.Reviewers.Remove, "remove-reviewer", nil, "Remove reviewers by their `login`. Use \"@copilot\" to remove review request from Copilot.")
|
||||
cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Add, "add-assignee", nil, "Add assigned users by their `login`. Use \"@me\" to assign yourself, or \"@copilot\" to assign Copilot.")
|
||||
cmd.Flags().StringSliceVar(&opts.Editable.Assignees.Remove, "remove-assignee", nil, "Remove assigned users by their `login`. Use \"@me\" to unassign yourself, or \"@copilot\" to unassign Copilot.")
|
||||
cmd.Flags().StringSliceVar(&opts.Editable.Labels.Add, "add-label", nil, "Add labels by `name`")
|
||||
|
|
@ -265,7 +267,8 @@ func editRun(opts *EditOptions) error {
|
|||
editable.Title.Default = pr.Title
|
||||
editable.Body.Default = pr.Body
|
||||
editable.Base.Default = pr.BaseRefName
|
||||
editable.Reviewers.Default = pr.ReviewRequests.Logins()
|
||||
editable.Reviewers.Default = pr.ReviewRequests.DisplayNames()
|
||||
editable.Reviewers.DefaultLogins = pr.ReviewRequests.Logins()
|
||||
if issueFeatures.ActorIsAssignable {
|
||||
editable.Assignees.ActorAssignees = true
|
||||
editable.Assignees.Default = pr.AssignedActors.DisplayNames()
|
||||
|
|
@ -294,9 +297,11 @@ func editRun(opts *EditOptions) error {
|
|||
apiClient := api.NewClientFromHTTP(httpClient)
|
||||
|
||||
// Wire up search functions for assignees and reviewers.
|
||||
// TODO KW: Wire up reviewer search func if/when it exists.
|
||||
// When these aren't wired up, it triggers a downstream fallback
|
||||
// to legacy reviewer/assignee fetching.
|
||||
if issueFeatures.ActorIsAssignable {
|
||||
editable.AssigneeSearchFunc = assigneeSearchFunc(apiClient, repo, &editable, pr.ID)
|
||||
editable.ReviewerSearchFunc = reviewerSearchFunc(apiClient, repo, &editable, pr.ID)
|
||||
}
|
||||
|
||||
opts.IO.StartProgressIndicator()
|
||||
|
|
@ -389,6 +394,51 @@ func assigneeSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable *
|
|||
return searchFunc
|
||||
}
|
||||
|
||||
// reviewerSearchFunc is intended to be an arg for MultiSelectWithSearch
|
||||
// to return potential reviewer candidates (users, bots, and teams).
|
||||
// It also updates the editable's metadata for later ID resolution.
|
||||
func reviewerSearchFunc(apiClient *api.Client, repo ghrepo.Interface, editable *shared.Editable, prID string) func(string) prompter.MultiSelectSearchResult {
|
||||
searchFunc := func(input string) prompter.MultiSelectSearchResult {
|
||||
candidates, moreResults, err := api.SuggestedReviewerActors(
|
||||
apiClient,
|
||||
repo,
|
||||
prID,
|
||||
input)
|
||||
if err != nil {
|
||||
return prompter.MultiSelectSearchResult{
|
||||
Keys: nil,
|
||||
Labels: nil,
|
||||
MoreResults: 0,
|
||||
Err: err,
|
||||
}
|
||||
}
|
||||
|
||||
keys := make([]string, 0, len(candidates))
|
||||
labels := make([]string, 0, len(candidates))
|
||||
|
||||
for _, c := range candidates {
|
||||
keys = append(keys, c.Login())
|
||||
labels = append(labels, c.DisplayName())
|
||||
|
||||
// Update the teams metadata in the editable struct
|
||||
// so that updating the PR later can resolve the team ID.
|
||||
if team, ok := c.(api.ReviewerTeam); ok {
|
||||
editable.Metadata.Teams = append(editable.Metadata.Teams, api.OrgTeam{
|
||||
ID: "", // ID not needed for REST API reviewer mutations
|
||||
Slug: team.Slug(),
|
||||
})
|
||||
}
|
||||
}
|
||||
return prompter.MultiSelectSearchResult{
|
||||
Keys: keys,
|
||||
Labels: labels,
|
||||
MoreResults: moreResults,
|
||||
Err: nil,
|
||||
}
|
||||
}
|
||||
return searchFunc
|
||||
}
|
||||
|
||||
func updatePullRequest(httpClient *http.Client, repo ghrepo.Interface, id string, number int, editable shared.Editable) error {
|
||||
var wg errgroup.Group
|
||||
wg.Go(func() error {
|
||||
|
|
@ -396,44 +446,82 @@ func updatePullRequest(httpClient *http.Client, repo ghrepo.Interface, id string
|
|||
})
|
||||
if editable.Reviewers.Edited {
|
||||
wg.Go(func() error {
|
||||
return updatePullRequestReviews(httpClient, repo, number, editable)
|
||||
return updatePullRequestReviews(httpClient, repo, id, number, editable)
|
||||
})
|
||||
}
|
||||
return wg.Wait()
|
||||
}
|
||||
|
||||
func updatePullRequestReviews(httpClient *http.Client, repo ghrepo.Interface, number int, editable shared.Editable) error {
|
||||
func updatePullRequestReviews(httpClient *http.Client, repo ghrepo.Interface, prID string, number int, editable shared.Editable) error {
|
||||
if !editable.Reviewers.Edited {
|
||||
return nil
|
||||
}
|
||||
|
||||
client := api.NewClientFromHTTP(httpClient)
|
||||
|
||||
// Rebuild the Value slice from non-interactive flag input.
|
||||
if len(editable.Reviewers.Add) != 0 || len(editable.Reviewers.Remove) != 0 {
|
||||
add := editable.Reviewers.Add
|
||||
remove := editable.Reviewers.Remove
|
||||
|
||||
// Replace @copilot with the Copilot reviewer login (only on github.com).
|
||||
// Also use DefaultLogins (not Default display names) for computing the set.
|
||||
var defaultLogins []string
|
||||
if editable.Assignees.ActorAssignees {
|
||||
copilotReplacer := shared.NewCopilotReviewerReplacer()
|
||||
add = copilotReplacer.ReplaceSlice(add)
|
||||
remove = copilotReplacer.ReplaceSlice(remove)
|
||||
defaultLogins = editable.Reviewers.DefaultLogins
|
||||
} else {
|
||||
// On GHES, Default already contains logins (no display name distinction)
|
||||
defaultLogins = editable.Reviewers.Default
|
||||
}
|
||||
|
||||
s := set.NewStringSet()
|
||||
s.AddValues(editable.Reviewers.Add)
|
||||
s.AddValues(editable.Reviewers.Default)
|
||||
s.RemoveValues(editable.Reviewers.Remove)
|
||||
s.AddValues(add)
|
||||
s.AddValues(defaultLogins)
|
||||
s.RemoveValues(remove)
|
||||
editable.Reviewers.Value = s.ToSlice()
|
||||
}
|
||||
|
||||
addUsers, addTeams := partitionUsersAndTeams(editable.Reviewers.Value)
|
||||
// On github.com, use the new GraphQL mutation which supports bots.
|
||||
// On GHES, fall back to REST API.
|
||||
if editable.Assignees.ActorAssignees {
|
||||
return updatePullRequestReviewsGraphQL(client, repo, prID, editable)
|
||||
}
|
||||
return updatePullRequestReviewsREST(client, repo, number, editable)
|
||||
}
|
||||
|
||||
// Reviewers in Default but not in the Value have been removed interactively.
|
||||
// updatePullRequestReviewsGraphQL uses the RequestReviewsByLogin mutation.
|
||||
// This mutation replaces the entire reviewer set (union: false).
|
||||
func updatePullRequestReviewsGraphQL(client *api.Client, repo ghrepo.Interface, prID string, editable shared.Editable) error {
|
||||
users, bots, teams := partitionReviewersByType(editable.Reviewers.Value)
|
||||
return api.RequestReviewsByLogin(client, repo, prID, users, bots, teams, false)
|
||||
}
|
||||
|
||||
// updatePullRequestReviewsREST uses the REST API to add/remove reviewers.
|
||||
// This is the legacy path for GHES compatibility.
|
||||
func updatePullRequestReviewsREST(client *api.Client, repo ghrepo.Interface, number int, editable shared.Editable) error {
|
||||
addUsers, addBots, addTeams := partitionReviewersByType(editable.Reviewers.Value)
|
||||
// REST API doesn't distinguish bots from users, so we need to combine them.
|
||||
allAddUsers := append(addUsers, addBots...)
|
||||
|
||||
// Reviewers in Default but not in Value have been removed interactively.
|
||||
var toRemove []string
|
||||
for _, r := range editable.Reviewers.Default {
|
||||
if !slices.Contains(editable.Reviewers.Value, r) {
|
||||
toRemove = append(toRemove, r)
|
||||
}
|
||||
}
|
||||
removeUsers, removeTeams := partitionUsersAndTeams(toRemove)
|
||||
removeUsers, removeBots, removeTeams := partitionReviewersByType(toRemove)
|
||||
allRemoveUsers := append(removeUsers, removeBots...)
|
||||
|
||||
client := api.NewClientFromHTTP(httpClient)
|
||||
wg := errgroup.Group{}
|
||||
wg.Go(func() error {
|
||||
return api.AddPullRequestReviews(client, repo, number, addUsers, addTeams)
|
||||
return api.AddPullRequestReviews(client, repo, number, allAddUsers, addTeams)
|
||||
})
|
||||
wg.Go(func() error {
|
||||
return api.RemovePullRequestReviews(client, repo, number, removeUsers, removeTeams)
|
||||
return api.RemovePullRequestReviews(client, repo, number, allRemoveUsers, removeTeams)
|
||||
})
|
||||
return wg.Wait()
|
||||
}
|
||||
|
|
@ -477,16 +565,20 @@ func (e editorRetriever) Retrieve() (string, error) {
|
|||
return cmdutil.DetermineEditor(e.config)
|
||||
}
|
||||
|
||||
// partitionUsersAndTeams splits reviewer identifiers into user logins and team slugs.
|
||||
// Team identifiers are in the form "org/slug"; only the slug portion is returned for teams.
|
||||
func partitionUsersAndTeams(values []string) (users []string, teams []string) {
|
||||
// partitionReviewersByType splits reviewer identifiers into users, bots, and teams.
|
||||
// Team identifiers are in the form "org/slug" and are returned as-is.
|
||||
// Bot logins (currently only Copilot) are identified and returned separately.
|
||||
func partitionReviewersByType(values []string) (users []string, bots []string, teams []string) {
|
||||
for _, v := range values {
|
||||
if v == "" {
|
||||
continue
|
||||
}
|
||||
if strings.ContainsRune(v, '/') {
|
||||
parts := strings.SplitN(v, "/", 2)
|
||||
if len(parts) == 2 && parts[1] != "" {
|
||||
teams = append(teams, parts[1])
|
||||
}
|
||||
} else if v != "" {
|
||||
// Team: org/slug format, pass as-is
|
||||
teams = append(teams, v)
|
||||
} else if v == api.CopilotReviewerLogin {
|
||||
bots = append(bots, v)
|
||||
} else {
|
||||
users = append(users, v)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -150,10 +150,10 @@ func TestNewCmdEdit(t *testing.T) {
|
|||
output: EditOptions{
|
||||
SelectorArg: "23",
|
||||
Editable: shared.Editable{
|
||||
Reviewers: shared.EditableSlice{
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{
|
||||
Add: []string{"monalisa", "owner/core"},
|
||||
Edited: true,
|
||||
},
|
||||
}},
|
||||
},
|
||||
},
|
||||
wantsErr: false,
|
||||
|
|
@ -164,10 +164,10 @@ func TestNewCmdEdit(t *testing.T) {
|
|||
output: EditOptions{
|
||||
SelectorArg: "23",
|
||||
Editable: shared.Editable{
|
||||
Reviewers: shared.EditableSlice{
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{
|
||||
Remove: []string{"monalisa", "owner/core"},
|
||||
Edited: true,
|
||||
},
|
||||
}},
|
||||
},
|
||||
},
|
||||
wantsErr: false,
|
||||
|
|
@ -381,11 +381,11 @@ func Test_editRun(t *testing.T) {
|
|||
Value: "base-branch-name",
|
||||
Edited: true,
|
||||
},
|
||||
Reviewers: shared.EditableSlice{
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{
|
||||
Add: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot"},
|
||||
Remove: []string{"dependabot"},
|
||||
Edited: true,
|
||||
},
|
||||
}},
|
||||
Assignees: shared.EditableAssignees{
|
||||
EditableSlice: shared.EditableSlice{
|
||||
Add: []string{"monalisa", "hubot"},
|
||||
|
|
@ -413,10 +413,12 @@ func Test_editRun(t *testing.T) {
|
|||
Fetcher: testFetcher{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true, teamReviewers: false, assignees: true, labels: true, projects: true, milestones: true})
|
||||
// Non-interactive with Add/Remove doesn't need reviewers/assignees metadata
|
||||
// REST API accepts logins and team slugs directly
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: true, labels: true, projects: true, milestones: true})
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestUpdateActorAssignees(reg)
|
||||
mockPullRequestAddReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
mockPullRequestUpdateLabels(reg)
|
||||
mockProjectV2ItemUpdate(reg)
|
||||
},
|
||||
|
|
@ -512,11 +514,11 @@ func Test_editRun(t *testing.T) {
|
|||
Value: "base-branch-name",
|
||||
Edited: true,
|
||||
},
|
||||
Reviewers: shared.EditableSlice{
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{
|
||||
Default: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot", "dependabot"},
|
||||
Remove: []string{"OWNER/core", "OWNER/external", "monalisa", "hubot", "dependabot"},
|
||||
Edited: true,
|
||||
},
|
||||
}},
|
||||
Assignees: shared.EditableAssignees{
|
||||
EditableSlice: shared.EditableSlice{
|
||||
Add: []string{"monalisa", "hubot"},
|
||||
|
|
@ -544,15 +546,71 @@ func Test_editRun(t *testing.T) {
|
|||
Fetcher: testFetcher{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true, teamReviewers: false, assignees: true, labels: true, projects: true, milestones: true})
|
||||
// Non-interactive with Remove doesn't need reviewers metadata
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: true, labels: true, projects: true, milestones: true})
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestRemoveReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
mockPullRequestUpdateLabels(reg)
|
||||
mockPullRequestUpdateActorAssignees(reg)
|
||||
mockProjectV2ItemUpdate(reg)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
{
|
||||
name: "remove all reviewers sends empty slices to mutation",
|
||||
input: &EditOptions{
|
||||
Detector: &fd.EnabledDetectorMock{},
|
||||
SelectorArg: "123",
|
||||
Finder: shared.NewMockFinder("123", &api.PullRequest{
|
||||
URL: "https://github.com/OWNER/REPO/pull/123",
|
||||
ReviewRequests: api.ReviewRequests{
|
||||
Nodes: []struct{ RequestedReviewer api.RequestedReviewer }{
|
||||
{
|
||||
RequestedReviewer: api.RequestedReviewer{
|
||||
TypeName: "Team",
|
||||
Slug: "core",
|
||||
Organization: struct {
|
||||
Login string `json:"login"`
|
||||
}{Login: "OWNER"},
|
||||
},
|
||||
},
|
||||
{
|
||||
RequestedReviewer: api.RequestedReviewer{
|
||||
TypeName: "User",
|
||||
Login: "monalisa",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}, ghrepo.New("OWNER", "REPO")),
|
||||
Interactive: false,
|
||||
Editable: shared.Editable{
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{
|
||||
Default: []string{"OWNER/core", "monalisa"},
|
||||
Remove: []string{"OWNER/core", "monalisa"},
|
||||
Edited: true,
|
||||
}},
|
||||
},
|
||||
Fetcher: testFetcher{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{})
|
||||
mockPullRequestUpdate(reg)
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`mutation RequestReviewsByLogin\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "requestReviewsByLogin": { "clientMutationId": "" } } }`,
|
||||
func(inputs map[string]interface{}) {
|
||||
// Verify that empty slices are sent to properly clear all reviewer types
|
||||
require.Equal(t, []interface{}{}, inputs["userLogins"], "userLogins should be an empty slice")
|
||||
require.Equal(t, []interface{}{}, inputs["botLogins"], "botLogins should be an empty slice")
|
||||
require.Equal(t, []interface{}{}, inputs["teamSlugs"], "teamSlugs should be an empty slice")
|
||||
require.Equal(t, false, inputs["union"], "union should be false for replace mode")
|
||||
}),
|
||||
)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
// Conditional team fetching cases
|
||||
{
|
||||
name: "non-interactive add only user reviewers skips team fetch",
|
||||
|
|
@ -562,17 +620,17 @@ func Test_editRun(t *testing.T) {
|
|||
Finder: shared.NewMockFinder("123", &api.PullRequest{URL: "https://github.com/OWNER/REPO/pull/123"}, ghrepo.New("OWNER", "REPO")),
|
||||
Interactive: false,
|
||||
Editable: shared.Editable{
|
||||
Reviewers: shared.EditableSlice{Add: []string{"monalisa", "hubot"}, Edited: true},
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{Add: []string{"monalisa", "hubot"}, Edited: true}},
|
||||
},
|
||||
Fetcher: testFetcher{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
// reviewers only (users), no team reviewers fetched
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true})
|
||||
// Non-interactive with Add/Remove doesn't need reviewer metadata
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{})
|
||||
// explicitly assert that no OrganizationTeamList query occurs
|
||||
reg.Exclude(t, httpmock.GraphQL(`query OrganizationTeamList\b`))
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestAddReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
|
|
@ -584,17 +642,17 @@ func Test_editRun(t *testing.T) {
|
|||
Finder: shared.NewMockFinder("123", &api.PullRequest{URL: "https://github.com/OWNER/REPO/pull/123"}, ghrepo.New("OWNER", "REPO")),
|
||||
Interactive: false,
|
||||
Editable: shared.Editable{
|
||||
Reviewers: shared.EditableSlice{Add: []string{"monalisa", "OWNER/core"}, Edited: true},
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{Add: []string{"monalisa", "OWNER/core"}, Edited: true}},
|
||||
},
|
||||
Fetcher: testFetcher{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
// reviewer add includes team but non-interactive Add/Remove provided -> no team fetch
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true})
|
||||
// Non-interactive with Add/Remove doesn't need reviewer metadata
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{})
|
||||
// explicitly assert that no OrganizationTeamList query occurs
|
||||
reg.Exclude(t, httpmock.GraphQL(`query OrganizationTeamList\b`))
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestAddReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
|
|
@ -611,16 +669,17 @@ func Test_editRun(t *testing.T) {
|
|||
}}}, ghrepo.New("OWNER", "REPO")),
|
||||
Interactive: false,
|
||||
Editable: shared.Editable{
|
||||
Reviewers: shared.EditableSlice{Remove: []string{"monalisa", "OWNER/core"}, Edited: true},
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{Remove: []string{"monalisa", "OWNER/core"}, Edited: true}},
|
||||
},
|
||||
Fetcher: testFetcher{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true})
|
||||
// Non-interactive with Add/Remove doesn't need reviewer metadata
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{})
|
||||
// explicitly assert that no OrganizationTeamList query occurs
|
||||
reg.Exclude(t, httpmock.GraphQL(`query OrganizationTeamList\b`))
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestRemoveReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
|
|
@ -632,17 +691,17 @@ func Test_editRun(t *testing.T) {
|
|||
Finder: shared.NewMockFinder("123", &api.PullRequest{URL: "https://github.com/OWNER/REPO/pull/123"}, ghrepo.New("OWNER", "REPO")),
|
||||
Interactive: false,
|
||||
Editable: shared.Editable{
|
||||
Reviewers: shared.EditableSlice{Add: []string{"monalisa"}, Remove: []string{"hubot"}, Default: []string{"OWNER/core"}, Edited: true},
|
||||
Reviewers: shared.EditableReviewers{EditableSlice: shared.EditableSlice{Add: []string{"monalisa"}, Remove: []string{"hubot"}, Default: []string{"OWNER/core"}, Edited: true}},
|
||||
},
|
||||
Fetcher: testFetcher{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
// reviewers only (users), no team reviewers fetched
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true})
|
||||
// Non-interactive with Add/Remove doesn't need reviewer metadata
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{})
|
||||
// explicitly assert that no OrganizationTeamList query occurs
|
||||
reg.Exclude(t, httpmock.GraphQL(`query OrganizationTeamList\b`))
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestAddReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
|
|
@ -671,6 +730,16 @@ func Test_editRun(t *testing.T) {
|
|||
e.Body.Value = "new body"
|
||||
e.Reviewers.Value = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external"}
|
||||
e.Assignees.Value = []string{"monalisa", "hubot"}
|
||||
// Populate metadata to simulate what searchFunc would do during prompting
|
||||
e.Metadata.AssignableActors = []api.AssignableActor{
|
||||
api.NewAssignableBot("HUBOTID", "hubot"),
|
||||
api.NewAssignableUser("MONAID", "monalisa", "Mona Display Name"),
|
||||
}
|
||||
// Populate team metadata for reviewer search
|
||||
e.Metadata.Teams = []api.OrgTeam{
|
||||
{ID: "COREID", Slug: "core"},
|
||||
{ID: "EXTERNALID", Slug: "external"},
|
||||
}
|
||||
e.Labels.Value = []string{"feature", "TODO", "bug"}
|
||||
e.Labels.Add = []string{"feature", "TODO", "bug"}
|
||||
e.Labels.Remove = []string{"docs"}
|
||||
|
|
@ -683,10 +752,12 @@ func Test_editRun(t *testing.T) {
|
|||
EditorRetriever: testEditorRetriever{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true, teamReviewers: true, assignees: true, labels: true, projects: true, milestones: true})
|
||||
// With search functions enabled, we don't fetch reviewers/assignees metadata
|
||||
// (searchFunc handles dynamic fetching, metadata populated in test mock)
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true})
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestUpdateActorAssignees(reg)
|
||||
mockPullRequestAddReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
mockPullRequestUpdateLabels(reg)
|
||||
mockProjectV2ItemUpdate(reg)
|
||||
},
|
||||
|
|
@ -779,6 +850,16 @@ func Test_editRun(t *testing.T) {
|
|||
e.Body.Value = "new body"
|
||||
e.Reviewers.Remove = []string{"monalisa", "hubot", "OWNER/core", "OWNER/external", "dependabot"}
|
||||
e.Assignees.Value = []string{"monalisa", "hubot"}
|
||||
// Populate metadata to simulate what searchFunc would do during prompting
|
||||
e.Metadata.AssignableActors = []api.AssignableActor{
|
||||
api.NewAssignableBot("HUBOTID", "hubot"),
|
||||
api.NewAssignableUser("MONAID", "monalisa", "Mona Display Name"),
|
||||
}
|
||||
// Populate team metadata for reviewer search
|
||||
e.Metadata.Teams = []api.OrgTeam{
|
||||
{ID: "COREID", Slug: "core"},
|
||||
{ID: "EXTERNALID", Slug: "external"},
|
||||
}
|
||||
e.Labels.Value = []string{"feature", "TODO", "bug"}
|
||||
e.Labels.Add = []string{"feature", "TODO", "bug"}
|
||||
e.Labels.Remove = []string{"docs"}
|
||||
|
|
@ -791,9 +872,10 @@ func Test_editRun(t *testing.T) {
|
|||
EditorRetriever: testEditorRetriever{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: true, teamReviewers: true, assignees: true, labels: true, projects: true, milestones: true})
|
||||
// With search functions enabled, we don't fetch reviewers/assignees metadata
|
||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true})
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestRemoveReviewers(reg)
|
||||
mockRequestReviewsByLogin(reg)
|
||||
mockPullRequestUpdateActorAssignees(reg)
|
||||
mockPullRequestUpdateLabels(reg)
|
||||
mockProjectV2ItemUpdate(reg)
|
||||
|
|
@ -945,6 +1027,78 @@ func Test_editRun(t *testing.T) {
|
|||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
{
|
||||
name: "interactive GHES uses legacy reviewer flow without search",
|
||||
input: &EditOptions{
|
||||
Detector: &fd.DisabledDetectorMock{},
|
||||
SelectorArg: "123",
|
||||
Finder: shared.NewMockFinder("123", &api.PullRequest{
|
||||
URL: "https://github.com/OWNER/REPO/pull/123",
|
||||
ReviewRequests: api.ReviewRequests{Nodes: []struct{ RequestedReviewer api.RequestedReviewer }{
|
||||
{RequestedReviewer: api.RequestedReviewer{TypeName: "User", Login: "octocat"}},
|
||||
}},
|
||||
}, ghrepo.New("OWNER", "REPO")),
|
||||
Interactive: true,
|
||||
Surveyor: testSurveyor{
|
||||
fieldsToEdit: func(e *shared.Editable) error {
|
||||
e.Reviewers.Edited = true
|
||||
return nil
|
||||
},
|
||||
editFields: func(e *shared.Editable, _ string) error {
|
||||
// Verify GHES uses legacy flow: ReviewerSearchFunc should be nil
|
||||
require.Nil(t, e.ReviewerSearchFunc)
|
||||
// Verify options are populated from fetched metadata
|
||||
require.Contains(t, e.Reviewers.Options, "monalisa")
|
||||
require.Contains(t, e.Reviewers.Options, "hubot")
|
||||
require.Contains(t, e.Reviewers.Options, "OWNER/core")
|
||||
|
||||
e.Reviewers.Value = []string{"monalisa", "OWNER/core"}
|
||||
return nil
|
||||
},
|
||||
},
|
||||
Fetcher: testFetcher{},
|
||||
EditorRetriever: testEditorRetriever{},
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
// GHES should NOT use the new SuggestedReviewerActors query
|
||||
reg.Exclude(t, httpmock.GraphQL(`query SuggestedReviewerActors\b`))
|
||||
// GHES should use legacy metadata fetch for reviewers (AssignableUsers, not Actors)
|
||||
reg.Exclude(t, httpmock.GraphQL(`query RepositoryAssignableActors\b`))
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query RepositoryAssignableUsers\b`),
|
||||
httpmock.StringResponse(`
|
||||
{ "data": { "repository": { "assignableUsers": {
|
||||
"nodes": [
|
||||
{ "login": "hubot", "id": "HUBOTID" },
|
||||
{ "login": "monalisa", "id": "MONAID" }
|
||||
],
|
||||
"pageInfo": { "hasNextPage": false }
|
||||
} } } }
|
||||
`))
|
||||
// GHES should fetch teams for interactive reviewer editing
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query OrganizationTeamList\b`),
|
||||
httpmock.StringResponse(`
|
||||
{ "data": { "organization": { "teams": {
|
||||
"nodes": [
|
||||
{ "slug": "external", "id": "EXTERNALID" },
|
||||
{ "slug": "core", "id": "COREID" }
|
||||
],
|
||||
"pageInfo": { "hasNextPage": false }
|
||||
} } } }
|
||||
`))
|
||||
// Current user fetched for reviewers
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`query UserCurrent\b`),
|
||||
httpmock.StringResponse(`
|
||||
{ "data": { "viewer": { "login": "monalisa" } } }
|
||||
`))
|
||||
mockPullRequestUpdate(reg)
|
||||
mockPullRequestAddReviewers(reg)
|
||||
mockPullRequestRemoveReviewers(reg)
|
||||
},
|
||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
||||
},
|
||||
{
|
||||
name: "non-interactive projects v1 unsupported doesn't fetch v1 metadata",
|
||||
input: &EditOptions{
|
||||
|
|
@ -1188,6 +1342,17 @@ func mockPullRequestRemoveReviewers(reg *httpmock.Registry) {
|
|||
httpmock.StringResponse(`{}`))
|
||||
}
|
||||
|
||||
// mockRequestReviewsByLogin mocks the RequestReviewsByLogin GraphQL mutation
|
||||
// used on github.com when ActorAssignees is enabled.
|
||||
func mockRequestReviewsByLogin(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`mutation RequestReviewsByLogin\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "requestReviewsByLogin": { "clientMutationId": "" } } }`,
|
||||
func(inputs map[string]interface{}) {}),
|
||||
)
|
||||
}
|
||||
|
||||
func mockPullRequestUpdateLabels(reg *httpmock.Registry) {
|
||||
reg.Register(
|
||||
httpmock.GraphQL(`mutation LabelAdd\b`),
|
||||
|
|
|
|||
|
|
@ -14,8 +14,8 @@ type Editable struct {
|
|||
Title EditableString
|
||||
Body EditableString
|
||||
Base EditableString
|
||||
Reviewers EditableSlice
|
||||
ReviewerSearchFunc func(string) ([]string, []string, error)
|
||||
Reviewers EditableReviewers
|
||||
ReviewerSearchFunc func(string) prompter.MultiSelectSearchResult
|
||||
Assignees EditableAssignees
|
||||
AssigneeSearchFunc func(string) prompter.MultiSelectSearchResult
|
||||
Labels EditableSlice
|
||||
|
|
@ -49,6 +49,12 @@ type EditableAssignees struct {
|
|||
DefaultLogins []string // For disambiguating actors from display names
|
||||
}
|
||||
|
||||
// EditableReviewers is a special case of EditableSlice.
|
||||
type EditableReviewers struct {
|
||||
EditableSlice
|
||||
DefaultLogins []string // For disambiguating actors from display names
|
||||
}
|
||||
|
||||
// ProjectsV2 mutations require a mapping of an item ID to a project ID.
|
||||
// Keep that map along with standard EditableSlice data.
|
||||
type EditableProjects struct {
|
||||
|
|
@ -268,6 +274,13 @@ func (ea *EditableAssignees) clone() EditableAssignees {
|
|||
}
|
||||
}
|
||||
|
||||
func (er *EditableReviewers) clone() EditableReviewers {
|
||||
return EditableReviewers{
|
||||
EditableSlice: er.EditableSlice.clone(),
|
||||
DefaultLogins: er.DefaultLogins,
|
||||
}
|
||||
}
|
||||
|
||||
func (ep *EditableProjects) clone() EditableProjects {
|
||||
return EditableProjects{
|
||||
EditableSlice: ep.EditableSlice.clone(),
|
||||
|
|
@ -299,10 +312,24 @@ func EditFieldsSurvey(p EditPrompter, editable *Editable, editorCommand string)
|
|||
}
|
||||
}
|
||||
if editable.Reviewers.Edited {
|
||||
editable.Reviewers.Value, err = multiSelectSurvey(
|
||||
p, "Reviewers", editable.Reviewers.Default, editable.Reviewers.Options)
|
||||
if err != nil {
|
||||
return err
|
||||
if editable.ReviewerSearchFunc != nil {
|
||||
editable.Reviewers.Options = []string{}
|
||||
editable.Reviewers.Value, err = p.MultiSelectWithSearch(
|
||||
"Reviewers",
|
||||
"Search reviewers",
|
||||
editable.Reviewers.Default,
|
||||
// No persistent options - teams are included in search results
|
||||
[]string{},
|
||||
editable.ReviewerSearchFunc)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
} else {
|
||||
editable.Reviewers.Value, err = multiSelectSurvey(
|
||||
p, "Reviewers", editable.Reviewers.Default, editable.Reviewers.Options)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
if editable.Assignees.Edited {
|
||||
|
|
@ -415,22 +442,29 @@ func FieldsToEditSurvey(p EditPrompter, editable *Editable) error {
|
|||
}
|
||||
|
||||
func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, projectV1Support gh.ProjectsV1Support) error {
|
||||
// Determine whether to fetch organization teams.
|
||||
// Determine whether to fetch organization teams and reviewers.
|
||||
// Interactive reviewer editing (Edited true, but no Add/Remove slices) still needs
|
||||
// team data for selection UI. For non-interactive flows, we never need to fetch teams.
|
||||
// team data for selection UI. For non-interactive flows, we never need to fetch teams
|
||||
// as the REST API accepts team slugs directly.
|
||||
// If we have a search func, we don't need to fetch teams/reviewers since we
|
||||
// assume that will be done dynamically in the prompting flow.
|
||||
teamReviewers := false
|
||||
fetchReviewers := false
|
||||
if editable.Reviewers.Edited {
|
||||
// This is likely an interactive flow since edited is set but no mutations to
|
||||
// Add/Remove slices, so we need to load the teams.
|
||||
if len(editable.Reviewers.Add) == 0 && len(editable.Reviewers.Remove) == 0 {
|
||||
// Add/Remove slices, so we need to load the teams and reviewers.
|
||||
// However, if we have a search func, skip fetching as it will be done dynamically.
|
||||
if len(editable.Reviewers.Add) == 0 && len(editable.Reviewers.Remove) == 0 && editable.ReviewerSearchFunc == nil {
|
||||
teamReviewers = true
|
||||
fetchReviewers = true
|
||||
}
|
||||
// Note: Non-interactive flows (with Add/Remove) don't need to fetch reviewers/teams
|
||||
// because the APIs in use for both GHES and GitHub.com accept user logins and team slugs directly.
|
||||
}
|
||||
|
||||
fetchAssignees := false
|
||||
if editable.Assignees.Edited {
|
||||
// Similar as above, this is likely an interactive flow if no Add/Remove slices are set.
|
||||
// The addition here is that we also check for an assignee search func.
|
||||
// If we have a search func, we don't need to fetch assignees since we
|
||||
// assume that will be done dynamically in the prompting flow.
|
||||
if len(editable.Assignees.Add) == 0 && len(editable.Assignees.Remove) == 0 && editable.AssigneeSearchFunc == nil {
|
||||
|
|
@ -438,13 +472,15 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable,
|
|||
}
|
||||
// However, if we have Add/Remove operations (non-interactive flow),
|
||||
// we do need to fetch the assignees.
|
||||
// TODO: KW noninteractive assignees need to migrate to directly use
|
||||
// new logins input with ReplaceActorsForAssignable to prevent fetching.
|
||||
if len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0 {
|
||||
fetchAssignees = true
|
||||
}
|
||||
}
|
||||
|
||||
input := api.RepoMetadataInput{
|
||||
Reviewers: editable.Reviewers.Edited,
|
||||
Reviewers: fetchReviewers,
|
||||
TeamReviewers: teamReviewers,
|
||||
Assignees: fetchAssignees,
|
||||
ActorAssignees: editable.Assignees.ActorAssignees,
|
||||
|
|
|
|||
|
|
@ -295,11 +295,24 @@ func (r *MeReplacer) ReplaceSlice(handles []string) ([]string, error) {
|
|||
// Login is generally needed for API calls; name is used when launching web browser.
|
||||
type CopilotReplacer struct {
|
||||
returnLogin bool
|
||||
// copilotLogin is the login to use when replacing @copilot.
|
||||
// Different Copilot features use different bot logins.
|
||||
copilotLogin string
|
||||
}
|
||||
|
||||
// NewCopilotReplacer creates a replacer for assignee @copilot references.
|
||||
func NewCopilotReplacer(returnLogin bool) *CopilotReplacer {
|
||||
return &CopilotReplacer{
|
||||
returnLogin: returnLogin,
|
||||
returnLogin: returnLogin,
|
||||
copilotLogin: api.CopilotAssigneeLogin,
|
||||
}
|
||||
}
|
||||
|
||||
// NewCopilotReviewerReplacer creates a replacer for reviewer @copilot references.
|
||||
func NewCopilotReviewerReplacer() *CopilotReplacer {
|
||||
return &CopilotReplacer{
|
||||
returnLogin: true,
|
||||
copilotLogin: api.CopilotReviewerLogin,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -308,7 +321,7 @@ func (r *CopilotReplacer) replace(handle string) string {
|
|||
return handle
|
||||
}
|
||||
if r.returnLogin {
|
||||
return api.CopilotActorLogin
|
||||
return r.copilotLogin
|
||||
}
|
||||
return api.CopilotActorName
|
||||
}
|
||||
|
|
|
|||
|
|
@ -334,6 +334,37 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestCopilotReviewerReplacer_ReplaceSlice(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
handles []string
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "replaces @copilot with reviewer login",
|
||||
handles: []string{"monalisa", "@copilot", "hubot"},
|
||||
want: []string{"monalisa", "copilot-pull-request-reviewer", "hubot"},
|
||||
},
|
||||
{
|
||||
name: "handles @copilot case-insensitively",
|
||||
handles: []string{"@Copilot", "user", "@CoPiLoT"},
|
||||
want: []string{"copilot-pull-request-reviewer", "user", "copilot-pull-request-reviewer"},
|
||||
},
|
||||
{
|
||||
name: "handles no @copilot mentions",
|
||||
handles: []string{"monalisa", "user", "hubot"},
|
||||
want: []string{"monalisa", "user", "hubot"},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := NewCopilotReviewerReplacer()
|
||||
got := r.ReplaceSlice(tt.handles)
|
||||
require.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_QueryHasStateClause(t *testing.T) {
|
||||
tests := []struct {
|
||||
searchQuery string
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue