From eb7397695f3c64909cd9550832150800a3b8d92e Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Tue, 12 May 2026 20:14:45 -0600 Subject: [PATCH] Apply deferred update mutations in parallel for gh issue create The post-creation Issues 2.0 mutations (issue type, parent, blocked-by, blocking) ran sequentially in three separate apply* helpers. Replace them with a single call to api.DeferredUpdateIssue, which fans the mutations out in parallel and joins their errors. The new newCreateDeferredOpts helper resolves the user-supplied refs to node IDs (re-using the cached opts.issueTypeID from the interactive prompt) and hands them to the orchestrator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/issue/create/create.go | 84 +++++++++-------------- pkg/cmd/issue/create/create_test.go | 101 ++++++++++++++++------------ 2 files changed, 89 insertions(+), 96 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 4b57cdd05..fb507cd5c 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -412,19 +412,12 @@ func createRun(opts *CreateOptions) (err error) { return } - // Post-creation mutations for Issues 2.0 fields - err = applyIssueType(apiClient, baseRepo, newIssue, opts) + var updateOpts api.DeferredUpdateIssueOptions + updateOpts, err = deferredUpdateIssueOptions(apiClient, baseRepo, newIssue, opts) if err != nil { return } - - err = applyParent(apiClient, baseRepo, newIssue, opts) - if err != nil { - return - } - - err = applyRelationships(apiClient, baseRepo, newIssue, opts) - if err != nil { + if err = api.DeferredUpdateIssue(apiClient, updateOpts); err != nil { return } @@ -441,63 +434,50 @@ func generatePreviewURL(apiClient *api.Client, baseRepo ghrepo.Interface, tb prS return prShared.WithPrAndIssueQueryParams(apiClient, baseRepo, openURL, tb, projectsV1Support) } -// applyIssueType resolves the --type flag and sets the issue type via mutation. -func applyIssueType(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, opts *CreateOptions) error { - if opts.IssueType == "" { - return nil +// deferredUpdateIssueOptions resolves the user-supplied --type / --parent / +// --blocked-by / --blocking flags into the IDs that DeferredUpdateIssue +// expects. +func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, opts *CreateOptions) (api.DeferredUpdateIssueOptions, error) { + updateOpts := api.DeferredUpdateIssueOptions{ + IssueID: issue.ID, + Hostname: baseRepo.RepoHost(), } - // Use pre-resolved ID from interactive flow if available - typeID := opts.issueTypeID - if typeID == "" { - var err error - typeID, err = issueShared.ResolveIssueTypeName(client, baseRepo, opts.IssueType) - if err != nil { - return err + if opts.IssueType != "" { + typeID := opts.issueTypeID + if typeID == "" { + var err error + typeID, err = issueShared.ResolveIssueTypeName(client, baseRepo, opts.IssueType) + if err != nil { + return updateOpts, err + } } + updateOpts.IssueTypeID = typeID } - return api.UpdateIssueIssueType(client, baseRepo.RepoHost(), issue.ID, typeID) -} - -// applyParent resolves the --parent flag and adds the issue as a sub-issue. -func applyParent(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, opts *CreateOptions) error { - if opts.Parent == "" { - return nil + if opts.Parent != "" { + parentID, err := issueShared.ResolveIssueRef(client, baseRepo, opts.Parent) + if err != nil { + return updateOpts, fmt.Errorf("resolving --parent reference %q: %w", opts.Parent, err) + } + updateOpts.ParentID = parentID } - parentID, err := issueShared.ResolveIssueRef(client, baseRepo, opts.Parent) - if err != nil { - return fmt.Errorf("resolving parent: %w", err) - } - - return api.AddSubIssue(client, baseRepo.RepoHost(), parentID, issue.ID, false) -} - -// applyRelationships resolves the --blocked-by and --blocking flags and creates relationships. -func applyRelationships(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, opts *CreateOptions) error { - hostname := baseRepo.RepoHost() - for _, ref := range opts.BlockedBy { - blockingID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) + id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return fmt.Errorf("resolving --blocked-by reference %q: %w", ref, err) - } - if err := api.AddBlockedBy(client, hostname, issue.ID, blockingID); err != nil { - return err + return updateOpts, fmt.Errorf("resolving --blocked-by reference %q: %w", ref, err) } + updateOpts.AddBlockedByIDs = append(updateOpts.AddBlockedByIDs, id) } for _, ref := range opts.Blocking { - // --blocking swaps the args: the OTHER issue is blocked by THIS issue - blockedID, err := issueShared.ResolveIssueRef(client, baseRepo, ref) + id, err := issueShared.ResolveIssueRef(client, baseRepo, ref) if err != nil { - return fmt.Errorf("resolving --blocking reference %q: %w", ref, err) - } - if err := api.AddBlockedBy(client, hostname, blockedID, issue.ID); err != nil { - return err + return updateOpts, fmt.Errorf("resolving --blocking reference %q: %w", ref, err) } + updateOpts.AddBlockingIDs = append(updateOpts.AddBlockingIDs, id) } - return nil + return updateOpts, nil } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 603bb7350..bd39dfd9b 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -885,58 +885,44 @@ func Test_createRun(t *testing.T) { "id": "ISSUE_ID_123", "URL": "https://github.com/OWNER/REPO/issues/123" } } } }`)) - // IssueNodeID + AddBlockedBy for --blocked-by 200 + // IssueNodeID lookups for each ref, routed by number so they + // don't depend on parallel ordering. r.Register( - httpmock.GraphQL(`query IssueNodeID\b`), - httpmock.StringResponse(` - { "data": { "repository": { "issue": { "id": "BLOCKER_ID_200" } } } }`)) + issueNodeIDByNumberMatcher(200), + httpmock.StringResponse(`{ "data": { "repository": { "issue": { "id": "BLOCKER_ID_200" } } } }`)) r.Register( - httpmock.GraphQL(`mutation AddBlockedBy\b`), - httpmock.GraphQLMutation(` - { "data": { "addBlockedBy": { "issue": { "id": "ISSUE_ID_123" } } } }`, - func(inputs map[string]interface{}) { - assert.Equal(t, "ISSUE_ID_123", inputs["issueId"]) - assert.Equal(t, "BLOCKER_ID_200", inputs["blockingIssueId"]) - })) - // IssueNodeID + AddBlockedBy for --blocked-by 201 + issueNodeIDByNumberMatcher(201), + httpmock.StringResponse(`{ "data": { "repository": { "issue": { "id": "BLOCKER_ID_201" } } } }`)) r.Register( - httpmock.GraphQL(`query IssueNodeID\b`), - httpmock.StringResponse(` - { "data": { "repository": { "issue": { "id": "BLOCKER_ID_201" } } } }`)) + issueNodeIDByNumberMatcher(300), + httpmock.StringResponse(`{ "data": { "repository": { "issue": { "id": "BLOCKED_ID_300" } } } }`)) r.Register( - httpmock.GraphQL(`mutation AddBlockedBy\b`), - httpmock.GraphQLMutation(` - { "data": { "addBlockedBy": { "issue": { "id": "ISSUE_ID_123" } } } }`, - func(inputs map[string]interface{}) { - assert.Equal(t, "ISSUE_ID_123", inputs["issueId"]) - assert.Equal(t, "BLOCKER_ID_201", inputs["blockingIssueId"]) - })) - // IssueNodeID + AddBlockedBy for --blocking 300 (swapped args) + issueNodeIDByNumberMatcher(301), + httpmock.StringResponse(`{ "data": { "repository": { "issue": { "id": "BLOCKED_ID_301" } } } }`)) + // AddBlockedBy mutations, routed by their inputs so they + // also don't depend on parallel ordering. + // --blocked-by N: this issue is blocked by N r.Register( - httpmock.GraphQL(`query IssueNodeID\b`), - httpmock.StringResponse(` - { "data": { "repository": { "issue": { "id": "BLOCKED_ID_300" } } } }`)) + httpmock.GraphQLMutationMatcher(`mutation AddBlockedBy\b`, func(input map[string]interface{}) bool { + return input["issueId"] == "ISSUE_ID_123" && input["blockingIssueId"] == "BLOCKER_ID_200" + }), + httpmock.StringResponse(`{ "data": { "addBlockedBy": { "issue": { "id": "ISSUE_ID_123" } } } }`)) r.Register( - httpmock.GraphQL(`mutation AddBlockedBy\b`), - httpmock.GraphQLMutation(` - { "data": { "addBlockedBy": { "issue": { "id": "BLOCKED_ID_300" } } } }`, - func(inputs map[string]interface{}) { - assert.Equal(t, "BLOCKED_ID_300", inputs["issueId"]) - assert.Equal(t, "ISSUE_ID_123", inputs["blockingIssueId"]) - })) - // IssueNodeID + AddBlockedBy for --blocking 301 (swapped args) + httpmock.GraphQLMutationMatcher(`mutation AddBlockedBy\b`, func(input map[string]interface{}) bool { + return input["issueId"] == "ISSUE_ID_123" && input["blockingIssueId"] == "BLOCKER_ID_201" + }), + httpmock.StringResponse(`{ "data": { "addBlockedBy": { "issue": { "id": "ISSUE_ID_123" } } } }`)) + // --blocking N: N is blocked by this issue (args swapped) r.Register( - httpmock.GraphQL(`query IssueNodeID\b`), - httpmock.StringResponse(` - { "data": { "repository": { "issue": { "id": "BLOCKED_ID_301" } } } }`)) + httpmock.GraphQLMutationMatcher(`mutation AddBlockedBy\b`, func(input map[string]interface{}) bool { + return input["issueId"] == "BLOCKED_ID_300" && input["blockingIssueId"] == "ISSUE_ID_123" + }), + httpmock.StringResponse(`{ "data": { "addBlockedBy": { "issue": { "id": "BLOCKED_ID_300" } } } }`)) r.Register( - httpmock.GraphQL(`mutation AddBlockedBy\b`), - httpmock.GraphQLMutation(` - { "data": { "addBlockedBy": { "issue": { "id": "BLOCKED_ID_301" } } } }`, - func(inputs map[string]interface{}) { - assert.Equal(t, "BLOCKED_ID_301", inputs["issueId"]) - assert.Equal(t, "ISSUE_ID_123", inputs["blockingIssueId"]) - })) + httpmock.GraphQLMutationMatcher(`mutation AddBlockedBy\b`, func(input map[string]interface{}) bool { + return input["issueId"] == "BLOCKED_ID_301" && input["blockingIssueId"] == "ISSUE_ID_123" + }), + httpmock.StringResponse(`{ "data": { "addBlockedBy": { "issue": { "id": "BLOCKED_ID_301" } } } }`)) }, wantsStdout: "https://github.com/OWNER/REPO/issues/123\n", wantsStderr: "\nCreating issue in OWNER/REPO\n\n", @@ -1716,3 +1702,30 @@ func TestProjectsV1Deprecation(t *testing.T) { }) }) } + +// issueNodeIDByNumberMatcher matches an IssueNodeID GraphQL query whose +// number variable equals the given value. Used by tests that issue +// multiple IssueNodeID lookups and need stubs to route by issue number +// rather than by registration order. +func issueNodeIDByNumberMatcher(number int) httpmock.Matcher { + queryMatcher := httpmock.GraphQL(`query IssueNodeID\b`) + return func(req *http.Request) bool { + if !queryMatcher(req) { + return false + } + body, err := io.ReadAll(req.Body) + if err != nil { + return false + } + req.Body = io.NopCloser(bytes.NewReader(body)) + var b struct { + Variables struct { + Number int `json:"number"` + } `json:"variables"` + } + if err := json.Unmarshal(body, &b); err != nil { + return false + } + return b.Variables.Number == number + } +}