Skip to content

Commit b38f677

Browse files
gunadhyagunadhya
committed
Fix issue develop repeated invocation with named branches
1 parent a2b8b68 commit b38f677

2 files changed

Lines changed: 274 additions & 13 deletions

File tree

‎pkg/cmd/issue/develop/develop.go‎

Lines changed: 85 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
ctx "context"
55
"fmt"
66
"net/http"
7+
"net/url"
8+
"strings"
79

810
"github.com/MakeNowJust/heredoc"
911
"github.com/cli/cli/v2/api"
@@ -174,7 +176,6 @@ func developRun(opts *DevelopOptions) error {
174176

175177
func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghrepo.Interface, issue *api.Issue) error {
176178
branchRepo := issueRepo
177-
var repoID string
178179
if opts.BranchRepo != "" {
179180
var err error
180181
branchRepo, err = ghrepo.FromFullName(opts.BranchRepo)
@@ -183,24 +184,66 @@ func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghr
183184
}
184185
}
185186

186-
opts.IO.StartProgressIndicator()
187-
repoID, branchID, err := api.FindRepoBranchID(apiClient, branchRepo, opts.BaseBranch)
188-
opts.IO.StopProgressIndicator()
189-
if err != nil {
190-
return err
187+
branchName := ""
188+
reusedExisting := false
189+
if opts.Name != "" {
190+
opts.IO.StartProgressIndicator()
191+
branches, err := api.ListLinkedBranches(apiClient, issueRepo, issue.Number)
192+
opts.IO.StopProgressIndicator()
193+
if err != nil {
194+
return err
195+
}
196+
branchName = findExistingLinkedBranchName(branches, branchRepo, opts.Name)
197+
reusedExisting = branchName != ""
191198
}
192199

193-
opts.IO.StartProgressIndicator()
194-
branchName, err := api.CreateLinkedBranch(apiClient, branchRepo.RepoHost(), repoID, issue.ID, branchID, opts.Name)
195-
opts.IO.StopProgressIndicator()
196-
if err != nil {
197-
return err
200+
repoID := ""
201+
branchID := ""
202+
baseValidated := false
203+
if opts.BaseBranch != "" {
204+
opts.IO.StartProgressIndicator()
205+
foundRepoID, foundBranchID, err := api.FindRepoBranchID(apiClient, branchRepo, opts.BaseBranch)
206+
opts.IO.StopProgressIndicator()
207+
if err != nil {
208+
return err
209+
}
210+
repoID = foundRepoID
211+
branchID = foundBranchID
212+
baseValidated = true
213+
}
214+
215+
if branchName == "" {
216+
if !baseValidated {
217+
opts.IO.StartProgressIndicator()
218+
foundRepoID, foundBranchID, err := api.FindRepoBranchID(apiClient, branchRepo, opts.BaseBranch)
219+
opts.IO.StopProgressIndicator()
220+
if err != nil {
221+
return err
222+
}
223+
repoID = foundRepoID
224+
branchID = foundBranchID
225+
}
226+
227+
opts.IO.StartProgressIndicator()
228+
createdBranchName, err := api.CreateLinkedBranch(apiClient, branchRepo.RepoHost(), repoID, issue.ID, branchID, opts.Name)
229+
opts.IO.StopProgressIndicator()
230+
if err != nil {
231+
return err
232+
}
233+
branchName = createdBranchName
234+
}
235+
236+
if branchName == "" {
237+
return fmt.Errorf("failed to create linked branch: API returned empty branch name")
238+
}
239+
240+
if reusedExisting && opts.IO.IsStdoutTTY() {
241+
fmt.Fprintf(opts.IO.ErrOut, "Using existing linked branch %q\n", branchName)
198242
}
199243

200244
// Remember which branch to target when creating a PR.
201245
if opts.BaseBranch != "" {
202-
err = opts.GitClient.SetBranchConfig(ctx.Background(), branchName, git.MergeBaseConfig, opts.BaseBranch)
203-
if err != nil {
246+
if err := opts.GitClient.SetBranchConfig(ctx.Background(), branchName, git.MergeBaseConfig, opts.BaseBranch); err != nil {
204247
return err
205248
}
206249
}
@@ -210,6 +253,35 @@ func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghr
210253
return checkoutBranch(opts, branchRepo, branchName)
211254
}
212255

256+
func findExistingLinkedBranchName(branches []api.LinkedBranch, branchRepo ghrepo.Interface, branchName string) string {
257+
for _, branch := range branches {
258+
if branch.BranchName != branchName {
259+
continue
260+
}
261+
linkedRepo, err := linkedBranchRepoFromURL(branch.URL)
262+
if err != nil {
263+
continue
264+
}
265+
if ghrepo.IsSame(linkedRepo, branchRepo) {
266+
return branch.BranchName
267+
}
268+
}
269+
return ""
270+
}
271+
272+
func linkedBranchRepoFromURL(branchURL string) (ghrepo.Interface, error) {
273+
u, err := url.Parse(branchURL)
274+
if err != nil {
275+
return nil, err
276+
}
277+
pathParts := strings.SplitN(strings.Trim(u.Path, "/"), "/", 3)
278+
if len(pathParts) < 2 {
279+
return nil, fmt.Errorf("invalid linked branch URL: %q", branchURL)
280+
}
281+
u.Path = "/" + strings.Join(pathParts[0:2], "/")
282+
return ghrepo.FromURL(u)
283+
}
284+
213285
func developRunList(opts *DevelopOptions, apiClient *api.Client, issueRepo ghrepo.Interface, issue *api.Issue) error {
214286
opts.IO.StartProgressIndicator()
215287
branches, err := api.ListLinkedBranches(apiClient, issueRepo, issue.Number)

‎pkg/cmd/issue/develop/develop_test.go‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,16 @@ func TestDevelopRun(t *testing.T) {
353353
reg.Register(
354354
httpmock.GraphQL(`query FindRepoBranchID\b`),
355355
httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`))
356+
reg.Register(
357+
httpmock.GraphQL(`query ListLinkedBranches\b`),
358+
httpmock.GraphQLQuery(`
359+
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}}
360+
`, func(query string, inputs map[string]interface{}) {
361+
assert.Equal(t, float64(123), inputs["number"])
362+
assert.Equal(t, "OWNER", inputs["owner"])
363+
assert.Equal(t, "REPO", inputs["name"])
364+
}),
365+
)
356366
reg.Register(
357367
httpmock.GraphQL(`mutation CreateLinkedBranch\b`),
358368
httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":"my-branch"}}}}}`,
@@ -370,6 +380,165 @@ func TestDevelopRun(t *testing.T) {
370380
},
371381
expectedOut: "github.com/OWNER/REPO/tree/my-branch\n",
372382
},
383+
{
384+
name: "develop existing linked branch with name and checkout",
385+
opts: &DevelopOptions{
386+
Name: "my-branch",
387+
BaseBranch: "main",
388+
IssueNumber: 123,
389+
Checkout: true,
390+
},
391+
remotes: map[string]string{
392+
"origin": "OWNER/REPO",
393+
},
394+
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
395+
reg.Register(
396+
httpmock.GraphQL(`query LinkedBranchFeature\b`),
397+
httpmock.StringResponse(featureEnabledPayload),
398+
)
399+
reg.Register(
400+
httpmock.GraphQL(`query IssueByNumber\b`),
401+
httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`),
402+
)
403+
reg.Register(
404+
httpmock.GraphQL(`query ListLinkedBranches\b`),
405+
httpmock.GraphQLQuery(`
406+
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[{"ref":{"name":"my-branch","repository":{"url":"https://github.com/OWNER/REPO"}}}]}}}}}
407+
`, func(query string, inputs map[string]interface{}) {
408+
assert.Equal(t, float64(123), inputs["number"])
409+
assert.Equal(t, "OWNER", inputs["owner"])
410+
assert.Equal(t, "REPO", inputs["name"])
411+
}),
412+
)
413+
reg.Register(
414+
httpmock.GraphQL(`query FindRepoBranchID\b`),
415+
httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`))
416+
},
417+
runStubs: func(cs *run.CommandStubber) {
418+
cs.Register(`git config branch\.my-branch\.gh-merge-base main`, 0, "")
419+
cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "")
420+
cs.Register(`git rev-parse --verify refs/heads/my-branch`, 0, "")
421+
cs.Register(`git checkout my-branch`, 0, "")
422+
cs.Register(`git pull --ff-only origin my-branch`, 0, "")
423+
},
424+
expectedOut: "github.com/OWNER/REPO/tree/my-branch\n",
425+
},
426+
{
427+
name: "develop existing linked branch with name in tty shows reuse message",
428+
opts: &DevelopOptions{
429+
Name: "my-branch",
430+
BaseBranch: "main",
431+
IssueNumber: 123,
432+
},
433+
tty: true,
434+
remotes: map[string]string{
435+
"origin": "OWNER/REPO",
436+
},
437+
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
438+
reg.Register(
439+
httpmock.GraphQL(`query LinkedBranchFeature\b`),
440+
httpmock.StringResponse(featureEnabledPayload),
441+
)
442+
reg.Register(
443+
httpmock.GraphQL(`query IssueByNumber\b`),
444+
httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`),
445+
)
446+
reg.Register(
447+
httpmock.GraphQL(`query ListLinkedBranches\b`),
448+
httpmock.GraphQLQuery(`
449+
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[{"ref":{"name":"my-branch","repository":{"url":"https://github.com/OWNER/REPO"}}}]}}}}}
450+
`, func(query string, inputs map[string]interface{}) {
451+
assert.Equal(t, float64(123), inputs["number"])
452+
assert.Equal(t, "OWNER", inputs["owner"])
453+
assert.Equal(t, "REPO", inputs["name"])
454+
}),
455+
)
456+
reg.Register(
457+
httpmock.GraphQL(`query FindRepoBranchID\b`),
458+
httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`))
459+
},
460+
runStubs: func(cs *run.CommandStubber) {
461+
cs.Register(`git config branch\.my-branch\.gh-merge-base main`, 0, "")
462+
cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "")
463+
},
464+
expectedOut: "github.com/OWNER/REPO/tree/my-branch\n",
465+
expectedErrOut: "Using existing linked branch \"my-branch\"\n",
466+
},
467+
{
468+
name: "develop existing linked branch with invalid base branch returns an error",
469+
opts: &DevelopOptions{
470+
Name: "my-branch",
471+
BaseBranch: "does-not-exist-branch",
472+
IssueNumber: 123,
473+
},
474+
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
475+
reg.Register(
476+
httpmock.GraphQL(`query LinkedBranchFeature\b`),
477+
httpmock.StringResponse(featureEnabledPayload),
478+
)
479+
reg.Register(
480+
httpmock.GraphQL(`query IssueByNumber\b`),
481+
httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`),
482+
)
483+
reg.Register(
484+
httpmock.GraphQL(`query ListLinkedBranches\b`),
485+
httpmock.GraphQLQuery(`
486+
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[{"ref":{"name":"my-branch","repository":{"url":"https://github.com/OWNER/REPO"}}}]}}}}}
487+
`, func(query string, inputs map[string]interface{}) {
488+
assert.Equal(t, float64(123), inputs["number"])
489+
assert.Equal(t, "OWNER", inputs["owner"])
490+
assert.Equal(t, "REPO", inputs["name"])
491+
}),
492+
)
493+
reg.Register(
494+
httpmock.GraphQL(`query FindRepoBranchID\b`),
495+
httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","defaultBranchRef":{"target":{"oid":"DEFAULTOID"}},"ref":null}}}`),
496+
)
497+
},
498+
wantErr: `could not find branch "does-not-exist-branch" in OWNER/REPO`,
499+
},
500+
{
501+
name: "develop with empty linked branch name response returns an error",
502+
opts: &DevelopOptions{
503+
Name: "my-branch",
504+
BaseBranch: "main",
505+
IssueNumber: 123,
506+
},
507+
httpStubs: func(reg *httpmock.Registry, t *testing.T) {
508+
reg.Register(
509+
httpmock.GraphQL(`query LinkedBranchFeature\b`),
510+
httpmock.StringResponse(featureEnabledPayload),
511+
)
512+
reg.Register(
513+
httpmock.GraphQL(`query IssueByNumber\b`),
514+
httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`),
515+
)
516+
reg.Register(
517+
httpmock.GraphQL(`query ListLinkedBranches\b`),
518+
httpmock.GraphQLQuery(`
519+
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}}
520+
`, func(query string, inputs map[string]interface{}) {
521+
assert.Equal(t, float64(123), inputs["number"])
522+
assert.Equal(t, "OWNER", inputs["owner"])
523+
assert.Equal(t, "REPO", inputs["name"])
524+
}),
525+
)
526+
reg.Register(
527+
httpmock.GraphQL(`query FindRepoBranchID\b`),
528+
httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`))
529+
reg.Register(
530+
httpmock.GraphQL(`mutation CreateLinkedBranch\b`),
531+
httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":""}}}}}`,
532+
func(inputs map[string]interface{}) {
533+
assert.Equal(t, "REPOID", inputs["repositoryId"])
534+
assert.Equal(t, "SOMEID", inputs["issueId"])
535+
assert.Equal(t, "OID", inputs["oid"])
536+
assert.Equal(t, "my-branch", inputs["name"])
537+
}),
538+
)
539+
},
540+
wantErr: "failed to create linked branch: API returned empty branch name",
541+
},
373542
{
374543
name: "develop new branch outside of local git repo",
375544
opts: &DevelopOptions{
@@ -426,6 +595,16 @@ func TestDevelopRun(t *testing.T) {
426595
httpmock.GraphQL(`query FindRepoBranchID\b`),
427596
httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`),
428597
)
598+
reg.Register(
599+
httpmock.GraphQL(`query ListLinkedBranches\b`),
600+
httpmock.GraphQLQuery(`
601+
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}}
602+
`, func(query string, inputs map[string]interface{}) {
603+
assert.Equal(t, float64(123), inputs["number"])
604+
assert.Equal(t, "OWNER", inputs["owner"])
605+
assert.Equal(t, "REPO", inputs["name"])
606+
}),
607+
)
429608
reg.Register(
430609
httpmock.GraphQL(`mutation CreateLinkedBranch\b`),
431610
httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":"my-branch"}}}}}`,
@@ -468,6 +647,16 @@ func TestDevelopRun(t *testing.T) {
468647
httpmock.GraphQL(`query FindRepoBranchID\b`),
469648
httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`),
470649
)
650+
reg.Register(
651+
httpmock.GraphQL(`query ListLinkedBranches\b`),
652+
httpmock.GraphQLQuery(`
653+
{"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}}
654+
`, func(query string, inputs map[string]interface{}) {
655+
assert.Equal(t, float64(123), inputs["number"])
656+
assert.Equal(t, "OWNER", inputs["owner"])
657+
assert.Equal(t, "REPO", inputs["name"])
658+
}),
659+
)
471660
reg.Register(
472661
httpmock.GraphQL(`mutation CreateLinkedBranch\b`),
473662
httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":"my-branch"}}}}}`,

0 commit comments

Comments
 (0)