fix(pr edit): send empty slices to clear reviewers in replace mode

Always send explicit lists for userLogins, botLogins, and teamSlugs
in RequestReviewsByLogin mutation, even when empty. Previously, empty
slices were omitted due to omitempty JSON behavior and len > 0 checks,
which prevented clearing all reviewers when using replace mode.

Empty slices are harmless no-ops in union mode, so we now send them
unconditionally for simpler logic.

Add test to verify the mutation receives empty slices when all
reviewers are removed.
This commit is contained in:
Kynan Ware 2026-01-29 10:37:11 -07:00
parent 7f8ca2ca81
commit d643d5386e
2 changed files with 68 additions and 19 deletions

View file

@ -772,30 +772,24 @@ func RequestReviewsByLogin(client *Client, repo ghrepo.Interface, prID string, u
Union: githubv4.Boolean(union),
}
if len(userLogins) > 0 {
logins := make([]githubv4.String, len(userLogins))
for i, l := range userLogins {
logins[i] = githubv4.String(l)
}
input.UserLogins = &logins
userLoginValues := make([]githubv4.String, len(userLogins))
for i, l := range userLogins {
userLoginValues[i] = githubv4.String(l)
}
input.UserLogins = &userLoginValues
if len(botLogins) > 0 {
logins := make([]githubv4.String, len(botLogins))
for i, l := range botLogins {
// Bot logins require the [bot] suffix for the mutation
logins[i] = githubv4.String(l + "[bot]")
}
input.BotLogins = &logins
botLoginValues := make([]githubv4.String, len(botLogins))
for i, l := range botLogins {
// Bot logins require the [bot] suffix for the mutation
botLoginValues[i] = githubv4.String(l + "[bot]")
}
input.BotLogins = &botLoginValues
if len(teamSlugs) > 0 {
slugs := make([]githubv4.String, len(teamSlugs))
for i, s := range teamSlugs {
slugs[i] = githubv4.String(s)
}
input.TeamSlugs = &slugs
teamSlugValues := make([]githubv4.String, len(teamSlugs))
for i, s := range teamSlugs {
teamSlugValues[i] = githubv4.String(s)
}
input.TeamSlugs = &teamSlugValues
variables := map[string]interface{}{
"input": input,

View file

@ -556,6 +556,61 @@ func Test_editRun(t *testing.T) {
},
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",