From ddb2fce07b0fbf73f38c43a3ba0cf65fc3fcef08 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Sun, 29 Mar 2026 17:06:25 -0600 Subject: [PATCH] refactor: extract shared helpers for issue ref resolution and type lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address code review findings: - Extract resolveIssueRef into shared.ResolveIssueRef (was duplicated between create.go and edit.go) - Extract issue type nameā†’ID resolution into shared.ResolveIssueTypeName (was duplicated between create applyIssueTypes and edit applyEditIssueType) - Fix double import of issue/shared in view.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/issue/create/create.go | 37 +++------------------------ pkg/cmd/issue/edit/edit.go | 41 ++++++------------------------ pkg/cmd/issue/shared/resolve.go | 44 +++++++++++++++++++++++++++++++++ pkg/cmd/issue/view/view.go | 3 +-- 4 files changed, 57 insertions(+), 68 deletions(-) create mode 100644 pkg/cmd/issue/shared/resolve.go diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index a8b4f5074..b0ee6634b 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net/http" - "strings" "time" "github.com/MakeNowJust/heredoc" @@ -452,24 +451,11 @@ func applyIssueTypes(client *api.Client, baseRepo ghrepo.Interface, issue *api.I return nil } - issueTypes, err := api.RepoIssueTypes(client, baseRepo) + issueTypeID, err := issueShared.ResolveIssueTypeName(client, baseRepo, opts.IssueType) if err != nil { return err } - issueTypeID := "" - typeNames := make([]string, len(issueTypes)) - for i, t := range issueTypes { - typeNames[i] = t.Name - if strings.EqualFold(t.Name, opts.IssueType) { - issueTypeID = t.ID - } - } - - if issueTypeID == "" { - return fmt.Errorf("type %q not found; available types: %s", opts.IssueType, strings.Join(typeNames, ", ")) - } - return api.UpdateIssueIssueType(client, baseRepo.RepoHost(), issue.ID, issueTypeID) } @@ -479,7 +465,7 @@ func applyParent(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue return nil } - parentID, err := resolveIssueRef(client, baseRepo, opts.Parent) + parentID, err := issueShared.ResolveIssueRef(client, baseRepo, opts.Parent) if err != nil { return fmt.Errorf("resolving parent: %w", err) } @@ -492,7 +478,7 @@ func applyRelationships(client *api.Client, baseRepo ghrepo.Interface, issue *ap hostname := baseRepo.RepoHost() for _, ref := range opts.BlockedBy { - blockingID, err := resolveIssueRef(client, baseRepo, ref) + blockingID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { return fmt.Errorf("resolving --blocked-by reference %q: %w", ref, err) } @@ -503,7 +489,7 @@ func applyRelationships(client *api.Client, baseRepo ghrepo.Interface, issue *ap for _, ref := range opts.Blocking { // --blocking swaps the args: the OTHER issue is blocked by THIS issue - blockedID, err := resolveIssueRef(client, baseRepo, ref) + blockedID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { return fmt.Errorf("resolving --blocking reference %q: %w", ref, err) } @@ -514,18 +500,3 @@ func applyRelationships(client *api.Client, baseRepo ghrepo.Interface, issue *ap return nil } - -// resolveIssueRef parses an issue reference (number or URL) and returns its node ID. -func resolveIssueRef(client *api.Client, baseRepo ghrepo.Interface, ref string) (string, error) { - number, repo, err := issueShared.ParseIssueFromArg(ref) - if err != nil { - return "", err - } - - targetRepo := baseRepo - if r, ok := repo.Value(); ok { - targetRepo = r - } - - return api.IssueNodeID(client, targetRepo, number) -} diff --git a/pkg/cmd/issue/edit/edit.go b/pkg/cmd/issue/edit/edit.go index 9e92f9624..6051ff479 100644 --- a/pkg/cmd/issue/edit/edit.go +++ b/pkg/cmd/issue/edit/edit.go @@ -4,7 +4,6 @@ import ( "fmt" "net/http" "sort" - "strings" "sync" "time" @@ -448,20 +447,11 @@ func editRun(opts *EditOptions) error { } func applyEditIssueType(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, typeName string) error { - issueTypes, err := api.RepoIssueTypes(client, baseRepo) + issueTypeID, err := issueShared.ResolveIssueTypeName(client, baseRepo, typeName) if err != nil { return err } - - typeNames := make([]string, len(issueTypes)) - for i, t := range issueTypes { - typeNames[i] = t.Name - if strings.EqualFold(t.Name, typeName) { - return api.UpdateIssueIssueType(client, baseRepo.RepoHost(), issue.ID, t.ID) - } - } - - return fmt.Errorf("type %q not found; available types: %s", typeName, strings.Join(typeNames, ", ")) + return api.UpdateIssueIssueType(client, baseRepo.RepoHost(), issue.ID, issueTypeID) } func applyEditParent(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, parentRef string) error { @@ -488,7 +478,7 @@ func applyEditParent(client *api.Client, baseRepo ghrepo.Interface, issue *api.I } // Set parent with replaceParent=true - parentID, err := resolveIssueRef(client, baseRepo, parentRef) + parentID, err := issueShared.ResolveIssueRef(client, baseRepo, parentRef) if err != nil { return fmt.Errorf("resolving parent: %w", err) } @@ -499,7 +489,7 @@ func applyEditSubIssues(client *api.Client, baseRepo ghrepo.Interface, issue *ap hostname := baseRepo.RepoHost() for _, ref := range opts.AddSubIssues { - subID, err := resolveIssueRef(client, baseRepo, ref) + subID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { return fmt.Errorf("resolving --add-sub-issue reference %q: %w", ref, err) } @@ -509,7 +499,7 @@ func applyEditSubIssues(client *api.Client, baseRepo ghrepo.Interface, issue *ap } for _, ref := range opts.RemoveSubIssues { - subID, err := resolveIssueRef(client, baseRepo, ref) + subID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { return fmt.Errorf("resolving --remove-sub-issue reference %q: %w", ref, err) } @@ -535,7 +525,7 @@ func applyEditRelationships(client *api.Client, baseRepo ghrepo.Interface, issue hostname := baseRepo.RepoHost() for _, ref := range opts.AddBlockedBy { - blockingID, err := resolveIssueRef(client, baseRepo, ref) + blockingID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { return fmt.Errorf("resolving --add-blocked-by reference %q: %w", ref, err) } @@ -545,7 +535,7 @@ func applyEditRelationships(client *api.Client, baseRepo ghrepo.Interface, issue } for _, ref := range opts.RemoveBlockedBy { - blockingID, err := resolveIssueRef(client, baseRepo, ref) + blockingID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { return fmt.Errorf("resolving --remove-blocked-by reference %q: %w", ref, err) } @@ -556,7 +546,7 @@ func applyEditRelationships(client *api.Client, baseRepo ghrepo.Interface, issue for _, ref := range opts.AddBlocking { // --add-blocking swaps args: the OTHER issue is blocked by THIS issue - blockedID, err := resolveIssueRef(client, baseRepo, ref) + blockedID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { return fmt.Errorf("resolving --add-blocking reference %q: %w", ref, err) } @@ -567,18 +557,3 @@ func applyEditRelationships(client *api.Client, baseRepo ghrepo.Interface, issue return nil } - -// resolveIssueRef parses an issue reference (number or URL) and returns its node ID. -func resolveIssueRef(client *api.Client, baseRepo ghrepo.Interface, ref string) (string, error) { - number, repo, err := issueShared.ParseIssueFromArg(ref) - if err != nil { - return "", err - } - - targetRepo := baseRepo - if r, ok := repo.Value(); ok { - targetRepo = r - } - - return api.IssueNodeID(client, targetRepo, number) -} diff --git a/pkg/cmd/issue/shared/resolve.go b/pkg/cmd/issue/shared/resolve.go new file mode 100644 index 000000000..291dc6393 --- /dev/null +++ b/pkg/cmd/issue/shared/resolve.go @@ -0,0 +1,44 @@ +package shared + +import ( + "fmt" + "strings" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghrepo" +) + +// ResolveIssueRef parses an issue reference (number or URL) and returns its node ID. +func ResolveIssueRef(client *api.Client, baseRepo ghrepo.Interface, ref string) (string, error) { + number, repo, err := ParseIssueFromArg(ref) + if err != nil { + return "", err + } + + targetRepo := baseRepo + if r, ok := repo.Value(); ok { + targetRepo = r + } + + return api.IssueNodeID(client, targetRepo, number) +} + +// ResolveIssueTypeName resolves an issue type name to its ID by fetching +// available types from the repository. Returns an error listing available +// types if the name is not found. +func ResolveIssueTypeName(client *api.Client, repo ghrepo.Interface, typeName string) (string, error) { + issueTypes, err := api.RepoIssueTypes(client, repo) + if err != nil { + return "", err + } + + typeNames := make([]string, len(issueTypes)) + for i, t := range issueTypes { + typeNames[i] = t.Name + if strings.EqualFold(t.Name, typeName) { + return t.ID, nil + } + } + + return "", fmt.Errorf("type %q not found; available types: %s", typeName, strings.Join(typeNames, ", ")) +} diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 993940c4f..a0d3b2aa3 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -15,7 +15,6 @@ import ( "github.com/cli/cli/v2/internal/gh" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/text" - "github.com/cli/cli/v2/pkg/cmd/issue/shared" issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" "github.com/cli/cli/v2/pkg/cmdutil" @@ -58,7 +57,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman `, "`"), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - issueNumber, baseRepo, err := shared.ParseIssueFromArg(args[0]) + issueNumber, baseRepo, err := issueShared.ParseIssueFromArg(args[0]) if err != nil { return err }