From 711591c92b3fe2b270a3ae9b0ff16aeab0f71073 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Fri, 26 Aug 2022 15:25:19 -0400 Subject: [PATCH 01/26] WIP --- pkg/cmd/issue/develop/develop.go | 68 ++++++++++++++++++++++++++++++++ pkg/cmd/issue/issue.go | 2 + 2 files changed, 70 insertions(+) create mode 100644 pkg/cmd/issue/develop/develop.go diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go new file mode 100644 index 000000000..e4fae8066 --- /dev/null +++ b/pkg/cmd/issue/develop/develop.go @@ -0,0 +1,68 @@ +package develop + +import ( + "fmt" + "net/http" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/internal/browser" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type DevelopOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Browser browser.Browser + + IssueRepo string + Name string + BaseBranch string + Checkout bool +} + +func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra.Command { + opts := &DevelopOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + Browser: f.Browser, + BaseRepo: f.BaseRepo, + } + + cmd := &cobra.Command{ + Use: "develop", + Short: "Manage branches for an issue", + Example: heredoc.Doc(` + $ gh issue develop --list 123 # list branches for issue 123 + $ gh issue develop --issue-repo "github/cli" 123 list branches for issue 123 in repo "github/cli" + $ gh issue develop 123 --name "my-branch" --head main + `), + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + if runF != nil { + return runF(opts) + } + return developRun(opts) + }, + } + fl := cmd.Flags() + fl.StringVarP(&opts.Name, "name", "n", "", "Name of the branch to create") + fl.StringVarP(&opts.BaseBranch, "base-branch", "b", "", "Name of the base branch") + fl.StringVarP(&opts.IssueRepo, "issue-repo", "i", "", "Name of the issue's repository") + return cmd +} + +func developRun(opts *DevelopOptions) (err error) { + // httpClient, err := opts.HttpClient() + // if err != nil { + // return err + // } + fmt.Fprintf(opts.IO.ErrOut, "hello world") + return +} diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go index 136f58d06..67e9eed12 100644 --- a/pkg/cmd/issue/issue.go +++ b/pkg/cmd/issue/issue.go @@ -6,6 +6,7 @@ import ( cmdComment "github.com/cli/cli/v2/pkg/cmd/issue/comment" cmdCreate "github.com/cli/cli/v2/pkg/cmd/issue/create" cmdDelete "github.com/cli/cli/v2/pkg/cmd/issue/delete" + cmdDevelop "github.com/cli/cli/v2/pkg/cmd/issue/develop" cmdEdit "github.com/cli/cli/v2/pkg/cmd/issue/edit" cmdList "github.com/cli/cli/v2/pkg/cmd/issue/list" cmdReopen "github.com/cli/cli/v2/pkg/cmd/issue/reopen" @@ -48,6 +49,7 @@ func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil)) cmd.AddCommand(cmdEdit.NewCmdEdit(f, nil)) cmd.AddCommand(cmdTransfer.NewCmdTransfer(f, nil)) + cmd.AddCommand(cmdDevelop.NewCmdDevelop(f, nil)) return cmd } From 152b8610019a50d18226178e87781019a4314e93 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Thu, 1 Sep 2022 16:36:48 -0400 Subject: [PATCH 02/26] WIP --- api/queries_branch_issue_reference.go | 93 +++++++++++++++++++++++++++ pkg/cmd/issue/develop/develop.go | 61 +++++++++++++++--- 2 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 api/queries_branch_issue_reference.go diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go new file mode 100644 index 000000000..bb168bb74 --- /dev/null +++ b/api/queries_branch_issue_reference.go @@ -0,0 +1,93 @@ +package api + +type BranchIssueReference struct { + ID int + BranchName string + BranchRepositoryId int + IssueId int + IssueRepositoryId int +} + +func CreateBranchIssueReference(client *Client, repo *Repository, params map[string]interface{}) (*BranchIssueReference, error) { + query := ` + mutation BranchIssueReferenceCreate($input: CreateBranchIssueReferenceInput!) { + mutation($issueId: ID!, $oid: GitObjectID!, $name: String, $repositoryId: ID) { + createLinkedBranch(input: { + issueId: $issueId, + name: $name, + oid: $oid + repositoryId: $repositoryId + }) { + linkedBranch { + ref { + name + repository { + name + } + } + } + } + } + }` + + inputParams := map[string]interface{}{ + "repositoryId": repo.ID, + } + for key, val := range params { + switch key { + case "issueId", "name", "oid": + inputParams[key] = val + } + } + variables := map[string]interface{}{ + "input": inputParams, + } + + result := struct { + createLinkedBranch struct { + BranchIssueReference BranchIssueReference + } + }{} + + err := client.GraphQL(repo.RepoHost(), query, variables, &result) + if err != nil { + return nil, err + } + ref := &result.createLinkedBranch.BranchIssueReference + return ref, nil + +} + +func FindBaseOid(client *Client, repo *Repository, ref string) (string, error) { + query := ` + query BranchIssueReferenceFindBaseOid($repositoryId: ID!, $ref: String!) { + repository(id: $repositoryId) { + ref(qualifiedName: $ref) { + target { + oid + } + } + } + }` + + variables := map[string]interface{}{ + "repositoryId": repo.ID, + "ref": ref, + } + + result := struct { + Repository struct { + Ref struct { + Target struct { + Oid string + } + } + } + }{} + + err := client.GraphQL(repo.RepoHost(), query, variables, &result) + if err != nil { + return "", err + } + return result.Repository.Ref.Target.Oid, nil +} diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index e4fae8066..96fdb4a0c 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -5,9 +5,11 @@ import ( "net/http" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" @@ -20,10 +22,11 @@ type DevelopOptions struct { BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser - IssueRepo string - Name string - BaseBranch string - Checkout bool + IssueRepo string + IssueNumber string + Name string + BaseBranch string + Checkout bool } func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra.Command { @@ -59,10 +62,50 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. } func developRun(opts *DevelopOptions) (err error) { - // httpClient, err := opts.HttpClient() - // if err != nil { - // return err - // } - fmt.Fprintf(opts.IO.ErrOut, "hello world") + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + opts.IO.StartProgressIndicator() + fmt.Fprintf(opts.IO.ErrOut, "running") + repo, err := api.GitHubRepo(apiClient, baseRepo) + fmt.Fprintf(opts.IO.ErrOut, "found your repo %s\n", repo.Name) + if err != nil { + return err + } + + oid, err := api.FindBaseOid(apiClient, repo, opts.BaseBranch) + if err != nil { + return err + } + + fmt.Fprintf(opts.IO.ErrOut, "found oid for %s", oid) + // get the id of the issue repo + issue, _, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueNumber, []string{"id", "number", "title", "state"}) + if err != nil { + return err + } + + // get the oid of the branch from the base repo + params := map[string]interface{}{ + "issueId": issue.ID, + "name": opts.Name, + "oid": oid, + "repositoryId": repo.ID, + } + + ref, err := api.CreateBranchIssueReference(apiClient, repo, params) + opts.IO.StopProgressIndicator() + if ref != nil { + fmt.Fprintf(opts.IO.Out, "Created %s\n", ref.BranchName) + } + if err != nil { + return err + } return } From 0e1bae7be04339f41b20a77f718e9a4323478cd7 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Tue, 6 Sep 2022 17:04:37 -0400 Subject: [PATCH 03/26] WIP --- api/queries_branch_issue_reference.go | 25 ++++++++++++++++++------- pkg/cmd/issue/develop/develop.go | 20 ++++++++++++-------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index bb168bb74..c14f4c800 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -58,10 +58,15 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } -func FindBaseOid(client *Client, repo *Repository, ref string) (string, error) { +func FindBaseOid(client *Client, repo *Repository, ref string) (string, string, error) { query := ` - query BranchIssueReferenceFindBaseOid($repositoryId: ID!, $ref: String!) { - repository(id: $repositoryId) { + query BranchIssueReferenceFindBaseOid($repositoryName: String!, $repositoryOwner: String!, $ref: String!) { + repository(name: $repositoryName, owner: $repositoryOwner) { + defaultBranchRef { + target { + oid + } + } ref(qualifiedName: $ref) { target { oid @@ -71,12 +76,18 @@ func FindBaseOid(client *Client, repo *Repository, ref string) (string, error) { }` variables := map[string]interface{}{ - "repositoryId": repo.ID, - "ref": ref, + "repositoryName": repo.Name, + "repositoryOwner": repo.RepoOwner(), + "ref": ref, } result := struct { Repository struct { + DefaultBranchRef struct { + Target struct { + Oid string + } + } Ref struct { Target struct { Oid string @@ -87,7 +98,7 @@ func FindBaseOid(client *Client, repo *Repository, ref string) (string, error) { err := client.GraphQL(repo.RepoHost(), query, variables, &result) if err != nil { - return "", err + return "", "", err } - return result.Repository.Ref.Target.Oid, nil + return result.Repository.Ref.Target.Oid, result.Repository.DefaultBranchRef.Target.Oid, nil } diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 96fdb4a0c..95be27218 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -22,11 +22,11 @@ type DevelopOptions struct { BaseRepo func() (ghrepo.Interface, error) Browser browser.Browser - IssueRepo string - IssueNumber string - Name string - BaseBranch string - Checkout bool + IssueRepo string + IssueSelector string + Name string + BaseBranch string + Checkout bool } func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra.Command { @@ -46,11 +46,12 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. $ gh issue develop --issue-repo "github/cli" 123 list branches for issue 123 in repo "github/cli" $ gh issue develop 123 --name "my-branch" --head main `), - Args: cmdutil.NoArgsQuoteReminder, + Args: cmdutil.ExactArgs(1, "issue number is required"), RunE: func(cmd *cobra.Command, args []string) error { if runF != nil { return runF(opts) } + opts.IssueSelector = args[0] return developRun(opts) }, } @@ -62,15 +63,18 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. } func developRun(opts *DevelopOptions) (err error) { + fmt.Printf("starting\n") httpClient, err := opts.HttpClient() if err != nil { return err } + fmt.Fprintf(opts.IO.ErrOut, "got the http client\n") apiClient := api.NewClientFromHTTP(httpClient) baseRepo, err := opts.BaseRepo() if err != nil { return err } + fmt.Fprintf(opts.IO.ErrOut, "got the baseRepo") opts.IO.StartProgressIndicator() fmt.Fprintf(opts.IO.ErrOut, "running") repo, err := api.GitHubRepo(apiClient, baseRepo) @@ -79,12 +83,12 @@ func developRun(opts *DevelopOptions) (err error) { return err } - oid, err := api.FindBaseOid(apiClient, repo, opts.BaseBranch) + oid, default_branch_oid, err := api.FindBaseOid(apiClient, repo, opts.BaseBranch) if err != nil { return err } - fmt.Fprintf(opts.IO.ErrOut, "found oid for %s", oid) + fmt.Fprintf(opts.IO.ErrOut, "found %s for ref %s, and found default branch oid %s\n", oid, opts.BaseBranch, default_branch_oid) // get the id of the issue repo issue, _, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueNumber, []string{"id", "number", "title", "state"}) if err != nil { From 7c7896859c9f96d0bb50a0235481657fa7def7ad Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Thu, 8 Sep 2022 10:02:00 -0400 Subject: [PATCH 04/26] Add flag to features list --- api/client.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/client.go b/api/client.go index 1bf861f9a..bf318dad9 100644 --- a/api/client.go +++ b/api/client.go @@ -19,7 +19,7 @@ const ( authorization = "Authorization" cacheTTL = "X-GH-CACHE-TTL" graphqlFeatures = "GraphQL-Features" - mergeQueue = "merge_queue" + features = "merge_queue,branch_for_issue_api" userAgent = "User-Agent" ) @@ -55,7 +55,7 @@ func (err HTTPError) ScopesSuggestion() string { // GraphQLError will be returned, but the data will also be parsed into the receiver. func (c Client) GraphQL(hostname string, query string, variables map[string]interface{}, data interface{}) error { opts := clientOptions(hostname, c.http.Transport) - opts.Headers[graphqlFeatures] = mergeQueue + opts.Headers[graphqlFeatures] = features gqlClient, err := gh.GQLClient(&opts) if err != nil { return err @@ -67,7 +67,7 @@ func (c Client) GraphQL(hostname string, query string, variables map[string]inte // GraphQLError will be returned, but the data will also be parsed into the receiver. func (c Client) Mutate(hostname, name string, mutation interface{}, variables map[string]interface{}) error { opts := clientOptions(hostname, c.http.Transport) - opts.Headers[graphqlFeatures] = mergeQueue + opts.Headers[graphqlFeatures] = features gqlClient, err := gh.GQLClient(&opts) if err != nil { return err @@ -79,7 +79,7 @@ func (c Client) Mutate(hostname, name string, mutation interface{}, variables ma // GraphQLError will be returned, but the data will also be parsed into the receiver. func (c Client) Query(hostname, name string, query interface{}, variables map[string]interface{}) error { opts := clientOptions(hostname, c.http.Transport) - opts.Headers[graphqlFeatures] = mergeQueue + opts.Headers[graphqlFeatures] = features gqlClient, err := gh.GQLClient(&opts) if err != nil { return err From 5f3f00f90943cbb9d371eb09c3b2b2e4999c53d3 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Thu, 8 Sep 2022 10:02:18 -0400 Subject: [PATCH 05/26] WIP --- api/queries_branch_issue_reference.go | 22 ++++++++-------------- api/queries_pr.go | 5 +---- pkg/cmd/issue/develop/develop.go | 12 +++++------- pkg/cmd/issue/develop/develop_test.go | 7 +++++++ 4 files changed, 21 insertions(+), 25 deletions(-) create mode 100644 pkg/cmd/issue/develop/develop_test.go diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index c14f4c800..2dd9cffcc 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -1,5 +1,7 @@ package api +import "fmt" + type BranchIssueReference struct { ID int BranchName string @@ -10,25 +12,19 @@ type BranchIssueReference struct { func CreateBranchIssueReference(client *Client, repo *Repository, params map[string]interface{}) (*BranchIssueReference, error) { query := ` - mutation BranchIssueReferenceCreate($input: CreateBranchIssueReferenceInput!) { - mutation($issueId: ID!, $oid: GitObjectID!, $name: String, $repositoryId: ID) { + mutation CreateLinkedBranch($issueId: ID!, $oid: GitObjectID!, $name: String, $repositoryId: ID) { createLinkedBranch(input: { issueId: $issueId, name: $name, - oid: $oid + oid: $oid, repositoryId: $repositoryId }) { linkedBranch { - ref { - name - repository { - name - } - } + id } } } - }` + ` inputParams := map[string]interface{}{ "repositoryId": repo.ID, @@ -39,9 +35,6 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str inputParams[key] = val } } - variables := map[string]interface{}{ - "input": inputParams, - } result := struct { createLinkedBranch struct { @@ -49,7 +42,8 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } }{} - err := client.GraphQL(repo.RepoHost(), query, variables, &result) + fmt.Printf("query variables %#v", inputParams) + err := client.GraphQL(repo.RepoHost(), query, inputParams, &result) if err != nil { return nil, err } diff --git a/api/queries_pr.go b/api/queries_pr.go index 18217a583..99f054b28 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -295,10 +295,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter inputParams[key] = val } } - variables := map[string]interface{}{ - "input": inputParams, - } - + variables := inputParams result := struct { CreatePullRequest struct { PullRequest PullRequest diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 95be27218..26c832ab5 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -63,22 +63,17 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. } func developRun(opts *DevelopOptions) (err error) { - fmt.Printf("starting\n") httpClient, err := opts.HttpClient() if err != nil { return err } - fmt.Fprintf(opts.IO.ErrOut, "got the http client\n") apiClient := api.NewClientFromHTTP(httpClient) baseRepo, err := opts.BaseRepo() if err != nil { return err } - fmt.Fprintf(opts.IO.ErrOut, "got the baseRepo") opts.IO.StartProgressIndicator() - fmt.Fprintf(opts.IO.ErrOut, "running") repo, err := api.GitHubRepo(apiClient, baseRepo) - fmt.Fprintf(opts.IO.ErrOut, "found your repo %s\n", repo.Name) if err != nil { return err } @@ -88,9 +83,12 @@ func developRun(opts *DevelopOptions) (err error) { return err } - fmt.Fprintf(opts.IO.ErrOut, "found %s for ref %s, and found default branch oid %s\n", oid, opts.BaseBranch, default_branch_oid) + if oid == "" { + oid = default_branch_oid + } + // get the id of the issue repo - issue, _, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueNumber, []string{"id", "number", "title", "state"}) + issue, _, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number", "title"}) if err != nil { return err } diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go new file mode 100644 index 000000000..41f3cbd37 --- /dev/null +++ b/pkg/cmd/issue/develop/develop_test.go @@ -0,0 +1,7 @@ +package develop + +import "testing" + +func TestNewCmdDevelop(t *testing.T) { + +} From 5734df806b5dadbe89304165e932cb1cb59dec62 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Thu, 8 Sep 2022 12:18:37 -0400 Subject: [PATCH 06/26] FIll in all the missing stuff in the test setup --- pkg/cmd/issue/develop/develop.go | 5 -- pkg/cmd/issue/develop/develop_test.go | 88 ++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 26c832ab5..a9073ffa0 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -6,7 +6,6 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/internal/browser" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" @@ -20,7 +19,6 @@ type DevelopOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) - Browser browser.Browser IssueRepo string IssueSelector string @@ -34,7 +32,6 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. IO: f.IOStreams, HttpClient: f.HttpClient, Config: f.Config, - Browser: f.Browser, BaseRepo: f.BaseRepo, } @@ -72,7 +69,6 @@ func developRun(opts *DevelopOptions) (err error) { if err != nil { return err } - opts.IO.StartProgressIndicator() repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err @@ -102,7 +98,6 @@ func developRun(opts *DevelopOptions) (err error) { } ref, err := api.CreateBranchIssueReference(apiClient, repo, params) - opts.IO.StopProgressIndicator() if ref != nil { fmt.Fprintf(opts.IO.Out, "Created %s\n", ref.BranchName) } diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 41f3cbd37..5967cedac 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -1,7 +1,91 @@ package develop -import "testing" +import ( + "net/http" + "testing" -func TestNewCmdDevelop(t *testing.T) { + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/run" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/prompt" + "github.com/stretchr/testify/assert" +) +func Test_developRun(t *testing.T) { + tests := []struct { + name string + setup func(*DevelopOptions, *testing.T) func() + cmdStubs func(*run.CommandStubber) + askStubs func(*prompt.AskStubber) // TODO eventually migrate to PrompterMock + httpStubs func(*httpmock.Registry, *testing.T) + expectedOut string + expectedErrOut string + expectedBrowse string + wantErr string + tty bool + }{ + {name: "develop new branch", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.Name = "my-branch" + opts.BaseBranch = "main" + opts.IssueSelector = "123" + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.StubRepoResponse("OWNER", "REPO") + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ + "hasIssuesEnabled": true, + "issue":{"id":1, "number":123, "title":"my issue"}, + }}}`)) + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceFindBaseOid\b`), + httpmock.StringResponse(`{"data":{"repository":{"ref":{"target":{"oid":"123"}}}}}`)) + + reg.Register( + httpmock.GraphQL(`mutation CreateLinkedBranch\b`), + httpmock.GraphQLMutation(` + { "data": { "createLinkedBranch": { "linkedBranch": 1 } } }`, + func(inputs map[string]interface{}) { + }), + ) + + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.httpStubs != nil { + tt.httpStubs(reg, t) + } + + opts := DevelopOptions{} + + opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + opts.Config = func() (config.Config, error) { + return config.NewBlankConfig(), nil + } + cleanSetup := func() {} + if tt.setup != nil { + cleanSetup = tt.setup(&opts, t) + } + defer cleanSetup() + + err := developRun(&opts) + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + assert.NoError(t, err) + } + }) + } } From 2e98e8a4a7b3944ff45929590977353cac9491f8 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Thu, 8 Sep 2022 17:17:06 -0400 Subject: [PATCH 07/26] Get that test mostly working --- api/queries_branch_issue_reference.go | 19 +++++++++++---- pkg/cmd/issue/develop/develop_test.go | 34 ++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index 2dd9cffcc..a3681421d 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -21,6 +21,9 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str }) { linkedBranch { id + ref { + name + } } } } @@ -38,17 +41,25 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str result := struct { createLinkedBranch struct { - BranchIssueReference BranchIssueReference + LinkedBranch struct { + ID int + Ref struct { + Name string + } + } } }{} - fmt.Printf("query variables %#v", inputParams) err := client.GraphQL(repo.RepoHost(), query, inputParams, &result) if err != nil { return nil, err } - ref := &result.createLinkedBranch.BranchIssueReference - return ref, nil + fmt.Printf("result %#v", &result) + ref := BranchIssueReference{ + ID: result.createLinkedBranch.LinkedBranch.ID, + BranchName: result.createLinkedBranch.LinkedBranch.Ref.Name, + } + return &ref, nil } diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 5967cedac..9ab2a15c5 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -8,7 +8,9 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/prompt" + "github.com/cli/cli/v2/test" "github.com/stretchr/testify/assert" ) @@ -34,25 +36,32 @@ func Test_developRun(t *testing.T) { }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { reg.StubRepoResponse("OWNER", "REPO") + + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`), + ) reg.Register( httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(`{"data":{"repository":{ - "hasIssuesEnabled": true, - "issue":{"id":1, "number":123, "title":"my issue"}, - }}}`)) + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled": true, "issue":{"id": "yar", "number":123, "title":"my issue"} }}}`)) reg.Register( httpmock.GraphQL(`query BranchIssueReferenceFindBaseOid\b`), httpmock.StringResponse(`{"data":{"repository":{"ref":{"target":{"oid":"123"}}}}}`)) reg.Register( httpmock.GraphQL(`mutation CreateLinkedBranch\b`), - httpmock.GraphQLMutation(` - { "data": { "createLinkedBranch": { "linkedBranch": 1 } } }`, + httpmock.GraphQLMutation(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": 2, "ref": {"name": "my-branch"} } } } }`, func(inputs map[string]interface{}) { + }), ) }, + expectedOut: "Created my-branch\n", }, } for _, tt := range tests { @@ -65,6 +74,13 @@ func Test_developRun(t *testing.T) { opts := DevelopOptions{} + ios, _, stdout, stderr := iostreams.Test() + + ios.SetStdoutTTY(tt.tty) + ios.SetStdinTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + opts.IO = ios + opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil } @@ -81,10 +97,16 @@ func Test_developRun(t *testing.T) { defer cleanSetup() err := developRun(&opts) + output := &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + } if tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) } else { assert.NoError(t, err) + assert.Equal(t, tt.expectedOut, output.String()) + assert.Equal(t, tt.expectedErrOut, output.Stderr()) } }) } From e1b83496fae41d5e5998b42f26ffb024f56b29df Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Fri, 9 Sep 2022 10:17:35 -0400 Subject: [PATCH 08/26] Make sure struct fields are public --- api/queries_branch_issue_reference.go | 10 ++++------ pkg/cmd/issue/develop/develop_test.go | 5 +++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index a3681421d..d4ca43233 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -1,7 +1,5 @@ package api -import "fmt" - type BranchIssueReference struct { ID int BranchName string @@ -40,7 +38,7 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } result := struct { - createLinkedBranch struct { + CreateLinkedBranch struct { LinkedBranch struct { ID int Ref struct { @@ -54,10 +52,10 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str if err != nil { return nil, err } - fmt.Printf("result %#v", &result) + ref := BranchIssueReference{ - ID: result.createLinkedBranch.LinkedBranch.ID, - BranchName: result.createLinkedBranch.LinkedBranch.Ref.Name, + ID: result.CreateLinkedBranch.LinkedBranch.ID, + BranchName: result.CreateLinkedBranch.LinkedBranch.Ref.Name, } return &ref, nil diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 9ab2a15c5..5a86a0a7f 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -35,7 +35,6 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { - reg.StubRepoResponse("OWNER", "REPO") reg.Register( httpmock.GraphQL(`query RepositoryInfo\b`), @@ -56,7 +55,9 @@ func Test_developRun(t *testing.T) { httpmock.GraphQL(`mutation CreateLinkedBranch\b`), httpmock.GraphQLMutation(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": 2, "ref": {"name": "my-branch"} } } } }`, func(inputs map[string]interface{}) { - + assert.Equal(t, "REPOID", inputs["repositoryId"]) + assert.Equal(t, "my-branch", inputs["name"]) + assert.Equal(t, "yar", inputs["issueId"]) }), ) From 6248e62611cba5649a4bf362dc5e2931f95d18e4 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Mon, 12 Sep 2022 11:28:49 -0400 Subject: [PATCH 09/26] Use GraphQLQuery test helper GraphQLMutation expects a single `input` variable --- api/queries_branch_issue_reference.go | 9 +++------ pkg/cmd/issue/develop/develop_test.go | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index d4ca43233..a05b88476 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -1,11 +1,8 @@ package api type BranchIssueReference struct { - ID int - BranchName string - BranchRepositoryId int - IssueId int - IssueRepositoryId int + ID string + BranchName string } func CreateBranchIssueReference(client *Client, repo *Repository, params map[string]interface{}) (*BranchIssueReference, error) { @@ -40,7 +37,7 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str result := struct { CreateLinkedBranch struct { LinkedBranch struct { - ID int + ID string Ref struct { Name string } diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 5a86a0a7f..4b9eb85f1 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -53,8 +53,8 @@ func Test_developRun(t *testing.T) { reg.Register( httpmock.GraphQL(`mutation CreateLinkedBranch\b`), - httpmock.GraphQLMutation(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": 2, "ref": {"name": "my-branch"} } } } }`, - func(inputs map[string]interface{}) { + httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": 2, "ref": {"name": "my-branch"} } } } }`, + func(query string, inputs map[string]interface{}) { assert.Equal(t, "REPOID", inputs["repositoryId"]) assert.Equal(t, "my-branch", inputs["name"]) assert.Equal(t, "yar", inputs["issueId"]) From 8cf8a16e6a48dc8378f7b30cdcb12dfca7950f5f Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Mon, 12 Sep 2022 17:20:24 -0400 Subject: [PATCH 10/26] WIP add --checkout option --- pkg/cmd/issue/develop/develop.go | 62 +++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index a9073ffa0..49b5c1874 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -6,8 +6,11 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -19,6 +22,7 @@ type DevelopOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Remotes func() (context.Remotes, error) IssueRepo string IssueSelector string @@ -33,15 +37,17 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. HttpClient: f.HttpClient, Config: f.Config, BaseRepo: f.BaseRepo, + Remotes: f.Remotes, } cmd := &cobra.Command{ - Use: "develop", - Short: "Manage branches for an issue", + Use: "develop [flags] { | }", + Short: "Manage linked branches for an issue", Example: heredoc.Doc(` $ gh issue develop --list 123 # list branches for issue 123 $ gh issue develop --issue-repo "github/cli" 123 list branches for issue 123 in repo "github/cli" $ gh issue develop 123 --name "my-branch" --head main + $ gh issue develop 123 --checkout # checkout the branch for issue 123 after creating it `), Args: cmdutil.ExactArgs(1, "issue number is required"), RunE: func(cmd *cobra.Command, args []string) error { @@ -53,9 +59,10 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. }, } fl := cmd.Flags() - fl.StringVarP(&opts.Name, "name", "n", "", "Name of the branch to create") fl.StringVarP(&opts.BaseBranch, "base-branch", "b", "", "Name of the base branch") - fl.StringVarP(&opts.IssueRepo, "issue-repo", "i", "", "Name of the issue's repository") + fl.BoolVarP(&opts.Checkout, "checkout", "c", false, "Checkout the branch after creating it") + fl.StringVarP(&opts.IssueRepo, "issue-repo", "i", "", "Name or URL of the issue's repository") + fl.StringVarP(&opts.Name, "name", "n", "", "Name of the branch to create") return cmd } @@ -69,6 +76,7 @@ func developRun(opts *DevelopOptions) (err error) { if err != nil { return err } + opts.IO.StartProgressIndicator() repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err @@ -98,11 +106,57 @@ func developRun(opts *DevelopOptions) (err error) { } ref, err := api.CreateBranchIssueReference(apiClient, repo, params) + opts.IO.StopProgressIndicator() if ref != nil { fmt.Fprintf(opts.IO.Out, "Created %s\n", ref.BranchName) + + if opts.Checkout { + return checkoutBranch(opts, baseRepo, ref.BranchName) + } } if err != nil { return err } return } + +func checkoutBranch(opts *DevelopOptions, baseRepo ghrepo.Interface, checkoutBranch string) (err error) { + + remotes, err := opts.Remotes() + if err != nil { + return err + } + + baseRemote, err := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName()) + if err != nil { + return err + } + + if git.HasLocalBranch(checkoutBranch) { + if err := git.CheckoutBranch(checkoutBranch); err != nil { + return err + } + } else { + gitFetch, err := git.GitCommand("fetch", "origin", fmt.Sprintf("+refs/heads/%[1]s:refs/remotes/origin/%[1]s", checkoutBranch)) + + if err != nil { + return err + } + + gitFetch.Stdout = opts.IO.Out + gitFetch.Stderr = opts.IO.ErrOut + err = run.PrepareCmd(gitFetch).Run() + if err != nil { + return err + } + if err := git.CheckoutNewBranch(baseRemote.Name, checkoutBranch); err != nil { + return err + } + } + + if err := git.Pull(baseRemote.Name, checkoutBranch); err != nil { + _, _ = fmt.Fprintf(opts.IO.ErrOut, "%s warning: not possible to fast-forward to: %q\n", opts.IO.ColorScheme().WarningIcon(), checkoutBranch) + } + + return nil +} From ac4fc388bf4913698c38f3ff9200610209989452 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Tue, 13 Sep 2022 07:28:33 -0400 Subject: [PATCH 11/26] List linked branches works --- api/queries_branch_issue_reference.go | 57 +++++++++++++++++++++ pkg/cmd/issue/develop/develop.go | 74 ++++++++++++++++++++++++--- pkg/cmd/issue/list/http.go | 1 + pkg/cmd/issue/shared/lookup.go | 20 ++++++++ 4 files changed, 144 insertions(+), 8 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index a05b88476..f1265b484 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -1,5 +1,7 @@ package api +import "github.com/cli/cli/v2/internal/ghrepo" + type BranchIssueReference struct { ID string BranchName string @@ -58,6 +60,61 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } +func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) ([]string, error) { + // query uses name and owner + query := ` + query BranchIssueReferenceListLinkedBranches($repositoryName: String!, $repositoryOwner: String!, $issueNumber: Int!) { + repository(name: $repositoryName, owner: $repositoryOwner) { + issue(number: $issueNumber) { + linkedBranches(first: 30) { + edges { + node { + ref { + name + } + } + } + } + } + } + } + ` + variables := map[string]interface{}{ + "repositoryName": repo.RepoName(), + "repositoryOwner": repo.RepoOwner(), + "issueNumber": issueNumber, + } + + result := struct { + Repository struct { + Issue struct { + LinkedBranches struct { + Edges []struct { + Node struct { + Ref struct { + Name string + } + } + } + } + } + } + }{} + + err := client.GraphQL(repo.RepoHost(), query, variables, &result) + var branchNames []string + if err != nil { + return branchNames, err + } + + for _, edge := range result.Repository.Issue.LinkedBranches.Edges { + branchNames = append(branchNames, edge.Node.Ref.Name) + } + + return branchNames, nil + +} + func FindBaseOid(client *Client, repo *Repository, ref string) (string, string, error) { query := ` query BranchIssueReferenceFindBaseOid($repositoryName: String!, $repositoryOwner: String!, $ref: String!) { diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 49b5c1874..f5681108e 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -24,11 +24,12 @@ type DevelopOptions struct { BaseRepo func() (ghrepo.Interface, error) Remotes func() (context.Remotes, error) - IssueRepo string - IssueSelector string - Name string - BaseBranch string - Checkout bool + IssueRepoSelector string + IssueSelector string + Name string + BaseBranch string + Checkout bool + List bool } func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra.Command { @@ -45,23 +46,28 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. Short: "Manage linked branches for an issue", Example: heredoc.Doc(` $ gh issue develop --list 123 # list branches for issue 123 - $ gh issue develop --issue-repo "github/cli" 123 list branches for issue 123 in repo "github/cli" + $ gh issue develop --list --issue-repo "github/cli" 123 list branches for issue 123 in repo "github/cli" + $ gh issue develop --list https://github.com/github/cli/issues/123 # list branches for issue 123 in repo "github/cli" $ gh issue develop 123 --name "my-branch" --head main $ gh issue develop 123 --checkout # checkout the branch for issue 123 after creating it `), - Args: cmdutil.ExactArgs(1, "issue number is required"), + Args: cmdutil.ExactArgs(1, "issue number or url is required"), RunE: func(cmd *cobra.Command, args []string) error { if runF != nil { return runF(opts) } opts.IssueSelector = args[0] + if opts.List { + return developRunList(opts) + } return developRun(opts) }, } fl := cmd.Flags() fl.StringVarP(&opts.BaseBranch, "base-branch", "b", "", "Name of the base branch") fl.BoolVarP(&opts.Checkout, "checkout", "c", false, "Checkout the branch after creating it") - fl.StringVarP(&opts.IssueRepo, "issue-repo", "i", "", "Name or URL of the issue's repository") + fl.StringVarP(&opts.IssueRepoSelector, "issue-repo", "i", "", "Name or URL of the issue's repository") + fl.BoolVarP(&opts.List, "list", "l", false, "List branches for the issue") fl.StringVarP(&opts.Name, "name", "n", "", "Name of the branch to create") return cmd } @@ -120,6 +126,58 @@ func developRun(opts *DevelopOptions) (err error) { return } +func developRunList(opts *DevelopOptions) (err error) { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + apiClient := api.NewClientFromHTTP(httpClient) + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + opts.IO.StartProgressIndicator() + var issueRepo ghrepo.Interface + if opts.IssueRepoSelector != "" { + issueRepo, err = ghrepo.FromFullNameWithHost(opts.IssueRepoSelector, baseRepo.RepoHost()) + if err != nil { + return err + } + } + + targetRepo := baseRepo + if issueRepo != nil { + targetRepo = issueRepo + } + issueNumber, issueRepo, err := shared.IssueNumberAndRepoFromArg(opts.IssueSelector, targetRepo) + if err != nil { + return err + } + + branches, err := api.ListLinkedBranches(apiClient, issueRepo, issueNumber) + if err != nil { + return err + } + + opts.IO.StopProgressIndicator() + if len(branches) == 0 { + return cmdutil.NewNoResultsError(fmt.Sprintf("no linked branches found for %s/%s#%d", issueRepo.RepoOwner(), issueRepo.RepoName(), issueNumber)) + } + + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "\nShowing linked branches for %s/%s#%d\n\n", issueRepo.RepoOwner(), issueRepo.RepoName(), issueNumber) + } + + for _, branch := range branches { + fmt.Fprintf(opts.IO.Out, "%s\n", branch) + } + + return nil + +} + func checkoutBranch(opts *DevelopOptions, baseRepo ghrepo.Interface, checkoutBranch string) (err error) { remotes, err := opts.Remotes() diff --git a/pkg/cmd/issue/list/http.go b/pkg/cmd/issue/list/http.go index db7633157..8dfcfe65e 100644 --- a/pkg/cmd/issue/list/http.go +++ b/pkg/cmd/issue/list/http.go @@ -22,6 +22,7 @@ func listIssues(client *api.Client, repo ghrepo.Interface, filters prShared.Filt } fragments := fmt.Sprintf("fragment issue on Issue {%s}", api.PullRequestGraphQL(filters.Fields)) + // TODO try to paginate like this query := fragments + ` query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $assignee: String, $author: String, $mention: String) { repository(owner: $owner, name: $repo) { diff --git a/pkg/cmd/issue/shared/lookup.go b/pkg/cmd/issue/shared/lookup.go index e802cada0..3a4d6594e 100644 --- a/pkg/cmd/issue/shared/lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -60,6 +60,26 @@ func issueMetadataFromURL(s string) (int, ghrepo.Interface) { return issueNumber, repo } +//TODO: Returns the issue number and base repo if the issue URL is provided, falling back to the +// supplied repo if the base repo is not specified in the URL. +func IssueNumberAndRepoFromArg(arg string, fallbackRepo ghrepo.Interface) (int, ghrepo.Interface, error) { + issueNumber, baseRepo := issueMetadataFromURL(arg) + + if issueNumber == 0 { + var err error + issueNumber, err = strconv.Atoi(strings.TrimPrefix(arg, "#")) + if err != nil { + return 0, nil, fmt.Errorf("invalid issue format: %q", arg) + } + } + + if baseRepo == nil { + baseRepo = fallbackRepo + } + + return issueNumber, baseRepo, nil +} + type PartialLoadError struct { error } From 398c86b8a2061c5670025718cdd5e52b7ed60533 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Tue, 13 Sep 2022 10:47:57 -0400 Subject: [PATCH 12/26] Use shared fn to look up issue and its repo --- pkg/cmd/issue/develop/develop.go | 50 ++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index f5681108e..e95d77cf0 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -88,6 +88,17 @@ func developRun(opts *DevelopOptions) (err error) { return err } + issueNumber, issueRepo, err := issueMetadata(opts.IssueSelector, opts.IssueRepoSelector, baseRepo) + if err != nil { + return err + } + + // get the id of the issue + issue, _, err := shared.IssueFromArgWithFields(httpClient, func() (ghrepo.Interface, error) { return issueRepo, nil }, fmt.Sprint(issueNumber), []string{"id"}) + if err != nil { + return err + } + oid, default_branch_oid, err := api.FindBaseOid(apiClient, repo, opts.BaseBranch) if err != nil { return err @@ -97,12 +108,6 @@ func developRun(opts *DevelopOptions) (err error) { oid = default_branch_oid } - // get the id of the issue repo - issue, _, err := shared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.IssueSelector, []string{"id", "number", "title"}) - if err != nil { - return err - } - // get the oid of the branch from the base repo params := map[string]interface{}{ "issueId": issue.ID, @@ -126,6 +131,25 @@ func developRun(opts *DevelopOptions) (err error) { return } +func issueMetadata(issueSelector string, issueRepoSelector string, baseRepo ghrepo.Interface) (issueNumber int, issueRepo ghrepo.Interface, err error) { + if issueRepoSelector != "" { + issueRepo, err = ghrepo.FromFullNameWithHost(issueRepoSelector, baseRepo.RepoHost()) + if err != nil { + return 0, nil, err + } + } + + targetRepo := baseRepo + if issueRepo != nil { + targetRepo = issueRepo + } + issueNumber, issueRepo, err = shared.IssueNumberAndRepoFromArg(issueSelector, targetRepo) + if err != nil { + return 0, nil, err + } + return issueNumber, issueRepo, nil +} + func developRunList(opts *DevelopOptions) (err error) { httpClient, err := opts.HttpClient() if err != nil { @@ -139,19 +163,7 @@ func developRunList(opts *DevelopOptions) (err error) { } opts.IO.StartProgressIndicator() - var issueRepo ghrepo.Interface - if opts.IssueRepoSelector != "" { - issueRepo, err = ghrepo.FromFullNameWithHost(opts.IssueRepoSelector, baseRepo.RepoHost()) - if err != nil { - return err - } - } - - targetRepo := baseRepo - if issueRepo != nil { - targetRepo = issueRepo - } - issueNumber, issueRepo, err := shared.IssueNumberAndRepoFromArg(opts.IssueSelector, targetRepo) + issueNumber, issueRepo, err := issueMetadata(opts.IssueSelector, opts.IssueRepoSelector, baseRepo) if err != nil { return err } From 07e84ea7a98254e4139c5648e369ef6d5e8262bb Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Tue, 13 Sep 2022 15:01:19 -0400 Subject: [PATCH 13/26] Add tests for checkout flag --- pkg/cmd/issue/develop/develop.go | 3 +- pkg/cmd/issue/develop/develop_test.go | 126 +++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index e95d77cf0..b6e0119f6 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -119,7 +119,8 @@ func developRun(opts *DevelopOptions) (err error) { ref, err := api.CreateBranchIssueReference(apiClient, repo, params) opts.IO.StopProgressIndicator() if ref != nil { - fmt.Fprintf(opts.IO.Out, "Created %s\n", ref.BranchName) + baseRepo.RepoHost() + fmt.Fprintf(opts.IO.Out, "%s/%s/%s/tree/%s\n", baseRepo.RepoHost(), baseRepo.RepoOwner(), baseRepo.RepoName(), ref.BranchName) if opts.Checkout { return checkoutBranch(opts, baseRepo, ref.BranchName) diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 4b9eb85f1..eb079323e 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -1,9 +1,12 @@ package develop import ( + "errors" "net/http" "testing" + "github.com/cli/cli/v2/context" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" @@ -19,6 +22,8 @@ func Test_developRun(t *testing.T) { name string setup func(*DevelopOptions, *testing.T) func() cmdStubs func(*run.CommandStubber) + runStubs func(*run.CommandStubber) + remotes map[string]string askStubs func(*prompt.AskStubber) // TODO eventually migrate to PrompterMock httpStubs func(*httpmock.Registry, *testing.T) expectedOut string @@ -53,7 +58,7 @@ func Test_developRun(t *testing.T) { reg.Register( httpmock.GraphQL(`mutation CreateLinkedBranch\b`), - httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": 2, "ref": {"name": "my-branch"} } } } }`, + httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": "2", "ref": {"name": "my-branch"} } } } }`, func(query string, inputs map[string]interface{}) { assert.Equal(t, "REPOID", inputs["repositoryId"]) assert.Equal(t, "my-branch", inputs["name"]) @@ -62,7 +67,99 @@ func Test_developRun(t *testing.T) { ) }, - expectedOut: "Created my-branch\n", + expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", + }, + {name: "develop new branch with checkout when the branch exists locally", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.Name = "my-branch" + opts.BaseBranch = "main" + opts.IssueSelector = "123" + opts.Checkout = true + return func() {} + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`), + ) + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled": true, "issue":{"id": "yar", "number":123, "title":"my issue"} }}}`)) + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceFindBaseOid\b`), + httpmock.StringResponse(`{"data":{"repository":{"ref":{"target":{"oid":"123"}}}}}`)) + + reg.Register( + httpmock.GraphQL(`mutation CreateLinkedBranch\b`), + httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": "2", "ref": {"name": "my-branch"} } } } }`, + func(query string, inputs map[string]interface{}) { + assert.Equal(t, "REPOID", inputs["repositoryId"]) + assert.Equal(t, "my-branch", inputs["name"]) + assert.Equal(t, "yar", inputs["issueId"]) + }), + ) + + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --verify refs/heads/my-branch`, 0, "") + cs.Register(`git checkout my-branch`, 0, "") + cs.Register(`git pull --ff-only origin my-branch`, 0, "") + }, + expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", + }, {name: "develop new branch with checkout when the branch does not exist locally", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.Name = "my-branch" + opts.BaseBranch = "main" + opts.IssueSelector = "123" + opts.Checkout = true + return func() {} + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`), + ) + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled": true, "issue":{"id": "yar", "number":123, "title":"my issue"} }}}`)) + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceFindBaseOid\b`), + httpmock.StringResponse(`{"data":{"repository":{"ref":{"target":{"oid":"123"}}}}}`)) + + reg.Register( + httpmock.GraphQL(`mutation CreateLinkedBranch\b`), + httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": "2", "ref": {"name": "my-branch"} } } } }`, + func(query string, inputs map[string]interface{}) { + assert.Equal(t, "REPOID", inputs["repositoryId"]) + assert.Equal(t, "my-branch", inputs["name"]) + assert.Equal(t, "yar", inputs["issueId"]) + }), + ) + + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git rev-parse --verify refs/heads/my-branch`, 1, "") + cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "") + cs.Register(`git checkout -b my-branch --track origin/my-branch`, 0, "") + cs.Register(`git pull --ff-only origin my-branch`, 0, "") + }, + expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", }, } for _, tt := range tests { @@ -91,6 +188,31 @@ func Test_developRun(t *testing.T) { opts.Config = func() (config.Config, error) { return config.NewBlankConfig(), nil } + + opts.Remotes = func() (context.Remotes, error) { + if len(tt.remotes) == 0 { + return nil, errors.New("no remotes") + } + var remotes context.Remotes + for name, repo := range tt.remotes { + r, err := ghrepo.FromFullName(repo) + if err != nil { + return remotes, err + } + remotes = append(remotes, &context.Remote{ + Remote: &git.Remote{Name: name}, + Repo: r, + }) + } + return remotes, nil + } + + cmdStubs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + if tt.runStubs != nil { + tt.runStubs(cmdStubs) + } + cleanSetup := func() {} if tt.setup != nil { cleanSetup = tt.setup(&opts, t) From 0f1924ba7ee6c53df88924edacba4af297f4b157 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Tue, 13 Sep 2022 17:06:27 -0400 Subject: [PATCH 14/26] WIP tests for --list flag --- pkg/cmd/issue/develop/develop_test.go | 182 +++++++++++++++++++++++++- 1 file changed, 181 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index eb079323e..2b8927e0a 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -32,6 +32,180 @@ func Test_developRun(t *testing.T) { wantErr string tty bool }{ + {name: "list branches for an issue", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "42" + opts.List = true + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), + httpmock.GraphQLQuery(`{ + "data": { + "repository": { + "issue": { + "linkedBranches": { + "edges": [ + { + "node": { + "ref": { + "name": "foo" + } + } + }, + { + "node": { + "ref": { + "name": "bar" + } + } + } + ] + } + } + } + } + } + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(42), inputs["issueNumber"]) + assert.Equal(t, "OWNER", inputs["repositoryOwner"]) + assert.Equal(t, "REPO", inputs["repositoryName"]) + })) + }, + expectedOut: "foo\nbar\n", + }, + {name: "list branches for an issue providing an issue url", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" + opts.List = true + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), + httpmock.GraphQLQuery(`{ + "data": { + "repository": { + "issue": { + "linkedBranches": { + "edges": [ + { + "node": { + "ref": { + "name": "foo" + } + } + }, + { + "node": { + "ref": { + "name": "bar" + } + } + } + ] + } + } + } + } + } + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(42), inputs["issueNumber"]) + assert.Equal(t, "cli", inputs["repositoryOwner"]) + assert.Equal(t, "test-repo", inputs["repositoryName"]) + })) + }, + expectedOut: "foo\nbar\n", + }, + {name: "list branches for an issue providing an issue repo", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "42" + opts.IssueRepoSelector = "cli/test-repo" + opts.List = true + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), + httpmock.GraphQLQuery(`{ + "data": { + "repository": { + "issue": { + "linkedBranches": { + "edges": [ + { + "node": { + "ref": { + "name": "foo" + } + } + }, + { + "node": { + "ref": { + "name": "bar" + } + } + } + ] + } + } + } + } + } + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(42), inputs["issueNumber"]) + assert.Equal(t, "cli", inputs["repositoryOwner"]) + assert.Equal(t, "test-repo", inputs["repositoryName"]) + })) + }, + expectedOut: "foo\nbar\n", + }, + {name: "list branches for an issue providing an issue url and specifying its repo returns an error", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" + opts.IssueRepoSelector = "cli/test-repo" + opts.List = true + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), + httpmock.GraphQLQuery(`{ + "data": { + "repository": { + "issue": { + "linkedBranches": { + "edges": [ + { + "node": { + "ref": { + "name": "foo" + } + } + }, + { + "node": { + "ref": { + "name": "bar" + } + } + } + ] + } + } + } + } + } + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(42), inputs["issueNumber"]) + assert.Equal(t, "cli", inputs["repositoryOwner"]) + assert.Equal(t, "test-repo", inputs["repositoryName"]) + })) + }, + expectedErrOut: "error: --issue-repo cannot be used when providing an issue URL\n", + }, {name: "develop new branch", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.Name = "my-branch" @@ -219,7 +393,13 @@ func Test_developRun(t *testing.T) { } defer cleanSetup() - err := developRun(&opts) + var err error + if opts.List { + err = developRunList(&opts) + } else { + + err = developRun(&opts) + } output := &test.CmdOut{ OutBuf: stdout, ErrBuf: stderr, From 48512176d8416c4f046cc49d88b13901acb253b1 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Wed, 14 Sep 2022 11:57:51 -0400 Subject: [PATCH 15/26] Error when issue-repo doesn't match issue url --- pkg/cmd/issue/develop/develop.go | 49 ++++++++++++++++++++------- pkg/cmd/issue/develop/develop_test.go | 26 +++++++++++--- pkg/cmd/issue/shared/lookup.go | 10 ++---- 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index b6e0119f6..a8e73add0 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -60,7 +60,7 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. if opts.List { return developRunList(opts) } - return developRun(opts) + return developRunCreate(opts) }, } fl := cmd.Flags() @@ -72,7 +72,7 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. return cmd } -func developRun(opts *DevelopOptions) (err error) { +func developRunCreate(opts *DevelopOptions) (err error) { httpClient, err := opts.HttpClient() if err != nil { return err @@ -82,13 +82,14 @@ func developRun(opts *DevelopOptions) (err error) { if err != nil { return err } - opts.IO.StartProgressIndicator() - repo, err := api.GitHubRepo(apiClient, baseRepo) + + issueNumber, issueRepo, err := issueMetadata(opts.IssueSelector, opts.IssueRepoSelector, baseRepo) if err != nil { return err } - issueNumber, issueRepo, err := issueMetadata(opts.IssueSelector, opts.IssueRepoSelector, baseRepo) + opts.IO.StartProgressIndicator() + repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err } @@ -132,23 +133,47 @@ func developRun(opts *DevelopOptions) (err error) { return } -func issueMetadata(issueSelector string, issueRepoSelector string, baseRepo ghrepo.Interface) (issueNumber int, issueRepo ghrepo.Interface, err error) { +// If the issue is in the base repo, we can use the issue number directly. Otherwise, we need to use the issue's url or the IssueRepoSelector argument. +// If the repo from the URL doesn't match the IssueRepoSelector argument, we error. +func issueMetadata(issueSelector string, issueRepoSelector string, baseRepo ghrepo.Interface) (issueNumber int, issueFlagRepo ghrepo.Interface, err error) { + var targetRepo ghrepo.Interface if issueRepoSelector != "" { - issueRepo, err = ghrepo.FromFullNameWithHost(issueRepoSelector, baseRepo.RepoHost()) + issueFlagRepo, err = ghrepo.FromFullNameWithHost(issueRepoSelector, baseRepo.RepoHost()) if err != nil { return 0, nil, err } } - targetRepo := baseRepo - if issueRepo != nil { - targetRepo = issueRepo + if issueFlagRepo != nil { + targetRepo = issueFlagRepo } - issueNumber, issueRepo, err = shared.IssueNumberAndRepoFromArg(issueSelector, targetRepo) + + issueNumber, issueArgRepo, err := shared.IssueNumberAndRepoFromArg(issueSelector) if err != nil { return 0, nil, err } - return issueNumber, issueRepo, nil + + if issueArgRepo != nil { + targetRepo = issueArgRepo + + if issueFlagRepo != nil { + differentOwner := (issueFlagRepo.RepoOwner() != issueArgRepo.RepoOwner()) + differentName := (issueFlagRepo.RepoName() != issueArgRepo.RepoName()) + if differentOwner || differentName { + return 0, nil, fmt.Errorf("issue repo in url %s/%s does not match the repo from --issue-repo %s/%s", issueArgRepo.RepoOwner(), issueArgRepo.RepoName(), issueFlagRepo.RepoOwner(), issueFlagRepo.RepoName()) + } + } + } + + if issueFlagRepo == nil && issueArgRepo == nil { + targetRepo = baseRepo + } + + if targetRepo == nil { + return 0, nil, fmt.Errorf("could not determine issue repo") + } + + return issueNumber, targetRepo, nil } func developRunList(opts *DevelopOptions) (err error) { diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 2b8927e0a..fd7296559 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -162,7 +162,7 @@ func Test_developRun(t *testing.T) { }, expectedOut: "foo\nbar\n", }, - {name: "list branches for an issue providing an issue url and specifying its repo returns an error", + {name: "list branches for an issue providing an issue url and specifying the same repo works", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" opts.IssueRepoSelector = "cli/test-repo" @@ -204,7 +204,16 @@ func Test_developRun(t *testing.T) { assert.Equal(t, "test-repo", inputs["repositoryName"]) })) }, - expectedErrOut: "error: --issue-repo cannot be used when providing an issue URL\n", + expectedOut: "foo\nbar\n", + }, + {name: "list branches for an issue providing an issue url and specifying a different repo returns an error", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" + opts.IssueRepoSelector = "cli/other" + opts.List = true + return func() {} + }, + wantErr: "issue repo in url cli/test-repo does not match the repo from --issue-repo cli/other", }, {name: "develop new branch", setup: func(opts *DevelopOptions, t *testing.T) func() { @@ -243,6 +252,14 @@ func Test_developRun(t *testing.T) { }, expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", }, + {name: "develop providing an issue url and specifying a different repo returns an error", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" + opts.IssueRepoSelector = "cli/other" + return func() {} + }, + wantErr: "issue repo in url cli/test-repo does not match the repo from --issue-repo cli/other", + }, {name: "develop new branch with checkout when the branch exists locally", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.Name = "my-branch" @@ -288,7 +305,8 @@ func Test_developRun(t *testing.T) { cs.Register(`git pull --ff-only origin my-branch`, 0, "") }, expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", - }, {name: "develop new branch with checkout when the branch does not exist locally", + }, + {name: "develop new branch with checkout when the branch does not exist locally", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.Name = "my-branch" opts.BaseBranch = "main" @@ -398,7 +416,7 @@ func Test_developRun(t *testing.T) { err = developRunList(&opts) } else { - err = developRun(&opts) + err = developRunCreate(&opts) } output := &test.CmdOut{ OutBuf: stdout, diff --git a/pkg/cmd/issue/shared/lookup.go b/pkg/cmd/issue/shared/lookup.go index 3a4d6594e..abd22e021 100644 --- a/pkg/cmd/issue/shared/lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -60,9 +60,9 @@ func issueMetadataFromURL(s string) (int, ghrepo.Interface) { return issueNumber, repo } -//TODO: Returns the issue number and base repo if the issue URL is provided, falling back to the -// supplied repo if the base repo is not specified in the URL. -func IssueNumberAndRepoFromArg(arg string, fallbackRepo ghrepo.Interface) (int, ghrepo.Interface, error) { +// Returns the issue number and repo if the issue URL is provided. +// If only the issue number is provided, returns the number and nil repo. +func IssueNumberAndRepoFromArg(arg string) (int, ghrepo.Interface, error) { issueNumber, baseRepo := issueMetadataFromURL(arg) if issueNumber == 0 { @@ -73,10 +73,6 @@ func IssueNumberAndRepoFromArg(arg string, fallbackRepo ghrepo.Interface) (int, } } - if baseRepo == nil { - baseRepo = fallbackRepo - } - return issueNumber, baseRepo, nil } From bc72d6b983a9afcb18dbf404e632ea2f688a50b6 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Wed, 14 Sep 2022 14:54:43 -0400 Subject: [PATCH 16/26] Omit name param from mutation when blank As it currently sits the backend won't ignore the param. I'll be looking to add a PR for this but for now we'll remove it. --- api/queries_branch_issue_reference.go | 20 +++++++++---- pkg/cmd/issue/develop/develop.go | 2 +- pkg/cmd/issue/develop/develop_test.go | 41 +++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index f1265b484..7c04110de 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -1,18 +1,29 @@ package api -import "github.com/cli/cli/v2/internal/ghrepo" +import ( + "fmt" + + "github.com/cli/cli/v2/internal/ghrepo" +) type BranchIssueReference struct { ID string BranchName string } +func nameParam(params map[string]interface{}) string { + if params["name"] != "" { + return "name: $name," + } + return "" +} + func CreateBranchIssueReference(client *Client, repo *Repository, params map[string]interface{}) (*BranchIssueReference, error) { - query := ` + query := fmt.Sprintf(` mutation CreateLinkedBranch($issueId: ID!, $oid: GitObjectID!, $name: String, $repositoryId: ID) { createLinkedBranch(input: { issueId: $issueId, - name: $name, + %[1]s oid: $oid, repositoryId: $repositoryId }) { @@ -23,8 +34,7 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } } } - } - ` + }`, nameParam(params)) inputParams := map[string]interface{}{ "repositoryId": repo.ID, diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index a8e73add0..99eec45b5 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -67,7 +67,7 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. fl.StringVarP(&opts.BaseBranch, "base-branch", "b", "", "Name of the base branch") fl.BoolVarP(&opts.Checkout, "checkout", "c", false, "Checkout the branch after creating it") fl.StringVarP(&opts.IssueRepoSelector, "issue-repo", "i", "", "Name or URL of the issue's repository") - fl.BoolVarP(&opts.List, "list", "l", false, "List branches for the issue") + fl.BoolVarP(&opts.List, "list", "l", false, "List linked branches for the issue") fl.StringVarP(&opts.Name, "name", "n", "", "Name of the branch to create") return cmd } diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index fd7296559..b14c2df9d 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -215,7 +215,7 @@ func Test_developRun(t *testing.T) { }, wantErr: "issue repo in url cli/test-repo does not match the repo from --issue-repo cli/other", }, - {name: "develop new branch", + {name: "develop new branch with a name provided", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.Name = "my-branch" opts.BaseBranch = "main" @@ -240,7 +240,7 @@ func Test_developRun(t *testing.T) { httpmock.StringResponse(`{"data":{"repository":{"ref":{"target":{"oid":"123"}}}}}`)) reg.Register( - httpmock.GraphQL(`mutation CreateLinkedBranch\b`), + httpmock.GraphQL(`(?s)mutation CreateLinkedBranch\b.*issueId: \$issueId,\s+name: \$name,\s+oid: \$oid,`), httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": "2", "ref": {"name": "my-branch"} } } } }`, func(query string, inputs map[string]interface{}) { assert.Equal(t, "REPOID", inputs["repositoryId"]) @@ -252,6 +252,43 @@ func Test_developRun(t *testing.T) { }, expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", }, + {name: "develop new branch without a name provided omits the param from the mutation", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.Name = "" + opts.BaseBranch = "main" + opts.IssueSelector = "123" + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`), + ) + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled": true, "issue":{"id": "yar", "number":123, "title":"my issue"} }}}`)) + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceFindBaseOid\b`), + httpmock.StringResponse(`{"data":{"repository":{"ref":{"target":{"oid":"123"}}}}}`)) + + reg.Register( + httpmock.GraphQL(`(?s)mutation CreateLinkedBranch\b.*issueId: \$issueId,\s+oid: \$oid,`), + httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": "2", "ref": {"name": "my-issue-1"} } } } }`, + func(query string, inputs map[string]interface{}) { + assert.Equal(t, "REPOID", inputs["repositoryId"]) + assert.Equal(t, "", inputs["name"]) + assert.Equal(t, "yar", inputs["issueId"]) + }), + ) + + }, + expectedOut: "github.com/OWNER/REPO/tree/my-issue-1\n", + }, {name: "develop providing an issue url and specifying a different repo returns an error", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" From 79783162f4de555a1f1ca182c8077ccc5b6c15aa Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Wed, 14 Sep 2022 16:21:15 -0400 Subject: [PATCH 17/26] self-review cleanup --- api/queries_branch_issue_reference.go | 2 +- pkg/cmd/issue/develop/develop.go | 21 +++++++++++---------- pkg/cmd/issue/list/http.go | 1 - 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index 7c04110de..24a14dad4 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -71,7 +71,6 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) ([]string, error) { - // query uses name and owner query := ` query BranchIssueReferenceListLinkedBranches($repositoryName: String!, $repositoryOwner: String!, $issueNumber: Int!) { repository(name: $repositoryName, owner: $repositoryOwner) { @@ -125,6 +124,7 @@ func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) } +// This fetches the oids for the repo's default branch (`main`, etc) and the name the user might have provided in one shot. func FindBaseOid(client *Client, repo *Repository, ref string) (string, string, error) { query := ` query BranchIssueReferenceFindBaseOid($repositoryName: String!, $repositoryOwner: String!, $ref: String!) { diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 99eec45b5..39dca8820 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -46,10 +46,10 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. Short: "Manage linked branches for an issue", Example: heredoc.Doc(` $ gh issue develop --list 123 # list branches for issue 123 - $ gh issue develop --list --issue-repo "github/cli" 123 list branches for issue 123 in repo "github/cli" + $ gh issue develop --list --issue-repo "github/cli" 123 # list branches for issue 123 in repo "github/cli" $ gh issue develop --list https://github.com/github/cli/issues/123 # list branches for issue 123 in repo "github/cli" - $ gh issue develop 123 --name "my-branch" --head main - $ gh issue develop 123 --checkout # checkout the branch for issue 123 after creating it + $ gh issue develop 123 --name "my-branch" --base my-feature # create a branch for issue 123 based on the my-feature branch + $ gh issue develop 123 --checkout # fetch and checkout the branch for issue 123 after creating it `), Args: cmdutil.ExactArgs(1, "issue number or url is required"), RunE: func(cmd *cobra.Command, args []string) error { @@ -64,7 +64,7 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. }, } fl := cmd.Flags() - fl.StringVarP(&opts.BaseBranch, "base-branch", "b", "", "Name of the base branch") + fl.StringVarP(&opts.BaseBranch, "base", "b", "", "Name of the base branch you want to make your new branch from") fl.BoolVarP(&opts.Checkout, "checkout", "c", false, "Checkout the branch after creating it") fl.StringVarP(&opts.IssueRepoSelector, "issue-repo", "i", "", "Name or URL of the issue's repository") fl.BoolVarP(&opts.List, "list", "l", false, "List linked branches for the issue") @@ -83,23 +83,24 @@ func developRunCreate(opts *DevelopOptions) (err error) { return err } - issueNumber, issueRepo, err := issueMetadata(opts.IssueSelector, opts.IssueRepoSelector, baseRepo) - if err != nil { - return err - } - opts.IO.StartProgressIndicator() repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err } - // get the id of the issue + issueNumber, issueRepo, err := issueMetadata(opts.IssueSelector, opts.IssueRepoSelector, baseRepo) + if err != nil { + return err + } + + // The mutation requires the issue id, not just its number issue, _, err := shared.IssueFromArgWithFields(httpClient, func() (ghrepo.Interface, error) { return issueRepo, nil }, fmt.Sprint(issueNumber), []string{"id"}) if err != nil { return err } + // The mutation takes an oid instead of a branch name as it's a more stable reference oid, default_branch_oid, err := api.FindBaseOid(apiClient, repo, opts.BaseBranch) if err != nil { return err diff --git a/pkg/cmd/issue/list/http.go b/pkg/cmd/issue/list/http.go index 8dfcfe65e..db7633157 100644 --- a/pkg/cmd/issue/list/http.go +++ b/pkg/cmd/issue/list/http.go @@ -22,7 +22,6 @@ func listIssues(client *api.Client, repo ghrepo.Interface, filters prShared.Filt } fragments := fmt.Sprintf("fragment issue on Issue {%s}", api.PullRequestGraphQL(filters.Fields)) - // TODO try to paginate like this query := fragments + ` query IssueList($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $assignee: String, $author: String, $mention: String) { repository(owner: $owner, name: $repo) { From 6f0bf826964163b1b9a12f578dc1da9870561912 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Fri, 16 Sep 2022 10:41:10 -0400 Subject: [PATCH 18/26] Properly check for omitted name param, fix stubs --- api/queries_branch_issue_reference.go | 14 +++++++++++--- pkg/cmd/issue/develop/develop_test.go | 13 ++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index 24a14dad4..3f6f66966 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -18,12 +18,20 @@ func nameParam(params map[string]interface{}) string { return "" } +func nameArg(params map[string]interface{}) string { + if params["name"] != "" { + return "$name: String, " + } + + return "" +} + func CreateBranchIssueReference(client *Client, repo *Repository, params map[string]interface{}) (*BranchIssueReference, error) { query := fmt.Sprintf(` - mutation CreateLinkedBranch($issueId: ID!, $oid: GitObjectID!, $name: String, $repositoryId: ID) { + mutation CreateLinkedBranch($issueId: ID!, $oid: GitObjectID!, %[1]s$repositoryId: ID) { createLinkedBranch(input: { issueId: $issueId, - %[1]s + %[2]s oid: $oid, repositoryId: $repositoryId }) { @@ -34,7 +42,7 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } } } - }`, nameParam(params)) + }`, nameArg(params), nameParam(params)) inputParams := map[string]interface{}{ "repositoryId": repo.ID, diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index b14c2df9d..90c9dc2e3 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -277,7 +277,7 @@ func Test_developRun(t *testing.T) { httpmock.StringResponse(`{"data":{"repository":{"ref":{"target":{"oid":"123"}}}}}`)) reg.Register( - httpmock.GraphQL(`(?s)mutation CreateLinkedBranch\b.*issueId: \$issueId,\s+oid: \$oid,`), + httpmock.GraphQL(`(?s)mutation CreateLinkedBranch\b.*\$oid: GitObjectID!, \$repositoryId:.*issueId: \$issueId,\s+oid: \$oid,`), httpmock.GraphQLQuery(`{ "data": { "createLinkedBranch": { "linkedBranch": {"id": "2", "ref": {"name": "my-issue-1"} } } } }`, func(query string, inputs map[string]interface{}) { assert.Equal(t, "REPOID", inputs["repositoryId"]) @@ -295,6 +295,17 @@ func Test_developRun(t *testing.T) { opts.IssueRepoSelector = "cli/other" return func() {} }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } }`), + ) + }, wantErr: "issue repo in url cli/test-repo does not match the repo from --issue-repo cli/other", }, {name: "develop new branch with checkout when the branch exists locally", From 76d975524e02b723dec19620f3a973df913ae309 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Fri, 16 Sep 2022 12:12:25 -0400 Subject: [PATCH 19/26] Add feature detection query --- api/queries_branch_issue_reference.go | 22 ++++++++++++++++++++++ pkg/cmd/issue/develop/develop.go | 10 ++++++++++ 2 files changed, 32 insertions(+) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index 3f6f66966..6626bea7f 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -132,6 +132,28 @@ func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) } +// introspects the schema to see if we expose the LinkedBranch type +func CheckLinkedBranchFeature(client *Client, host string) (err error) { + var featureDetection struct { + Name struct { + Fields []struct { + Name string + } + } `graphql:"LinkedBranch: __type(name: \"LinkedBranch\")"` + } + + err = client.Query(host, "LinkedBranch_fields", &featureDetection, nil) + + if err != nil { + return err + } + + if len(featureDetection.Name.Fields) == 0 { + return fmt.Errorf("the `gh issue develop` command is not currently available") + } + return nil +} + // This fetches the oids for the repo's default branch (`main`, etc) and the name the user might have provided in one shot. func FindBaseOid(client *Client, repo *Repository, ref string) (string, string, error) { query := ` diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 39dca8820..d209121f7 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -84,6 +84,11 @@ func developRunCreate(opts *DevelopOptions) (err error) { } opts.IO.StartProgressIndicator() + err = api.CheckLinkedBranchFeature(apiClient, baseRepo.RepoHost()) + if err != nil { + return err + } + repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err @@ -190,6 +195,11 @@ func developRunList(opts *DevelopOptions) (err error) { } opts.IO.StartProgressIndicator() + + err = api.CheckLinkedBranchFeature(apiClient, baseRepo.RepoHost()) + if err != nil { + return err + } issueNumber, issueRepo, err := issueMetadata(opts.IssueSelector, opts.IssueRepoSelector, baseRepo) if err != nil { return err From 0073de492716a8eb693605981431d6e6d5697cf5 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Fri, 16 Sep 2022 12:14:14 -0400 Subject: [PATCH 20/26] Revert accidental `queries_pr` change --- api/queries_pr.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 5914fce73..df96569d4 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -304,7 +304,10 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter inputParams[key] = val } } - variables := inputParams + variables := map[string]interface{}{ + "input": inputParams, + } + result := struct { CreatePullRequest struct { PullRequest PullRequest From 9a38142a0ff4a405578b2b90d1b745133e418c4a Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Fri, 16 Sep 2022 13:38:51 -0400 Subject: [PATCH 21/26] Add tests, update stubs for feature detection --- pkg/cmd/issue/develop/develop.go | 4 ++ pkg/cmd/issue/develop/develop_test.go | 78 +++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index d209121f7..aba4b5d8f 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -223,6 +223,10 @@ func developRunList(opts *DevelopOptions) (err error) { fmt.Fprintf(opts.IO.Out, "%s\n", branch) } + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "\nShowing linked branches for %s/%s#%d\n\n", issueRepo.RepoOwner(), issueRepo.RepoName(), issueNumber) + } + return nil } diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 90c9dc2e3..dce1d9387 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -18,6 +18,23 @@ import ( ) func Test_developRun(t *testing.T) { + featureEnabledPayload := `{ + "data": { + "LinkedBranch": { + "fields": [ + { + "name": "id" + }, + { + "name": "ref" + } + ] + } + } + }` + + featureDisabledPayload := `{ "data": { "LinkedBranch": null } }` + tests := []struct { name string setup func(*DevelopOptions, *testing.T) func() @@ -39,6 +56,10 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), httpmock.GraphQLQuery(`{ @@ -82,6 +103,10 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), httpmock.GraphQLQuery(`{ @@ -126,6 +151,10 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), httpmock.GraphQLQuery(`{ @@ -170,6 +199,10 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), httpmock.GraphQLQuery(`{ @@ -213,8 +246,28 @@ func Test_developRun(t *testing.T) { opts.List = true return func() {} }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) + }, wantErr: "issue repo in url cli/test-repo does not match the repo from --issue-repo cli/other", }, + {name: "returns an error when the feature isn't enabled in the GraphQL API", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" + opts.List = true + return func() {} + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureDisabledPayload), + ) + }, + wantErr: "the `gh issue develop` command is not currently available", + }, {name: "develop new branch with a name provided", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.Name = "my-branch" @@ -223,7 +276,10 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { - + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` @@ -260,7 +316,10 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { - + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` @@ -296,7 +355,10 @@ func Test_developRun(t *testing.T) { return func() {} }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { - + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` @@ -320,7 +382,10 @@ func Test_developRun(t *testing.T) { "origin": "OWNER/REPO", }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { - + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` @@ -366,7 +431,10 @@ func Test_developRun(t *testing.T) { "origin": "OWNER/REPO", }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { - + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) reg.Register( httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` From 48f527fdd3d43e1a64fea57c92ba398080a60b42 Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Mon, 26 Sep 2022 14:39:04 -0400 Subject: [PATCH 22/26] Show repo url for linked branches in tty --- api/queries_branch_issue_reference.go | 32 ++++++++++++---- pkg/cmd/issue/develop/develop.go | 25 +++++++++---- pkg/cmd/issue/develop/develop_test.go | 54 +++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 14 deletions(-) diff --git a/api/queries_branch_issue_reference.go b/api/queries_branch_issue_reference.go index 6626bea7f..5393b484d 100644 --- a/api/queries_branch_issue_reference.go +++ b/api/queries_branch_issue_reference.go @@ -6,9 +6,15 @@ import ( "github.com/cli/cli/v2/internal/ghrepo" ) -type BranchIssueReference struct { +type LinkedBranch struct { ID string BranchName string + RepoUrl string +} + +// method to return url of linked branch, adds the branch name to the end of the repo url +func (b *LinkedBranch) Url() string { + return fmt.Sprintf("%s/tree/%s", b.RepoUrl, b.BranchName) } func nameParam(params map[string]interface{}) string { @@ -26,7 +32,7 @@ func nameArg(params map[string]interface{}) string { return "" } -func CreateBranchIssueReference(client *Client, repo *Repository, params map[string]interface{}) (*BranchIssueReference, error) { +func CreateBranchIssueReference(client *Client, repo *Repository, params map[string]interface{}) (*LinkedBranch, error) { query := fmt.Sprintf(` mutation CreateLinkedBranch($issueId: ID!, $oid: GitObjectID!, %[1]s$repositoryId: ID) { createLinkedBranch(input: { @@ -70,7 +76,7 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str return nil, err } - ref := BranchIssueReference{ + ref := LinkedBranch{ ID: result.CreateLinkedBranch.LinkedBranch.ID, BranchName: result.CreateLinkedBranch.LinkedBranch.Ref.Name, } @@ -78,7 +84,7 @@ func CreateBranchIssueReference(client *Client, repo *Repository, params map[str } -func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) ([]string, error) { +func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) ([]LinkedBranch, error) { query := ` query BranchIssueReferenceListLinkedBranches($repositoryName: String!, $repositoryOwner: String!, $issueNumber: Int!) { repository(name: $repositoryName, owner: $repositoryOwner) { @@ -88,6 +94,9 @@ func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) node { ref { name + repository { + url + } } } } @@ -109,7 +118,11 @@ func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) Edges []struct { Node struct { Ref struct { - Name string + Name string + Repository struct { + NameWithOwner string + Url string + } } } } @@ -119,13 +132,18 @@ func ListLinkedBranches(client *Client, repo ghrepo.Interface, issueNumber int) }{} err := client.GraphQL(repo.RepoHost(), query, variables, &result) - var branchNames []string + var branchNames []LinkedBranch if err != nil { return branchNames, err } for _, edge := range result.Repository.Issue.LinkedBranches.Edges { - branchNames = append(branchNames, edge.Node.Ref.Name) + branch := LinkedBranch{ + BranchName: edge.Node.Ref.Name, + RepoUrl: edge.Node.Ref.Repository.Url, + } + + branchNames = append(branchNames, branch) } return branchNames, nil diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index aba4b5d8f..eeda7bd19 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/utils" "github.com/spf13/cobra" ) @@ -182,6 +183,22 @@ func issueMetadata(issueSelector string, issueRepoSelector string, baseRepo ghre return issueNumber, targetRepo, nil } +func printLinkedBranches(io *iostreams.IOStreams, branches []api.LinkedBranch) { + + cs := io.ColorScheme() + table := utils.NewTablePrinter(io) + + for _, branch := range branches { + table.AddField(branch.BranchName, nil, cs.ColorFromString("cyan")) + if table.IsTTY() { + table.AddField(branch.Url(), nil, nil) + } + table.EndRow() + } + + _ = table.Render() +} + func developRunList(opts *DevelopOptions) (err error) { httpClient, err := opts.HttpClient() if err != nil { @@ -219,13 +236,7 @@ func developRunList(opts *DevelopOptions) (err error) { fmt.Fprintf(opts.IO.Out, "\nShowing linked branches for %s/%s#%d\n\n", issueRepo.RepoOwner(), issueRepo.RepoName(), issueNumber) } - for _, branch := range branches { - fmt.Fprintf(opts.IO.Out, "%s\n", branch) - } - - if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.Out, "\nShowing linked branches for %s/%s#%d\n\n", issueRepo.RepoOwner(), issueRepo.RepoName(), issueNumber) - } + printLinkedBranches(opts.IO, branches) return nil diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index dce1d9387..20d34cce0 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -96,6 +96,60 @@ func Test_developRun(t *testing.T) { }, expectedOut: "foo\nbar\n", }, + {name: "list branches for an issue in tty", + setup: func(opts *DevelopOptions, t *testing.T) func() { + opts.IssueSelector = "42" + opts.List = true + return func() {} + }, + tty: true, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranch_fields\b`), + httpmock.StringResponse(featureEnabledPayload), + ) + reg.Register( + httpmock.GraphQL(`query BranchIssueReferenceListLinkedBranches\b`), + httpmock.GraphQLQuery(`{ + "data": { + "repository": { + "issue": { + "linkedBranches": { + "edges": [ + { + "node": { + "ref": { + "name": "foo", + "repository": { + "url": "http://github.localhost/OWNER/REPO" + } + } + } + }, + { + "node": { + "ref": { + "name": "bar", + "repository": { + "url": "http://github.localhost/OWNER/OTHER-REPO" + } + } + } + } + ] + } + } + } + } + } + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(42), inputs["issueNumber"]) + assert.Equal(t, "OWNER", inputs["repositoryOwner"]) + assert.Equal(t, "REPO", inputs["repositoryName"]) + })) + }, + expectedOut: "\nShowing linked branches for OWNER/REPO#42\n\nfoo http://github.localhost/OWNER/REPO/tree/foo\nbar http://github.localhost/OWNER/OTHER-REPO/tree/bar\n", + }, {name: "list branches for an issue providing an issue url", setup: func(opts *DevelopOptions, t *testing.T) func() { opts.IssueSelector = "https://github.com/cli/test-repo/issues/42" From 4daac4991761e3d43d2bc7a2d0b144f7cc483aac Mon Sep 17 00:00:00 2001 From: Chris Westra Date: Wed, 5 Oct 2022 07:45:15 -0400 Subject: [PATCH 23/26] Remove released feature flag `branch_for_issue_api` is no longer in the dotcom codebase so we can remove it from here --- api/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/client.go b/api/client.go index bf318dad9..61b5d844b 100644 --- a/api/client.go +++ b/api/client.go @@ -19,7 +19,7 @@ const ( authorization = "Authorization" cacheTTL = "X-GH-CACHE-TTL" graphqlFeatures = "GraphQL-Features" - features = "merge_queue,branch_for_issue_api" + features = "merge_queue" userAgent = "User-Agent" ) From 96372e5ac83f8dbf880ad21c9a881d4ce030dd61 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 24 Oct 2022 15:02:24 -0700 Subject: [PATCH 24/26] use new GitClient in issue develop --- pkg/cmd/issue/develop/develop.go | 16 +++++++++------- pkg/cmd/issue/develop/develop_test.go | 2 ++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index eeda7bd19..4dcea2b12 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -1,6 +1,7 @@ package develop import ( + ctx "context" "fmt" "net/http" @@ -10,7 +11,6 @@ import ( "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" - "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" @@ -20,6 +20,7 @@ import ( type DevelopOptions struct { HttpClient func() (*http.Client, error) + GitClient *git.Client Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) @@ -37,6 +38,7 @@ func NewCmdDevelop(f *cmdutil.Factory, runF func(*DevelopOptions) error) *cobra. opts := &DevelopOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + GitClient: f.GitClient, Config: f.Config, BaseRepo: f.BaseRepo, Remotes: f.Remotes, @@ -254,12 +256,12 @@ func checkoutBranch(opts *DevelopOptions, baseRepo ghrepo.Interface, checkoutBra return err } - if git.HasLocalBranch(checkoutBranch) { - if err := git.CheckoutBranch(checkoutBranch); err != nil { + if opts.GitClient.HasLocalBranch(ctx.Background(), checkoutBranch) { + if err := opts.GitClient.CheckoutBranch(ctx.Background(), checkoutBranch); err != nil { return err } } else { - gitFetch, err := git.GitCommand("fetch", "origin", fmt.Sprintf("+refs/heads/%[1]s:refs/remotes/origin/%[1]s", checkoutBranch)) + gitFetch, err := opts.GitClient.Command(ctx.Background(), "fetch", "origin", fmt.Sprintf("+refs/heads/%[1]s:refs/remotes/origin/%[1]s", checkoutBranch)) if err != nil { return err @@ -267,16 +269,16 @@ func checkoutBranch(opts *DevelopOptions, baseRepo ghrepo.Interface, checkoutBra gitFetch.Stdout = opts.IO.Out gitFetch.Stderr = opts.IO.ErrOut - err = run.PrepareCmd(gitFetch).Run() + err = gitFetch.Run() if err != nil { return err } - if err := git.CheckoutNewBranch(baseRemote.Name, checkoutBranch); err != nil { + if err := opts.GitClient.CheckoutNewBranch(ctx.Background(), baseRemote.Name, checkoutBranch); err != nil { return err } } - if err := git.Pull(baseRemote.Name, checkoutBranch); err != nil { + if err := opts.GitClient.Pull(ctx.Background(), baseRemote.Name, checkoutBranch); err != nil { _, _ = fmt.Fprintf(opts.IO.ErrOut, "%s warning: not possible to fast-forward to: %q\n", opts.IO.ColorScheme().WarningIcon(), checkoutBranch) } diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 20d34cce0..2bf11e0d7 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -569,6 +569,8 @@ func Test_developRun(t *testing.T) { return remotes, nil } + opts.GitClient = &git.Client{GitPath: "some/path/git"} + cmdStubs, cmdTeardown := run.Stub() defer cmdTeardown(t) if tt.runStubs != nil { From afecf7e5d02d0344221153e47b09c58338176cfe Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 24 Oct 2022 15:23:40 -0700 Subject: [PATCH 25/26] use tableprinter in issue develop --- pkg/cmd/issue/develop/develop.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 4dcea2b12..5d2fa2d6a 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -11,10 +11,10 @@ import ( "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/tableprinter" "github.com/cli/cli/v2/pkg/cmd/issue/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/cli/v2/utils" "github.com/spf13/cobra" ) @@ -188,12 +188,12 @@ func issueMetadata(issueSelector string, issueRepoSelector string, baseRepo ghre func printLinkedBranches(io *iostreams.IOStreams, branches []api.LinkedBranch) { cs := io.ColorScheme() - table := utils.NewTablePrinter(io) + table := tableprinter.New(io) for _, branch := range branches { - table.AddField(branch.BranchName, nil, cs.ColorFromString("cyan")) - if table.IsTTY() { - table.AddField(branch.Url(), nil, nil) + table.AddField(branch.BranchName, tableprinter.WithColor(cs.ColorFromString("cyan"))) + if io.CanPrompt() { + table.AddField(branch.Url()) } table.EndRow() } From d0955ee077a656322e4e3a44f389863a6fffbe29 Mon Sep 17 00:00:00 2001 From: dojutsu-user Date: Tue, 25 Oct 2022 22:57:00 +0530 Subject: [PATCH 26/26] disable colors in ansi --- cmd/gh/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 750233ba7..09beb5a35 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -70,6 +70,7 @@ func mainRun() exitCode { } if !cmdFactory.IOStreams.ColorEnabled() { surveyCore.DisableColor = true + ansi.DisableColors(true) } else { // override survey's poor choice of color surveyCore.TemplateFuncsWithColor["color"] = func(style string) string {