Skip to content

Commit

Permalink
feat: Save PR message if creation fails (#113)
Browse files Browse the repository at this point in the history
Saves the pull request description (entered via editor) to a temporary file if creating the pull request fails. It will re-use the title/description if the user tries to re-create the PR (also just logs the file path to the console in case they want that).

The most common cause of this error (in my experience) is expired auth tokens and/or the base branch of a stack having been deleted on GitHub. (we should provide a better experience for the latter but that's outside the scope of this PR)
  • Loading branch information
twavv authored May 15, 2023
1 parent 7d8e52e commit f5b183a
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 8 deletions.
2 changes: 1 addition & 1 deletion cmd/av/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func getRepo() (*git.Repo, error) {
}

func getDB(repo *git.Repo) (meta.DB, error) {
dbPath := path.Join(repo.GitDir(), "av", "av.db")
dbPath := path.Join(repo.AvDir(), "av.db")
existingStat, _ := os.Stat(dbPath)
db, err := jsonfiledb.OpenPath(dbPath)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cmd/av/stack_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ const stackSyncStateFile = "stack-sync.state.json"

func readStackSyncState(repo *git.Repo) (stackSyncState, error) {
var state stackSyncState
data, err := os.ReadFile(path.Join(repo.GitDir(), "av", stackSyncStateFile))
data, err := os.ReadFile(path.Join(repo.AvDir(), stackSyncStateFile))
if err != nil {
return state, err
}
Expand All @@ -348,7 +348,7 @@ func readStackSyncState(repo *git.Repo) (stackSyncState, error) {
}

func writeStackSyncState(repo *git.Repo, state *stackSyncState) error {
avDir := path.Join(repo.GitDir(), "av")
avDir := repo.AvDir()
if _, err := os.Stat(avDir); err != nil {
if !os.IsNotExist(err) {
return err
Expand Down
50 changes: 46 additions & 4 deletions internal/actions/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/aviator-co/av/internal/utils/sanitize"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -234,8 +235,25 @@ func CreatePullRequest(
commits = append(commits, *commit)
}

// We need to open an editor to ask the user. Try to populate with
// reasonable defaults here.
// If a saved pull request description exists, use that.
saveFile := filepath.Join(repo.AvTmpDir(), fmt.Sprintf("av-pr-%s.md", sanitize.FileName(opts.BranchName)))
if _, err := os.Stat(saveFile); err == nil {
contents, err := os.ReadFile(saveFile)
if err != nil {
logrus.WithError(err).Warn("failed to read saved pull request description")
} else {
title, body := stringutils.ParseSubjectBody(string(contents))
if opts.Title == "" {
opts.Title = title
}
if opts.Body == "" {
opts.Body = body
}
}
}

// Try to populate the editor text using contextual information from the
// repository and commits included in this pull request.
if opts.Title == "" {
opts.Title = commits[0].Subject
}
Expand All @@ -255,6 +273,7 @@ func CreatePullRequest(
Body: opts.Body,
Commits: commits,
})

res, err := editor.Launch(repo, editor.Config{
Text: editorText,
TmpFilePattern: "pr-*.md",
Expand All @@ -267,6 +286,26 @@ func CreatePullRequest(
if opts.Title == "" {
return nil, errors.New("aborting pull request due to empty message")
}

defer func() {
// If we created the PR successfully, just make sure to clean up any
// lingering files.
if reterr == nil {
_ = os.Remove(saveFile)
return
}

// Otherwise, save what the user entered to a file so that it's not
// lost forever (and we can re-use it if they try again).
if err := os.WriteFile(saveFile, []byte(res), 0644); err != nil {
logrus.WithError(err).Error("failed to write pull request description to temporary file")
return
}
_, _ = fmt.Fprint(os.Stderr,
" - saved pull request description to ", colors.UserInput(saveFile),
" (it will be automatically re-used if you try again)\n",
)
}()
}

prMeta, err := getPRMetadata(tx, branchMeta, &parentMeta)
Expand All @@ -284,6 +323,9 @@ func CreatePullRequest(
existingPR: existingPR,
})
if err != nil {
_, _ = fmt.Fprint(os.Stderr,
colors.Failure(" - failed to create pull request: "), err, "\n",
)
return nil, errors.WrapIf(err, "failed to create PR")
}

Expand Down Expand Up @@ -382,7 +424,7 @@ func ensurePR(ctx context.Context, client *gh.Client, repoMeta meta.Repository,
Body: gh.Ptr(githubv4.String(newBody)),
})
if err != nil {
return nil, false, errors.WrapIf(err, "updating PR")
return nil, false, errors.WithStack(err)
}
return updatedPR, false, nil
}
Expand All @@ -395,7 +437,7 @@ func ensurePR(ctx context.Context, client *gh.Client, repoMeta meta.Repository,
Draft: gh.Ptr(githubv4.Boolean(opts.draft)),
})
if err != nil {
return nil, false, errors.WrapIf(err, "opening pull request")
return nil, false, errors.WithStack(err)
}
return pull, true, nil
}
Expand Down
12 changes: 12 additions & 0 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ func (r *Repo) GitDir() string {
return path.Join(r.repoDir, ".git")
}

func (r *Repo) AvDir() string {
return path.Join(r.GitDir(), "av")
}

func (r *Repo) AvTmpDir() string {
dir := path.Join(r.AvDir(), "tmp")
// Try to create the directory, but swallow the error since it will
// ultimately be surfaced when trying to create a file in the directory.
_ = os.MkdirAll(dir, 0755)
return dir
}

func (r *Repo) DefaultBranch() (string, error) {
ref, err := r.Git("symbolic-ref", "refs/remotes/origin/HEAD")
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/meta/jsonfiledb/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type DB struct {
}

func RepoPath(repo *git.Repo) string {
return path.Join(repo.GitDir(), "av", "av.db")
return path.Join(repo.AvDir(), "av.db")
}

func OpenRepo(repo *git.Repo) (*DB, error) {
Expand Down
24 changes: 24 additions & 0 deletions internal/utils/sanitize/filename.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package sanitize

import (
"regexp"
"strings"
)

const (
fileNameMax = 100
)

var (
filenameReplacePattern = regexp.MustCompile(`[^a-zA-Z0-9]+`)
)

func FileName(name string) string {
name = strings.ToLower(name)
name = filenameReplacePattern.ReplaceAllString(name, "-")
if len(name) > fileNameMax {
name = name[:fileNameMax]
}
name = strings.Trim(name, "-")
return name
}
25 changes: 25 additions & 0 deletions internal/utils/sanitize/filename_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package sanitize

import (
"fmt"
"testing"
)

func TestFileName(t *testing.T) {
for _, tt := range []struct{ Input, Output string }{
{"", ""},
{" hello world ", "hello-world"},
{"hello world", "hello-world"},
{"hello-world", "hello-world"},
{"hello_world", "hello-world"},
{"hello-world-123", "hello-world-123"},
{"hello-/-world-123!!!", "hello-world-123"},
} {
name := fmt.Sprintf("%q=>%q", tt.Input, tt.Output)
t.Run(name, func(t *testing.T) {
if got := FileName(tt.Input); got != tt.Output {
t.Errorf("FileName() = %q, want %q", got, tt.Output)
}
})
}
}

0 comments on commit f5b183a

Please sign in to comment.