Merge pull request #8826 from lucasmelin/lucasmelin/add-pr-revert-command
feat: implement gh `pr revert`
This commit is contained in:
Kynan Ware
GitHub
commit
b90e39ea91
4 changed files with 490 additions and 0 deletions
|
|
@ -764,6 +764,37 @@ func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) er
|
|||
return client.Mutate(repo.RepoHost(), "PullRequestReadyForReview", &mutation, variables)
|
||||
}
|
||||
|
||||
func PullRequestRevert(client *Client, repo ghrepo.Interface, params githubv4.RevertPullRequestInput) (*PullRequest, error) {
|
||||
var mutation struct {
|
||||
RevertPullRequest struct {
|
||||
PullRequest struct {
|
||||
ID githubv4.ID
|
||||
}
|
||||
RevertPullRequest struct {
|
||||
ID string
|
||||
Number int
|
||||
URL string
|
||||
}
|
||||
} `graphql:"revertPullRequest(input: $input)"`
|
||||
}
|
||||
|
||||
variables := map[string]interface{}{
|
||||
"input": params,
|
||||
}
|
||||
err := client.Mutate(repo.RepoHost(), "PullRequestRevert", &mutation, variables)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
pr := &mutation.RevertPullRequest.RevertPullRequest
|
||||
revertPR := &PullRequest{
|
||||
ID: pr.ID,
|
||||
Number: pr.Number,
|
||||
URL: pr.URL,
|
||||
}
|
||||
|
||||
return revertPR, nil
|
||||
}
|
||||
|
||||
func ConvertPullRequestToDraft(client *Client, repo ghrepo.Interface, pr *PullRequest) error {
|
||||
var mutation struct {
|
||||
ConvertPullRequestToDraft struct {
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import (
|
|||
cmdMerge "github.com/cli/cli/v2/pkg/cmd/pr/merge"
|
||||
cmdReady "github.com/cli/cli/v2/pkg/cmd/pr/ready"
|
||||
cmdReopen "github.com/cli/cli/v2/pkg/cmd/pr/reopen"
|
||||
cmdRevert "github.com/cli/cli/v2/pkg/cmd/pr/revert"
|
||||
cmdReview "github.com/cli/cli/v2/pkg/cmd/pr/review"
|
||||
cmdStatus "github.com/cli/cli/v2/pkg/cmd/pr/status"
|
||||
cmdUpdateBranch "github.com/cli/cli/v2/pkg/cmd/pr/update-branch"
|
||||
|
|
@ -63,6 +64,7 @@ func NewCmdPR(f *cmdutil.Factory) *cobra.Command {
|
|||
cmdComment.NewCmdComment(f, nil),
|
||||
cmdClose.NewCmdClose(f, nil),
|
||||
cmdReopen.NewCmdReopen(f, nil),
|
||||
cmdRevert.NewCmdRevert(f, nil),
|
||||
cmdEdit.NewCmdEdit(f, nil),
|
||||
cmdLock.NewCmdLock(f, cmd.Name(), nil),
|
||||
cmdLock.NewCmdUnlock(f, cmd.Name(), nil),
|
||||
|
|
|
|||
132
pkg/cmd/pr/revert/revert.go
Normal file
132
pkg/cmd/pr/revert/revert.go
Normal file
|
|
@ -0,0 +1,132 @@
|
|||
package revert
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
"github.com/cli/cli/v2/api"
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
|
||||
"github.com/cli/cli/v2/pkg/cmdutil"
|
||||
"github.com/cli/cli/v2/pkg/iostreams"
|
||||
"github.com/shurcooL/githubv4"
|
||||
"github.com/spf13/cobra"
|
||||
)
|
||||
|
||||
type RevertOptions struct {
|
||||
HttpClient func() (*http.Client, error)
|
||||
IO *iostreams.IOStreams
|
||||
|
||||
Finder shared.PRFinder
|
||||
|
||||
SelectorArg string
|
||||
|
||||
Body string
|
||||
BodySet bool
|
||||
Title string
|
||||
IsDraft bool
|
||||
}
|
||||
|
||||
func NewCmdRevert(f *cmdutil.Factory, runF func(*RevertOptions) error) *cobra.Command {
|
||||
opts := &RevertOptions{
|
||||
IO: f.IOStreams,
|
||||
HttpClient: f.HttpClient,
|
||||
}
|
||||
|
||||
var bodyFile string
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Use: "revert {<number> | <url> | <branch>}",
|
||||
Short: "Revert a pull request",
|
||||
Args: cmdutil.ExactArgs(1, "cannot revert pull request: number, url, or branch required"),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
opts.Finder = shared.NewFinder(f)
|
||||
|
||||
if len(args) > 0 {
|
||||
opts.SelectorArg = args[0]
|
||||
}
|
||||
|
||||
bodyProvided := cmd.Flags().Changed("body")
|
||||
bodyFileProvided := bodyFile != ""
|
||||
|
||||
if err := cmdutil.MutuallyExclusive(
|
||||
"specify only one of `--body` or `--body-file`",
|
||||
bodyProvided,
|
||||
bodyFileProvided,
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if bodyProvided || bodyFileProvided {
|
||||
opts.BodySet = true
|
||||
if bodyFileProvided {
|
||||
b, err := cmdutil.ReadFile(bodyFile, opts.IO.In)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
opts.Body = string(b)
|
||||
}
|
||||
}
|
||||
|
||||
if runF != nil {
|
||||
return runF(opts)
|
||||
}
|
||||
return revertRun(opts)
|
||||
},
|
||||
}
|
||||
|
||||
cmd.Flags().BoolVarP(&opts.IsDraft, "draft", "d", false, "Mark revert pull request as a draft")
|
||||
cmd.Flags().StringVarP(&opts.Title, "title", "t", "", "Title for the revert pull request")
|
||||
cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Body for the revert pull request")
|
||||
cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)")
|
||||
return cmd
|
||||
}
|
||||
|
||||
func revertRun(opts *RevertOptions) error {
|
||||
cs := opts.IO.ColorScheme()
|
||||
|
||||
findOptions := shared.FindOptions{
|
||||
Selector: opts.SelectorArg,
|
||||
Fields: []string{"id", "number", "state", "title"},
|
||||
}
|
||||
pr, baseRepo, err := opts.Finder.Find(findOptions)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if pr.State != "MERGED" {
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s Pull request %s#%d (%s) can't be reverted because it has not been merged\n", cs.FailureIcon(), ghrepo.FullName(baseRepo), pr.Number, pr.Title)
|
||||
return cmdutil.SilentError
|
||||
}
|
||||
|
||||
httpClient, err := opts.HttpClient()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
apiClient := api.NewClientFromHTTP(httpClient)
|
||||
|
||||
params := githubv4.RevertPullRequestInput{
|
||||
PullRequestID: pr.ID,
|
||||
Draft: githubv4.NewBoolean(githubv4.Boolean(opts.IsDraft)),
|
||||
}
|
||||
// Only set the Body field when opts.BodySet is true to avoid overriding
|
||||
// GitHub's default revert body generation.
|
||||
if opts.BodySet {
|
||||
params.Body = githubv4.NewString(githubv4.String(opts.Body))
|
||||
}
|
||||
// Only set the Title field when opts.Title is not empty to avoid overriding
|
||||
// GitHub's default revert title generation.
|
||||
if opts.Title != "" {
|
||||
params.Title = githubv4.NewString(githubv4.String(opts.Title))
|
||||
}
|
||||
|
||||
revertPR, err := api.PullRequestRevert(apiClient, baseRepo, params)
|
||||
if err != nil {
|
||||
fmt.Fprintf(opts.IO.ErrOut, "%s %s\n", cs.FailureIcon(), err)
|
||||
return fmt.Errorf("API call failed: %w", err)
|
||||
}
|
||||
|
||||
if revertPR != nil {
|
||||
fmt.Fprintln(opts.IO.Out, revertPR.URL)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
325
pkg/cmd/pr/revert/revert_test.go
Normal file
325
pkg/cmd/pr/revert/revert_test.go
Normal file
|
|
@ -0,0 +1,325 @@
|
|||
package revert
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"io"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/cli/cli/v2/api"
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
|
||||
"github.com/cli/cli/v2/pkg/cmdutil"
|
||||
"github.com/cli/cli/v2/pkg/httpmock"
|
||||
"github.com/cli/cli/v2/pkg/iostreams"
|
||||
"github.com/cli/cli/v2/test"
|
||||
"github.com/google/shlex"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) {
|
||||
ios, _, stdout, stderr := iostreams.Test()
|
||||
ios.SetStdoutTTY(isTTY)
|
||||
ios.SetStdinTTY(isTTY)
|
||||
ios.SetStderrTTY(isTTY)
|
||||
|
||||
factory := &cmdutil.Factory{
|
||||
IOStreams: ios,
|
||||
HttpClient: func() (*http.Client, error) {
|
||||
return &http.Client{Transport: rt}, nil
|
||||
},
|
||||
}
|
||||
|
||||
cmd := NewCmdRevert(factory, nil)
|
||||
|
||||
argv, err := shlex.Split(cli)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
cmd.SetArgs(argv)
|
||||
|
||||
cmd.SetIn(&bytes.Buffer{})
|
||||
cmd.SetOut(io.Discard)
|
||||
cmd.SetErr(io.Discard)
|
||||
|
||||
_, err = cmd.ExecuteC()
|
||||
return &test.CmdOut{
|
||||
OutBuf: stdout,
|
||||
ErrBuf: stderr,
|
||||
}, err
|
||||
}
|
||||
|
||||
func TestPRRevert_missingArgument(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "MERGED",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
// No arguments provided.
|
||||
_, err := runCommand(http, true, "")
|
||||
// Exits non-zero and prints an argument error.
|
||||
assert.EqualError(t, err, "cannot revert pull request: number, url, or branch required")
|
||||
}
|
||||
|
||||
func TestPRRevert_acceptedIdentifierFormats(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
args string
|
||||
}{
|
||||
{
|
||||
name: "Revert by pull request number",
|
||||
args: "123",
|
||||
},
|
||||
{
|
||||
name: "Revert by pull request identifier",
|
||||
args: "owner/repo#123",
|
||||
},
|
||||
{
|
||||
name: "Revert by pull request URL",
|
||||
args: "https://github.com/owner/repo/pull/123",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
shared.StubFinderForRunCommandStyleTests(t, tt.args, &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "MERGED",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
http.Register(
|
||||
httpmock.GraphQL(`mutation PullRequestRevert\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "revertPullRequest": { "pullRequest": {
|
||||
"ID": "SOME-ID"
|
||||
}, "revertPullRequest": {
|
||||
"ID": "NEW-ID",
|
||||
"Number": 456,
|
||||
"URL": "https://github.com/OWNER/REPO/pull/456"
|
||||
} } } }
|
||||
`,
|
||||
func(inputs map[string]interface{}) {
|
||||
assert.Equal(t, inputs["pullRequestId"], "SOME-ID")
|
||||
}),
|
||||
)
|
||||
|
||||
output, err := runCommand(http, true, tt.args)
|
||||
// Revert PR is created and only its URL is printed.
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "https://github.com/OWNER/REPO/pull/456\n", output.String())
|
||||
assert.Equal(t, "", output.Stderr())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPRRevert_notRevertable(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "OPEN",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
// Target PR is not merged.
|
||||
output, err := runCommand(http, true, "123")
|
||||
// API error, non-zero exit.
|
||||
assert.EqualError(t, err, "SilentError")
|
||||
assert.Equal(t, "X Pull request OWNER/REPO#123 (The title of the PR) can't be reverted because it has not been merged\n", output.Stderr())
|
||||
// No URL printed.
|
||||
assert.Equal(t, "", output.String())
|
||||
}
|
||||
|
||||
func TestPRRevert_withTitleAndBody(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "MERGED",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
http.Register(
|
||||
httpmock.GraphQL(`mutation PullRequestRevert\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "revertPullRequest": { "pullRequest": {
|
||||
"ID": "SOME-ID"
|
||||
}, "revertPullRequest": {
|
||||
"ID": "NEW-ID",
|
||||
"Number": 456,
|
||||
"URL": "https://github.com/OWNER/REPO/pull/456"
|
||||
} } } }
|
||||
`,
|
||||
func(inputs map[string]interface{}) {
|
||||
assert.Equal(t, inputs["pullRequestId"], "SOME-ID")
|
||||
assert.Equal(t, inputs["title"], "Revert PR title")
|
||||
assert.Equal(t, inputs["body"], "Revert PR body")
|
||||
}),
|
||||
)
|
||||
|
||||
output, err := runCommand(http, true, "123 --title 'Revert PR title' --body 'Revert PR body'")
|
||||
// Revert PR created.
|
||||
assert.NoError(t, err)
|
||||
// Only URL printed.
|
||||
assert.Equal(t, "https://github.com/OWNER/REPO/pull/456\n", output.String())
|
||||
assert.Equal(t, "", output.Stderr())
|
||||
}
|
||||
|
||||
func TestPRRevert_withDraft(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "MERGED",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
http.Register(
|
||||
httpmock.GraphQL(`mutation PullRequestRevert\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "revertPullRequest": { "pullRequest": {
|
||||
"ID": "SOME-ID"
|
||||
}, "revertPullRequest": {
|
||||
"ID": "NEW-ID",
|
||||
"Number": 456,
|
||||
"URL": "https://github.com/OWNER/REPO/pull/456"
|
||||
} } } }
|
||||
`,
|
||||
func(inputs map[string]interface{}) {
|
||||
assert.Equal(t, inputs["pullRequestId"], "SOME-ID")
|
||||
assert.Equal(t, inputs["draft"], true)
|
||||
}),
|
||||
)
|
||||
|
||||
output, err := runCommand(http, true, "123 --draft")
|
||||
// Revert PR created as a draft.
|
||||
assert.NoError(t, err)
|
||||
// Only URL printed.
|
||||
assert.Equal(t, "https://github.com/OWNER/REPO/pull/456\n", output.String())
|
||||
assert.Equal(t, "", output.Stderr())
|
||||
}
|
||||
|
||||
func TestPRRevert_APIFailure(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "MERGED",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
http.Register(
|
||||
httpmock.GraphQL(`mutation PullRequestRevert\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "errors": [{
|
||||
"message": "Authorization error"
|
||||
}]}`,
|
||||
func(inputs map[string]interface{}) {
|
||||
assert.Equal(t, inputs["pullRequestId"], "SOME-ID")
|
||||
}),
|
||||
)
|
||||
|
||||
output, err := runCommand(http, true, "123")
|
||||
// Non-zero exit, stderr shows the API error, stdout empty.
|
||||
assert.EqualError(t, err, "API call failed: GraphQL: Authorization error")
|
||||
assert.Equal(t, "X GraphQL: Authorization error\n", output.Stderr())
|
||||
assert.Equal(t, "", output.String())
|
||||
}
|
||||
|
||||
func TestPRRevert_multipleInvocations(t *testing.T) {
|
||||
http := &httpmock.Registry{}
|
||||
defer http.Verify(t)
|
||||
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "MERGED",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
http.Register(
|
||||
httpmock.GraphQL(`mutation PullRequestRevert\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "revertPullRequest": { "pullRequest": {
|
||||
"ID": "SOME-ID"
|
||||
}, "revertPullRequest": {
|
||||
"ID": "NEW-ID",
|
||||
"Number": 456,
|
||||
"URL": "https://github.com/OWNER/REPO/pull/456"
|
||||
} } } }
|
||||
`,
|
||||
func(inputs map[string]interface{}) {
|
||||
assert.Equal(t, inputs["pullRequestId"], "SOME-ID")
|
||||
}),
|
||||
)
|
||||
|
||||
output, err := runCommand(http, true, "123")
|
||||
// Revert PR is created and only its URL is printed.
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "https://github.com/OWNER/REPO/pull/456\n", output.String())
|
||||
assert.Equal(t, "", output.Stderr())
|
||||
|
||||
// Invoke the same command, behavior depends solely on API response
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "MERGED",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
http.Register(
|
||||
httpmock.GraphQL(`mutation PullRequestRevert\b`),
|
||||
httpmock.GraphQLMutation(`
|
||||
{ "data": { "revertPullRequest": { "pullRequest": {
|
||||
"ID": "SOME-ID"
|
||||
}, "revertPullRequest": {
|
||||
"ID": "NEW-ID",
|
||||
"Number": 456,
|
||||
"URL": "https://github.com/OWNER/REPO/pull/456"
|
||||
} } } }
|
||||
`,
|
||||
func(inputs map[string]interface{}) {
|
||||
assert.Equal(t, inputs["pullRequestId"], "SOME-ID")
|
||||
}),
|
||||
)
|
||||
|
||||
output, err = runCommand(http, true, "123")
|
||||
// Revert PR is created and only its URL is printed.
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "https://github.com/OWNER/REPO/pull/456\n", output.String())
|
||||
assert.Equal(t, "", output.Stderr())
|
||||
|
||||
// Invoke the same command, behavior depends solely on API response.
|
||||
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
|
||||
ID: "SOME-ID",
|
||||
Number: 123,
|
||||
State: "OPEN",
|
||||
Title: "The title of the PR",
|
||||
}, ghrepo.New("OWNER", "REPO"))
|
||||
|
||||
output, err = runCommand(http, true, "123")
|
||||
// Revert PR is not created, API error, non-zero exit.
|
||||
assert.EqualError(t, err, "SilentError")
|
||||
assert.Equal(t, "X Pull request OWNER/REPO#123 (The title of the PR) can't be reverted because it has not been merged\n", output.Stderr())
|
||||
// No URL printed.
|
||||
assert.Equal(t, "", output.String())
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue