From 3c6c7b097e3ee2c5717c4a938eef54b711e1f47d Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev Date: Wed, 28 Jun 2023 09:13:04 +0000 Subject: [PATCH] feat: add incident note command --- commands/incident/incident.go | 2 + .../incident/note/incident_note_create.go | 13 + .../issuable/note/issuable_note_create.go | 74 ++++++ .../note/issuable_note_create_test.go | 236 ++++++++++++++++++ commands/issue/note/issue_note_create.go | 63 +---- commands/issue/note/issue_note_create_test.go | 170 ------------- docs/source/incident/index.md | 1 + docs/source/incident/note.md | 37 +++ docs/source/issue/note.md | 2 +- 9 files changed, 369 insertions(+), 229 deletions(-) create mode 100644 commands/incident/note/incident_note_create.go create mode 100644 commands/issuable/note/issuable_note_create.go create mode 100644 commands/issuable/note/issuable_note_create_test.go delete mode 100644 commands/issue/note/issue_note_create_test.go create mode 100644 docs/source/incident/note.md diff --git a/commands/incident/incident.go b/commands/incident/incident.go index 102d1921..bf4da842 100644 --- a/commands/incident/incident.go +++ b/commands/incident/incident.go @@ -6,6 +6,7 @@ import ( incidentCloseCmd "gitlab.com/gitlab-org/cli/commands/incident/close" incidentListCmd "gitlab.com/gitlab-org/cli/commands/incident/list" + incidentNoteCmd "gitlab.com/gitlab-org/cli/commands/incident/note" incidentReopenCmd "gitlab.com/gitlab-org/cli/commands/incident/reopen" incidentSubscribeCmd "gitlab.com/gitlab-org/cli/commands/incident/subscribe" incidentUnsubscribeCmd "gitlab.com/gitlab-org/cli/commands/incident/unsubscribe" @@ -34,6 +35,7 @@ func NewCmdIncident(f *cmdutils.Factory) *cobra.Command { cmdutils.EnableRepoOverride(incidentCmd, f) incidentCmd.AddCommand(incidentListCmd.NewCmdList(f, nil)) + incidentCmd.AddCommand(incidentNoteCmd.NewCmdNote(f)) incidentCmd.AddCommand(incidentViewCmd.NewCmdView(f)) incidentCmd.AddCommand(incidentCloseCmd.NewCmdClose(f)) incidentCmd.AddCommand(incidentReopenCmd.NewCmdReopen(f)) diff --git a/commands/incident/note/incident_note_create.go b/commands/incident/note/incident_note_create.go new file mode 100644 index 00000000..ba1d59ff --- /dev/null +++ b/commands/incident/note/incident_note_create.go @@ -0,0 +1,13 @@ +package note + +import ( + "github.com/spf13/cobra" + "gitlab.com/gitlab-org/cli/commands/cmdutils" + "gitlab.com/gitlab-org/cli/commands/issuable" + + issuableNoteCmd "gitlab.com/gitlab-org/cli/commands/issuable/note" +) + +func NewCmdNote(f *cmdutils.Factory) *cobra.Command { + return issuableNoteCmd.NewCmdNote(f, issuable.TypeIncident) +} diff --git a/commands/issuable/note/issuable_note_create.go b/commands/issuable/note/issuable_note_create.go new file mode 100644 index 00000000..bf8937ec --- /dev/null +++ b/commands/issuable/note/issuable_note_create.go @@ -0,0 +1,74 @@ +package note + +import ( + "errors" + "fmt" + "strings" + + "gitlab.com/gitlab-org/cli/commands/issuable" + "gitlab.com/gitlab-org/cli/commands/issue/issueutils" + + "gitlab.com/gitlab-org/cli/api" + "gitlab.com/gitlab-org/cli/commands/cmdutils" + "gitlab.com/gitlab-org/cli/pkg/utils" + + "github.com/spf13/cobra" + gitlab "github.com/xanzy/go-gitlab" +) + +func NewCmdNote(f *cmdutils.Factory, issueType issuable.IssueType) *cobra.Command { + issueNoteCreateCmd := &cobra.Command{ + Use: fmt.Sprintf("note <%s-id>", issueType), + Aliases: []string{"comment"}, + Short: fmt.Sprintf("Comment on an %s in GitLab", issueType), + Long: ``, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + var err error + out := f.IO.StdOut + + apiClient, err := f.HttpClient() + if err != nil { + return err + } + + issue, repo, err := issueutils.IssueFromArg(apiClient, f.BaseRepo, args[0]) + if err != nil { + return err + } + + valid, msg := issuable.ValidateIncidentCmd(issueType, "comment", issue) + if !valid { + fmt.Fprintln(f.IO.StdOut, msg) + return nil + } + + body, _ := cmd.Flags().GetString("message") + + if strings.TrimSpace(body) == "" { + body = utils.Editor(utils.EditorOptions{ + Label: "Note Message:", + Help: "Enter the note message. ", + FileName: "ISSUE_NOTE_EDITMSG", + }) + } + + if strings.TrimSpace(body) == "" { + return errors.New("aborted... Note is empty") + } + + noteInfo, err := api.CreateIssueNote(apiClient, repo.FullName(), issue.IID, &gitlab.CreateIssueNoteOptions{ + Body: &body, + }) + if err != nil { + return err + } + + fmt.Fprintf(out, "%s#note_%d\n", issue.WebURL, noteInfo.ID) + return nil + }, + } + issueNoteCreateCmd.Flags().StringP("message", "m", "", "Comment/Note message") + + return issueNoteCreateCmd +} diff --git a/commands/issuable/note/issuable_note_create_test.go b/commands/issuable/note/issuable_note_create_test.go new file mode 100644 index 00000000..01baca3b --- /dev/null +++ b/commands/issuable/note/issuable_note_create_test.go @@ -0,0 +1,236 @@ +package note + +import ( + "fmt" + "net/http" + "testing" + + "github.com/alecthomas/assert" + "gitlab.com/gitlab-org/cli/commands/cmdtest" + "gitlab.com/gitlab-org/cli/commands/issuable" + "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 runCommand(rt http.RoundTripper, isTTY bool, cli string, issueType issuable.IssueType) (*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 := NewCmdNote(factory, issueType) + + return cmdtest.ExecuteCommand(cmd, cli, stdout, stderr) +} + +func Test_NewCmdNote(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + commands := []struct { + name string + issueType issuable.IssueType + }{ + {"issue", issuable.TypeIssue}, + {"incident", issuable.TypeIncident}, + } + + for _, cc := range commands { + t.Run("--message flag specified", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodPost, "/projects/OWNER/REPO/issues/1/notes", + 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/issues/1", + httpmock.NewStringResponse(http.StatusOK, fmt.Sprintf(` + { + "id": 1, + "iid": 1, + "issue_type": "%s", + "web_url": "https://gitlab.com/OWNER/REPO/issues/1" + } + `, cc.issueType))) + + // glab issue note 1 --message "Here is my note" + // glab incident note 1 --message "Here is my note" + output, err := runCommand(fakeHTTP, true, `1 --message "Here is my note"`, cc.issueType) + if err != nil { + t.Error(err) + return + } + assert.Equal(t, output.Stderr(), "") + assert.Equal(t, output.String(), "https://gitlab.com/OWNER/REPO/issues/1#note_301\n") + }) + + t.Run("issue not found", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/issues/122", + httpmock.NewStringResponse(http.StatusNotFound, ` + { + "message" : "issue not found" + } + `)) + + // glab issue note 1 --message "Here is my note" + // glab incident note 1 --message "Here is my note" + _, err := runCommand(fakeHTTP, true, `122`, cc.issueType) + assert.NotNil(t, err) + assert.Equal(t, "GET https://gitlab.com/api/v4/projects/OWNER/REPO/issues/122: 404 {message: issue not found}", err.Error()) + }) + } +} + +func Test_NewCmdNote_error(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + commands := []struct { + name string + issueType issuable.IssueType + }{ + {"issue", issuable.TypeIssue}, + {"incident", issuable.TypeIncident}, + } + + for _, cc := range commands { + t.Run("note could not be created", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodPost, "/projects/OWNER/REPO/issues/1/notes", + httpmock.NewStringResponse(http.StatusUnauthorized, ` + { + "message" : "Unauthorized" + } + `)) + + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/issues/1", + httpmock.NewStringResponse(http.StatusOK, fmt.Sprintf(` + { + "id": 1, + "iid": 1, + "issue_type": "%s", + "web_url": "https://gitlab.com/OWNER/REPO/issues/1" + } + `, cc.issueType))) + + // glab issue note 1 --message "Here is my note" + // glab incident note 1 --message "Here is my note" + _, err := runCommand(fakeHTTP, true, `1 -m "Some message"`, cc.issueType) + assert.NotNil(t, err) + assert.Equal(t, "POST https://gitlab.com/api/v4/projects/OWNER/REPO/issues/1/notes: 401 {message: Unauthorized}", err.Error()) + }) + } + + t.Run("using incident note command with issue ID", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/issues/1", + httpmock.NewStringResponse(http.StatusOK, ` + { + "id": 1, + "iid": 1, + "issue_type": "issue", + "web_url": "https://gitlab.com/OWNER/REPO/issues/1" + } + `)) + + output, err := runCommand(fakeHTTP, true, `1 -m "Some message"`, issuable.TypeIncident) + assert.Nil(t, err) + assert.Equal(t, "Incident not found, but an issue with the provided ID exists. Run `glab issue comment ` to comment.\n", output.String()) + }) +} + +func Test_IssuableNoteCreate_prompt(t *testing.T) { + fakeHTTP := httpmock.New() + defer fakeHTTP.Verify(t) + + commands := []struct { + name string + issueType issuable.IssueType + }{ + {"issue", issuable.TypeIssue}, + {"incident", issuable.TypeIncident}, + } + + for _, cc := range commands { + t.Run("message provided", func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodPost, "/projects/OWNER/REPO/issues/1/notes", + 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/issues/1", + httpmock.NewStringResponse(http.StatusOK, fmt.Sprintf(` + { + "id": 1, + "iid": 1, + "issue_type": "%s", + "web_url": "https://gitlab.com/OWNER/REPO/issues/1" + } + `, cc.issueType))) + as, teardown := prompt.InitAskStubber() + defer teardown() + as.StubOne("some note message") + + // glab issue note 1 + // glab incident note 1 + output, err := runCommand(fakeHTTP, true, `1`, cc.issueType) + if err != nil { + t.Error(err) + return + } + assert.Equal(t, output.Stderr(), "") + assert.Equal(t, output.String(), "https://gitlab.com/OWNER/REPO/issues/1#note_301\n") + }) + + tests := []struct { + name string + message string + }{ + {"message is empty", ""}, + {"message contains only spaces", " "}, + {"message contains only line breaks", "\n\n"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/issues/1", + httpmock.NewStringResponse(http.StatusOK, fmt.Sprintf(` + { + "id": 1, + "iid": 1, + "issue_type": "%s", + "web_url": "https://gitlab.com/OWNER/REPO/issues/1" + } + `, cc.issueType))) + + as, teardown := prompt.InitAskStubber() + defer teardown() + as.StubOne(tt.message) + + _, err := runCommand(fakeHTTP, true, `1`, cc.issueType) + if err == nil { + t.Error("expected error") + return + } + assert.Equal(t, "aborted... Note is empty", err.Error()) + }) + } + } +} diff --git a/commands/issue/note/issue_note_create.go b/commands/issue/note/issue_note_create.go index 0264f7f8..a0165efd 100644 --- a/commands/issue/note/issue_note_create.go +++ b/commands/issue/note/issue_note_create.go @@ -1,66 +1,13 @@ package note import ( - "errors" - "fmt" - - "gitlab.com/gitlab-org/cli/commands/issue/issueutils" - - "gitlab.com/gitlab-org/cli/api" - "gitlab.com/gitlab-org/cli/commands/cmdutils" - "gitlab.com/gitlab-org/cli/pkg/utils" - "github.com/spf13/cobra" - gitlab "github.com/xanzy/go-gitlab" + "gitlab.com/gitlab-org/cli/commands/cmdutils" + "gitlab.com/gitlab-org/cli/commands/issuable" + + issuableNoteCmd "gitlab.com/gitlab-org/cli/commands/issuable/note" ) func NewCmdNote(f *cmdutils.Factory) *cobra.Command { - issueNoteCreateCmd := &cobra.Command{ - Use: "note ", - Aliases: []string{"comment"}, - Short: "Add a comment or note to an issue on GitLab", - Long: ``, - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - var err error - out := f.IO.StdOut - - apiClient, err := f.HttpClient() - if err != nil { - return err - } - - issue, repo, err := issueutils.IssueFromArg(apiClient, f.BaseRepo, args[0]) - if err != nil { - return err - } - - body, _ := cmd.Flags().GetString("message") - - if body == "" { - body = utils.Editor(utils.EditorOptions{ - Label: "Note Message:", - Help: "Enter the note message. ", - FileName: "ISSUE_NOTE_EDITMSG", - }) - } - - if body == "" { - return errors.New("aborted... Note is empty") - } - - noteInfo, err := api.CreateIssueNote(apiClient, repo.FullName(), issue.IID, &gitlab.CreateIssueNoteOptions{ - Body: &body, - }) - if err != nil { - return err - } - - fmt.Fprintf(out, "%s#note_%d\n", issue.WebURL, noteInfo.ID) - return nil - }, - } - issueNoteCreateCmd.Flags().StringP("message", "m", "", "Comment/Note message") - - return issueNoteCreateCmd + return issuableNoteCmd.NewCmdNote(f, issuable.TypeIssue) } diff --git a/commands/issue/note/issue_note_create_test.go b/commands/issue/note/issue_note_create_test.go deleted file mode 100644 index 58903916..00000000 --- a/commands/issue/note/issue_note_create_test.go +++ /dev/null @@ -1,170 +0,0 @@ -package note - -import ( - "net/http" - "testing" - - "github.com/alecthomas/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 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 := NewCmdNote(factory) - - return cmdtest.ExecuteCommand(cmd, cli, stdout, stderr) -} - -func Test_NewCmdNote(t *testing.T) { - fakeHTTP := httpmock.New() - defer fakeHTTP.Verify(t) - - t.Run("--message flag specified", func(t *testing.T) { - fakeHTTP.RegisterResponder(http.MethodPost, "/projects/OWNER/REPO/issues/1/notes", - 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/issues/1", - httpmock.NewStringResponse(http.StatusOK, ` - { - "id": 1, - "iid": 1, - "web_url": "https://gitlab.com/OWNER/REPO/issues/1" - } - `)) - - // glab mr note 1 --message "Here is my note" - output, err := runCommand(fakeHTTP, true, `1 --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/issues/1#note_301\n") - }) - - t.Run("issue not found", func(t *testing.T) { - fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/issues/122", - httpmock.NewStringResponse(http.StatusNotFound, ` - { - "message" : "issue not found" - } - `)) - - // glab mr note 1 --message "Here is my note" - _, err := runCommand(fakeHTTP, true, `122`) - assert.NotNil(t, err) - assert.Equal(t, "GET https://gitlab.com/api/v4/projects/OWNER/REPO/issues/122: 404 {message: issue not found}", err.Error()) - }) -} - -func Test_NewCmdNote_error(t *testing.T) { - fakeHTTP := httpmock.New() - defer fakeHTTP.Verify(t) - - t.Run("note could not be created", func(t *testing.T) { - fakeHTTP.RegisterResponder(http.MethodPost, "/projects/OWNER/REPO/issues/1/notes", - httpmock.NewStringResponse(http.StatusUnauthorized, ` - { - "message" : "Unauthorized" - } - `)) - - fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/issues/1", - httpmock.NewStringResponse(http.StatusOK, ` - { - "id": 1, - "iid": 1, - "web_url": "https://gitlab.com/OWNER/REPO/issues/1" - } - `)) - - // glab mr note 1 --message "Here is my note" - _, err := runCommand(fakeHTTP, true, `1 -m "Some message"`) - assert.NotNil(t, err) - assert.Equal(t, "POST https://gitlab.com/api/v4/projects/OWNER/REPO/issues/1/notes: 401 {message: Unauthorized}", 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.MethodPost, "/projects/OWNER/REPO/issues/1/notes", - 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/issues/1", - httpmock.NewStringResponse(http.StatusOK, ` - { - "id": 1, - "iid": 1, - "web_url": "https://gitlab.com/OWNER/REPO/issues/1" - } - `)) - as, teardown := prompt.InitAskStubber() - defer teardown() - as.StubOne("some note message") - - // glab mr note 1 - output, err := runCommand(fakeHTTP, true, `1`) - if err != nil { - t.Error(err) - return - } - assert.Equal(t, output.Stderr(), "") - assert.Equal(t, output.String(), "https://gitlab.com/OWNER/REPO/issues/1#note_301\n") - }) - - t.Run("message is empty", func(t *testing.T) { - fakeHTTP.RegisterResponder(http.MethodGet, "/projects/OWNER/REPO/issues/1", - httpmock.NewStringResponse(http.StatusOK, ` - { - "id": 1, - "iid": 1, - "web_url": "https://gitlab.com/OWNER/REPO/issues/1" - } - `)) - - as, teardown := prompt.InitAskStubber() - defer teardown() - as.StubOne("") - - // glab mr note 1 - _, err := runCommand(fakeHTTP, true, `1`) - if err == nil { - t.Error("expected error") - return - } - assert.Equal(t, "aborted... Note is empty", err.Error()) - }) -} diff --git a/docs/source/incident/index.md b/docs/source/incident/index.md index e4ea3f21..073e7ce3 100644 --- a/docs/source/incident/index.md +++ b/docs/source/incident/index.md @@ -36,6 +36,7 @@ glab incident list - [close](close.md) - [list](list.md) +- [note](note.md) - [reopen](reopen.md) - [subscribe](subscribe.md) - [unsubscribe](unsubscribe.md) diff --git a/docs/source/incident/note.md b/docs/source/incident/note.md new file mode 100644 index 00000000..a1349997 --- /dev/null +++ b/docs/source/incident/note.md @@ -0,0 +1,37 @@ +--- +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 incident note` + +Comment on an incident in GitLab + +```plaintext +glab incident note [flags] +``` + +## Aliases + +```plaintext +comment +``` + +## Options + +```plaintext + -m, --message string Comment/Note message +``` + +## Options inherited from parent commands + +```plaintext + --help Show help for command + -R, --repo OWNER/REPO Select another repository using the OWNER/REPO or `GROUP/NAMESPACE/REPO` format or full URL or git URL +``` diff --git a/docs/source/issue/note.md b/docs/source/issue/note.md index 56da31ea..2e9e8cec 100644 --- a/docs/source/issue/note.md +++ b/docs/source/issue/note.md @@ -11,7 +11,7 @@ Please do not edit this file directly. Run `make gen-docs` instead. # `glab issue note` -Add a comment or note to an issue on GitLab +Comment on an issue in GitLab ```plaintext glab issue note [flags]