From d643d5386e25c7f7c9d4f716a1c4356262a251f6 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Thu, 29 Jan 2026 10:37:11 -0700 Subject: [PATCH] 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. --- api/queries_pr.go | 32 +++++++++------------ pkg/cmd/pr/edit/edit_test.go | 55 ++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 19 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index a84582c42..aa584cf5d 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -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, diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index ddb166317..e38fac3ca 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -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",