From 3ddd93793c3060ee61907822344b469cbb186921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 10 Feb 2021 17:32:00 +0100 Subject: [PATCH] Port `issue create` to using templates API --- api/cache.go | 2 +- api/queries_issue.go | 32 ---- api/queries_pr.go | 4 +- pkg/cmd/issue/create/create.go | 30 +++- pkg/cmd/issue/create/create_test.go | 22 +-- pkg/cmd/pr/shared/survey.go | 1 + pkg/cmd/pr/shared/templates.go | 252 ++++++++++++++++++++++++++++ 7 files changed, 289 insertions(+), 54 deletions(-) create mode 100644 pkg/cmd/pr/shared/templates.go diff --git a/api/cache.go b/api/cache.go index 1f6d8896b..1c9936d37 100644 --- a/api/cache.go +++ b/api/cache.go @@ -16,7 +16,7 @@ import ( "time" ) -func makeCachedClient(httpClient *http.Client, cacheTTL time.Duration) *http.Client { +func NewCachedClient(httpClient *http.Client, cacheTTL time.Duration) *http.Client { cacheDir := filepath.Join(os.TempDir(), "gh-cli-cache") return &http.Client{ Transport: CacheResponse(cacheTTL, cacheDir)(httpClient.Transport), diff --git a/api/queries_issue.go b/api/queries_issue.go index 3f05c261f..22f08287b 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -23,13 +23,6 @@ type IssuesAndTotalCount struct { TotalCount int } -type IssueTemplate struct { - About string - Body string - Name string - Title string -} - type Issue struct { ID string Number int @@ -495,31 +488,6 @@ func IssueDelete(client *Client, repo ghrepo.Interface, issue Issue) error { return err } -func IssueTemplates(client *Client, repo *Repository) ([]IssueTemplate, error) { - var query struct { - Repository struct { - IssueTemplates []IssueTemplate - } `graphql:"repository(owner: $owner, name: $name)"` - } - - variables := map[string]interface{}{ - "owner": githubv4.String(repo.RepoOwner()), - "name": githubv4.String(repo.RepoName()), - } - - gql := graphQLClient(client.http, repo.RepoHost()) - - err := gql.QueryNamed(context.Background(), "RepositoryIssueTemplates", &query, variables) - if err != nil { - return nil, err - } - if query.Repository.IssueTemplates == nil { - return nil, fmt.Errorf("no issue templates found ") - } - - return query.Repository.IssueTemplates, nil -} - // milestoneNodeIdToDatabaseId extracts the REST Database ID from the GraphQL Node ID // This conversion is necessary since the GraphQL API requires the use of the milestone's database ID // for querying the related issues. diff --git a/api/queries_pr.go b/api/queries_pr.go index 8b526bdc6..3698891d8 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -310,7 +310,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu ReviewRequested edges } - cachedClient := makeCachedClient(client.http, time.Hour*24) + cachedClient := NewCachedClient(client.http, time.Hour*24) prFeatures, err := determinePullRequestFeatures(cachedClient, repo.RepoHost()) if err != nil { return nil, err @@ -479,7 +479,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu } func prCommitsFragment(httpClient *http.Client, hostname string) (string, error) { - cachedClient := makeCachedClient(httpClient, time.Hour*24) + cachedClient := NewCachedClient(httpClient, time.Hour*24) if prFeatures, err := determinePullRequestFeatures(cachedClient, hostname); err != nil { return "", err } else if !prFeatures.HasStatusCheckRollup { diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index ce95c20eb..d2cbd5306 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -25,9 +25,9 @@ type CreateOptions struct { RootDirOverride string - RepoOverride string - WebMode bool - RecoverFile string + HasRepoOverride bool + WebMode bool + RecoverFile string Title string Body string @@ -64,7 +64,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co titleProvided := cmd.Flags().Changed("title") bodyProvided := cmd.Flags().Changed("body") - opts.RepoOverride, _ = cmd.Flags().GetString("repo") + opts.HasRepoOverride = cmd.Flags().Changed("repo") if !opts.IO.CanPrompt() && opts.RecoverFile != "" { return &cmdutil.FlagError{Err: errors.New("--recover only supported when running interactively")} @@ -107,8 +107,6 @@ func createRun(opts *CreateOptions) (err error) { return } - templateFiles, legacyTemplate := prShared.FindTemplates(opts.RootDirOverride, "ISSUE_TEMPLATE") - isTerminal := opts.IO.IsStdoutTTY() var milestones []string @@ -140,6 +138,8 @@ func createRun(opts *CreateOptions) (err error) { } } + tpl := shared.NewTemplateManager(httpClient, baseRepo, opts.RootDirOverride, !opts.HasRepoOverride, false) + if opts.WebMode { openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") if opts.Title != "" || opts.Body != "" || tb.HasMetadata() { @@ -154,7 +154,7 @@ func createRun(opts *CreateOptions) (err error) { if err != nil { return } - } else if len(templateFiles) > 1 { + } else if ok, _ := tpl.HasTemplates(); ok { openURL += "/choose" } if isTerminal { @@ -177,6 +177,7 @@ func createRun(opts *CreateOptions) (err error) { } action := prShared.SubmitAction + templateNameForSubmit := "" if opts.Interactive { var editorCommand string @@ -198,10 +199,20 @@ func createRun(opts *CreateOptions) (err error) { templateContent := "" if opts.RecoverFile == "" { - templateContent, err = prShared.TemplateSurvey(templateFiles, legacyTemplate, tb) + var template shared.Template + template, err = tpl.Choose() if err != nil { return } + + if template != nil { + templateContent = string(template.Body()) + if ok, _ := tpl.HasAPI(); ok { + templateNameForSubmit = template.Name() + } + } else { + templateContent = string(tpl.LegacyBody()) + } } err = prShared.BodySurvey(&tb, templateContent, editorCommand) @@ -271,6 +282,9 @@ func createRun(opts *CreateOptions) (err error) { "title": tb.Title, "body": tb.Body, } + if templateNameForSubmit != "" { + params["issueTemplate"] = templateNameForSubmit + } err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb) if err != nil { diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 3bbec9d05..abffff3fa 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -203,6 +203,16 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { "hasIssuesEnabled": true } } }`), ) + http.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" } + ] } } }`), + ) http.Register( httpmock.GraphQL(`mutation IssueCreate\b`), httpmock.GraphQLMutation(` @@ -220,12 +230,7 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { defer teardown() // template - as.Stub([]*prompt.QuestionStub{ - { - Name: "index", - Value: 1, - }, - }) + as.StubOne(1) // body as.Stub([]*prompt.QuestionStub{ { @@ -283,7 +288,6 @@ func TestIssueCreate_continueInBrowser(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --show-toplevel`, 0, "") cs.Register(`https://github\.com`, 0, "", func(args []string) { url := strings.ReplaceAll(args[len(args)-1], "^", "") assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url) @@ -416,7 +420,6 @@ func TestIssueCreate_web(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --show-toplevel`, 0, "") cs.Register(`https://github\.com`, 0, "", func(args []string) { url := strings.ReplaceAll(args[len(args)-1], "^", "") assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", url) @@ -438,7 +441,6 @@ func TestIssueCreate_webTitleBody(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --show-toplevel`, 0, "") cs.Register(`https://github\.com`, 0, "", func(args []string) { url := strings.ReplaceAll(args[len(args)-1], "^", "") assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle", url) @@ -469,7 +471,6 @@ func TestIssueCreate_webTitleBodyAtMeAssignee(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --show-toplevel`, 0, "") cs.Register(`https://github\.com`, 0, "", func(args []string) { url := strings.ReplaceAll(args[len(args)-1], "^", "") assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=mybody&title=mytitle", url) @@ -565,7 +566,6 @@ func TestIssueCreate_webProject(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git rev-parse --show-toplevel`, 0, "") cs.Register(`https://github\.com`, 0, "", func(args []string) { url := strings.ReplaceAll(args[len(args)-1], "^", "") assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1&title=Title", url) diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 8ef40d047..6c3bdfbe9 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -74,6 +74,7 @@ func ConfirmSubmission(allowPreview bool, allowMetadata bool) (Action, error) { } } +// Deprecated: use SelectTemplate instead func TemplateSurvey(templateFiles []string, legacyTemplate string, state IssueMetadataState) (templateContent string, err error) { if len(templateFiles) == 0 && legacyTemplate == "" { return diff --git a/pkg/cmd/pr/shared/templates.go b/pkg/cmd/pr/shared/templates.go new file mode 100644 index 000000000..7c5871431 --- /dev/null +++ b/pkg/cmd/pr/shared/templates.go @@ -0,0 +1,252 @@ +package shared + +import ( + "context" + "fmt" + "net/http" + "time" + + "github.com/AlecAivazis/survey/v2" + "github.com/cli/cli/api" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/githubtemplate" + "github.com/cli/cli/pkg/prompt" + "github.com/shurcooL/githubv4" + "github.com/shurcooL/graphql" +) + +type issueTemplate struct { + // I would have un-exported these fields, except `shurcool/graphql` then cannot unmarshal them :/ + Gname string `graphql:"name"` + Gbody string `graphql:"body"` +} + +func (t *issueTemplate) Name() string { + return t.Gname +} + +func (t *issueTemplate) Body() []byte { + return []byte(t.Gbody) +} + +func listIssueTemplates(httpClient *http.Client, repo ghrepo.Interface) ([]issueTemplate, error) { + var query struct { + Repository struct { + IssueTemplates []issueTemplate + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "name": githubv4.String(repo.RepoName()), + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient) + + err := gql.QueryNamed(context.Background(), "IssueTemplates", &query, variables) + if err != nil { + return nil, err + } + + return query.Repository.IssueTemplates, nil +} + +func hasIssueTemplateSupport(httpClient *http.Client, hostname string) (bool, error) { + if !ghinstance.IsEnterprise(hostname) { + return true, nil + } + + var featureDetection struct { + Repository struct { + Fields []struct { + Name string + } `graphql:"fields(includeDeprecated: true)"` + } `graphql:"Repository: __type(name: \"Repository\")"` + CreateIssueInput struct { + InputFields []struct { + Name string + } + } `graphql:"CreateIssueInput: __type(name: \"CreateIssueInput\")"` + } + + gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), httpClient) + err := gql.QueryNamed(context.Background(), "IssueTemplates_fields", &featureDetection, nil) + if err != nil { + return false, err + } + + var hasQuerySupport bool + var hasMutationSupport bool + for _, field := range featureDetection.Repository.Fields { + if field.Name == "issueTemplates" { + hasQuerySupport = true + } + } + for _, field := range featureDetection.CreateIssueInput.InputFields { + if field.Name == "issueTemplate" { + hasMutationSupport = true + } + } + + return hasQuerySupport && hasMutationSupport, nil +} + +type Template interface { + Name() string + Body() []byte +} + +type templateManager struct { + repo ghrepo.Interface + rootDir string + allowFS bool + isPR bool + httpClient *http.Client + + cachedClient *http.Client + templates []Template + legacyTemplate Template + + didFetch bool + fetchError error +} + +func NewTemplateManager(httpClient *http.Client, repo ghrepo.Interface, dir string, allowFS bool, isPR bool) *templateManager { + return &templateManager{ + repo: repo, + rootDir: dir, + allowFS: allowFS, + isPR: isPR, + httpClient: httpClient, + } +} + +func (m *templateManager) HasAPI() (bool, error) { + if m.isPR { + return false, nil + } + if m.cachedClient == nil { + m.cachedClient = api.NewCachedClient(m.httpClient, time.Hour*24) + } + return hasIssueTemplateSupport(m.cachedClient, m.repo.RepoHost()) +} + +func (m *templateManager) HasTemplates() (bool, error) { + if err := m.memoizedFetch(); err != nil { + return false, err + } + return len(m.templates) > 0, nil +} + +func (m *templateManager) LegacyBody() []byte { + if m.legacyTemplate == nil { + return nil + } + return m.legacyTemplate.Body() +} + +func (m *templateManager) Choose() (Template, error) { + if err := m.memoizedFetch(); err != nil { + return nil, err + } + if len(m.templates) == 0 { + return nil, nil + } + + names := make([]string, len(m.templates)) + for i, t := range m.templates { + names[i] = t.Name() + } + + blankOption := "Open a blank issue" + if m.isPR { + blankOption = "Open a blank pull request" + } + + var selectedOption int + err := prompt.SurveyAskOne(&survey.Select{ + Message: "Choose a template", + Options: append(names, blankOption), + }, &selectedOption) + if err != nil { + return nil, fmt.Errorf("could not prompt: %w", err) + } + + if selectedOption == len(names) { + return nil, nil + } + return m.templates[selectedOption], nil +} + +func (m *templateManager) memoizedFetch() error { + if m.didFetch { + return m.fetchError + } + m.fetchError = m.fetch() + m.didFetch = true + return m.fetchError +} + +func (m *templateManager) fetch() error { + hasAPI, err := m.HasAPI() + if err != nil { + return err + } + + if hasAPI { + issueTemplates, err := listIssueTemplates(m.httpClient, m.repo) + if err != nil { + return err + } + m.templates = make([]Template, len(issueTemplates)) + for i := range issueTemplates { + m.templates[i] = &issueTemplates[i] + } + } + + if !m.allowFS { + return nil + } + + dir := m.rootDir + if dir == "" { + var err error + dir, err = git.ToplevelDir() + if err != nil { + return nil // abort silently + } + } + + filePattern := "ISSUE_TEMPLATE" + if m.isPR { + filePattern = "PULL_REQUEST_TEMPLATE" + } + + if !hasAPI { + issueTemplates := githubtemplate.FindNonLegacy(dir, filePattern) + m.templates = make([]Template, len(issueTemplates)) + for i, t := range issueTemplates { + m.templates[i] = &filesystemTemplate{path: t} + } + } + + if legacyTemplate := githubtemplate.FindLegacy(dir, filePattern); legacyTemplate != "" { + m.legacyTemplate = &filesystemTemplate{path: legacyTemplate} + } + + return nil +} + +type filesystemTemplate struct { + path string +} + +func (t *filesystemTemplate) Name() string { + return githubtemplate.ExtractName(t.path) +} + +func (t *filesystemTemplate) Body() []byte { + return githubtemplate.ExtractContents(t.path) +}