From 0c48bd09819719de7907d580df6e002f636e2f20 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 6 May 2026 17:12:29 -0600 Subject: [PATCH] Cache issue-type name->ID map and use it in issue edit prShared.FetchOptions already fetches RepoIssueTypes when IssueType is edited, but only kept the names. editRun then called RepoIssueTypes a second time via ResolveIssueTypeName. Have FetchOptions store the name->ID map on Editable, and look the ID up directly in editRun. The lookup now also lives inside the per-issue loop, which fixes a bug where the interactive type prompt's chosen value was set after the upfront resolve, sending an empty issueTypeId on the mutation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/issue/create/create.go | 4 +- pkg/cmd/issue/edit/edit.go | 22 ++++--- pkg/cmd/issue/edit/edit_test.go | 102 +++++++++++++++++++++++++++++--- pkg/cmd/pr/shared/editable.go | 5 ++ 4 files changed, 113 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index cfb98b5f5..09643d4b7 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -52,8 +52,8 @@ type CreateOptions struct { IssueType string issueTypeID string // resolved during interactive flow to avoid double API call Parent string - BlockedBy []string - Blocking []string + BlockedBy []string + Blocking []string } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 13a5adaef..2236d2a83 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "sort" + "strings" "sync" "time" @@ -328,15 +329,6 @@ func editRun(opts *EditOptions) error { opts.IO.StartProgressIndicatorWithLabel(fmt.Sprintf("Updating %d issues", len(issues))) } - // Resolve issue type ID once for all issues to avoid redundant API calls. - var issueTypeID string - if editable.IssueType.Edited && editable.IssueType.Value != "" { - issueTypeID, err = issueShared.ResolveIssueTypeName(apiClient, baseRepo, editable.IssueType.Value) - if err != nil { - return err - } - } - for _, issue := range issues { // Copy variables to capture in the go routine below. editable := editable.Clone() @@ -381,6 +373,18 @@ func editRun(opts *EditOptions) error { } } + // Look up the issue type ID using the map populated by FetchOptions + var issueTypeID string + if editable.IssueType.Edited && editable.IssueType.Value != "" { + id, ok := editable.IssueTypeNameToID[editable.IssueType.Value] + if !ok { + return fmt.Errorf("type %q not found; available types: %s", + editable.IssueType.Value, + strings.Join(editable.IssueType.Options, ", ")) + } + issueTypeID = id + } + g.Add(1) go func(issue *api.Issue) { defer g.Done() diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index 7dcca6206..345af7fd3 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -340,7 +340,7 @@ func TestNewCmdEdit(t *testing.T) { name: "remove-sub-issue flag", input: "23 --remove-sub-issue 50", output: EditOptions{ - IssueNumbers: []int{23}, + IssueNumbers: []int{23}, RemoveSubIssues: []string{"50"}, }, }, @@ -356,7 +356,7 @@ func TestNewCmdEdit(t *testing.T) { name: "remove-blocked-by flag", input: "23 --remove-blocked-by 201", output: EditOptions{ - IssueNumbers: []int{23}, + IssueNumbers: []int{23}, RemoveBlockedBy: []string{"201"}, }, }, @@ -365,7 +365,7 @@ func TestNewCmdEdit(t *testing.T) { input: "23 --add-blocking 300,301", output: EditOptions{ IssueNumbers: []int{23}, - AddBlocking: []string{"300", "301"}, + AddBlocking: []string{"300", "301"}, }, }, { @@ -835,9 +835,7 @@ func Test_editRun(t *testing.T) { Edited: true, }, }, - FetchOptions: func(_ *api.Client, _ ghrepo.Interface, _ *prShared.Editable, _ gh.ProjectsV1Support) error { - return nil - }, + FetchOptions: prShared.FetchOptions, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { mockIssueGet(t, reg) @@ -862,6 +860,94 @@ func Test_editRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/issue/123\n", }, + { + name: "interactive edit type prompt", + input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, + IssueNumbers: []int{123}, + Interactive: true, + FieldsToEditSurvey: func(_ prShared.EditPrompter, eo *prShared.Editable) error { + // Verify the survey is allowed to offer Type as an option for issue edit. + assert.True(t, eo.IssueType.Allowed) + eo.IssueType.Edited = true + return nil + }, + EditFieldsSurvey: func(_ prShared.EditPrompter, eo *prShared.Editable, _ string) error { + // FetchOptions populated Options and IssueTypeNameToID from + // the RepositoryIssueTypes stub below. + assert.Equal(t, []string{"Bug", "Feature"}, eo.IssueType.Options) + assert.Equal(t, "FEATURE_TYPE_ID", eo.IssueTypeNameToID["Feature"]) + eo.IssueType.Value = "Feature" + return nil + }, + FetchOptions: prShared.FetchOptions, + DetermineEditor: func() (string, error) { return "vim", nil }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockIssueGet(t, reg) + reg.Register( + httpmock.GraphQL(`query RepositoryIssueTypes\b`), + httpmock.StringResponse(` + { "data": { "repository": { "issueTypes": { "nodes": [ + { "id": "BUG_TYPE_ID", "name": "Bug", "description": "", "color": "" }, + { "id": "FEATURE_TYPE_ID", "name": "Feature", "description": "", "color": "" } + ] } } } } + `), + ) + reg.Register( + httpmock.GraphQL(`mutation UpdateIssueIssueType\b`), + httpmock.GraphQLMutation(` + { "data": { "updateIssueIssueType": { "issue": { "id": "123" } } } }`, + func(inputs map[string]interface{}) { + assert.Equal(t, "123", inputs["issueId"]) + assert.Equal(t, "FEATURE_TYPE_ID", inputs["issueTypeId"]) + }), + ) + }, + stdout: "https://github.com/OWNER/REPO/issue/123\n", + }, + { + name: "interactive edit parent prompt", + input: &EditOptions{ + Detector: &fd.EnabledDetectorMock{}, + IssueNumbers: []int{123}, + Interactive: true, + FieldsToEditSurvey: func(_ prShared.EditPrompter, eo *prShared.Editable) error { + // Verify the survey is allowed to offer Parent as an option for issue edit. + assert.True(t, eo.Parent.Allowed) + eo.Parent.Edited = true + return nil + }, + EditFieldsSurvey: func(_ prShared.EditPrompter, eo *prShared.Editable, _ string) error { + eo.Parent.Value = "100" + return nil + }, + FetchOptions: func(_ *api.Client, _ ghrepo.Interface, _ *prShared.Editable, _ gh.ProjectsV1Support) error { + return nil + }, + DetermineEditor: func() (string, error) { return "vim", nil }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockIssueGet(t, reg) + reg.Register( + httpmock.GraphQL(`query IssueNodeID\b`), + httpmock.StringResponse(` + { "data": { "repository": { "issue": { "id": "PARENT_100_ID" } } } } + `), + ) + reg.Register( + httpmock.GraphQL(`mutation AddSubIssue\b`), + httpmock.GraphQLMutation(` + { "data": { "addSubIssue": { "issue": { "id": "PARENT_100_ID" } } } }`, + func(inputs map[string]interface{}) { + assert.Equal(t, "PARENT_100_ID", inputs["issueId"]) + assert.Equal(t, "123", inputs["subIssueId"]) + assert.Equal(t, true, inputs["replaceParent"]) + }), + ) + }, + stdout: "https://github.com/OWNER/REPO/issue/123\n", + }, { name: "edit set parent", input: &EditOptions{ @@ -1164,9 +1250,7 @@ func Test_editRun(t *testing.T) { Edited: true, }, }, - FetchOptions: func(_ *api.Client, _ ghrepo.Interface, _ *prShared.Editable, _ gh.ProjectsV1Support) error { - return nil - }, + FetchOptions: prShared.FetchOptions, }, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( diff --git a/pkg/cmd/pr/shared/editable.go b/pkg/cmd/pr/shared/editable.go index 326636d66..2b6bbe15c 100644 --- a/pkg/cmd/pr/shared/editable.go +++ b/pkg/cmd/pr/shared/editable.go @@ -22,6 +22,7 @@ type Editable struct { Projects EditableProjects Milestone EditableString IssueType EditableString + IssueTypeNameToID map[string]string Parent EditableString Metadata api.RepoMetadataResult @@ -296,6 +297,7 @@ func (e *Editable) Clone() Editable { Projects: e.Projects.clone(), Milestone: e.Milestone.clone(), IssueType: e.IssueType.clone(), + IssueTypeNameToID: e.IssueTypeNameToID, Parent: e.Parent.clone(), ApiActorsSupported: e.ApiActorsSupported, // Shallow copy since no mutation. @@ -634,10 +636,13 @@ func FetchOptions(client *api.Client, repo ghrepo.Interface, editable *Editable, issueTypes, err := api.RepoIssueTypes(client, repo) if err == nil { typeNames := make([]string, len(issueTypes)) + ids := make(map[string]string, len(issueTypes)) for i, t := range issueTypes { typeNames[i] = t.Name + ids[t.Name] = t.ID } editable.IssueType.Options = typeNames + editable.IssueTypeNameToID = ids } }