From a2ff97d73f6758a3b9d992cb629759c0c7d04100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 20 Apr 2021 13:50:01 +0200 Subject: [PATCH] Fix `issue create --web` --- pkg/cmd/issue/create/create.go | 6 +- pkg/cmd/issue/create/create_test.go | 267 ++++++++++++++++------------ 2 files changed, 155 insertions(+), 118 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index da15716ca..a974d8d67 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -157,9 +157,8 @@ func createRun(opts *CreateOptions) (err error) { tpl := shared.NewTemplateManager(httpClient, baseRepo, opts.RootDirOverride, !opts.HasRepoOverride, false) - var openURL string - if opts.WebMode { + var openURL string if opts.Title != "" || opts.Body != "" || tb.HasMetadata() { openURL, err = generatePreviewURL(apiClient, baseRepo, tb) if err != nil { @@ -171,6 +170,8 @@ func createRun(opts *CreateOptions) (err error) { } } else if ok, _ := tpl.HasTemplates(); ok { openURL = ghrepo.GenerateRepoURL(baseRepo, "issues/new/choose") + } else { + openURL = ghrepo.GenerateRepoURL(baseRepo, "issues/new") } if isTerminal { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) @@ -193,6 +194,7 @@ func createRun(opts *CreateOptions) (err error) { action := prShared.SubmitAction templateNameForSubmit := "" + var openURL string if opts.Interactive { var editorCommand string diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 5de909ad3..eece13daf 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "testing" "github.com/MakeNowJust/heredoc" @@ -134,6 +135,156 @@ func TestNewCmdCreate(t *testing.T) { } } +func Test_createRun(t *testing.T) { + tests := []struct { + name string + opts CreateOptions + httpStubs func(*httpmock.Registry) + wantsStdout string + wantsStderr string + wantsBrowse string + wantsErr string + }{ + { + name: "no args", + opts: CreateOptions{ + WebMode: true, + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new", + wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", + }, + { + name: "title and body", + opts: CreateOptions{ + WebMode: true, + Title: "myissue", + Body: "hello cli", + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?body=hello+cli&title=myissue", + wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", + }, + { + name: "assignee", + opts: CreateOptions{ + WebMode: true, + Assignees: []string{"monalisa"}, + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=monalisa", + wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", + }, + { + name: "@me", + opts: CreateOptions{ + WebMode: true, + Assignees: []string{"@me"}, + }, + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(` + { "data": { + "viewer": { "login": "MonaLisa" } + } }`)) + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", + wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", + }, + { + name: "project", + opts: CreateOptions{ + WebMode: true, + Projects: []string{"cleanup"}, + }, + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query RepositoryProjectList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "projects": { + "nodes": [ + { "name": "Cleanup", "id": "CLEANUPID", "resourcePath": "/OWNER/REPO/projects/1" } + ], + "pageInfo": { "hasNextPage": false } + } } } }`)) + r.Register( + httpmock.GraphQL(`query OrganizationProjectList\b`), + httpmock.StringResponse(` + { "data": { "organization": { "projects": { + "nodes": [ + { "name": "Triage", "id": "TRIAGEID", "resourcePath": "/orgs/ORG/projects/1" } + ], + "pageInfo": { "hasNextPage": false } + } } } }`)) + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1", + wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", + }, + { + name: "has templates", + opts: CreateOptions{ + WebMode: true, + }, + httpStubs: func(r *httpmock.Registry) { + r.Register( + httpmock.GraphQL(`query IssueTemplates\b`), + httpmock.StringResponse(` + { "data": { "repository": { "issueTemplates": [ + { "name": "Bug report", + "body": "Does not work :((" }, + { "name": "Submit a request", + "body": "I have a suggestion for an enhancement" } + ] } } }`), + ) + }, + wantsBrowse: "https://github.com/OWNER/REPO/issues/new/choose", + wantsStderr: "Opening github.com/OWNER/REPO/issues/new/choose in your browser.\n", + }, + { + name: "too long body", + opts: CreateOptions{ + WebMode: true, + Body: strings.Repeat("A", 9216), + }, + wantsErr: "cannot open in browser: maximum URL length exceeded", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + httpReg := &httpmock.Registry{} + defer httpReg.Verify(t) + if tt.httpStubs != nil { + tt.httpStubs(httpReg) + } + + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(true) + opts := &tt.opts + opts.IO = io + opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: httpReg}, nil + } + opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + } + browser := &cmdutil.TestBrowser{} + opts.Browser = browser + + err := createRun(opts) + if tt.wantsErr == "" { + require.NoError(t, err) + } else { + assert.EqualError(t, err, tt.wantsErr) + return + } + + assert.Equal(t, tt.wantsStdout, stdout.String()) + assert.Equal(t, tt.wantsStderr, stderr.String()) + browser.Verify(t, tt.wantsBrowse) + }) + } +} + +/*** LEGACY TESTS ***/ + func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { return runCommandWithRootDirOverridden(rt, isTTY, cli, "") } @@ -506,84 +657,6 @@ func TestIssueCreate_disabledIssues(t *testing.T) { } } -func TestIssueCreate_web(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - http.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(` - { "data": { - "viewer": { "login": "MonaLisa" } - } } - `), - ) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - output, err := runCommand(http, true, `--web -a @me`) - if err != nil { - t.Errorf("error running command `issue create`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", output.BrowsedURL) -} - -func TestIssueCreate_webLongURL(t *testing.T) { - longBodyFile := filepath.Join(t.TempDir(), "long-body.txt") - err := ioutil.WriteFile(longBodyFile, make([]byte, 9216), 0600) - require.NoError(t, err) - - _, err = runCommand(nil, true, fmt.Sprintf("-F '%s' --web", longBodyFile)) - require.EqualError(t, err, "cannot open in browser: maximum URL length exceeded") -} - -func TestIssueCreate_webTitleBody(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - output, err := runCommand(http, true, `-w -t mytitle -b mybody`) - if err != nil { - t.Errorf("error running command `issue create`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", output.BrowsedURL) -} - -func TestIssueCreate_webTitleBodyAtMeAssignee(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - http.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(` - { "data": { - "viewer": { "login": "MonaLisa" } - } } - `), - ) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - output, err := runCommand(http, true, `-w -t mytitle -b mybody -a @me`) - if err != nil { - t.Errorf("error running command `issue create`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", output.BrowsedURL) -} - func TestIssueCreate_AtMeAssignee(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) @@ -636,41 +709,3 @@ func TestIssueCreate_AtMeAssignee(t *testing.T) { assert.Equal(t, "https://github.com/OWNER/REPO/issues/12\n", output.String()) } - -func TestIssueCreate_webProject(t *testing.T) { - http := &httpmock.Registry{} - defer http.Verify(t) - - http.Register( - httpmock.GraphQL(`query RepositoryProjectList\b`), - httpmock.StringResponse(` - { "data": { "repository": { "projects": { - "nodes": [ - { "name": "Cleanup", "id": "CLEANUPID", "resourcePath": "/OWNER/REPO/projects/1" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - http.Register( - httpmock.GraphQL(`query OrganizationProjectList\b`), - httpmock.StringResponse(` - { "data": { "organization": { "projects": { - "nodes": [ - { "name": "Triage", "id": "TRIAGEID", "resourcePath": "/orgs/ORG/projects/1" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - - _, cmdTeardown := run.Stub() - defer cmdTeardown(t) - - output, err := runCommand(http, true, `-w -t Title -p Cleanup`) - if err != nil { - t.Errorf("error running command `issue create`: %v", err) - } - - assert.Equal(t, "", output.String()) - assert.Equal(t, "Opening github.com/OWNER/REPO/issues/new in your browser.\n", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", output.BrowsedURL) -}