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>
This commit is contained in:
Kynan Ware
parent
5c783a229b
commit
eb7397695f
2 changed files with 89 additions and 96 deletions
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue