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>
This commit is contained in:
parent
1df6f84d70
commit
3c00ffdade
15 changed files with 119 additions and 92 deletions
|
|
@ -311,7 +311,7 @@ func IssueCreate(client *Client, repo *Repository, params map[string]interface{}
|
||||||
}
|
}
|
||||||
issue := &result.CreateIssue.Issue
|
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 {
|
if assigneeLogins, ok := params["assigneeLogins"].([]string); ok && len(assigneeLogins) > 0 {
|
||||||
err := ReplaceActorsForAssignableByLogin(client, repo, issue.ID, assigneeLogins)
|
err := ReplaceActorsForAssignableByLogin(client, repo, issue.ID, assigneeLogins)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
||||||
|
|
@ -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 {
|
if assigneeLogins, ok := params["assigneeLogins"].([]string); ok && len(assigneeLogins) > 0 {
|
||||||
err := ReplaceActorsForAssignableByLogin(client, repo, pr.ID, assigneeLogins)
|
err := ReplaceActorsForAssignableByLogin(client, repo, pr.ID, assigneeLogins)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
||||||
|
|
@ -922,14 +922,15 @@ func (m *RepoMetadataResult) Merge(m2 *RepoMetadataResult) {
|
||||||
}
|
}
|
||||||
|
|
||||||
type RepoMetadataInput struct {
|
type RepoMetadataInput struct {
|
||||||
Assignees bool
|
Assignees bool
|
||||||
ActorAssignees bool
|
Reviewers bool
|
||||||
Reviewers bool
|
TeamReviewers bool
|
||||||
TeamReviewers bool
|
// TODO ApiActorsSupported
|
||||||
Labels bool
|
ApiActorsSupported bool
|
||||||
ProjectsV1 bool
|
Labels bool
|
||||||
ProjectsV2 bool
|
ProjectsV1 bool
|
||||||
Milestones bool
|
ProjectsV2 bool
|
||||||
|
Milestones bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// RepoMetadata pre-fetches the metadata for attaching to issues and pull requests
|
// 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
|
var g errgroup.Group
|
||||||
|
|
||||||
if input.Assignees || input.Reviewers {
|
if input.Assignees || input.Reviewers {
|
||||||
if input.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if input.ApiActorsSupported {
|
||||||
g.Go(func() error {
|
g.Go(func() error {
|
||||||
actors, err := RepoAssignableActors(client, repo)
|
actors, err := RepoAssignableActors(client, repo)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
||||||
|
|
@ -188,14 +188,14 @@ func createRun(opts *CreateOptions) (err error) {
|
||||||
assigneeSet.AddValues(assignees)
|
assigneeSet.AddValues(assignees)
|
||||||
|
|
||||||
tb := prShared.IssueMetadataState{
|
tb := prShared.IssueMetadataState{
|
||||||
Type: prShared.IssueMetadata,
|
Type: prShared.IssueMetadata,
|
||||||
ActorAssignees: issueFeatures.ActorIsAssignable,
|
ApiActorsSupported: issueFeatures.ActorIsAssignable, // TODO ApiActorsSupported
|
||||||
Assignees: assigneeSet.ToSlice(),
|
Assignees: assigneeSet.ToSlice(),
|
||||||
Labels: opts.Labels,
|
Labels: opts.Labels,
|
||||||
ProjectTitles: opts.Projects,
|
ProjectTitles: opts.Projects,
|
||||||
Milestones: milestones,
|
Milestones: milestones,
|
||||||
Title: opts.Title,
|
Title: opts.Title,
|
||||||
Body: opts.Body,
|
Body: opts.Body,
|
||||||
}
|
}
|
||||||
|
|
||||||
if opts.RecoverFile != "" {
|
if opts.RecoverFile != "" {
|
||||||
|
|
|
||||||
|
|
@ -215,9 +215,9 @@ func editRun(opts *EditOptions) error {
|
||||||
|
|
||||||
lookupFields := []string{"id", "number", "title", "body", "url"}
|
lookupFields := []string{"id", "number", "title", "body", "url"}
|
||||||
if editable.Assignees.Edited {
|
if editable.Assignees.Edited {
|
||||||
// TODO actorIsAssignableCleanup
|
// TODO ApiActorsSupported
|
||||||
if issueFeatures.ActorIsAssignable {
|
if issueFeatures.ActorIsAssignable {
|
||||||
editable.Assignees.ActorAssignees = true
|
editable.ApiActorsSupported = true
|
||||||
lookupFields = append(lookupFields, "assignedActors")
|
lookupFields = append(lookupFields, "assignedActors")
|
||||||
} else {
|
} else {
|
||||||
lookupFields = append(lookupFields, "assignees")
|
lookupFields = append(lookupFields, "assignees")
|
||||||
|
|
@ -280,7 +280,8 @@ func editRun(opts *EditOptions) error {
|
||||||
editable.Body.Default = issue.Body
|
editable.Body.Default = issue.Body
|
||||||
// We use Actors as the default assignees if Actors are assignable
|
// We use Actors as the default assignees if Actors are assignable
|
||||||
// on this GitHub host.
|
// on this GitHub host.
|
||||||
if editable.Assignees.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if editable.ApiActorsSupported {
|
||||||
editable.Assignees.Default = issue.AssignedActors.DisplayNames()
|
editable.Assignees.Default = issue.AssignedActors.DisplayNames()
|
||||||
editable.Assignees.DefaultLogins = issue.AssignedActors.Logins()
|
editable.Assignees.DefaultLogins = issue.AssignedActors.Logins()
|
||||||
} else {
|
} else {
|
||||||
|
|
|
||||||
|
|
@ -395,7 +395,7 @@ func Test_editRun(t *testing.T) {
|
||||||
mockIssueProjectItemsGet(t, reg)
|
mockIssueProjectItemsGet(t, reg)
|
||||||
mockRepoMetadata(t, reg)
|
mockRepoMetadata(t, reg)
|
||||||
mockIssueUpdate(t, reg)
|
mockIssueUpdate(t, reg)
|
||||||
mockIssueUpdateActorAssignees(t, reg)
|
mockIssueUpdateApiActors(t, reg)
|
||||||
mockIssueUpdateLabels(t, reg)
|
mockIssueUpdateLabels(t, reg)
|
||||||
mockProjectV2ItemUpdate(t, reg)
|
mockProjectV2ItemUpdate(t, reg)
|
||||||
},
|
},
|
||||||
|
|
@ -444,8 +444,8 @@ func Test_editRun(t *testing.T) {
|
||||||
mockIssueProjectItemsGet(t, reg)
|
mockIssueProjectItemsGet(t, reg)
|
||||||
mockIssueUpdate(t, reg)
|
mockIssueUpdate(t, reg)
|
||||||
mockIssueUpdate(t, reg)
|
mockIssueUpdate(t, reg)
|
||||||
mockIssueUpdateActorAssignees(t, reg)
|
mockIssueUpdateApiActors(t, reg)
|
||||||
mockIssueUpdateActorAssignees(t, reg)
|
mockIssueUpdateApiActors(t, reg)
|
||||||
mockIssueUpdateLabels(t, reg)
|
mockIssueUpdateLabels(t, reg)
|
||||||
mockIssueUpdateLabels(t, reg)
|
mockIssueUpdateLabels(t, reg)
|
||||||
mockProjectV2ItemUpdate(t, reg)
|
mockProjectV2ItemUpdate(t, reg)
|
||||||
|
|
@ -608,7 +608,7 @@ func Test_editRun(t *testing.T) {
|
||||||
mockIssueProjectItemsGet(t, reg)
|
mockIssueProjectItemsGet(t, reg)
|
||||||
mockRepoMetadata(t, reg)
|
mockRepoMetadata(t, reg)
|
||||||
mockIssueUpdate(t, reg)
|
mockIssueUpdate(t, reg)
|
||||||
mockIssueUpdateActorAssignees(t, reg)
|
mockIssueUpdateApiActors(t, reg)
|
||||||
mockIssueUpdateLabels(t, reg)
|
mockIssueUpdateLabels(t, reg)
|
||||||
mockProjectV2ItemUpdate(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(
|
reg.Register(
|
||||||
httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
|
httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
|
||||||
httpmock.GraphQLMutation(`
|
httpmock.GraphQLMutation(`
|
||||||
|
|
|
||||||
|
|
@ -429,9 +429,9 @@ func createRun(opts *CreateOptions) error {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TODO ApiActorsSupported
|
||||||
if issueFeatures.ActorIsAssignable {
|
if issueFeatures.ActorIsAssignable {
|
||||||
state.ActorReviewers = true
|
state.ApiActorsSupported = true
|
||||||
state.ActorAssignees = true
|
|
||||||
}
|
}
|
||||||
|
|
||||||
var openURL string
|
var openURL string
|
||||||
|
|
|
||||||
|
|
@ -965,7 +965,7 @@ func Test_createRun(t *testing.T) {
|
||||||
`, func(inputs map[string]interface{}) {
|
`, func(inputs map[string]interface{}) {
|
||||||
assert.Equal(t, "NEWPULLID", inputs["pullRequestId"])
|
assert.Equal(t, "NEWPULLID", inputs["pullRequestId"])
|
||||||
if _, ok := inputs["assigneeIds"]; ok {
|
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{}{"BUGID", "TODOID"}, inputs["labelIds"])
|
||||||
assert.Equal(t, []interface{}{"ROADMAPID"}, inputs["projectIds"])
|
assert.Equal(t, []interface{}{"ROADMAPID"}, inputs["projectIds"])
|
||||||
|
|
|
||||||
|
|
@ -289,9 +289,9 @@ func editRun(opts *EditOptions) error {
|
||||||
editable.Base.Default = pr.BaseRefName
|
editable.Base.Default = pr.BaseRefName
|
||||||
editable.Reviewers.Default = pr.ReviewRequests.DisplayNames()
|
editable.Reviewers.Default = pr.ReviewRequests.DisplayNames()
|
||||||
editable.Reviewers.DefaultLogins = pr.ReviewRequests.Logins()
|
editable.Reviewers.DefaultLogins = pr.ReviewRequests.Logins()
|
||||||
// TODO actorIsAssignableCleanup
|
// TODO ApiActorsSupported
|
||||||
if issueFeatures.ActorIsAssignable {
|
if issueFeatures.ActorIsAssignable {
|
||||||
editable.Assignees.ActorAssignees = true
|
editable.ApiActorsSupported = true
|
||||||
editable.Assignees.Default = pr.AssignedActors.DisplayNames()
|
editable.Assignees.Default = pr.AssignedActors.DisplayNames()
|
||||||
editable.Assignees.DefaultLogins = pr.AssignedActors.Logins()
|
editable.Assignees.DefaultLogins = pr.AssignedActors.Logins()
|
||||||
} else {
|
} 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).
|
// Replace @copilot with the Copilot reviewer login (only on github.com).
|
||||||
// Also use DefaultLogins (not Default display names) for computing the set.
|
// Also use DefaultLogins (not Default display names) for computing the set.
|
||||||
var defaultLogins []string
|
var defaultLogins []string
|
||||||
if editable.Assignees.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if editable.ApiActorsSupported {
|
||||||
copilotReplacer := shared.NewCopilotReviewerReplacer()
|
copilotReplacer := shared.NewCopilotReviewerReplacer()
|
||||||
add = copilotReplacer.ReplaceSlice(add)
|
add = copilotReplacer.ReplaceSlice(add)
|
||||||
remove = copilotReplacer.ReplaceSlice(remove)
|
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 github.com, use the new GraphQL mutation which supports bots.
|
||||||
// On GHES, fall back to REST API.
|
// On GHES, fall back to REST API.
|
||||||
if editable.Assignees.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if editable.ApiActorsSupported {
|
||||||
return updatePullRequestReviewsGraphQL(client, repo, prID, editable)
|
return updatePullRequestReviewsGraphQL(client, repo, prID, editable)
|
||||||
}
|
}
|
||||||
return updatePullRequestReviewsREST(client, repo, number, editable)
|
return updatePullRequestReviewsREST(client, repo, number, editable)
|
||||||
|
|
|
||||||
|
|
@ -417,7 +417,7 @@ func Test_editRun(t *testing.T) {
|
||||||
// REST API accepts logins and team slugs directly
|
// REST API accepts logins and team slugs directly
|
||||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true})
|
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true})
|
||||||
mockPullRequestUpdate(reg)
|
mockPullRequestUpdate(reg)
|
||||||
mockPullRequestUpdateActorAssignees(reg)
|
mockPullRequestUpdateApiActors(reg)
|
||||||
mockRequestReviewsByLogin(reg)
|
mockRequestReviewsByLogin(reg)
|
||||||
mockPullRequestUpdateLabels(reg)
|
mockPullRequestUpdateLabels(reg)
|
||||||
mockProjectV2ItemUpdate(reg)
|
mockProjectV2ItemUpdate(reg)
|
||||||
|
|
@ -475,7 +475,7 @@ func Test_editRun(t *testing.T) {
|
||||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||||
mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true})
|
mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true})
|
||||||
mockPullRequestUpdate(reg)
|
mockPullRequestUpdate(reg)
|
||||||
mockPullRequestUpdateActorAssignees(reg)
|
mockPullRequestUpdateApiActors(reg)
|
||||||
mockPullRequestUpdateLabels(reg)
|
mockPullRequestUpdateLabels(reg)
|
||||||
mockProjectV2ItemUpdate(reg)
|
mockProjectV2ItemUpdate(reg)
|
||||||
},
|
},
|
||||||
|
|
@ -551,7 +551,7 @@ func Test_editRun(t *testing.T) {
|
||||||
mockPullRequestUpdate(reg)
|
mockPullRequestUpdate(reg)
|
||||||
mockRequestReviewsByLogin(reg)
|
mockRequestReviewsByLogin(reg)
|
||||||
mockPullRequestUpdateLabels(reg)
|
mockPullRequestUpdateLabels(reg)
|
||||||
mockPullRequestUpdateActorAssignees(reg)
|
mockPullRequestUpdateApiActors(reg)
|
||||||
mockProjectV2ItemUpdate(reg)
|
mockProjectV2ItemUpdate(reg)
|
||||||
},
|
},
|
||||||
stdout: "https://github.com/OWNER/REPO/pull/123\n",
|
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)
|
// (searchFunc handles dynamic fetching, metadata populated in test mock)
|
||||||
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true})
|
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true})
|
||||||
mockPullRequestUpdate(reg)
|
mockPullRequestUpdate(reg)
|
||||||
mockPullRequestUpdateActorAssignees(reg)
|
mockPullRequestUpdateApiActors(reg)
|
||||||
mockRequestReviewsByLogin(reg)
|
mockRequestReviewsByLogin(reg)
|
||||||
mockPullRequestUpdateLabels(reg)
|
mockPullRequestUpdateLabels(reg)
|
||||||
mockProjectV2ItemUpdate(reg)
|
mockProjectV2ItemUpdate(reg)
|
||||||
|
|
@ -785,7 +785,7 @@ func Test_editRun(t *testing.T) {
|
||||||
editFields: func(e *shared.Editable, _ string) error {
|
editFields: func(e *shared.Editable, _ string) error {
|
||||||
e.Title.Value = "new title"
|
e.Title.Value = "new title"
|
||||||
e.Body.Value = "new body"
|
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"}
|
e.Assignees.Value = []string{"monalisa (Mona Display Name)", "hubot"}
|
||||||
// Populate metadata to simulate what searchFunc would do during prompting
|
// Populate metadata to simulate what searchFunc would do during prompting
|
||||||
e.Metadata.AssignableActors = []api.AssignableActor{
|
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)
|
// assignees: false because searchFunc handles dynamic fetching (metadata populated in test mock)
|
||||||
mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true})
|
mockRepoMetadata(reg, mockRepoMetadataOptions{assignees: false, labels: true, projects: true, milestones: true})
|
||||||
mockPullRequestUpdate(reg)
|
mockPullRequestUpdate(reg)
|
||||||
mockPullRequestUpdateActorAssignees(reg)
|
mockPullRequestUpdateApiActors(reg)
|
||||||
mockPullRequestUpdateLabels(reg)
|
mockPullRequestUpdateLabels(reg)
|
||||||
mockProjectV2ItemUpdate(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})
|
mockRepoMetadata(reg, mockRepoMetadataOptions{reviewers: false, teamReviewers: false, assignees: false, labels: true, projects: true, milestones: true})
|
||||||
mockPullRequestUpdate(reg)
|
mockPullRequestUpdate(reg)
|
||||||
mockRequestReviewsByLogin(reg)
|
mockRequestReviewsByLogin(reg)
|
||||||
mockPullRequestUpdateActorAssignees(reg)
|
mockPullRequestUpdateApiActors(reg)
|
||||||
mockPullRequestUpdateLabels(reg)
|
mockPullRequestUpdateLabels(reg)
|
||||||
mockProjectV2ItemUpdate(reg)
|
mockProjectV2ItemUpdate(reg)
|
||||||
},
|
},
|
||||||
|
|
@ -990,7 +990,7 @@ func Test_editRun(t *testing.T) {
|
||||||
return nil
|
return nil
|
||||||
},
|
},
|
||||||
editFields: func(e *shared.Editable, _ string) error {
|
editFields: func(e *shared.Editable, _ string) error {
|
||||||
require.False(t, e.Assignees.ActorAssignees)
|
require.False(t, e.ApiActorsSupported)
|
||||||
require.Nil(t, e.AssigneeSearchFunc)
|
require.Nil(t, e.AssigneeSearchFunc)
|
||||||
require.Contains(t, e.Assignees.Options, "monalisa")
|
require.Contains(t, e.Assignees.Options, "monalisa")
|
||||||
require.Contains(t, e.Assignees.Options, "hubot")
|
require.Contains(t, e.Assignees.Options, "hubot")
|
||||||
|
|
@ -1190,7 +1190,7 @@ type mockRepoMetadataOptions struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
func mockRepoMetadata(reg *httpmock.Registry, opt mockRepoMetadataOptions) {
|
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 {
|
if opt.reviewers || opt.assignees {
|
||||||
reg.Register(
|
reg.Register(
|
||||||
httpmock.GraphQL(`query RepositoryAssignableActors\b`),
|
httpmock.GraphQL(`query RepositoryAssignableActors\b`),
|
||||||
|
|
@ -1314,7 +1314,7 @@ func mockPullRequestUpdate(reg *httpmock.Registry) {
|
||||||
httpmock.StringResponse(`{}`))
|
httpmock.StringResponse(`{}`))
|
||||||
}
|
}
|
||||||
|
|
||||||
func mockPullRequestUpdateActorAssignees(reg *httpmock.Registry) {
|
func mockPullRequestUpdateApiActors(reg *httpmock.Registry) {
|
||||||
reg.Register(
|
reg.Register(
|
||||||
httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
|
httpmock.GraphQL(`mutation ReplaceActorsForAssignable\b`),
|
||||||
httpmock.GraphQLMutation(`
|
httpmock.GraphQLMutation(`
|
||||||
|
|
@ -1336,7 +1336,7 @@ func mockPullRequestRemoveReviewers(reg *httpmock.Registry) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// mockRequestReviewsByLogin mocks the RequestReviewsByLogin GraphQL mutation
|
// 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) {
|
func mockRequestReviewsByLogin(reg *httpmock.Registry) {
|
||||||
reg.Register(
|
reg.Register(
|
||||||
httpmock.GraphQL(`mutation RequestReviewsByLogin\b`),
|
httpmock.GraphQL(`mutation RequestReviewsByLogin\b`),
|
||||||
|
|
|
||||||
|
|
@ -22,6 +22,13 @@ type Editable struct {
|
||||||
Projects EditableProjects
|
Projects EditableProjects
|
||||||
Milestone EditableString
|
Milestone EditableString
|
||||||
Metadata api.RepoMetadataResult
|
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 {
|
type EditableString struct {
|
||||||
|
|
@ -42,11 +49,9 @@ type EditableSlice struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
// EditableAssignees is a special case of EditableSlice.
|
// EditableAssignees is a special case of EditableSlice.
|
||||||
// It contains a flag to indicate whether the assignees are actors or not.
|
|
||||||
type EditableAssignees struct {
|
type EditableAssignees struct {
|
||||||
EditableSlice
|
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.
|
// 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
|
// If assignees came in from command line flags, we need to
|
||||||
// curate the final list of assignees from the default list.
|
// curate the final list of assignees from the default list.
|
||||||
if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 {
|
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()
|
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.
|
// 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
|
// Otherwise, the value the user provided won't be found in the
|
||||||
// set to be added or removed, causing unexpected behavior.
|
// set to be added or removed, causing unexpected behavior.
|
||||||
if e.Assignees.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if e.ApiActorsSupported {
|
||||||
assigneeSet.AddValues(e.Assignees.DefaultLogins)
|
assigneeSet.AddValues(e.Assignees.DefaultLogins)
|
||||||
} else {
|
} else {
|
||||||
assigneeSet.AddValues(e.Assignees.Default)
|
assigneeSet.AddValues(e.Assignees.Default)
|
||||||
|
|
@ -283,6 +290,7 @@ func (e *Editable) Clone() Editable {
|
||||||
Labels: e.Labels.clone(),
|
Labels: e.Labels.clone(),
|
||||||
Projects: e.Projects.clone(),
|
Projects: e.Projects.clone(),
|
||||||
Milestone: e.Milestone.clone(),
|
Milestone: e.Milestone.clone(),
|
||||||
|
ApiActorsSupported: e.ApiActorsSupported,
|
||||||
// Shallow copy since no mutation.
|
// Shallow copy since no mutation.
|
||||||
Metadata: e.Metadata,
|
Metadata: e.Metadata,
|
||||||
}
|
}
|
||||||
|
|
@ -319,9 +327,8 @@ func (es *EditableSlice) clone() EditableSlice {
|
||||||
|
|
||||||
func (ea *EditableAssignees) clone() EditableAssignees {
|
func (ea *EditableAssignees) clone() EditableAssignees {
|
||||||
return EditableAssignees{
|
return EditableAssignees{
|
||||||
EditableSlice: ea.EditableSlice.clone(),
|
EditableSlice: ea.EditableSlice.clone(),
|
||||||
ActorAssignees: ea.ActorAssignees,
|
DefaultLogins: ea.DefaultLogins,
|
||||||
DefaultLogins: ea.DefaultLogins,
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -522,22 +529,23 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable,
|
||||||
fetchAssignees = true
|
fetchAssignees = true
|
||||||
}
|
}
|
||||||
// For non-interactive Add/Remove operations, we only need to fetch assignees
|
// 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.
|
// 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
|
fetchAssignees = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
input := api.RepoMetadataInput{
|
input := api.RepoMetadataInput{
|
||||||
Reviewers: fetchReviewers,
|
Reviewers: fetchReviewers,
|
||||||
TeamReviewers: teamReviewers,
|
TeamReviewers: teamReviewers,
|
||||||
Assignees: fetchAssignees,
|
Assignees: fetchAssignees,
|
||||||
ActorAssignees: editable.Assignees.ActorAssignees,
|
ApiActorsSupported: editable.ApiActorsSupported,
|
||||||
Labels: editable.Labels.Edited,
|
Labels: editable.Labels.Edited,
|
||||||
ProjectsV1: editable.Projects.Edited && projectV1Support == gh.ProjectsV1Supported,
|
ProjectsV1: editable.Projects.Edited && projectV1Support == gh.ProjectsV1Supported,
|
||||||
ProjectsV2: editable.Projects.Edited,
|
ProjectsV2: editable.Projects.Edited,
|
||||||
Milestones: editable.Milestone.Edited,
|
Milestones: editable.Milestone.Edited,
|
||||||
}
|
}
|
||||||
metadata, err := api.RepoMetadata(client, repo, input)
|
metadata, err := api.RepoMetadata(client, repo, input)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -574,7 +582,8 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable,
|
||||||
|
|
||||||
editable.Metadata = *metadata
|
editable.Metadata = *metadata
|
||||||
editable.Reviewers.Options = append(users, teams...)
|
editable.Reviewers.Options = append(users, teams...)
|
||||||
if editable.Assignees.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if editable.ApiActorsSupported {
|
||||||
editable.Assignees.Options = actors
|
editable.Assignees.Options = actors
|
||||||
} else {
|
} else {
|
||||||
editable.Assignees.Options = users
|
editable.Assignees.Options = users
|
||||||
|
|
|
||||||
|
|
@ -66,7 +66,8 @@ func UpdateIssue(httpClient *http.Client, repo ghrepo.Interface, id string, isPR
|
||||||
// other issue fields to ensure consistency with how legacy
|
// other issue fields to ensure consistency with how legacy
|
||||||
// user assignees are handled.
|
// user assignees are handled.
|
||||||
// https://github.com/cli/cli/pull/10960#discussion_r2086725348
|
// 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)
|
apiClient := api.NewClientFromHTTP(httpClient)
|
||||||
logins, err := options.AssigneeLogins(apiClient, repo)
|
logins, err := options.AssigneeLogins(apiClient, repo)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -99,7 +100,8 @@ func replaceIssueFields(httpClient *http.Client, repo ghrepo.Interface, id strin
|
||||||
}
|
}
|
||||||
|
|
||||||
var assigneeIds *[]string
|
var assigneeIds *[]string
|
||||||
if !options.Assignees.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if !options.ApiActorsSupported {
|
||||||
assigneeIds, err = options.AssigneeIds(apiClient, repo)
|
assigneeIds, err = options.AssigneeIds(apiClient, repo)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
|
|
||||||
|
|
@ -61,11 +61,13 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// When ActorReviewers is true, we use login-based mutation and don't need to resolve reviewer IDs.
|
// TODO ApiActorsSupported
|
||||||
needReviewerIDs := len(tb.Reviewers) > 0 && !tb.ActorReviewers
|
// 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.
|
// TODO ApiActorsSupported
|
||||||
needAssigneeIDs := len(tb.Assignees) > 0 && !tb.ActorAssignees
|
// 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.
|
// Retrieve minimal information needed to resolve metadata if this was not previously cached from additional metadata survey.
|
||||||
if tb.MetadataResult == nil {
|
if tb.MetadataResult == nil {
|
||||||
|
|
@ -88,9 +90,10 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
|
||||||
tb.MetadataResult = metadataResult
|
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.
|
// ReplaceActorsForAssignable mutation. The ID-based else branch is for GHES compatibility.
|
||||||
if tb.ActorAssignees {
|
if tb.ApiActorsSupported {
|
||||||
params["assigneeLogins"] = tb.Assignees
|
params["assigneeLogins"] = tb.Assignees
|
||||||
} else {
|
} else {
|
||||||
assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees)
|
assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees)
|
||||||
|
|
@ -138,11 +141,11 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO requestReviewsByLoginCleanup
|
// TODO ApiActorsSupported
|
||||||
// When ActorReviewers is true (github.com), pass logins directly for use with
|
// When ApiActorsSupported is true (github.com), pass logins directly for use with
|
||||||
// RequestReviewsByLogin mutation. The ID-based else branch can be removed once
|
// RequestReviewsByLogin mutation. The ID-based else branch can be removed once
|
||||||
// GHES supports requestReviewsByLogin.
|
// GHES supports requestReviewsByLogin.
|
||||||
if tb.ActorReviewers {
|
if tb.ApiActorsSupported {
|
||||||
params["userReviewerLogins"] = userReviewers
|
params["userReviewerLogins"] = userReviewers
|
||||||
if len(botReviewers) > 0 {
|
if len(botReviewers) > 0 {
|
||||||
params["botReviewerLogins"] = botReviewers
|
params["botReviewerLogins"] = botReviewers
|
||||||
|
|
|
||||||
|
|
@ -18,9 +18,14 @@ const (
|
||||||
type IssueMetadataState struct {
|
type IssueMetadataState struct {
|
||||||
Type metadataStateType
|
Type metadataStateType
|
||||||
|
|
||||||
Draft bool
|
Draft bool
|
||||||
ActorAssignees bool
|
|
||||||
ActorReviewers 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
|
Body string
|
||||||
Title string
|
Title string
|
||||||
|
|
|
||||||
|
|
@ -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.
|
// 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
|
// When search-based selection is available, skip the expensive assignable-users
|
||||||
// and teams fetch since reviewers are found dynamically via the search function.
|
// and teams fetch since they are found dynamically via the search function.
|
||||||
useReviewerSearch := state.ActorReviewers && reviewerSearchFunc != nil
|
// TODO ApiActorsSupported
|
||||||
useAssigneeSearch := state.ActorAssignees && assigneeSearchFunc != nil
|
useReviewerSearch := state.ApiActorsSupported && reviewerSearchFunc != nil
|
||||||
|
useAssigneeSearch := state.ApiActorsSupported && assigneeSearchFunc != nil
|
||||||
metadataInput := api.RepoMetadataInput{
|
metadataInput := api.RepoMetadataInput{
|
||||||
Reviewers: isChosen("Reviewers") && !useReviewerSearch,
|
Reviewers: isChosen("Reviewers") && !useReviewerSearch,
|
||||||
TeamReviewers: isChosen("Reviewers") && !useReviewerSearch,
|
TeamReviewers: isChosen("Reviewers") && !useReviewerSearch,
|
||||||
Assignees: isChosen("Assignees") && !useAssigneeSearch,
|
Assignees: isChosen("Assignees") && !useAssigneeSearch,
|
||||||
ActorAssignees: isChosen("Assignees") && !useAssigneeSearch && state.ActorAssignees,
|
ApiActorsSupported: state.ApiActorsSupported && ((isChosen("Assignees") && !useAssigneeSearch) || (isChosen("Reviewers") && !useReviewerSearch)),
|
||||||
Labels: isChosen("Labels"),
|
Labels: isChosen("Labels"),
|
||||||
ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported,
|
ProjectsV1: isChosen("Projects") && projectsV1Support == gh.ProjectsV1Supported,
|
||||||
ProjectsV2: isChosen("Projects"),
|
ProjectsV2: isChosen("Projects"),
|
||||||
Milestones: isChosen("Milestone"),
|
Milestones: isChosen("Milestone"),
|
||||||
}
|
}
|
||||||
metadataResult, err := fetcher.RepoMetadataFetch(metadataInput)
|
metadataResult, err := fetcher.RepoMetadataFetch(metadataInput)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -217,7 +218,8 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface
|
||||||
var assignees []string
|
var assignees []string
|
||||||
var assigneesDefault []string
|
var assigneesDefault []string
|
||||||
if !useAssigneeSearch {
|
if !useAssigneeSearch {
|
||||||
if state.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if state.ApiActorsSupported {
|
||||||
for _, u := range metadataResult.AssignableActors {
|
for _, u := range metadataResult.AssignableActors {
|
||||||
assignees = append(assignees, u.DisplayName())
|
assignees = append(assignees, u.DisplayName())
|
||||||
|
|
||||||
|
|
@ -305,7 +307,8 @@ func MetadataSurvey(p Prompt, io *iostreams.IOStreams, baseRepo ghrepo.Interface
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
for _, i := range selected {
|
for _, i := range selected {
|
||||||
if state.ActorAssignees {
|
// TODO ApiActorsSupported
|
||||||
|
if state.ApiActorsSupported {
|
||||||
values.Assignees = append(values.Assignees, metadataResult.AssignableActors[i].Login())
|
values.Assignees = append(values.Assignees, metadataResult.AssignableActors[i].Login())
|
||||||
} else {
|
} else {
|
||||||
values.Assignees = append(values.Assignees, metadataResult.AssignableUsers[i].Login())
|
values.Assignees = append(values.Assignees, metadataResult.AssignableUsers[i].Login())
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue