From 86607d8aeb78dad3306d58e1e7db7d4172cd0015 Mon Sep 17 00:00:00 2001 From: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Date: Tue, 13 Jun 2023 04:27:42 +0100 Subject: [PATCH] feat: Add GitLab Reaction Emojis on MR Comments (#3456) * Add GitLab Reaction Emoji * Update docs * update docs * Update event_parser_test --- runatlantis.io/docs/server-configuration.md | 9 ++++++ .../controllers/events/events_controller.go | 6 ++-- .../events/events_controller_test.go | 16 +++++++++-- server/events/event_parser.go | 6 ++-- server/events/event_parser_test.go | 6 ++-- server/events/mocks/mock_event_parsing.go | 18 +++++++----- server/events/vcs/azuredevops_client.go | 2 +- server/events/vcs/bitbucketcloud/client.go | 2 +- server/events/vcs/bitbucketserver/client.go | 2 +- server/events/vcs/client.go | 2 +- server/events/vcs/github_client.go | 2 +- server/events/vcs/gitlab_client.go | 6 ++-- server/events/vcs/instrumented_client.go | 4 +-- server/events/vcs/mocks/mock_client.go | 28 +++++++++++-------- .../events/vcs/not_configured_vcs_client.go | 2 +- server/events/vcs/proxy.go | 4 +-- 16 files changed, 75 insertions(+), 40 deletions(-) diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 5a9271d314..920b39f178 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -371,6 +371,15 @@ and set `--autoplan-modules` to `false`. ``` Stops atlantis from locking projects and or workspaces when running terraform. +### `--emoji-reaction` + ```bash + atlantis server --emoji-reaction thumbsup + # or + ATLANTIS_EMOJI_REACTION=thumbsup + ``` + The emoji reaction to use for marking processed comments. Currently supported on Azure DevOps, GitHub and GitLab. + Defaults to `eyes`. + ### `--enable-policy-checks` ```bash atlantis server --enable-policy-checks diff --git a/server/controllers/events/events_controller.go b/server/controllers/events/events_controller.go index be2d548c85..0742e09065 100644 --- a/server/controllers/events/events_controller.go +++ b/server/controllers/events/events_controller.go @@ -515,12 +515,12 @@ func (e *VCSEventsController) handleGitlabPost(w http.ResponseWriter, r *http.Re // commands can come from. It's exported to make testing easier. func (e *VCSEventsController) HandleGitlabCommentEvent(w http.ResponseWriter, event gitlab.MergeCommentEvent) { // todo: can gitlab return the pull request here too? - baseRepo, headRepo, user, err := e.Parser.ParseGitlabMergeRequestCommentEvent(event) + baseRepo, headRepo, commentID, user, err := e.Parser.ParseGitlabMergeRequestCommentEvent(event) if err != nil { e.respond(w, logging.Error, http.StatusBadRequest, "Error parsing webhook: %s", err) return } - resp := e.handleCommentEvent(e.Logger, baseRepo, &headRepo, nil, user, event.MergeRequest.IID, event.ObjectAttributes.Note, -1, models.Gitlab) + resp := e.handleCommentEvent(e.Logger, baseRepo, &headRepo, nil, user, event.MergeRequest.IID, event.ObjectAttributes.Note, int64(commentID), models.Gitlab) //TODO: move this to the outer most function similar to github lvl := logging.Debug @@ -567,7 +567,7 @@ func (e *VCSEventsController) handleCommentEvent(logger logging.SimpleLogging, b // It's a comment we're gonna react to, so add a reaction. if e.EmojiReaction != "" { - err := e.VCSClient.ReactToComment(baseRepo, commentID, e.EmojiReaction) + err := e.VCSClient.ReactToComment(baseRepo, pullNum, commentID, e.EmojiReaction) if err != nil { logger.Warn("Failed to react to comment: %s", err) } diff --git a/server/controllers/events/events_controller_test.go b/server/controllers/events/events_controller_test.go index 62be8b261a..2b1c9d5f2e 100644 --- a/server/controllers/events/events_controller_test.go +++ b/server/controllers/events/events_controller_test.go @@ -189,7 +189,7 @@ func TestPost_GithubCommentInvalidCommand(t *testing.T) { w := httptest.NewRecorder() e.Post(w, req) ResponseContains(t, w, http.StatusOK, "Ignoring non-command comment: \"\"") - vcsClient.VerifyWasCalled(Never()).ReactToComment(models.Repo{}, 1, "eyes") + vcsClient.VerifyWasCalled(Never()).ReactToComment(models.Repo{}, 1, 1, "eyes") } func TestPost_GitlabCommentNotAllowlisted(t *testing.T) { @@ -398,7 +398,19 @@ func TestPost_GithubCommentReaction(t *testing.T) { e.Post(w, req) ResponseContains(t, w, http.StatusOK, "Processing...") - vcsClient.VerifyWasCalledOnce().ReactToComment(baseRepo, 1, "eyes") + vcsClient.VerifyWasCalledOnce().ReactToComment(baseRepo, 1, 1, "eyes") +} + +func TestPost_GilabCommentReaction(t *testing.T) { + t.Log("when the event is a gitlab comment with a valid command we call the ReactToComment handler") + e, _, gl, _, _, _, _, vcsClient, _ := setup(t) + req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) + req.Header.Set(gitlabHeader, "value") + When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil) + w := httptest.NewRecorder() + e.Post(w, req) + ResponseContains(t, w, http.StatusOK, "Processing...") + vcsClient.VerifyWasCalledOnce().ReactToComment(models.Repo{}, 0, 0, "eyes") } func TestPost_GithubPullRequestInvalid(t *testing.T) { diff --git a/server/events/event_parser.go b/server/events/event_parser.go index 2ccd6cad2b..48d4978adc 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -236,7 +236,7 @@ type EventParsing interface { // headRepo is the repo the merge request branch is from. // user is the pull request author. ParseGitlabMergeRequestCommentEvent(event gitlab.MergeCommentEvent) ( - baseRepo models.Repo, headRepo models.Repo, user models.User, err error) + baseRepo models.Repo, headRepo models.Repo, commentID int, user models.User, err error) // ParseGitlabMergeRequest parses the response from the GitLab API endpoint // that returns a merge request. @@ -672,10 +672,12 @@ func (e *EventParser) ParseGitlabMergeRequestEvent(event gitlab.MergeEvent) (pul // ParseGitlabMergeRequestCommentEvent parses GitLab merge request comment // events. // See EventParsing for return value docs. -func (e *EventParser) ParseGitlabMergeRequestCommentEvent(event gitlab.MergeCommentEvent) (baseRepo models.Repo, headRepo models.Repo, user models.User, err error) { +func (e *EventParser) ParseGitlabMergeRequestCommentEvent(event gitlab.MergeCommentEvent) (baseRepo models.Repo, headRepo models.Repo, commentID int, user models.User, err error) { // Parse the base repo first. + repoFullName := event.Project.PathWithNamespace cloneURL := event.Project.GitHTTPURL + commentID = event.ObjectAttributes.ID baseRepo, err = models.NewRepo(models.Gitlab, repoFullName, cloneURL, e.GitlabUser, e.GitlabToken) if err != nil { return diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go index 2806f03186..37c189a602 100644 --- a/server/events/event_parser_test.go +++ b/server/events/event_parser_test.go @@ -636,7 +636,7 @@ func TestParseGitlabMergeCommentEvent(t *testing.T) { var event *gitlab.MergeCommentEvent err = json.Unmarshal(bytes, &event) Ok(t, err) - baseRepo, headRepo, user, err := parser.ParseGitlabMergeRequestCommentEvent(*event) + baseRepo, headRepo, commentID, user, err := parser.ParseGitlabMergeRequestCommentEvent(*event) Ok(t, err) Equals(t, models.Repo{ FullName: "gitlabhq/gitlab-test", @@ -660,6 +660,7 @@ func TestParseGitlabMergeCommentEvent(t *testing.T) { Type: models.Gitlab, }, }, headRepo) + Equals(t, 1244, commentID) Equals(t, models.User{ Username: "root", }, user) @@ -673,7 +674,7 @@ func TestParseGitlabMergeCommentEvent_Subgroup(t *testing.T) { var event *gitlab.MergeCommentEvent err = json.Unmarshal(bytes, &event) Ok(t, err) - baseRepo, headRepo, user, err := parser.ParseGitlabMergeRequestCommentEvent(*event) + baseRepo, headRepo, commentID, user, err := parser.ParseGitlabMergeRequestCommentEvent(*event) Ok(t, err) Equals(t, models.Repo{ @@ -698,6 +699,7 @@ func TestParseGitlabMergeCommentEvent_Subgroup(t *testing.T) { Type: models.Gitlab, }, }, headRepo) + Equals(t, 96056916, commentID) Equals(t, models.User{ Username: "lkysow", }, user) diff --git a/server/events/mocks/mock_event_parsing.go b/server/events/mocks/mock_event_parsing.go index 2ea6f3f385..ce9881a5fd 100644 --- a/server/events/mocks/mock_event_parsing.go +++ b/server/events/mocks/mock_event_parsing.go @@ -413,16 +413,17 @@ func (mock *MockEventParsing) ParseGitlabMergeRequest(_param0 *go_gitlab.MergeRe return ret0 } -func (mock *MockEventParsing) ParseGitlabMergeRequestCommentEvent(_param0 go_gitlab.MergeCommentEvent) (models.Repo, models.Repo, models.User, error) { +func (mock *MockEventParsing) ParseGitlabMergeRequestCommentEvent(_param0 go_gitlab.MergeCommentEvent) (models.Repo, models.Repo, int, models.User, error) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockEventParsing().") } params := []pegomock.Param{_param0} - result := pegomock.GetGenericMockFrom(mock).Invoke("ParseGitlabMergeRequestCommentEvent", params, []reflect.Type{reflect.TypeOf((*models.Repo)(nil)).Elem(), reflect.TypeOf((*models.Repo)(nil)).Elem(), reflect.TypeOf((*models.User)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + result := pegomock.GetGenericMockFrom(mock).Invoke("ParseGitlabMergeRequestCommentEvent", params, []reflect.Type{reflect.TypeOf((*models.Repo)(nil)).Elem(), reflect.TypeOf((*models.Repo)(nil)).Elem(), reflect.TypeOf((*int)(nil)).Elem(), reflect.TypeOf((*models.User)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) var ret0 models.Repo var ret1 models.Repo - var ret2 models.User - var ret3 error + var ret2 int + var ret3 models.User + var ret4 error if len(result) != 0 { if result[0] != nil { ret0 = result[0].(models.Repo) @@ -431,13 +432,16 @@ func (mock *MockEventParsing) ParseGitlabMergeRequestCommentEvent(_param0 go_git ret1 = result[1].(models.Repo) } if result[2] != nil { - ret2 = result[2].(models.User) + ret2 = result[2].(int) } if result[3] != nil { - ret3 = result[3].(error) + ret3 = result[3].(models.User) + } + if result[4] != nil { + ret4 = result[4].(error) } } - return ret0, ret1, ret2, ret3 + return ret0, ret1, ret2, ret3, ret4 } func (mock *MockEventParsing) ParseGitlabMergeRequestEvent(_param0 go_gitlab.MergeEvent) (models.PullRequest, models.PullRequestEventType, models.Repo, models.Repo, models.User, error) { diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go index 5a5fa62485..2d48e5aba2 100644 --- a/server/events/vcs/azuredevops_client.go +++ b/server/events/vcs/azuredevops_client.go @@ -130,7 +130,7 @@ func (g *AzureDevopsClient) CreateComment(repo models.Repo, pullNum int, comment return nil } -func (g *AzureDevopsClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { //nolint: revive +func (g *AzureDevopsClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { //nolint: revive return nil } diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 977c68905a..38179590fa 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -101,7 +101,7 @@ func (b *Client) CreateComment(repo models.Repo, pullNum int, comment string, co } // UpdateComment updates the body of a comment on the merge request. -func (b *Client) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint revive +func (b *Client) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { // nolint revive // TODO: Bitbucket support for reactions return nil } diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index dabbad52b5..6df5c7c9a2 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -145,7 +145,7 @@ func (b *Client) CreateComment(repo models.Repo, pullNum int, comment string, co return nil } -func (b *Client) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint: revive +func (b *Client) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { // nolint: revive return nil } diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index d2b30bfea0..6d7421b12e 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -26,7 +26,7 @@ type Client interface { GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) CreateComment(repo models.Repo, pullNum int, comment string, command string) error - ReactToComment(repo models.Repo, commentID int64, reaction string) error + ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error HidePrevCommandComments(repo models.Repo, pullNum int, command string) error PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 5d0b688b43..7f67cb3dcc 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -199,7 +199,7 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri } // ReactToComment adds a reaction to a comment. -func (g *GithubClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { +func (g *GithubClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { g.logger.Debug("POST /repos/%v/%v/issues/comments/%d/reactions", repo.Owner, repo.Name, commentID) _, _, err := g.client.Reactions.CreateIssueCommentReaction(g.ctx, repo.Owner, repo.Name, commentID, reaction) return err diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index c24e0ad357..d9a769e362 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -180,8 +180,10 @@ func (g *GitlabClient) CreateComment(repo models.Repo, pullNum int, comment stri return nil } -func (g *GitlabClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint: revive - return nil +// ReactToComment adds a reaction to a comment. +func (g *GitlabClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { + _, _, err := g.Client.AwardEmoji.CreateMergeRequestAwardEmojiOnNote(repo.FullName, pullNum, int(commentID), &gitlab.CreateAwardEmojiOptions{Name: reaction}) + return err } func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { diff --git a/server/events/vcs/instrumented_client.go b/server/events/vcs/instrumented_client.go index 46e9b74b2f..bcb2524124 100644 --- a/server/events/vcs/instrumented_client.go +++ b/server/events/vcs/instrumented_client.go @@ -128,7 +128,7 @@ func (c *InstrumentedClient) CreateComment(repo models.Repo, pullNum int, commen return nil } -func (c *InstrumentedClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { +func (c *InstrumentedClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { scope := c.StatsScope.SubScope("react_to_comment") executionTime := scope.Timer(metrics.ExecutionTimeMetric).Start() @@ -137,7 +137,7 @@ func (c *InstrumentedClient) ReactToComment(repo models.Repo, commentID int64, r executionSuccess := scope.Counter(metrics.ExecutionSuccessMetric) executionError := scope.Counter(metrics.ExecutionErrorMetric) - if err := c.Client.ReactToComment(repo, commentID, reaction); err != nil { + if err := c.Client.ReactToComment(repo, pullNum, commentID, reaction); err != nil { executionError.Inc(1) c.Logger.Err("Unable to react to comment, error: %s", err.Error()) return err diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index 5a3cb9a432..0307ffd5d6 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -222,11 +222,11 @@ func (mock *MockClient) PullIsMergeable(_param0 models.Repo, _param1 models.Pull return ret0, ret1 } -func (mock *MockClient) ReactToComment(_param0 models.Repo, _param1 int64, _param2 string) error { +func (mock *MockClient) ReactToComment(_param0 models.Repo, _param1 int, _param2 int64, _param3 string) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") } - params := []pegomock.Param{_param0, _param1, _param2} + params := []pegomock.Param{_param0, _param1, _param2, _param3} result := pegomock.GetGenericMockFrom(mock).Invoke("ReactToComment", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -657,8 +657,8 @@ func (c *MockClient_PullIsMergeable_OngoingVerification) GetAllCapturedArguments return } -func (verifier *VerifierMockClient) ReactToComment(_param0 models.Repo, _param1 int64, _param2 string) *MockClient_ReactToComment_OngoingVerification { - params := []pegomock.Param{_param0, _param1, _param2} +func (verifier *VerifierMockClient) ReactToComment(_param0 models.Repo, _param1 int, _param2 int64, _param3 string) *MockClient_ReactToComment_OngoingVerification { + params := []pegomock.Param{_param0, _param1, _param2, _param3} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "ReactToComment", params, verifier.timeout) return &MockClient_ReactToComment_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -668,25 +668,29 @@ type MockClient_ReactToComment_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockClient_ReactToComment_OngoingVerification) GetCapturedArguments() (models.Repo, int64, string) { - _param0, _param1, _param2 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] +func (c *MockClient_ReactToComment_OngoingVerification) GetCapturedArguments() (models.Repo, int, int64, string) { + _param0, _param1, _param2, _param3 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1], _param3[len(_param3)-1] } -func (c *MockClient_ReactToComment_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []int64, _param2 []string) { +func (c *MockClient_ReactToComment_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []int, _param2 []int64, _param3 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.Repo, len(c.methodInvocations)) for u, param := range params[0] { _param0[u] = param.(models.Repo) } - _param1 = make([]int64, len(c.methodInvocations)) + _param1 = make([]int, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(int64) + _param1[u] = param.(int) } - _param2 = make([]string, len(c.methodInvocations)) + _param2 = make([]int64, len(c.methodInvocations)) for u, param := range params[2] { - _param2[u] = param.(string) + _param2[u] = param.(int64) + } + _param3 = make([]string, len(c.methodInvocations)) + for u, param := range params[3] { + _param3[u] = param.(string) } } return diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index 2594d9efe8..8ab7e03e89 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -35,7 +35,7 @@ func (a *NotConfiguredVCSClient) CreateComment(repo models.Repo, pullNum int, co func (a *NotConfiguredVCSClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { return nil } -func (a *NotConfiguredVCSClient) ReactToComment(repo models.Repo, commentID int64, reaction string) error { // nolint: revive +func (a *NotConfiguredVCSClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { // nolint: revive return nil } func (a *NotConfiguredVCSClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) { diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index a0afd43bc7..64fb8fa8ef 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -64,8 +64,8 @@ func (d *ClientProxy) HidePrevCommandComments(repo models.Repo, pullNum int, com return d.clients[repo.VCSHost.Type].HidePrevCommandComments(repo, pullNum, command) } -func (d *ClientProxy) ReactToComment(repo models.Repo, commentID int64, reaction string) error { - return d.clients[repo.VCSHost.Type].ReactToComment(repo, commentID, reaction) +func (d *ClientProxy) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error { + return d.clients[repo.VCSHost.Type].ReactToComment(repo, pullNum, commentID, reaction) } func (d *ClientProxy) PullIsApproved(repo models.Repo, pull models.PullRequest) (models.ApprovalStatus, error) {