preserve and restore issue/pr input on failure

This commit is contained in:
vilmibm 2020-11-13 10:11:39 -08:00 committed by vilmibm
parent e92cd43259
commit d300526318
10 changed files with 457 additions and 71 deletions

View file

@ -26,6 +26,8 @@ type CreateOptions struct {
RepoOverride string
WebMode bool
JSONFill bool
JSONInput string
Title string
Body string
@ -62,6 +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.JSONFill = cmd.Flags().Changed("json")
opts.Interactive = !(titleProvided && bodyProvided)
@ -69,6 +72,14 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
return &cmdutil.FlagError{Err: errors.New("must provide --title and --body when not running interactively")}
}
if opts.JSONFill {
opts.Interactive = false
if opts.WebMode {
return errors.New("--web and --json are mutually exclusive")
}
}
if runF != nil {
return runF(opts)
}
@ -83,20 +94,21 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`")
cmd.Flags().StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the issue to projects by `name`")
cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`")
cmd.Flags().StringVarP(&opts.JSONInput, "json", "j", "", "Use JSON to populate and submit issue")
return cmd
}
func createRun(opts *CreateOptions) error {
func createRun(opts *CreateOptions) (err error) {
httpClient, err := opts.HttpClient()
if err != nil {
return err
return
}
apiClient := api.NewClientFromHTTP(httpClient)
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
return
}
templateFiles, legacyTemplate := prShared.FindTemplates(opts.RootDirOverride, "ISSUE_TEMPLATE")
@ -123,7 +135,7 @@ func createRun(opts *CreateOptions) error {
if opts.Title != "" || opts.Body != "" {
openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb)
if err != nil {
return err
return
}
} else if len(templateFiles) > 1 {
openURL += "/choose"
@ -140,24 +152,28 @@ func createRun(opts *CreateOptions) error {
repo, err := api.GitHubRepo(apiClient, baseRepo)
if err != nil {
return err
return
}
if !repo.HasIssuesEnabled {
return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo))
err = fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo))
return
}
action := prShared.SubmitAction
if opts.Interactive {
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
var editorCommand string
editorCommand, err = cmdutil.DetermineEditor(opts.Config)
if err != nil {
return err
return
}
defer prShared.PreserveInput(opts.IO, &tb, &err)()
if tb.Title == "" {
err = prShared.TitleSurvey(&tb)
if err != nil {
return err
return
}
}
@ -166,12 +182,12 @@ func createRun(opts *CreateOptions) error {
templateContent, err = prShared.TemplateSurvey(templateFiles, legacyTemplate, tb)
if err != nil {
return err
return
}
err = prShared.BodySurvey(&tb, templateContent, editorCommand)
if err != nil {
return err
return
}
if tb.Body == "" {
@ -179,31 +195,40 @@ func createRun(opts *CreateOptions) error {
}
}
action, err := prShared.ConfirmSubmission(!tb.HasMetadata(), repo.ViewerCanTriage())
var action prShared.Action
action, err = prShared.ConfirmSubmission(!tb.HasMetadata(), repo.ViewerCanTriage())
if err != nil {
return fmt.Errorf("unable to confirm: %w", err)
err = fmt.Errorf("unable to confirm: %w", err)
return
}
if action == prShared.MetadataAction {
err = prShared.MetadataSurvey(opts.IO, apiClient, baseRepo, &tb)
if err != nil {
return err
return
}
action, err = prShared.ConfirmSubmission(!tb.HasMetadata(), false)
if err != nil {
return err
return
}
}
if action == prShared.CancelAction {
fmt.Fprintln(opts.IO.ErrOut, "Discarding.")
return nil
return
}
} else if opts.JSONFill {
err = prShared.FillFromJSON(opts.IO, opts.JSONInput, &tb)
if err != nil {
return
}
action = prShared.SubmitAction
} else {
if tb.Title == "" {
return fmt.Errorf("title can't be blank")
err = fmt.Errorf("title can't be blank")
return
}
}
@ -211,7 +236,7 @@ func createRun(opts *CreateOptions) error {
openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new")
openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb)
if err != nil {
return err
return
}
if isTerminal {
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL))
@ -225,12 +250,13 @@ func createRun(opts *CreateOptions) error {
err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb)
if err != nil {
return err
return
}
newIssue, err := api.IssueCreate(apiClient, repo, params)
var newIssue *api.Issue
newIssue, err = api.IssueCreate(apiClient, repo, params)
if err != nil {
return err
return
}
fmt.Fprintln(opts.IO.Out, newIssue.URL)
@ -238,5 +264,5 @@ func createRun(opts *CreateOptions) error {
panic("Unreachable state")
}
return nil
return
}

View file

@ -126,6 +126,46 @@ func TestIssueCreate(t *testing.T) {
eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n")
}
func TestIssueCreate_JSON(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true
} } }
`))
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "createIssue": { "issue": {
"URL": "https://github.com/OWNER/REPO/issues/12"
} } } }
`))
output, err := runCommand(http, true, `-j'{"title":"cool", "body":"issue"}'`)
if err != nil {
t.Errorf("error running command `issue create`: %v", err)
}
bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body)
reqBody := struct {
Variables struct {
Input struct {
RepositoryID string
Title string
Body string
}
}
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
eq(t, reqBody.Variables.Input.RepositoryID, "REPOID")
eq(t, reqBody.Variables.Input.Title, "cool")
eq(t, reqBody.Variables.Input.Body, "issue")
eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n")
}
func TestIssueCreate_nonLegacyTemplate(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)

View file

@ -38,8 +38,10 @@ type CreateOptions struct {
RootDirOverride string
RepoOverride string
Autofill bool
WebMode bool
Autofill bool
WebMode bool
JSONFill bool
JSONInput string
IsDraft bool
Title string
@ -99,11 +101,12 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
`),
Args: cmdutil.NoArgsQuoteReminder,
RunE: func(cmd *cobra.Command, args []string) error {
opts.JSONFill = cmd.Flags().Changed("json")
opts.TitleProvided = cmd.Flags().Changed("title")
opts.BodyProvided = cmd.Flags().Changed("body")
opts.RepoOverride, _ = cmd.Flags().GetString("repo")
if !opts.IO.CanPrompt() && !opts.WebMode && !opts.TitleProvided && !opts.Autofill {
if !opts.IO.CanPrompt() && !opts.JSONFill && !opts.WebMode && !opts.TitleProvided && !opts.Autofill {
return &cmdutil.FlagError{Err: errors.New("--title or --fill required when not running interactively")}
}
@ -114,6 +117,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
return errors.New("the --reviewer flag is not supported with --web")
}
if opts.JSONFill && opts.WebMode {
return errors.New("--web and --json are mutually exclusive")
}
if runF != nil {
return runF(opts)
}
@ -134,6 +141,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
fl.StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`")
fl.StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the pull request to projects by `name`")
fl.StringVarP(&opts.Milestone, "milestone", "m", "", "Add the pull request to a milestone by `name`")
fl.StringVarP(&opts.JSONInput, "json", "j", "", "Use JSON to populate and submit PR")
return cmd
}
@ -141,14 +149,14 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
func createRun(opts *CreateOptions) (err error) {
ctx, err := NewCreateContext(opts)
if err != nil {
return err
return
}
client := ctx.Client
state, err := NewIssueState(*ctx, *opts)
if err != nil {
return err
return
}
if opts.WebMode {
@ -156,9 +164,9 @@ func createRun(opts *CreateOptions) (err error) {
state.Title = opts.Title
state.Body = opts.Body
}
err := handlePush(*opts, *ctx)
err = handlePush(*opts, *ctx)
if err != nil {
return err
return
}
return previewPR(*opts, *ctx, *state)
}
@ -199,35 +207,51 @@ func createRun(opts *CreateOptions) (err error) {
if opts.Autofill || (opts.TitleProvided && opts.BodyProvided) {
err = handlePush(*opts, *ctx)
if err != nil {
return err
return
}
return submitPR(*opts, *ctx, *state)
}
if opts.JSONFill {
err = shared.FillFromJSON(opts.IO, opts.JSONInput, state)
if err != nil {
return fmt.Errorf("could not use JSON input: %w", err)
}
err = handlePush(*opts, *ctx)
if err != nil {
return
}
return submitPR(*opts, *ctx, *state)
}
if !opts.TitleProvided {
err = shared.TitleSurvey(state)
if err != nil {
return err
return
}
}
editorCommand, err := cmdutil.DetermineEditor(opts.Config)
if err != nil {
return err
return
}
defer shared.PreserveInput(opts.IO, state, &err)()
templateContent := ""
if !opts.BodyProvided {
templateFiles, legacyTemplate := shared.FindTemplates(opts.RootDirOverride, "PULL_REQUEST_TEMPLATE")
templateContent, err = shared.TemplateSurvey(templateFiles, legacyTemplate, *state)
if err != nil {
return err
return
}
err = shared.BodySurvey(state, templateContent, editorCommand)
if err != nil {
return err
return
}
if state.Body == "" {
@ -244,12 +268,12 @@ func createRun(opts *CreateOptions) (err error) {
if action == shared.MetadataAction {
err = shared.MetadataSurvey(opts.IO, client, ctx.BaseRepo, state)
if err != nil {
return err
return
}
action, err = shared.ConfirmSubmission(!state.HasMetadata(), false)
if err != nil {
return err
return
}
}
@ -260,7 +284,7 @@ func createRun(opts *CreateOptions) (err error) {
err = handlePush(*opts, *ctx)
if err != nil {
return err
return
}
if action == shared.PreviewAction {
@ -271,7 +295,8 @@ func createRun(opts *CreateOptions) (err error) {
return submitPR(*opts, *ctx, *state)
}
return errors.New("expected to cancel, preview, or submit")
err = errors.New("expected to cancel, preview, or submit")
return
}
func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) error {

View file

@ -136,6 +136,54 @@ func TestPRCreate_nontty_insufficient_flags(t *testing.T) {
assert.Equal(t, "", output.String())
}
func TestPRCreate_json(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
http.StubRepoInfoResponse("OWNER", "REPO", "master")
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "repository": { "pullRequests": { "nodes" : [
] } } } }
`))
http.StubResponse(200, bytes.NewBufferString(`
{ "data": { "createPullRequest": { "pullRequest": {
"URL": "https://github.com/OWNER/REPO/pull/12"
} } } }
`))
cs, cmdTeardown := test.InitCmdStubber()
defer cmdTeardown()
cs.Stub("") // git status
cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log
output, err := runCommand(http, nil, "feature", false, `-j'{"title":"cool", "body":"pr"}' -H feature`)
require.NoError(t, err)
bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body)
reqBody := struct {
Variables struct {
Input struct {
RepositoryID string
Title string
Body string
BaseRefName string
HeadRefName string
}
}
}{}
_ = json.Unmarshal(bodyBytes, &reqBody)
assert.Equal(t, "REPOID", reqBody.Variables.Input.RepositoryID)
assert.Equal(t, "cool", reqBody.Variables.Input.Title)
assert.Equal(t, "pr", reqBody.Variables.Input.Body)
assert.Equal(t, "master", reqBody.Variables.Input.BaseRefName)
assert.Equal(t, "feature", reqBody.Variables.Input.HeadRefName)
assert.Equal(t, "", output.Stderr())
assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String())
}
func TestPRCreate_nontty(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)

View file

@ -0,0 +1,66 @@
package shared
import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"time"
"github.com/cli/cli/pkg/iostreams"
)
func dumpPath(random int64) string {
r := fmt.Sprintf("%x", random)
r = r[len(r)-5:]
dumpFilename := fmt.Sprintf("gh%s.json", r)
return filepath.Join(os.TempDir(), dumpFilename)
}
func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr *error) func() {
return func() {
if !state.IsDirty() {
return
}
if *createErr == nil {
return
}
out := io.ErrOut
// this extra newline guards against appending to the end of a survey line
fmt.Fprintln(out)
data, err := json.Marshal(state)
if err != nil {
fmt.Fprintf(out, "failed to save input to file: %s\n", err)
fmt.Fprintln(out, "would have saved:")
fmt.Fprintf(out, "%v\n", state)
return
}
dp := dumpPath(time.Now().UnixNano())
err = io.WriteFile(dp, data)
if err != nil {
fmt.Fprintf(out, "failed to save input to file: %s\n", err)
fmt.Fprintln(out, "would have saved:")
fmt.Fprintln(out, string(data))
return
}
cs := io.ColorScheme()
issueType := "pr"
if state.Type == IssueMetadata {
issueType = "issue"
}
fmt.Fprintf(out, "%s operation failed. input saved to: %s\n", cs.FailureIcon(), dp)
fmt.Fprintf(out, "resubmit with: gh %s create -j@%s\n", issueType, dp)
// some whitespace before the actual error
fmt.Fprintln(out)
}
}

View file

@ -0,0 +1,114 @@
package shared
import (
"encoding/json"
"errors"
"os"
"testing"
"github.com/cli/cli/pkg/iostreams"
"github.com/cli/cli/test"
"github.com/stretchr/testify/assert"
)
func Test_dumpPath(t *testing.T) {
// mostly pointless test
var random int64 = 1234567890
tempDir := os.TempDir()
assert.Equal(t, dumpPath(random), tempDir+"/gh602d2.json")
}
func Test_PreserveInput(t *testing.T) {
tests := []struct {
name string
state *IssueMetadataState
err bool
wantErrLines []string
wantPreservation bool
}{
{
name: "err, no changes to state",
err: true,
},
{
name: "no err, no changes to state",
err: false,
},
{
name: "no err, changes to state",
state: &IssueMetadataState{
dirty: true,
},
},
{
name: "err, title/body input received",
state: &IssueMetadataState{
dirty: true,
Title: "almost a",
Body: "jill sandwich",
Reviewers: []string{"barry", "chris"},
Labels: []string{"sandwich"},
},
wantErrLines: []string{
`X operation failed. input saved to:.*\.json`,
`resubmit with: gh issue create -j@.*\.json`,
},
err: true,
wantPreservation: true,
},
{
name: "err, metadata received",
state: &IssueMetadataState{
Reviewers: []string{"barry", "chris"},
Labels: []string{"sandwich"},
},
wantErrLines: []string{
`X operation failed. input saved to:.*\.json`,
`resubmit with: gh issue create -j@.*\.json`,
},
err: true,
wantPreservation: true,
},
{
name: "err, dirty, pull request",
state: &IssueMetadataState{
dirty: true,
Title: "a pull request",
Type: PRMetadata,
},
wantErrLines: []string{
`X operation failed. input saved to:.*\.json`,
`resubmit with: gh pr create -j@.*\.json`,
},
err: true,
wantPreservation: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.state == nil {
tt.state = &IssueMetadataState{}
}
io, _, _, errOut := iostreams.Test()
io.WriteOverride = []byte{}
var err error
if tt.err {
err = errors.New("error during creation")
}
PreserveInput(io, tt.state, &err)()
if tt.wantPreservation {
test.ExpectLines(t, errOut.String(), tt.wantErrLines...)
preserved := &IssueMetadataState{}
assert.NoError(t, json.Unmarshal(io.WriteOverride, preserved))
preserved.dirty = tt.state.dirty
assert.Equal(t, preserved, tt.state)
} else {
assert.Equal(t, errOut.String(), "")
assert.Equal(t, string(io.WriteOverride), "")
}
})
}
}

View file

@ -0,0 +1,73 @@
package shared
import (
"encoding/json"
"fmt"
"strings"
"github.com/cli/cli/api"
"github.com/cli/cli/pkg/iostreams"
)
type metadataStateType int
const (
IssueMetadata metadataStateType = iota
PRMetadata
)
type IssueMetadataState struct {
Type metadataStateType
Draft bool
Body string
Title string
Metadata []string
Reviewers []string
Assignees []string
Labels []string
Projects []string
Milestones []string
MetadataResult *api.RepoMetadataResult
dirty bool // whether user i/o has modified this
}
func (tb *IssueMetadataState) MarkDirty() {
tb.dirty = true
}
func (tb *IssueMetadataState) IsDirty() bool {
return tb.dirty || tb.HasMetadata()
}
func (tb *IssueMetadataState) HasMetadata() bool {
return len(tb.Reviewers) > 0 ||
len(tb.Assignees) > 0 ||
len(tb.Labels) > 0 ||
len(tb.Projects) > 0 ||
len(tb.Milestones) > 0
}
func FillFromJSON(io *iostreams.IOStreams, JSONInput string, state *IssueMetadataState) error {
var data []byte
var err error
if strings.HasPrefix(JSONInput, "@") {
data, err = io.ReadUserFile(JSONInput[1:])
if err != nil {
return fmt.Errorf("failed to read file %s: %w", JSONInput[1:], err)
}
} else {
data = []byte(JSONInput)
}
err = json.Unmarshal(data, state)
if err != nil {
return fmt.Errorf("JSON parsing failure: %w", err)
}
return nil
}

View file

@ -16,38 +16,6 @@ import (
)
type Action int
type metadataStateType int
const (
IssueMetadata metadataStateType = iota
PRMetadata
)
type IssueMetadataState struct {
Type metadataStateType
Draft bool
Body string
Title string
Metadata []string
Reviewers []string
Assignees []string
Labels []string
Projects []string
Milestones []string
MetadataResult *api.RepoMetadataResult
}
func (tb *IssueMetadataState) HasMetadata() bool {
return len(tb.Reviewers) > 0 ||
len(tb.Assignees) > 0 ||
len(tb.Labels) > 0 ||
len(tb.Projects) > 0 ||
len(tb.Milestones) > 0
}
const (
SubmitAction Action = iota
@ -170,6 +138,8 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string
state.Body += templateContent
}
preBody := state.Body
// TODO should just be an AskOne but ran into problems with the stubber
qs := []*survey.Question{
{
@ -193,10 +163,16 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string
return err
}
if state.Body != "" && preBody != state.Body {
state.MarkDirty()
}
return nil
}
func TitleSurvey(state *IssueMetadataState) error {
preTitle := state.Title
// TODO should just be an AskOne but ran into problems with the stubber
qs := []*survey.Question{
{
@ -213,6 +189,10 @@ func TitleSurvey(state *IssueMetadataState) error {
return err
}
if preTitle != state.Title {
state.MarkDirty()
}
return nil
}

View file

@ -122,6 +122,10 @@ func (c *ColorScheme) WarningIcon() string {
return c.Yellow("!")
}
func (c *ColorScheme) FailureIcon() string {
return c.Red("X")
}
func (c *ColorScheme) ColorFromString(s string) func(string) string {
s = strings.ToLower(s)
var fn func(string) string

View file

@ -45,6 +45,8 @@ type IOStreams struct {
pagerProcess *os.Process
neverPrompt bool
WriteOverride []byte
}
func (s *IOStreams) ColorEnabled() bool {
@ -268,6 +270,14 @@ func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) {
return ioutil.ReadAll(r)
}
func (s *IOStreams) WriteFile(fn string, data []byte) error {
if s.WriteOverride != nil {
s.WriteOverride = data
return nil
}
return ioutil.WriteFile(fn, data, 0660)
}
func System() *IOStreams {
stdoutIsTTY := isTerminal(os.Stdout)
stderrIsTTY := isTerminal(os.Stderr)