From f9d3b71c3b68e8e608d8c83e9f8184dafcafbfd4 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Fri, 17 Mar 2023 22:35:37 +0800 Subject: [PATCH 01/11] feat: add gistRenameCmd to gist.go --- pkg/cmd/gist/gist.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index f637a0ffb..1f7fc9ea0 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -7,6 +7,7 @@ import ( gistDeleteCmd "github.com/cli/cli/v2/pkg/cmd/gist/delete" gistEditCmd "github.com/cli/cli/v2/pkg/cmd/gist/edit" gistListCmd "github.com/cli/cli/v2/pkg/cmd/gist/list" + gistRenameCmd "github.com/cli/cli/v2/pkg/cmd/gist/rename" gistViewCmd "github.com/cli/cli/v2/pkg/cmd/gist/view" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/spf13/cobra" @@ -33,6 +34,7 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(gistViewCmd.NewCmdView(f, nil)) cmd.AddCommand(gistEditCmd.NewCmdEdit(f, nil)) cmd.AddCommand(gistDeleteCmd.NewCmdDelete(f, nil)) + cmd.AddCommand(gistRenameCmd.NewCmdRename(f, nil)) return cmd } From 54de75db82b5281499dbfc1cbeab909511dbebbf Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sat, 18 Mar 2023 01:16:59 +0800 Subject: [PATCH 02/11] feat: add gist rename logic --- pkg/cmd/gist/rename/rename.go | 156 ++++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 pkg/cmd/gist/rename/rename.go diff --git a/pkg/cmd/gist/rename/rename.go b/pkg/cmd/gist/rename/rename.go new file mode 100644 index 000000000..5ccd81e90 --- /dev/null +++ b/pkg/cmd/gist/rename/rename.go @@ -0,0 +1,156 @@ +package rename + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/pkg/cmd/gist/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type RenameOptions struct { + IO *iostreams.IOStreams + Config func() (config.Config, error) + HttpClient func() (*http.Client, error) + Prompter iprompter + DoConfirm bool + + Selector string + OldFileName string + NewFileName string +} + +type iprompter interface { + Input(string, string) (string, error) + Confirm(string, bool) (bool, error) +} + +func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Command { + opts := &RenameOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + Prompter: f.Prompter, + } + + var confirm bool + + cmd := &cobra.Command{ + Use: "rename [ | ] ", + Short: "Rename a file in a gist", + Long: heredoc.Doc(`Rename a file in the given gist ID / URL.`), + Args: cobra.MaximumNArgs(3), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 3 { + opts.Selector = args[0] + opts.OldFileName = args[1] + opts.NewFileName = args[2] + } else { + return cmdutil.FlagErrorf("not enough arguments") + } + + if runf != nil { + return runf(opts) + } + + return renameRun(opts) + }, + } + + cmd.Flags().BoolVarP(&confirm, "yes", "y", false, "Skip the confirmation prompt") + + return cmd +} + +func renameRun(opts *RenameOptions) error { + gistID := opts.Selector + + if strings.Contains(gistID, "/") { + id, err := shared.GistIDFromURL(gistID) + if err != nil { + return err + } + gistID = id + } + + client, err := opts.HttpClient() + if err != nil { + return err + } + + apiClient := api.NewClientFromHTTP(client) + + cfg, err := opts.Config() + if err != nil { + return err + } + + host, _ := cfg.Authentication().DefaultHost() + + gist, err := shared.GetGist(client, host, gistID) + if err != nil { + if errors.Is(err, shared.NotFoundErr) { + return fmt.Errorf("gist not found: %s", gistID) + } + return err + } + username, err := api.CurrentLoginName(apiClient, host) + if err != nil { + return err + } + + if username != gist.Owner.Login { + return errors.New("you do not own this gist") + } + + _, ok := gist.Files[opts.OldFileName] + if !ok { + return fmt.Errorf("File %s not found in gist", opts.OldFileName) + } + + _, ok = gist.Files[opts.NewFileName] + if ok { + return fmt.Errorf("File %s already exists in gist", opts.NewFileName) + } + + gist.Files[opts.NewFileName] = gist.Files[opts.OldFileName] + gist.Files[opts.NewFileName].Filename = opts.NewFileName + gist.Files[opts.OldFileName] = &shared.GistFile{} + + return updateGist(apiClient, host, gist) +} + +func updateGist(apiClient *api.Client, hostname string, gist *shared.Gist) error { + body := shared.Gist{ + Description: gist.Description, + Files: gist.Files, + } + + path := "gists/" + gist.ID + + requestByte, err := json.Marshal(body) + if err != nil { + return err + } + + requestBody := bytes.NewReader(requestByte) + + result := shared.Gist{} + + err = apiClient.REST(hostname, "POST", path, requestBody, &result) + + if err != nil { + return err + } + + return nil +} From daf49221768e9abe674883c41e0c4d2da36face1 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 19 Mar 2023 17:28:02 +0800 Subject: [PATCH 03/11] refactor: remove unused code --- pkg/cmd/gist/rename/rename.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pkg/cmd/gist/rename/rename.go b/pkg/cmd/gist/rename/rename.go index 5ccd81e90..6cf509632 100644 --- a/pkg/cmd/gist/rename/rename.go +++ b/pkg/cmd/gist/rename/rename.go @@ -21,29 +21,19 @@ type RenameOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) HttpClient func() (*http.Client, error) - Prompter iprompter - DoConfirm bool Selector string OldFileName string NewFileName string } -type iprompter interface { - Input(string, string) (string, error) - Confirm(string, bool) (bool, error) -} - func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Command { opts := &RenameOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, Config: f.Config, - Prompter: f.Prompter, } - var confirm bool - cmd := &cobra.Command{ Use: "rename [ | ] ", Short: "Rename a file in a gist", @@ -66,8 +56,6 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co }, } - cmd.Flags().BoolVarP(&confirm, "yes", "y", false, "Skip the confirmation prompt") - return cmd } From eb2da16b601c37858ace926ac9963ff8a6f9f0e5 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 19 Mar 2023 19:43:34 +0800 Subject: [PATCH 04/11] feat: TestNewCmdRename for gist rename --- pkg/cmd/gist/rename/rename_test.go | 71 ++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 pkg/cmd/gist/rename/rename_test.go diff --git a/pkg/cmd/gist/rename/rename_test.go b/pkg/cmd/gist/rename/rename_test.go new file mode 100644 index 000000000..ef5121857 --- /dev/null +++ b/pkg/cmd/gist/rename/rename_test.go @@ -0,0 +1,71 @@ +package rename + +import ( + "bytes" + "testing" + + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdRename(t *testing.T) { + tests := []struct { + name string + input string + output RenameOptions + errMsg string + tty bool + wantErr bool + }{ + { + name: "no flags", + input: "123", + output: RenameOptions{ + Selector: "123", + }, + }, + { + name: "no new filename", + input: "123 old.md", + output: RenameOptions{ + Selector: "123", + OldFileName: "old.md", + }, + }, + { + name: "with both old and new filename", + input: "123 old.md new.md", + output: RenameOptions{ + Selector: "123", + OldFileName: "old.md", + NewFileName: "new.md", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{} + + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + + var gotOpts *RenameOptions + cmd := NewCmdRename(f, func(opts *RenameOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + assert.NoError(t, err) + + assert.Equal(t, tt.wants.Selector, gotOpts.Selector) + assert.Equal(t, tt.wants.OldFileName, gotOpts.OldFileName) + assert.Equal(t, tt.wants.NewFileName, gotOpts.NewFileName) + }) + } +} From 9e1d34f473c28bc7722e4dc3f7ff077f94fdca3e Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 19 Mar 2023 21:31:23 +0800 Subject: [PATCH 05/11] feat: add FlagErrorf for missing flag cases --- pkg/cmd/gist/rename/rename.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/gist/rename/rename.go b/pkg/cmd/gist/rename/rename.go index 6cf509632..adec3032a 100644 --- a/pkg/cmd/gist/rename/rename.go +++ b/pkg/cmd/gist/rename/rename.go @@ -44,6 +44,10 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co opts.Selector = args[0] opts.OldFileName = args[1] opts.NewFileName = args[2] + } else if len(args) == 2 { + return cmdutil.FlagErrorf(" is missing") + } else if len(args) == 1 { + return cmdutil.FlagErrorf(" and are missing") } else { return cmdutil.FlagErrorf("not enough arguments") } From b705ea94a0ad41271aea94038af7175669a58f64 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Sun, 19 Mar 2023 21:31:58 +0800 Subject: [PATCH 06/11] fix: revise test cases in TestNewCmdRename in rename_test.go --- pkg/cmd/gist/rename/rename_test.go | 52 ++++++++++++++++++------------ 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/gist/rename/rename_test.go b/pkg/cmd/gist/rename/rename_test.go index ef5121857..6e79685e3 100644 --- a/pkg/cmd/gist/rename/rename_test.go +++ b/pkg/cmd/gist/rename/rename_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -19,37 +20,44 @@ func TestNewCmdRename(t *testing.T) { wantErr bool }{ { - name: "no flags", - input: "123", - output: RenameOptions{ - Selector: "123", - }, + name: "no arguments", + input: "", + errMsg: "not enough arguments", + wantErr: true, }, { - name: "no new filename", - input: "123 old.md", - output: RenameOptions{ - Selector: "123", - OldFileName: "old.md", - }, + name: "missing old filename and new filename", + input: "123", + errMsg: " and are missing", + wantErr: true, }, { - name: "with both old and new filename", - input: "123 old.md new.md", + name: "missing new filename", + input: "123 old.txt", + errMsg: " is missing", + wantErr: true, + }, + { + name: "rename", + input: "123 old.txt new.txt", output: RenameOptions{ Selector: "123", - OldFileName: "old.md", - NewFileName: "new.md", + OldFileName: "old.txt", + NewFileName: "new.txt", }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - f := &cmdutil.Factory{} + ios, _, _, _ := iostreams.Test() + ios.SetStdinTTY(tt.tty) + ios.SetStdoutTTY(tt.tty) + f := &cmdutil.Factory{ + IOStreams: ios, + } argv, err := shlex.Split(tt.input) assert.NoError(t, err) - var gotOpts *RenameOptions cmd := NewCmdRename(f, func(opts *RenameOptions) error { gotOpts = opts @@ -61,11 +69,15 @@ func TestNewCmdRename(t *testing.T) { cmd.SetErr(&bytes.Buffer{}) _, err = cmd.ExecuteC() + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + return + } assert.NoError(t, err) - assert.Equal(t, tt.wants.Selector, gotOpts.Selector) - assert.Equal(t, tt.wants.OldFileName, gotOpts.OldFileName) - assert.Equal(t, tt.wants.NewFileName, gotOpts.NewFileName) + assert.Equal(t, tt.output.Selector, gotOpts.Selector) + assert.Equal(t, tt.output.OldFileName, gotOpts.OldFileName) + assert.Equal(t, tt.output.NewFileName, gotOpts.NewFileName) }) } } From 3b27e068f24061cc22194223232b8edb601049e5 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Wed, 22 Mar 2023 00:11:40 +0800 Subject: [PATCH 07/11] feat: gist rename test --- pkg/cmd/gist/rename/rename_test.go | 102 +++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/pkg/cmd/gist/rename/rename_test.go b/pkg/cmd/gist/rename/rename_test.go index 6e79685e3..4f64873ac 100644 --- a/pkg/cmd/gist/rename/rename_test.go +++ b/pkg/cmd/gist/rename/rename_test.go @@ -2,9 +2,13 @@ package rename import ( "bytes" + "net/http" "testing" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/pkg/cmd/gist/shared" "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" @@ -81,3 +85,101 @@ func TestNewCmdRename(t *testing.T) { }) } } + +func TestRenameRun(t *testing.T) { + tests := []struct { + name string + opts *RenameOptions + gist *shared.Gist + httpStubs func(*httpmock.Registry) + nontty bool + stdin string + wantOut string + wantParams map[string]interface{} + }{ + { + name: "no such gist", + wantOut: "gist not found: 1234", + }, + { + name: "rename from old.txt to new.txt", + opts: &RenameOptions{ + Selector: "1234", + OldFileName: "old.txt", + NewFileName: "new.txt", + }, + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "old.txt": { + Filename: "old.txt", + Type: "text/plain", + }, + }, + Owner: &shared.GistOwner{Login: "octocat"}, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + wantParams: map[string]interface{}{ + "files": map[string]interface{}{ + "new.txt": map[string]interface{}{ + "filename": "new.txt", + "type": "text/plain", + }, + }, + }, + }, + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + if tt.gist == nil { + reg.Register(httpmock.REST("GET", "gists/1234"), + httpmock.StatusStringResponse(404, "Not Found")) + } else { + reg.Register(httpmock.REST("GET", "gists/1234"), + httpmock.JSONResponse(tt.gist)) + reg.Register(httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) + } + + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + + if tt.opts == nil { + tt.opts = &RenameOptions{} + } + + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + ios, stdin, stdout, stderr := iostreams.Test() + stdin.WriteString(tt.stdin) + ios.SetStdoutTTY(!tt.nontty) + ios.SetStdinTTY(!tt.nontty) + + tt.opts.Selector = "1234" + tt.opts.OldFileName = "old.txt" + tt.opts.NewFileName = "new.txt" + tt.opts.IO = ios + + tt.opts.Config = func() (config.Config, error) { + return config.NewBlankConfig(), nil + } + + t.Run(tt.name, func(t *testing.T) { + err := renameRun(tt.opts) + reg.Verify(t) + if tt.wantOut != "" { + assert.EqualError(t, err, tt.wantOut) + return + } + assert.NoError(t, err) + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + }) + } +} From 5699daef2722801a9952bea9aab0efd1db3bfeff Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Thu, 23 Mar 2023 11:51:55 +0800 Subject: [PATCH 08/11] refactor: change from [] to {} --- pkg/cmd/gist/rename/rename.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/rename/rename.go b/pkg/cmd/gist/rename/rename.go index adec3032a..5acedf595 100644 --- a/pkg/cmd/gist/rename/rename.go +++ b/pkg/cmd/gist/rename/rename.go @@ -35,7 +35,7 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co } cmd := &cobra.Command{ - Use: "rename [ | ] ", + Use: "rename { | } ", Short: "Rename a file in a gist", Long: heredoc.Doc(`Rename a file in the given gist ID / URL.`), Args: cobra.MaximumNArgs(3), From 951d1aaa1fafbb133cfaf7ed1b2f1302c59bec9b Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Thu, 23 Mar 2023 11:52:14 +0800 Subject: [PATCH 09/11] refactor: use ExactArgs instead of MaximumNArgs --- pkg/cmd/gist/rename/rename.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/rename/rename.go b/pkg/cmd/gist/rename/rename.go index 5acedf595..3c968bfed 100644 --- a/pkg/cmd/gist/rename/rename.go +++ b/pkg/cmd/gist/rename/rename.go @@ -38,7 +38,7 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co Use: "rename { | } ", Short: "Rename a file in a gist", Long: heredoc.Doc(`Rename a file in the given gist ID / URL.`), - Args: cobra.MaximumNArgs(3), + Args: cobra.ExactArgs(3), RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 3 { opts.Selector = args[0] From 6854f0f63c4dc473dc3bcf3a4294aafb575d6aa2 Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Thu, 23 Mar 2023 11:58:00 +0800 Subject: [PATCH 10/11] refactor: remove unnecessary validations --- pkg/cmd/gist/rename/rename.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/gist/rename/rename.go b/pkg/cmd/gist/rename/rename.go index 3c968bfed..c23aca579 100644 --- a/pkg/cmd/gist/rename/rename.go +++ b/pkg/cmd/gist/rename/rename.go @@ -40,17 +40,9 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co Long: heredoc.Doc(`Rename a file in the given gist ID / URL.`), Args: cobra.ExactArgs(3), RunE: func(cmd *cobra.Command, args []string) error { - if len(args) == 3 { - opts.Selector = args[0] - opts.OldFileName = args[1] - opts.NewFileName = args[2] - } else if len(args) == 2 { - return cmdutil.FlagErrorf(" is missing") - } else if len(args) == 1 { - return cmdutil.FlagErrorf(" and are missing") - } else { - return cmdutil.FlagErrorf("not enough arguments") - } + opts.Selector = args[0] + opts.OldFileName = args[1] + opts.NewFileName = args[2] if runf != nil { return runf(opts) From c6454064a4cc17f737df53d63bb6bf5467038d4c Mon Sep 17 00:00:00 2001 From: Wing-Kam Wong Date: Thu, 23 Mar 2023 11:58:13 +0800 Subject: [PATCH 11/11] refactor: revise errMsg in rename_test.go --- pkg/cmd/gist/rename/rename_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/gist/rename/rename_test.go b/pkg/cmd/gist/rename/rename_test.go index 4f64873ac..5413d87c9 100644 --- a/pkg/cmd/gist/rename/rename_test.go +++ b/pkg/cmd/gist/rename/rename_test.go @@ -26,19 +26,19 @@ func TestNewCmdRename(t *testing.T) { { name: "no arguments", input: "", - errMsg: "not enough arguments", + errMsg: "accepts 3 arg(s), received 0", wantErr: true, }, { name: "missing old filename and new filename", input: "123", - errMsg: " and are missing", + errMsg: "accepts 3 arg(s), received 1", wantErr: true, }, { name: "missing new filename", input: "123 old.txt", - errMsg: " is missing", + errMsg: "accepts 3 arg(s), received 2", wantErr: true, }, {