From 3c00ffdade319d47505479edf215909762d46bac Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 24 Mar 2026 21:04:41 -0600 Subject: [PATCH] refactor(pr shared): consolidate ActorAssignees and ActorReviewers into ApiActorsSupported The CLI had two per-entity flags (ActorAssignees on EditableAssignees and IssueMetadataState, ActorReviewers on IssueMetadataState) threaded through different layers of the stack to distinguish github.com from GHES. Both flags were always set from the same source (issueFeatures.ActorIsAssignable) and never had different values, but they were carried independently on different structs. This led to a confusing asymmetry where: - EditableAssignees had ActorAssignees but EditableReviewers had nothing - The PR edit flow piggybacked on editable.Assignees.ActorAssignees to make reviewer mutation decisions, which was misleading - RepoMetadataInput only had ActorAssignees with no reviewer equivalent This commit replaces all per-entity flags with a single ApiActorsSupported bool hoisted to the shared level on Editable, IssueMetadataState, and RepoMetadataInput. Both assignees and reviewers now key off the same signal. Every branch site is marked with // TODO ApiActorsSupported so we can grep for cleanup sites when GHES eventually supports the actor-based mutations (replaceActorsForAssignable, requestReviewsByLogin). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/queries_issue.go | 2 +- api/queries_pr.go | 2 +- api/queries_repo.go | 20 +++++++------ pkg/cmd/issue/create/create.go | 16 +++++----- pkg/cmd/issue/edit/edit.go | 7 +++-- pkg/cmd/issue/edit/edit_test.go | 10 +++---- pkg/cmd/pr/create/create.go | 4 +-- pkg/cmd/pr/create/create_test.go | 2 +- pkg/cmd/pr/edit/edit.go | 10 ++++--- pkg/cmd/pr/edit/edit_test.go | 22 +++++++------- pkg/cmd/pr/shared/editable.go | 47 ++++++++++++++++++------------ pkg/cmd/pr/shared/editable_http.go | 6 ++-- pkg/cmd/pr/shared/params.go | 21 +++++++------ pkg/cmd/pr/shared/state.go | 11 +++++-- pkg/cmd/pr/shared/survey.go | 31 +++++++++++--------- 15 files changed, 119 insertions(+), 92 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index f719e52f9..bff84029d 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -311,7 +311,7 @@ func IssueCreate(client *Client, repo *Repository, params map[string]interface{} } issue := &result.CreateIssue.Issue - // Assign users using login-based mutation when ActorAssignees is true (github.com). + // Assign users using login-based mutation when ApiActorsSupported is true (github.com). if assigneeLogins, ok := params["assigneeLogins"].([]string); ok && len(assigneeLogins) > 0 { err := ReplaceActorsForAssignableByLogin(client, repo, issue.ID, assigneeLogins) if err != nil { diff --git a/api/queries_pr.go b/api/queries_pr.go index 01846a9e6..1b044a439 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -524,7 +524,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter } } - // Assign users using login-based mutation when ActorAssignees is true (github.com). + // Assign users using login-based mutation when ApiActorsSupported is true (github.com). if assigneeLogins, ok := params["assigneeLogins"].([]string); ok && len(assigneeLogins) > 0 { err := ReplaceActorsForAssignableByLogin(client, repo, pr.ID, assigneeLogins) if err != nil { diff --git a/api/queries_repo.go b/api/queries_repo.go index e5806b91e..c3a072961 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -922,14 +922,15 @@ func (m *RepoMetadataResult) Merge(m2 *RepoMetadataResult) { } type RepoMetadataInput struct { - Assignees bool - ActorAssignees bool - Reviewers bool - TeamReviewers bool - Labels bool - ProjectsV1 bool - ProjectsV2 bool - Milestones bool + Assignees bool + Reviewers bool + TeamReviewers bool + // TODO ApiActorsSupported + ApiActorsSupported bool + Labels bool + ProjectsV1 bool + ProjectsV2 bool + Milestones bool } // RepoMetadata pre-fetches the metadata for attaching to issues and pull requests @@ -938,7 +939,8 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput var g errgroup.Group if input.Assignees || input.Reviewers { - if input.ActorAssignees { + // TODO ApiActorsSupported + if input.ApiActorsSupported { g.Go(func() error { actors, err := RepoAssignableActors(client, repo) if err != nil { diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index f9fab11a1..831495891 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -188,14 +188,14 @@ func createRun(opts *CreateOptions) (err error) { assigneeSet.AddValues(assignees) tb := prShared.IssueMetadataState{ - Type: prShared.IssueMetadata, - ActorAssignees: issueFeatures.ActorIsAssignable, - Assignees: assigneeSet.ToSlice(), - Labels: opts.Labels, - ProjectTitles: opts.Projects, - Milestones: milestones, - Title: opts.Title, - Body: opts.Body, + Type: prShared.IssueMetadata, + ApiActorsSupported: issueFeatures.ActorIsAssignable, // TODO ApiActorsSupported + Assignees: assigneeSet.ToSlice(), + Labels: opts.Labels, + ProjectTitles: opts.Projects, + Milestones: milestones, + Title: opts.Title, + Body: opts.Body, } if opts.RecoverFile != "" { diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 2fd632b5e..965fc0b49 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -215,9 +215,9 @@ func editRun(opts *EditOptions) error { lookupFields := []string{"id", "number", "title", "body", "url"} if editable.Assignees.Edited { - // TODO actorIsAssignableCleanup + // TODO ApiActorsSupported if issueFeatures.ActorIsAssignable { - editable.Assignees.ActorAssignees = true + editable.ApiActorsSupported = true lookupFields = append(lookupFields, "assignedActors") } else { lookupFields = append(lookupFields, "assignees") @@ -280,7 +280,8 @@ func editRun(opts *EditOptions) error { editable.Body.Default = issue.Body // We use Actors as the default assignees if Actors are assignable // on this GitHub host. - if editable.Assignees.ActorAssignees { + // TODO ApiActorsSupported + if editable.ApiActorsSupported { editable.Assignees.Default = issue.AssignedActors.DisplayNames() editable.Assignees.DefaultLogins = issue.AssignedActors.Logins() } else { diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index ef496d1de..41af97fef 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -395,7 +395,7 @@ func Test_editRun(t *testing.T) { mockIssueProjectItemsGet(t, reg) mockRepoMetadata(t, reg) mockIssueUpdate(t, reg) - mockIssueUpdateActorAssignees(t, reg) + mockIssueUpdateApiActors(t, reg) mockIssueUpdateLabels(t, reg) mockProjectV2ItemUpdate(t, reg) }, @@ -444,8 +444,8 @@ func Test_editRun(t *testing.T) { mockIssueProjectItemsGet(t, reg) mockIssueUpdate(t, reg) mockIssueUpdate(t, reg) - mockIssueUpdateActorAssignees(t, reg) - mockIssueUpdateActorAssignees(t, reg) + mockIssueUpdateApiActors(t, reg) + mockIssueUpdateApiActors(t, reg) mockIssueUpdateLabels(t, reg) mockIssueUpdateLabels(t, reg) mockProjectV2ItemUpdate(t, reg) @@ -608,7 +608,7 @@ func Test_editRun(t *testing.T) { mockIssueProjectItemsGet(t, reg) mockRepoMetadata(t, reg) mockIssueUpdate(t, reg) - mockIssueUpdateActorAssignees(t, reg) + mockIssueUpdateApiActors(t, reg) mockIssueUpdateLabels(t, reg) mockProjectV2ItemUpdate(t, reg) }, @@ -902,7 +902,7 @@ func mockIssueUpdate(t *testing.T, reg *httpmock.Registry) { ) } -func mockIssueUpdateActorAssignees(t *testing.T, reg *httpmock.Registry) { +func mockIssueUpdateApiActors(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), httpmock.GraphQLMutation(` diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index d8230dc3c..b62e04d73 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -429,9 +429,9 @@ func createRun(opts *CreateOptions) error { return err } + // TODO ApiActorsSupported if issueFeatures.ActorIsAssignable { - state.ActorReviewers = true - state.ActorAssignees = true + state.ApiActorsSupported = true } var openURL string diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index fd53a92fc..5bad889b6 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -965,7 +965,7 @@ func Test_createRun(t *testing.T) { `, func(inputs map[string]interface{}) { assert.Equal(t, "NEWPULLID", inputs["pullRequestId"]) if _, ok := inputs["assigneeIds"]; ok { - t.Error("did not expect assigneeIds in updatePullRequest when ActorAssignees is true") + t.Error("did not expect assigneeIds in updatePullRequest when ApiActorsSupported is true") } assert.Equal(t, []interface{}{"BUGID", "TODOID"}, inputs["labelIds"]) assert.Equal(t, []interface{}{"ROADMAPID"}, inputs["projectIds"]) diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index 12c197b5a..c2535019b 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -289,9 +289,9 @@ func editRun(opts *EditOptions) error { editable.Base.Default = pr.BaseRefName editable.Reviewers.Default = pr.ReviewRequests.DisplayNames() editable.Reviewers.DefaultLogins = pr.ReviewRequests.Logins() - // TODO actorIsAssignableCleanup + // TODO ApiActorsSupported if issueFeatures.ActorIsAssignable { - editable.Assignees.ActorAssignees = true + editable.ApiActorsSupported = true editable.Assignees.Default = pr.AssignedActors.DisplayNames() editable.Assignees.DefaultLogins = pr.AssignedActors.Logins() } else { @@ -438,7 +438,8 @@ func updatePullRequestReviews(httpClient *http.Client, repo ghrepo.Interface, pr // 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 { + // TODO ApiActorsSupported + if editable.ApiActorsSupported { copilotReplacer := shared.NewCopilotReviewerReplacer() add = copilotReplacer.ReplaceSlice(add) remove = copilotReplacer.ReplaceSlice(remove) @@ -457,7 +458,8 @@ func updatePullRequestReviews(httpClient *http.Client, repo ghrepo.Interface, pr // On github.com, use the new GraphQL mutation which supports bots. // On GHES, fall back to REST API. - if editable.Assignees.ActorAssignees { + // TODO ApiActorsSupported + if editable.ApiActorsSupported { return updatePullRequestReviewsGraphQL(client, repo, prID, editable) } return updatePullRequestReviewsREST(client, repo, number, editable) diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 01acb7e0f..abe764158 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -417,7 +417,7 @@ func Test_editRun(t *testing.T) { // REST API accepts logins and team slugs directly mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) - mockPullRequestUpdateActorAssignees(reg) + mockPullRequestUpdateApiActors(reg) mockRequestReviewsByLogin(reg) mockPullRequestUpdateLabels(reg) mockProjectV2ItemUpdate(reg) @@ -475,7 +475,7 @@ func Test_editRun(t *testing.T) { httpStubs: func(t *testing.T, reg *httpmock.Registry) { mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) - mockPullRequestUpdateActorAssignees(reg) + mockPullRequestUpdateApiActors(reg) mockPullRequestUpdateLabels(reg) mockProjectV2ItemUpdate(reg) }, @@ -551,7 +551,7 @@ func Test_editRun(t *testing.T) { mockPullRequestUpdate(reg) mockRequestReviewsByLogin(reg) mockPullRequestUpdateLabels(reg) - mockPullRequestUpdateActorAssignees(reg) + mockPullRequestUpdateApiActors(reg) mockProjectV2ItemUpdate(reg) }, stdout: "https://github.com/OWNER/REPO/pull/123\n", @@ -756,7 +756,7 @@ func Test_editRun(t *testing.T) { // (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) + mockPullRequestUpdateApiActors(reg) mockRequestReviewsByLogin(reg) mockPullRequestUpdateLabels(reg) mockProjectV2ItemUpdate(reg) @@ -785,7 +785,7 @@ func Test_editRun(t *testing.T) { editFields: func(e *shared.Editable, _ string) error { e.Title.Value = "new title" e.Body.Value = "new body" - // When ActorAssignees is enabled, the interactive flow returns display names (or logins for non-users) + // When ApiActorsSupported is enabled, the interactive flow returns display names (or logins for non-users) e.Assignees.Value = []string{"monalisa (Mona Display Name)", "hubot"} // Populate metadata to simulate what searchFunc would do during prompting e.Metadata.AssignableActors = []api.AssignableActor{ @@ -808,7 +808,7 @@ func Test_editRun(t *testing.T) { // assignees: false because searchFunc handles dynamic fetching (metadata populated in test mock) mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) - mockPullRequestUpdateActorAssignees(reg) + mockPullRequestUpdateApiActors(reg) mockPullRequestUpdateLabels(reg) mockProjectV2ItemUpdate(reg) }, @@ -876,7 +876,7 @@ func Test_editRun(t *testing.T) { mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true}) mockPullRequestUpdate(reg) mockRequestReviewsByLogin(reg) - mockPullRequestUpdateActorAssignees(reg) + mockPullRequestUpdateApiActors(reg) mockPullRequestUpdateLabels(reg) mockProjectV2ItemUpdate(reg) }, @@ -990,7 +990,7 @@ func Test_editRun(t *testing.T) { return nil }, editFields: func(e *shared.Editable, _ string) error { - require.False(t, e.Assignees.ActorAssignees) + require.False(t, e.ApiActorsSupported) require.Nil(t, e.AssigneeSearchFunc) require.Contains(t, e.Assignees.Options, "monalisa") require.Contains(t, e.Assignees.Options, "hubot") @@ -1190,7 +1190,7 @@ type mockRepoMetadataOptions struct { } func mockRepoMetadata(reg *httpmock.Registry, opt mockRepoMetadataOptions) { - // Assignable actors (users/bots) are fetched when reviewers OR assignees edited with ActorAssignees enabled. + // Assignable actors (users/bots) are fetched when reviewers OR assignees edited with ApiActorsSupported enabled. if opt.reviewers || opt.assignees { reg.Register( httpmock.GraphQL(`query RepositoryAssignableActors\b`), @@ -1314,7 +1314,7 @@ func mockPullRequestUpdate(reg *httpmock.Registry) { httpmock.StringResponse(`{}`)) } -func mockPullRequestUpdateActorAssignees(reg *httpmock.Registry) { +func mockPullRequestUpdateApiActors(reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`), httpmock.GraphQLMutation(` @@ -1336,7 +1336,7 @@ func mockPullRequestRemoveReviewers(reg *httpmock.Registry) { } // mockRequestReviewsByLogin mocks the RequestReviewsByLogin GraphQL mutation -// used on github.com when ActorAssignees is enabled. +// used on github.com when ApiActorsSupported is enabled. func mockRequestReviewsByLogin(reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`mutation RequestReviewsByLogin\b`), diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 02fe5d33c..d29b6d4c4 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -22,6 +22,13 @@ type Editable struct { Projects EditableProjects Milestone EditableString Metadata api.RepoMetadataResult + + // TODO ApiActorsSupported + // ApiActorsSupported indicates the host supports actor-based APIs (github.com, ghe.com). + // When true, mutations use logins directly instead of resolving node IDs. + // Remove this flag (and collapse to actor-only paths) once GHES supports + // replaceActorsForAssignable and requestReviewsByLogin mutations. + ApiActorsSupported bool } type EditableString struct { @@ -42,11 +49,9 @@ type EditableSlice struct { } // EditableAssignees is a special case of EditableSlice. -// It contains a flag to indicate whether the assignees are actors or not. type EditableAssignees struct { EditableSlice - ActorAssignees bool - DefaultLogins []string // For disambiguating actors from display names + DefaultLogins []string // For disambiguating actors from display names } // EditableReviewers is a special case of EditableSlice. @@ -95,7 +100,8 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str // If assignees came in from command line flags, we need to // curate the final list of assignees from the default list. if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 { - replacer := NewSpecialAssigneeReplacer(client, repo.RepoHost(), e.Assignees.ActorAssignees, true) + // TODO ApiActorsSupported + replacer := NewSpecialAssigneeReplacer(client, repo.RepoHost(), e.ApiActorsSupported, true) assigneeSet := set.NewStringSet() @@ -107,7 +113,8 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str // So, we need to add the default logins here instead of the DisplayNames. // Otherwise, the value the user provided won't be found in the // set to be added or removed, causing unexpected behavior. - if e.Assignees.ActorAssignees { + // TODO ApiActorsSupported + if e.ApiActorsSupported { assigneeSet.AddValues(e.Assignees.DefaultLogins) } else { assigneeSet.AddValues(e.Assignees.Default) @@ -283,6 +290,7 @@ func (e *Editable) Clone() Editable { Labels: e.Labels.clone(), Projects: e.Projects.clone(), Milestone: e.Milestone.clone(), + ApiActorsSupported: e.ApiActorsSupported, // Shallow copy since no mutation. Metadata: e.Metadata, } @@ -319,9 +327,8 @@ func (es *EditableSlice) clone() EditableSlice { func (ea *EditableAssignees) clone() EditableAssignees { return EditableAssignees{ - EditableSlice: ea.EditableSlice.clone(), - ActorAssignees: ea.ActorAssignees, - DefaultLogins: ea.DefaultLogins, + EditableSlice: ea.EditableSlice.clone(), + DefaultLogins: ea.DefaultLogins, } } @@ -522,22 +529,23 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, fetchAssignees = true } // For non-interactive Add/Remove operations, we only need to fetch assignees - // on GHES where ID resolution is required. On github.com (ActorAssignees), + // on GHES where ID resolution is required. On github.com (ApiActorsSupported), // logins are passed directly to the mutation. - if (len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0) && !editable.Assignees.ActorAssignees { + // TODO ApiActorsSupported + if (len(editable.Assignees.Add) > 0 || len(editable.Assignees.Remove) > 0) && !editable.ApiActorsSupported { fetchAssignees = true } } input := api.RepoMetadataInput{ - Reviewers: fetchReviewers, - TeamReviewers: teamReviewers, - Assignees: fetchAssignees, - ActorAssignees: editable.Assignees.ActorAssignees, - Labels: editable.Labels.Edited, - ProjectsV1: editable.Projects.Edited && projectV1Support == gh.ProjectsV1Supported, - ProjectsV2: editable.Projects.Edited, - Milestones: editable.Milestone.Edited, + Reviewers: fetchReviewers, + TeamReviewers: teamReviewers, + Assignees: fetchAssignees, + ApiActorsSupported: editable.ApiActorsSupported, + Labels: editable.Labels.Edited, + ProjectsV1: editable.Projects.Edited && projectV1Support == gh.ProjectsV1Supported, + ProjectsV2: editable.Projects.Edited, + Milestones: editable.Milestone.Edited, } metadata, err := api.RepoMetadata(client, repo, input) if err != nil { @@ -574,7 +582,8 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, editable.Metadata = *metadata editable.Reviewers.Options = append(users, teams...) - if editable.Assignees.ActorAssignees { + // TODO ApiActorsSupported + if editable.ApiActorsSupported { editable.Assignees.Options = actors } else { editable.Assignees.Options = users diff --git a/pkg/cmd/pr/shared/editable_http.go b/pkg/cmd/pr/shared/editable_http.go index bb63ceb0f..39140eefd 100644 --- a/pkg/cmd/pr/shared/editable_http.go +++ b/pkg/cmd/pr/shared/editable_http.go @@ -66,7 +66,8 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR // other issue fields to ensure consistency with how legacy // user assignees are handled. // https://github.com/cli/cli/pull/10960#discussion_r2086725348 - if options.Assignees.Edited && options.Assignees.ActorAssignees { + // TODO ApiActorsSupported + if options.Assignees.Edited && options.ApiActorsSupported { apiClient := api.NewClientFromHTTP(httpClient) logins, err := options.AssigneeLogins(apiClient, repo) if err != nil { @@ -99,7 +100,8 @@ func replaceIssueFields(httpClient *http.Client, repo ghrepo.Interface, id strin } var assigneeIds *[]string - if !options.Assignees.ActorAssignees { + // TODO ApiActorsSupported + if !options.ApiActorsSupported { assigneeIds, err = options.AssigneeIds(apiClient, repo) if err != nil { return err diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 17d40d639..76f096efd 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -61,11 +61,13 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par return nil } - // When ActorReviewers is true, we use login-based mutation and don't need to resolve reviewer IDs. - needReviewerIDs := len(tb.Reviewers) > 0 && !tb.ActorReviewers + // TODO ApiActorsSupported + // When ApiActorsSupported is true, we use login-based mutation and don't need to resolve reviewer IDs. + needReviewerIDs := len(tb.Reviewers) > 0 && !tb.ApiActorsSupported - // When ActorAssignees is true, we use login-based mutation and don't need to resolve assignee IDs. - needAssigneeIDs := len(tb.Assignees) > 0 && !tb.ActorAssignees + // TODO ApiActorsSupported + // When ApiActorsSupported is true, we use login-based mutation and don't need to resolve assignee IDs. + needAssigneeIDs := len(tb.Assignees) > 0 && !tb.ApiActorsSupported // Retrieve minimal information needed to resolve metadata if this was not previously cached from additional metadata survey. if tb.MetadataResult == nil { @@ -88,9 +90,10 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par tb.MetadataResult = metadataResult } - // When ActorAssignees is true (github.com), pass logins directly for use with + // TODO ApiActorsSupported + // When ApiActorsSupported is true (github.com), pass logins directly for use with // ReplaceActorsForAssignable mutation. The ID-based else branch is for GHES compatibility. - if tb.ActorAssignees { + if tb.ApiActorsSupported { params["assigneeLogins"] = tb.Assignees } else { assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) @@ -138,11 +141,11 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par } } - // TODO requestReviewsByLoginCleanup - // When ActorReviewers is true (github.com), pass logins directly for use with + // TODO ApiActorsSupported + // When ApiActorsSupported is true (github.com), pass logins directly for use with // RequestReviewsByLogin mutation. The ID-based else branch can be removed once // GHES supports requestReviewsByLogin. - if tb.ActorReviewers { + if tb.ApiActorsSupported { params["userReviewerLogins"] = userReviewers if len(botReviewers) > 0 { params["botReviewerLogins"] = botReviewers diff --git a/pkg/cmd/pr/shared/state.go b/pkg/cmd/pr/shared/state.go index 0e5c31cdd..32807ae4d 100644 --- a/pkg/cmd/pr/shared/state.go +++ b/pkg/cmd/pr/shared/state.go @@ -18,9 +18,14 @@ const ( type IssueMetadataState struct { Type metadataStateType - Draft bool - ActorAssignees bool - ActorReviewers bool + Draft bool + + // TODO ApiActorsSupported + // ApiActorsSupported indicates the host supports actor-based APIs (github.com, ghe.com). + // When true, mutations use logins directly instead of resolving node IDs. + // Remove this flag (and collapse to actor-only paths) once GHES supports + // replaceActorsForAssignable and requestReviewsByLogin mutations. + ApiActorsSupported bool Body string Title string diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 71cfcd063..544c04bc8 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -181,19 +181,20 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface } // Retrieve and process data for survey prompts based on the extra fields selected. - // When search-based reviewer selection is available, skip the expensive assignable-users - // and teams fetch since reviewers are found dynamically via the search function. - useReviewerSearch := state.ActorReviewers && reviewerSearchFunc != nil - useAssigneeSearch := state.ActorAssignees && assigneeSearchFunc != nil + // When search-based selection is available, skip the expensive assignable-users + // and teams fetch since they are found dynamically via the search function. + // TODO ApiActorsSupported + useReviewerSearch := state.ApiActorsSupported && reviewerSearchFunc != nil + useAssigneeSearch := state.ApiActorsSupported && assigneeSearchFunc != nil metadataInput := api.RepoMetadataInput{ - Reviewers: isChosen("Reviewers") && !useReviewerSearch, - TeamReviewers: isChosen("Reviewers") && !useReviewerSearch, - Assignees: isChosen("Assignees") && !useAssigneeSearch, - ActorAssignees: isChosen("Assignees") && !useAssigneeSearch && state.ActorAssignees, - Labels: isChosen("Labels"), - ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported, - ProjectsV2: isChosen("Projects"), - Milestones: isChosen("Milestone"), + Reviewers: isChosen("Reviewers") && !useReviewerSearch, + TeamReviewers: isChosen("Reviewers") && !useReviewerSearch, + Assignees: isChosen("Assignees") && !useAssigneeSearch, + ApiActorsSupported: state.ApiActorsSupported && ((isChosen("Assignees") && !useAssigneeSearch) || (isChosen("Reviewers") && !useReviewerSearch)), + Labels: isChosen("Labels"), + ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported, + ProjectsV2: isChosen("Projects"), + Milestones: isChosen("Milestone"), } metadataResult, err := fetcher.RepoMetadataFetch(metadataInput) if err != nil { @@ -217,7 +218,8 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface var assignees []string var assigneesDefault []string if !useAssigneeSearch { - if state.ActorAssignees { + // TODO ApiActorsSupported + if state.ApiActorsSupported { for _, u := range metadataResult.AssignableActors { assignees = append(assignees, u.DisplayName()) @@ -305,7 +307,8 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface return err } for _, i := range selected { - if state.ActorAssignees { + // TODO ApiActorsSupported + if state.ApiActorsSupported { values.Assignees = append(values.Assignees, metadataResult.AssignableActors[i].Login()) } else { values.Assignees = append(values.Assignees, metadataResult.AssignableUsers[i].Login())