diff --git a/api/merge_request.go b/api/merge_request.go index 77fca0f948bce846866b158271fe05ccb003c195..d9593f814a85bc2d7f0d6a938dcbcbd9fdca5974 100644 --- a/api/merge_request.go +++ b/api/merge_request.go @@ -339,6 +339,19 @@ var ListMRNotes = func(client *gitlab.Client, projectID interface{}, mrID int, o return notes, nil } +var UpdateMRNotes = func(client *gitlab.Client, projectID interface{}, mrID int, noteID int, opts *gitlab.UpdateMergeRequestNoteOptions) (*gitlab.Note, error) { + if client == nil { + client = apiClient.Lab() + } + + note, _, err := client.Notes.UpdateMergeRequestNote(projectID, mrID, noteID, opts) + if err != nil { + return nil, err + } + + return note, nil +} + var RebaseMR = func(client *gitlab.Client, projectID interface{}, mrID int, opts *gitlab.RebaseMergeRequestOptions) error { if client == nil { client = apiClient.Lab() diff --git a/commands/mr/note/mr_note_create.go b/commands/mr/note/mr_note_create.go index 5e9e55f7d4518ab3aa3c837b9544b0bf032e7368..f853c6ea77ca1f415d1f9ea31b0f3fad4a36ed0a 100644 --- a/commands/mr/note/mr_note_create.go +++ b/commands/mr/note/mr_note_create.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" "github.com/xanzy/go-gitlab" + updateNoteCmd "gitlab.com/gitlab-org/cli/commands/mr/note/update" ) func NewCmdNote(f *cmdutils.Factory) *cobra.Command { @@ -78,6 +79,7 @@ func NewCmdNote(f *cmdutils.Factory) *cobra.Command { }, } + mrCreateNoteCmd.AddCommand(updateNoteCmd.UpdateCmdNote(f)) mrCreateNoteCmd.Flags().StringP("message", "m", "", "Comment or note message.") mrCreateNoteCmd.Flags().Bool("unique", false, "Don't create a comment or note if it already exists.") return mrCreateNoteCmd diff --git a/commands/mr/note/update/mr_note_update.go b/commands/mr/note/update/mr_note_update.go new file mode 100644 index 0000000000000000000000000000000000000000..482c5ff18c08471089239958d931ff1347966b98 --- /dev/null +++ b/commands/mr/note/update/mr_note_update.go @@ -0,0 +1,74 @@ +package note + +import ( + "fmt" + "strconv" + + "gitlab.com/gitlab-org/cli/api" + "gitlab.com/gitlab-org/cli/commands/cmdutils" + "gitlab.com/gitlab-org/cli/commands/mr/mrutils" + "gitlab.com/gitlab-org/cli/pkg/utils" + + "github.com/spf13/cobra" + "github.com/xanzy/go-gitlab" +) + +func UpdateCmdNote(f *cmdutils.Factory) *cobra.Command { + mrCreateNoteCmd := &cobra.Command{ + Use: "update [ | ] [note-id]", + Short: "Update a comment or note on a merge request.", + Long: ``, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + apiClient, err := f.HttpClient() + if err != nil { + return err + } + + mr, repo, err := mrutils.MRFromArgs(f, args, "any") + if err != nil { + return err + } + + body, _ := cmd.Flags().GetString("message") + + if body == "" { + editor, err := cmdutils.GetEditor(f.Config) + if err != nil { + return err + } + + body = utils.Editor(utils.EditorOptions{ + Label: "Note message:", + Help: "Enter the note message for the merge request. ", + FileName: "*_MR_NOTE_EDITMSG.md", + EditorCommand: editor, + }) + } + if body == "" { + return fmt.Errorf("aborted... Note has an empty message.") + } + + noteId, err := strconv.Atoi(args[1]) + if err != nil { + return err + } + + if noteId < 0 { + return fmt.Errorf("aborted... Note ID must not be negative.") + } + + noteInfo, err := api.UpdateMRNotes(apiClient, repo.FullName(), mr.IID, noteId, &gitlab.UpdateMergeRequestNoteOptions{ + Body: &body, + }) + if err != nil { + return err + } + fmt.Fprintf(f.IO.StdOut, "%s#note_%d\n", mr.WebURL, noteInfo.ID) + return nil + }, + } + + mrCreateNoteCmd.Flags().StringP("message", "m", "", "Comment or note message.") + return mrCreateNoteCmd +} diff --git a/commands/mr/note/update/mr_note_update_test.go b/commands/mr/note/update/mr_note_update_test.go new file mode 100644 index 0000000000000000000000000000000000000000..ebaa53b02d1696cce65fbf11bebc58bf56ee9011 --- /dev/null +++ b/commands/mr/note/update/mr_note_update_test.go @@ -0,0 +1,179 @@ +package note + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "gitlab.com/gitlab-org/cli/commands/cmdtest" + "gitlab.com/gitlab-org/cli/pkg/git" + "gitlab.com/gitlab-org/cli/pkg/httpmock" + "gitlab.com/gitlab-org/cli/pkg/prompt" + "gitlab.com/gitlab-org/cli/test" +) + +func TestMain(m *testing.M) { + cmdtest.InitTest(m, "mr_note_update_test") +} + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + ios, _, stdout, stderr := cmdtest.InitIOStreams(isTTY, "") + factory := cmdtest.InitFactory(ios, rt) + factory.Branch = git.CurrentBranch + + // TODO: shouldn't be there but the stub doesn't work without it + _, _ = factory.HttpClient() + + cmd := UpdateCmdNote(factory) + + return cmdtest.ExecuteCommand(cmd, cli, stdout, stderr) +} + +func Test_UpdateCmdNote(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + t.Run("--message flag specified", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodPut, "/projects/OWNER/REPO/merge_requests/1/notes/301", + httpmock.NewStringResponse(http.StatusOK, ` + { + "id": 301, + "created_at": "2013-10-02T08:57:14Z", + "updated_at": "2013-10-02T08:57:14Z", + "system": false, + "noteable_id": 1, + "noteable_type": "MergeRequest", + "noteable_iid": 1 + } + `)) + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/merge_requests/1", + httpmock.NewStringResponse(http.StatusOK, ` + { + "id": 1, + "iid": 1, + "web_url": "https://gitlab.com/OWNER/REPO/merge_requests/1" + } + `)) + // glab mr note update 1 301 --message "Here is my note" + output, err := runCommand(fakeHTTP, true, `1 301 --message "Here is my note"`) + if err != nil { + t.Error(err) + return + } + assert.Equal(t, output.Stderr(), "") + assert.Equal(t, output.String(), "https://gitlab.com/OWNER/REPO/merge_requests/1#note_301\n") + }) +} + +func Test_UpdateCmdNote_error(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + t.Run("note does not exist", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodPut, "/projects/OWNER/REPO/merge_requests/1/notes/301", + httpmock.NewStringResponse(http.StatusNotFound, ` + { + "message": "Not Found" + } + `)) + + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/merge_requests/1", + httpmock.NewStringResponse(http.StatusOK, ` + { + "id": 1, + "iid": 1, + "web_url": "https://gitlab.com/OWNER/REPO/merge_requests/1" + } + `)) + + // glab mr note update 1 301 --message "Here is my note" + _, err := runCommand(fakeHTTP, true, `1 301 --message "Some message"`) + assert.NotNil(t, err) + assert.Equal(t, "404 Not Found", err.Error()) + }) + + t.Run("note could not be updated", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodPut, "/projects/OWNER/REPO/merge_requests/1/notes/301", + httpmock.NewStringResponse(http.StatusUnauthorized, ` + { + "message": "note not found" + } + `)) + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/merge_requests/1", + httpmock.NewStringResponse(http.StatusOK, ` + { + "id": 1, + "iid": 1, + "web_url": "https://gitlab.com/OWNER/REPO/merge_requests/1" + } + `)) + // glab mr note 1 301 --message "Here is my note" + _, err := runCommand(fakeHTTP, true, `1 301 --message "Here is my note"`) + assert.NotNil(t, err) + assert.Equal(t, "PUT https://gitlab.com/api/v4/projects/OWNER/REPO/merge_requests/1/notes/301: 401 {message: note not found}", err.Error()) + }) +} + +func Test_mrNoteCreate_prompt(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + t.Run("message provided", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodPut, "/projects/OWNER/REPO/merge_requests/1/notes/301", + httpmock.NewStringResponse(http.StatusCreated, ` + { + "id": 301, + "created_at": "2013-10-02T08:57:14Z", + "updated_at": "2013-10-02T08:57:14Z", + "system": false, + "noteable_id": 1, + "noteable_type": "MergeRequest", + "noteable_iid": 1 + } + `)) + + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/merge_requests/1", + httpmock.NewStringResponse(http.StatusOK, ` + { + "id": 1, + "iid": 1, + "web_url": "https://gitlab.com/OWNER/REPO/merge_requests/1" + } + `)) + as, teardown := prompt.InitAskStubber() + defer teardown() + as.StubOne("some note message") + + // glab mr note update 1 + output, err := runCommand(fakeHTTP, true, `1 301`) + if err != nil { + t.Error(err) + return + } + assert.Equal(t, output.Stderr(), "") + assert.Equal(t, output.String(), "https://gitlab.com/OWNER/REPO/merge_requests/1#note_301\n") + }) + + t.Run("message is empty", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/merge_requests/1", + httpmock.NewStringResponse(http.StatusOK, ` + { + "id": 1, + "iid": 1, + "web_url": "https://gitlab.com/OWNER/REPO/merge_requests/1" + } + `)) + + as, teardown := prompt.InitAskStubber() + defer teardown() + as.StubOne("") + + // glab mr note update 1 + _, err := runCommand(fakeHTTP, true, `1 301`) + if err == nil { + t.Error("expected error") + return + } + assert.Equal(t, err.Error(), "aborted... Note has an empty message.") + }) +} diff --git a/docs/source/mr/index.md b/docs/source/mr/index.md index b04af820bbaf90deefc0572b5bc5594d377b3105..84787718f4f855a9d77f6e45b3721111cf06fa57 100644 --- a/docs/source/mr/index.md +++ b/docs/source/mr/index.md @@ -47,7 +47,7 @@ glab mr note -m "needs to do X before it can be merged" branch-foo - [`issues`](issues.md) - [`list`](list.md) - [`merge`](merge.md) -- [`note`](note.md) +- [`note`](note/index.md) - [`rebase`](rebase.md) - [`reopen`](reopen.md) - [`revoke`](revoke.md) diff --git a/docs/source/mr/note.md b/docs/source/mr/note/index.md similarity index 95% rename from docs/source/mr/note.md rename to docs/source/mr/note/index.md index 16bf36035b9ec0c333bcd98677d7868cd184ca4e..c10b3179bebbf3d9088925582d88964447a5761b 100644 --- a/docs/source/mr/note.md +++ b/docs/source/mr/note/index.md @@ -36,3 +36,7 @@ comment --help Show help for this command. -R, --repo OWNER/REPO Select another repository. Can use either OWNER/REPO or `GROUP/NAMESPACE/REPO` format. Also accepts full URL or Git URL. ``` + +## Subcommands + +- [`update`](update.md) diff --git a/docs/source/mr/note/update.md b/docs/source/mr/note/update.md new file mode 100644 index 0000000000000000000000000000000000000000..2aab6636438d5a96c7cdfe202790e59ae1dc8e39 --- /dev/null +++ b/docs/source/mr/note/update.md @@ -0,0 +1,31 @@ +--- +stage: Create +group: Code Review +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + + + +# `glab mr note update` + +Update a comment or note on a merge request. + +```plaintext +glab mr note update [ | ] [note-id] [flags] +``` + +## Options + +```plaintext + -m, --message string Comment or note message. +``` + +## Options inherited from parent commands + +```plaintext + --help Show help for this command. + -R, --repo OWNER/REPO Select another repository. Can use either OWNER/REPO or `GROUP/NAMESPACE/REPO` format. Also accepts full URL or Git URL. +```