diff --git a/README.md b/README.md index 25a7b54e..424d8e36 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,11 @@ Currently supported providers are: [GitHub](#github), [Bitbucket Server](#bitbuc - [List Open Pull Requests](#list-open-pull-requests) - [List Open Pull Requests With Body](#list-open-pull-requests-with-body) - [Add Pull Request Comment](#add-pull-request-comment) + - [Add Pull Request Review Comments](#add-pull-request-review-comments) - [List Pull Request Comments](#list-pull-request-comments) + - [List Pull Request Review Comments](#list-pull-request-review-comments) + - [Delete Pull Request Comment](#delete-pull-request-comment) + - [Delete Pull Request Review Comments](#delete-pull-request-review-comments) - [Get Commits](#get-commits) - [Get Latest Commit](#get-latest-commit) - [Get Commit By SHA](#get-commit-by-sha) @@ -417,6 +421,42 @@ pullRequestID := 5 err := client.AddPullRequestComment(ctx, owner, repository, content, pullRequestID) ``` +##### Add Pull Request Review Comments + +```go +// Go context +ctx := context.Background() +// Organization or username +owner := "jfrog" +// VCS repository +repository := "jfrog-cli" +// Pull Request ID +pullRequestID := 5 +// Pull Request Comment +comments := []PullRequestComment{ + { + CommentInfo: CommentInfo{ + Content: "content", + }, + PullRequestDiff: PullRequestDiff{ + OriginalFilePath: index.js + OriginalStartLine: 1 + OriginalEndLine: 1 + OriginalStartColumn: 1 + OriginalEndColumn: 1 + NewFilePath: index.js + NewStartLine: 1 + NewEndLine: 1 + NewStartColumn: 1 + NewEndColumn: 1 + }, + } +} + + +err := client.AddPullRequestReviewComments(ctx, owner, repository, pullRequestID, comments...) +``` + ##### List Pull Request Comments ```go @@ -432,6 +472,62 @@ pullRequestID := 5 pullRequestComments, err := client.ListPullRequestComment(ctx, owner, repository, pullRequestID) ``` +##### List Pull Request Review Comments + +```go +// Go context +ctx := context.Background() +// Organization or username +owner := "jfrog" +// VCS repository +repository := "jfrog-cli" +// Pull Request ID +pullRequestID := 5 + +pullRequestComments, err := client.ListPullRequestReviewComments(ctx, owner, repository, pullRequestID) +``` + +##### Delete Pull Request Comment + +```go +// Go context +ctx := context.Background() +// Organization or username +owner := "jfrog" +// VCS repository +repository := "jfrog-cli" +// Pull Request ID +pullRequestID := 5 +// Comment ID +commentID := 17 + +err := client.DeletePullRequestComment(ctx, owner, repository, pullRequestID, commentID) +``` + +##### Delete Pull Request Review Comments + +```go +// Go context +ctx := context.Background() +// Organization or username +owner := "jfrog" +// VCS repository +repository := "jfrog-cli" +// Pull Request ID +pullRequestID := 5 +// Comment ID +comments := []CommentInfo{ + { + ID: 2 + // For GitLab + ThreadID: 7 + } +} + +err := client.DeletePullRequestComment(ctx, owner, repository, pullRequestID, comments...) +``` + + #### Get Commits ```go diff --git a/vcsclient/azurerepos.go b/vcsclient/azurerepos.go index b3d40152..9631c6e3 100644 --- a/vcsclient/azurerepos.go +++ b/vcsclient/azurerepos.go @@ -207,24 +207,58 @@ func (client *AzureReposClient) UpdatePullRequest(ctx context.Context, owner, re // AddPullRequestComment on Azure Repos func (client *AzureReposClient) AddPullRequestComment(ctx context.Context, _, repository, content string, pullRequestID int) error { + return client.addPullRequestComment(ctx, repository, pullRequestID, PullRequestComment{CommentInfo: CommentInfo{Content: content}}) +} + +// AddPullRequestReviewComments on Azure Repos +func (client *AzureReposClient) AddPullRequestReviewComments(ctx context.Context, _, repository string, pullRequestID int, comments ...PullRequestComment) error { + if len(comments) == 0 { + return errors.New(vcsutils.ErrNoCommentsProvided) + } + for _, comment := range comments { + if err := client.addPullRequestComment(ctx, repository, pullRequestID, comment); err != nil { + return err + } + } + return nil +} + +func (client *AzureReposClient) addPullRequestComment(ctx context.Context, repository string, pullRequestID int, comment PullRequestComment) error { azureReposGitClient, err := client.buildAzureReposClient(ctx) if err != nil { return err } - // To add a new comment to the pull request, we need to open a new thread, and add a comment inside this thread. - _, err = azureReposGitClient.CreateThread(ctx, git.CreateThreadArgs{ + threadArgs := getThreadArgs(repository, client.vcsInfo.Project, pullRequestID, comment) + _, err = azureReposGitClient.CreateThread(ctx, threadArgs) + return err +} + +func getThreadArgs(repository, project string, prId int, comment PullRequestComment) git.CreateThreadArgs { + filePath := vcsutils.GetPullRequestFilePath(comment.NewFilePath) + return git.CreateThreadArgs{ CommentThread: &git.GitPullRequestCommentThread{ - Comments: &[]git.Comment{{Content: &content}}, + Comments: &[]git.Comment{{Content: &comment.Content}}, Status: &git.CommentThreadStatusValues.Active, + ThreadContext: &git.CommentThreadContext{ + FilePath: &filePath, + LeftFileStart: &git.CommentPosition{Line: &comment.OriginalStartLine, Offset: &comment.OriginalStartColumn}, + LeftFileEnd: &git.CommentPosition{Line: &comment.OriginalEndLine, Offset: &comment.OriginalEndColumn}, + RightFileStart: &git.CommentPosition{Line: &comment.NewStartLine, Offset: &comment.NewStartColumn}, + RightFileEnd: &git.CommentPosition{Line: &comment.NewEndLine, Offset: &comment.NewEndColumn}, + }, }, RepositoryId: &repository, - PullRequestId: &pullRequestID, - Project: &client.vcsInfo.Project, - }) - return err + PullRequestId: &prId, + Project: &project, + } } -// ListPullRequestComments returns all the pull request threads with their comments. +// ListPullRequestReviewComments on Azure Repos +func (client *AzureReposClient) ListPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) { + return client.ListPullRequestComments(ctx, owner, repository, pullRequestID) +} + +// ListPullRequestComments on Azure Repos func (client *AzureReposClient) ListPullRequestComments(ctx context.Context, _, repository string, pullRequestID int) ([]CommentInfo, error) { azureReposGitClient, err := client.buildAzureReposClient(ctx) if err != nil { @@ -266,6 +300,16 @@ func (client *AzureReposClient) ListPullRequestComments(ctx context.Context, _, return commentInfo, nil } +// DeletePullRequestReviewComments on Azure Repos +func (client *AzureReposClient) DeletePullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...CommentInfo) error { + for _, comment := range comments { + if err := client.DeletePullRequestComment(ctx, owner, repository, pullRequestID, int(comment.ID)); err != nil { + return err + } + } + return nil +} + // DeletePullRequestComment on Azure Repos func (client *AzureReposClient) DeletePullRequestComment(ctx context.Context, _, repository string, pullRequestID, commentID int) error { azureReposGitClient, err := client.buildAzureReposClient(ctx) @@ -314,6 +358,7 @@ func (client *AzureReposClient) getOpenPullRequests(ctx context.Context, owner, return pullRequestsInfo, nil } +// GetPullRequestById in Azure Repos func (client *AzureReposClient) GetPullRequestByID(ctx context.Context, owner, repository string, pullRequestId int) (pullRequestInfo PullRequestInfo, err error) { azureReposGitClient, err := client.buildAzureReposClient(ctx) if err != nil { diff --git a/vcsclient/azurerepos_test.go b/vcsclient/azurerepos_test.go index 00faa054..bfa8448f 100644 --- a/vcsclient/azurerepos_test.go +++ b/vcsclient/azurerepos_test.go @@ -179,6 +179,67 @@ func TestAzureRepos_TestAddPullRequestComment(t *testing.T) { assert.Error(t, err) } +func TestAzureRepos_TestAddPullRequestReviewComments(t *testing.T) { + type AddPullRequestCommentResponse struct { + Value git.GitPullRequestCommentThread + Count int + } + id := 123 + pom := "/pom.xml" + startLine := 1 + endLine := 5 + startColumn := 1 + endColumn := 13 + res := AddPullRequestCommentResponse{ + Value: git.GitPullRequestCommentThread{Id: &id, ThreadContext: &git.CommentThreadContext{ + FilePath: &pom, + LeftFileEnd: &git.CommentPosition{ + Line: &endLine, + Offset: &endColumn, + }, + LeftFileStart: &git.CommentPosition{ + Line: &startLine, + Offset: &startColumn, + }, + RightFileEnd: &git.CommentPosition{ + Line: &endLine, + Offset: &endColumn, + }, + RightFileStart: &git.CommentPosition{ + Line: &startLine, + Offset: &startColumn, + }, + }}, + Count: 1, + } + jsonRes, err := json.Marshal(res) + assert.NoError(t, err) + ctx := context.Background() + client, cleanUp := createServerAndClient(t, vcsutils.AzureRepos, true, jsonRes, "pullRequestComments", createAzureReposHandler) + defer cleanUp() + err = client.AddPullRequestReviewComments(ctx, "", repo1, 2, PullRequestComment{ + CommentInfo: CommentInfo{Content: "test"}, + PullRequestDiff: PullRequestDiff{ + OriginalFilePath: pom, + OriginalStartLine: startLine, + OriginalEndLine: endLine, + OriginalStartColumn: startColumn, + OriginalEndColumn: startColumn, + NewFilePath: pom, + NewStartLine: startLine, + NewEndLine: endLine, + NewStartColumn: startColumn, + NewEndColumn: endColumn, + }, + }) + assert.NoError(t, err) + + badClient, cleanUp := createBadAzureReposClient(t, []byte{}) + defer cleanUp() + err = badClient.AddPullRequestReviewComments(ctx, "", repo1, 2, PullRequestComment{CommentInfo: CommentInfo{Content: "test"}}) + assert.Error(t, err) +} + func TestAzureRepos_TestListOpenPullRequests(t *testing.T) { type ListOpenPullRequestsResponse struct { Value []git.GitPullRequest @@ -320,6 +381,10 @@ func TestAzureReposClient_GetPullRequest(t *testing.T) { assert.Error(t, err) } +func TestListPullRequestReviewComments(t *testing.T) { + TestListPullRequestComments(t) +} + func TestListPullRequestComments(t *testing.T) { type ListPullRequestCommentsResponse struct { Value []git.GitPullRequestCommentThread @@ -635,6 +700,17 @@ func TestAzureReposClient_GetModifiedFiles(t *testing.T) { }) } +func TestAzureReposClient_DeletePullRequestReviewComments(t *testing.T) { + client, cleanUp := createServerAndClient(t, vcsutils.AzureRepos, true, "", "deletePullRequestComments", createAzureReposHandler) + defer cleanUp() + err := client.DeletePullRequestReviewComments(context.Background(), "", repo1, 1, []CommentInfo{{ID: 1}, {ID: 2}}...) + assert.NoError(t, err) + badClient, badClientCleanup := createBadAzureReposClient(t, []byte{}) + defer badClientCleanup() + err = badClient.DeletePullRequestReviewComments(context.Background(), "", repo1, 1, []CommentInfo{{ID: 1}, {ID: 2}}...) + assert.Error(t, err) +} + func TestAzureReposClient_DeletePullRequestComment(t *testing.T) { client, cleanUp := createServerAndClient(t, vcsutils.AzureRepos, true, "", "deletePullRequestComments", createAzureReposHandler) defer cleanUp() diff --git a/vcsclient/bitbucketcloud.go b/vcsclient/bitbucketcloud.go index 3d767a06..636bb782 100644 --- a/vcsclient/bitbucketcloud.go +++ b/vcsclient/bitbucketcloud.go @@ -409,6 +409,16 @@ func (client *BitbucketCloudClient) AddPullRequestComment(ctx context.Context, o return err } +// AddPullRequestReviewComments on Bitbucket cloud +func (client *BitbucketCloudClient) AddPullRequestReviewComments(_ context.Context, _, _ string, _ int, _ ...PullRequestComment) error { + return errBitbucketAddPullRequestReviewCommentsNotSupported +} + +// ListPullRequestReviewComments on Bitbucket cloud +func (client *BitbucketCloudClient) ListPullRequestReviewComments(_ context.Context, _, _ string, _ int) ([]CommentInfo, error) { + return nil, errBitbucketListPullRequestReviewCommentsNotSupported +} + // ListPullRequestComments on Bitbucket cloud func (client *BitbucketCloudClient) ListPullRequestComments(ctx context.Context, owner, repository string, pullRequestID int) (res []CommentInfo, err error) { err = validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository}) @@ -432,9 +442,14 @@ func (client *BitbucketCloudClient) ListPullRequestComments(ctx context.Context, return mapBitbucketCloudCommentToCommentInfo(&parsedComments), nil } +// DeletePullRequestReviewComments on Bitbucket cloud +func (client *BitbucketCloudClient) DeletePullRequestReviewComments(_ context.Context, _, _ string, _ int, _ ...CommentInfo) error { + return errBitbucketDeletePullRequestComment +} + // DeletePullRequestComment on Bitbucket cloud func (client *BitbucketCloudClient) DeletePullRequestComment(_ context.Context, _, _ string, _, _ int) error { - return nil + return errBitbucketDeletePullRequestComment } // GetLatestCommit on Bitbucket cloud diff --git a/vcsclient/bitbucketcloud_test.go b/vcsclient/bitbucketcloud_test.go index 06c2380a..951fcaa8 100644 --- a/vcsclient/bitbucketcloud_test.go +++ b/vcsclient/bitbucketcloud_test.go @@ -464,6 +464,42 @@ func TestBitbucketCloud_CreateLabel(t *testing.T) { assert.ErrorIs(t, err, errLabelsNotSupported) } +func TestBitbucketCloud_AddPullRequestReviewComments(t *testing.T) { + ctx := context.Background() + client, err := NewClientBuilder(vcsutils.BitbucketCloud).Build() + assert.NoError(t, err) + + err = client.AddPullRequestReviewComments(ctx, owner, repo1, 1) + assert.ErrorIs(t, err, errBitbucketAddPullRequestReviewCommentsNotSupported) +} + +func TestBitbucketCloudClient_ListPullRequestReviewComments(t *testing.T) { + ctx := context.Background() + client, err := NewClientBuilder(vcsutils.BitbucketCloud).Build() + assert.NoError(t, err) + + _, err = client.ListPullRequestReviewComments(ctx, owner, repo1, 1) + assert.ErrorIs(t, err, errBitbucketListPullRequestReviewCommentsNotSupported) +} + +func TestBitbucketCloudClient_DeletePullRequestComment(t *testing.T) { + ctx := context.Background() + client, err := NewClientBuilder(vcsutils.BitbucketCloud).Build() + assert.NoError(t, err) + + err = client.DeletePullRequestComment(ctx, owner, repo1, 1, 1) + assert.ErrorIs(t, err, errBitbucketDeletePullRequestComment) +} + +func TestBitbucketCloudClient_DeletePullRequestReviewComment(t *testing.T) { + ctx := context.Background() + client, err := NewClientBuilder(vcsutils.BitbucketCloud).Build() + assert.NoError(t, err) + + err = client.DeletePullRequestReviewComments(ctx, owner, repo1, 1, CommentInfo{}) + assert.ErrorIs(t, err, errBitbucketDeletePullRequestComment) +} + func TestBitbucketCloudClient_DownloadFileFromRepo(t *testing.T) { ctx := context.Background() client, err := NewClientBuilder(vcsutils.BitbucketCloud).Build() diff --git a/vcsclient/bitbucketcommon.go b/vcsclient/bitbucketcommon.go index 9c23cf2e..c33c125d 100644 --- a/vcsclient/bitbucketcommon.go +++ b/vcsclient/bitbucketcommon.go @@ -1,19 +1,23 @@ package vcsclient import ( - "errors" "fmt" "github.com/jfrog/froggit-go/vcsutils" "github.com/mitchellh/mapstructure" "time" ) +const notSupportedOnBitbucket = "currently not supported on Bitbucket" + var ( - errLabelsNotSupported = errors.New("labels are not supported on Bitbucket") - errBitbucketCodeScanningNotSupported = errors.New("code scanning is not supported on Bitbucket") - errBitbucketDownloadFileFromRepoNotSupported = errors.New("download file from repo is currently not supported on Bitbucket") - errBitbucketGetCommitsNotSupported = errors.New("get commits is currently not supported on Bitbucket") - errBitbucketGetRepoEnvironmentInfoNotSupported = errors.New("get repository environment info is currently not supported on Bitbucket") + errLabelsNotSupported = fmt.Errorf("labels are %s", notSupportedOnBitbucket) + errBitbucketCodeScanningNotSupported = fmt.Errorf("code scanning is %s", notSupportedOnBitbucket) + errBitbucketDownloadFileFromRepoNotSupported = fmt.Errorf("download file from repo is %s", notSupportedOnBitbucket) + errBitbucketGetCommitsNotSupported = fmt.Errorf("get commits is %s", notSupportedOnBitbucket) + errBitbucketGetRepoEnvironmentInfoNotSupported = fmt.Errorf("get repository environment info is %s", notSupportedOnBitbucket) + errBitbucketListPullRequestReviewCommentsNotSupported = fmt.Errorf("list pull request review comments is %s", notSupportedOnBitbucket) + errBitbucketAddPullRequestReviewCommentsNotSupported = fmt.Errorf("add pull request review comment is %s", notSupportedOnBitbucket) + errBitbucketDeletePullRequestComment = fmt.Errorf("delete pull request comment is %s", notSupportedOnBitbucket) ) type BitbucketCommitInfo struct { diff --git a/vcsclient/bitbucketserver.go b/vcsclient/bitbucketserver.go index ce179ae0..5ae7c4a8 100644 --- a/vcsclient/bitbucketserver.go +++ b/vcsclient/bitbucketserver.go @@ -402,18 +402,54 @@ func mapBitbucketServerPullRequestToPullRequestInfo(pullRequest bitbucketv1.Pull // AddPullRequestComment on Bitbucket server func (client *BitbucketServerClient) AddPullRequestComment(ctx context.Context, owner, repository, content string, pullRequestID int) error { - err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository, "content": content}) + return client.addPullRequestComment(ctx, owner, repository, pullRequestID, PullRequestComment{CommentInfo: CommentInfo{Content: content}}) +} + +// AddPullRequestReviewComments on Bitbucket server +func (client *BitbucketServerClient) AddPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...PullRequestComment) error { + if len(comments) == 0 { + return errors.New(vcsutils.ErrNoCommentsProvided) + } + for _, comment := range comments { + if err := client.addPullRequestComment(ctx, owner, repository, pullRequestID, comment); err != nil { + return err + } + } + return nil +} + +func (client *BitbucketServerClient) addPullRequestComment(ctx context.Context, owner, repository string, pullRequestID int, comment PullRequestComment) error { + err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository, "content": comment.Content}) if err != nil { return err } bitbucketClient := client.buildBitbucketClient(ctx) - _, err = bitbucketClient.CreatePullRequestComment(owner, repository, pullRequestID, bitbucketv1.Comment{ - Text: content, - }, []string{"application/json"}) + // Determine the file path and anchor + var anchor *bitbucketv1.Anchor + if filePath := vcsutils.GetPullRequestFilePath(comment.NewFilePath); filePath != "" { + anchor = &bitbucketv1.Anchor{ + Line: comment.NewStartLine, + LineType: "CONTEXT", + FileType: "FROM", + Path: filePath, + SrcPath: filePath, + } + } + // Create the pull request comment + commentData := bitbucketv1.Comment{ + Text: comment.Content, + Anchor: anchor, + } + _, err = bitbucketClient.CreatePullRequestComment(owner, repository, pullRequestID, commentData, []string{"application/json"}) return err } +// ListPullRequestReviewComments on Bitbucket server +func (client *BitbucketServerClient) ListPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) { + return client.ListPullRequestComments(ctx, owner, repository, pullRequestID) +} + // ListPullRequestComments on Bitbucket server func (client *BitbucketServerClient) ListPullRequestComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) { bitbucketClient := client.buildBitbucketClient(ctx) @@ -444,6 +480,16 @@ func (client *BitbucketServerClient) ListPullRequestComments(ctx context.Context return results, nil } +// DeletePullRequestReviewComments on Bitbucket server +func (client *BitbucketServerClient) DeletePullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...CommentInfo) error { + for _, comment := range comments { + if err := client.DeletePullRequestComment(ctx, owner, repository, pullRequestID, int(comment.ID)); err != nil { + return err + } + } + return nil +} + // DeletePullRequestComment on Bitbucket Server func (client *BitbucketServerClient) DeletePullRequestComment(ctx context.Context, owner, repository string, pullRequestID, commentID int) error { bitbucketClient := client.buildBitbucketClient(ctx) diff --git a/vcsclient/bitbucketserver_test.go b/vcsclient/bitbucketserver_test.go index 33060652..c6cd1310 100644 --- a/vcsclient/bitbucketserver_test.go +++ b/vcsclient/bitbucketserver_test.go @@ -209,6 +209,18 @@ func TestBitbucketServer_AddPullRequestComment(t *testing.T) { assert.Error(t, err) } +func TestBitbucketServer_AddPullRequestReviewComment(t *testing.T) { + ctx := context.Background() + client, cleanUp := createServerAndClient(t, vcsutils.BitbucketServer, true, nil, "/rest/api/1.0/projects/jfrog/repos/repo-1/pull-requests/1/comments", createBitbucketServerHandler) + defer cleanUp() + + err := client.AddPullRequestReviewComments(ctx, owner, repo1, 1, PullRequestComment{CommentInfo: CommentInfo{Content: "Comment content"}, PullRequestDiff: PullRequestDiff{OriginalStartLine: 7, NewStartLine: 7}}) + assert.NoError(t, err) + + err = createBadBitbucketServerClient(t).AddPullRequestReviewComments(ctx, owner, repo1, 1, PullRequestComment{CommentInfo: CommentInfo{Content: "Comment content"}, PullRequestDiff: PullRequestDiff{OriginalStartLine: 7, NewStartLine: 7}}) + assert.Error(t, err) +} + func TestBitbucketServer_ListOpenPullRequests(t *testing.T) { ctx := context.Background() response, err := os.ReadFile(filepath.Join("testdata", "bitbucketserver", "pull_requests_list_response.json")) @@ -282,6 +294,10 @@ func TestBitbucketServerClient_GetPullRequest(t *testing.T) { assert.Error(t, err) } +func TestBitbucketServer_ListPullRequestReviewComments(t *testing.T) { + TestBitbucketServer_ListPullRequestComments(t) +} + func TestBitbucketServer_ListPullRequestComments(t *testing.T) { ctx := context.Background() response, err := os.ReadFile(filepath.Join("testdata", "bitbucketserver", "pull_request_comments_list_response.json")) @@ -829,6 +845,22 @@ func TestBitbucketServer_TestGetCommitStatus(t *testing.T) { }) } +func TestBitbucketServerClient_DeletePullRequestReviewComments(t *testing.T) { + ctx := context.Background() + prId := 4 + commentId := 10 + version := 0 + client, cleanUp := createServerAndClient(t, vcsutils.BitbucketServer, true, nil, fmt.Sprintf("/rest/api/1.0/projects/jfrog/repos/repo-1/pull-requests/%v/activities?start=%v", prId, version)+ + fmt.Sprintf("/rest/api/1.0/projects/jfrog/repos/repo-1/pull-requests/%v/comments/%v?version=%v", prId, commentId, version), createBitbucketServerHandler) + defer cleanUp() + + err := client.DeletePullRequestReviewComments(ctx, owner, repo1, prId, CommentInfo{ID: int64(commentId)}) + assert.NoError(t, err) + + err = createBadBitbucketServerClient(t).DeletePullRequestReviewComments(ctx, owner, repo1, prId, CommentInfo{ID: int64(commentId)}) + assert.Error(t, err) +} + func TestBitbucketServerClient_DeletePullRequestComment(t *testing.T) { ctx := context.Background() prId := 4 diff --git a/vcsclient/github.go b/vcsclient/github.go index 70a19a91..5e6bca03 100644 --- a/vcsclient/github.go +++ b/vcsclient/github.go @@ -5,19 +5,19 @@ import ( "encoding/json" "errors" "fmt" + "github.com/google/go-github/v45/github" + "github.com/grokify/mogo/encoding/base64" + "github.com/jfrog/froggit-go/vcsutils" "github.com/jfrog/gofrog/datastructures" + "github.com/mitchellh/mapstructure" + "golang.org/x/oauth2" "io" "net/http" "net/url" + "path/filepath" "sort" "strconv" "strings" - - "github.com/google/go-github/v45/github" - "github.com/grokify/mogo/encoding/base64" - "github.com/jfrog/froggit-go/vcsutils" - "github.com/mitchellh/mapstructure" - "golang.org/x/oauth2" ) // GitHubClient API version 3 @@ -418,6 +418,76 @@ func (client *GitHubClient) AddPullRequestComment(ctx context.Context, owner, re return err } +// AddPullRequestReviewComment on GitHub +func (client *GitHubClient) AddPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...PullRequestComment) error { + prID := strconv.Itoa(pullRequestID) + if err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository, "pullRequestID": prID}); err != nil { + return err + } + if len(comments) == 0 { + return errors.New(vcsutils.ErrNoCommentsProvided) + } + ghClient, err := client.buildGithubClient(ctx) + if err != nil { + return err + } + + commits, _, err := ghClient.PullRequests.ListCommits(ctx, owner, repository, pullRequestID, nil) + if err != nil { + return err + } + if len(commits) == 0 { + return errors.New("could not fetch the commits list for pull request " + prID) + } + + latestCommitSHA := commits[len(commits)-1].GetSHA() + + for _, comment := range comments { + filePath := filepath.Clean(comment.NewFilePath) + startLine := &comment.NewStartLine + // GitHub API won't accept 'start_line' if it equals the end line + if *startLine == comment.NewEndLine { + startLine = nil + } + if _, _, err = ghClient.PullRequests.CreateComment(ctx, owner, repository, pullRequestID, &github.PullRequestComment{ + CommitID: &latestCommitSHA, + Body: &comment.Content, + StartLine: startLine, + Line: &comment.NewEndLine, + Path: &filePath, + }); err != nil { + return fmt.Errorf("could not create a code review comment for <%s/%s> in pull request %s. error received: %w", + owner, repository, prID, err) + } + } + return nil +} + +// ListPullRequestReviewComments on GitHub +func (client *GitHubClient) ListPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) { + err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository}) + if err != nil { + return nil, err + } + ghClient, err := client.buildGithubClient(ctx) + if err != nil { + return nil, err + } + commentsList, _, err := ghClient.PullRequests.ListReviews(ctx, owner, repository, pullRequestID, nil) + if err != nil { + return []CommentInfo{}, err + } + commentsInfoList := []CommentInfo{} + for _, comment := range commentsList { + commentsInfoList = append(commentsInfoList, CommentInfo{ + ID: comment.GetID(), + Content: comment.GetBody(), + Created: comment.GetSubmittedAt(), + }) + } + return commentsInfoList, nil +} + // ListPullRequestComments on GitHub func (client *GitHubClient) ListPullRequestComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) { err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository}) @@ -432,7 +502,25 @@ func (client *GitHubClient) ListPullRequestComments(ctx context.Context, owner, if err != nil { return []CommentInfo{}, err } - return mapGitHubCommentToCommentInfoList(commentsList) + return mapGitHubIssuesCommentToCommentInfoList(commentsList) +} + +// DeletePullRequestReviewComments on GitHub +func (client *GitHubClient) DeletePullRequestReviewComments(ctx context.Context, owner, repository string, _ int, comments ...CommentInfo) error { + for _, comment := range comments { + commentID := comment.ID + if err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository, "commentID": strconv.FormatInt(commentID, 10)}); err != nil { + return err + } + ghClient, err := client.buildGithubClient(ctx) + if err != nil { + return err + } + if _, err = ghClient.PullRequests.DeleteComment(ctx, owner, repository, commentID); err != nil { + return fmt.Errorf("could not delete pull request review comment: %w", err) + } + } + return nil } // DeletePullRequestComment on GitHub @@ -866,7 +954,7 @@ func mapGitHubCommitToCommitInfo(commit *github.RepositoryCommit) CommitInfo { } } -func mapGitHubCommentToCommentInfoList(commentsList []*github.IssueComment) (res []CommentInfo, err error) { +func mapGitHubIssuesCommentToCommentInfoList(commentsList []*github.IssueComment) (res []CommentInfo, err error) { for _, comment := range commentsList { res = append(res, CommentInfo{ ID: *comment.ID, diff --git a/vcsclient/github_test.go b/vcsclient/github_test.go index 27c15d3b..2e0843f3 100644 --- a/vcsclient/github_test.go +++ b/vcsclient/github_test.go @@ -277,6 +277,57 @@ func TestGitHubClient_AddPullRequestComment(t *testing.T) { assert.Error(t, err) } +func TestGitHubClient_AddPullRequestReviewComments(t *testing.T) { + ctx := context.Background() + client, cleanUp := createServerAndClient(t, vcsutils.GitHub, false, github.PullRequestReview{}, "/repos/jfrog/repo-1/pulls/1/comments", createAddPullRequestReviewCommentHandler) + defer cleanUp() + + err := client.AddPullRequestReviewComments(ctx, owner, repo1, 1, []PullRequestComment{ + { + CommentInfo: CommentInfo{Content: "test1"}, + PullRequestDiff: PullRequestDiff{ + NewFilePath: "requirements.txt", + NewStartLine: 3, + }, + }, + { + CommentInfo: CommentInfo{Content: "test2"}, + PullRequestDiff: PullRequestDiff{ + NewFilePath: "requirements.txt", + NewStartLine: 1, + }, + }, + }...) + assert.NoError(t, err) + + err = createBadGitHubClient(t).AddPullRequestReviewComments(ctx, owner, repo1, 1, PullRequestComment{ + CommentInfo: CommentInfo{Content: "test1"}, + PullRequestDiff: PullRequestDiff{ + NewFilePath: "requirements.txt", + NewStartLine: 3, + }, + }) + assert.Error(t, err) +} + +func TestGitHubClient_ListPullRequestReviewComments(t *testing.T) { + ctx := context.Background() + id := int64(1) + body := "test" + client, cleanUp := createServerAndClient(t, vcsutils.GitHub, false, []*github.PullRequestReview{{ID: &id, Body: &body}}, "/repos/jfrog/repo-1/pulls/1/reviews", createGitHubHandler) + defer cleanUp() + + commentInfo, err := client.ListPullRequestReviewComments(ctx, owner, repo1, 1) + assert.NoError(t, err) + assert.Len(t, commentInfo, 1) + assert.Equal(t, id, commentInfo[0].ID) + assert.Equal(t, body, commentInfo[0].Content) + + commentInfo, err = createBadGitHubClient(t).ListPullRequestReviewComments(ctx, owner, repo1, 1) + assert.Nil(t, commentInfo) + assert.Error(t, err) +} + func TestGitHubClient_GetLatestCommit(t *testing.T) { ctx := context.Background() response, err := os.ReadFile(filepath.Join("testdata", "github", "commit_list_response.json")) @@ -844,6 +895,17 @@ func TestGitHubClient_TestGetCommitStatus(t *testing.T) { }) } +func TestGitHubClient_DeletePullRequestReviewComments(t *testing.T) { + ctx := context.Background() + client, cleanUp := createServerAndClient(t, vcsutils.GitHub, false, nil, "", createGitHubHandlerWithoutExpectedURI) + defer cleanUp() + err := client.DeletePullRequestReviewComments(ctx, owner, repo1, 1, []CommentInfo{{ID: 1}, {ID: 2}}...) + assert.NoError(t, err) + client = createBadGitHubClient(t) + err = client.DeletePullRequestReviewComments(ctx, owner, repo1, 1, CommentInfo{ID: 1}) + assert.Error(t, err) +} + func TestGitHubClient_DeletePullRequestComment(t *testing.T) { ctx := context.Background() client, cleanUp := createServerAndClient(t, vcsutils.GitHub, false, nil, fmt.Sprintf("/repos/%v/%v/issues/comments/1", owner, repo1), createGitHubHandler) @@ -946,6 +1008,36 @@ func createGitHubHandler(t *testing.T, expectedURI string, response []byte, expe } } +func createGitHubHandlerWithoutExpectedURI(t *testing.T, _ string, response []byte, expectedStatusCode int) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "Bearer "+token, r.Header.Get("Authorization")) + if strings.Contains(r.RequestURI, "tarball") { + w.Header().Add("Location", string(response)) + w.WriteHeader(expectedStatusCode) + return + } + w.WriteHeader(expectedStatusCode) + _, err := w.Write(response) + assert.NoError(t, err) + } +} + +func createAddPullRequestReviewCommentHandler(t *testing.T, expectedURI string, response []byte, expectedStatusCode int) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if r.RequestURI == "/repos/jfrog/repo-1/pulls/1/commits" { + commits, err := os.ReadFile(filepath.Join("testdata", "github", "commit_list_response.json")) + assert.NoError(t, err) + _, err = w.Write(commits) + assert.NoError(t, err) + return + } + assert.Equal(t, expectedURI, r.RequestURI) + w.WriteHeader(expectedStatusCode) + _, err := w.Write(response) + assert.NoError(t, err) + } +} + func createDownloadRepositoryGitHubHandler(t *testing.T, expectedURI string, response []byte, expectedStatusCode int) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.RequestURI == "/repos/jfrog/Hello-World" { diff --git a/vcsclient/gitlab.go b/vcsclient/gitlab.go index 4a994825..643f0006 100644 --- a/vcsclient/gitlab.go +++ b/vcsclient/gitlab.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/base64" + "errors" "fmt" "github.com/jfrog/froggit-go/vcsutils" "github.com/jfrog/gofrog/datastructures" @@ -307,18 +308,177 @@ func (client *GitLabClient) AddPullRequestComment(ctx context.Context, owner, re return err } +// AddPullRequestReviewComments adds comments to a pull request on GitLab. +func (client *GitLabClient) AddPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...PullRequestComment) error { + // Validate parameters + if err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository}); err != nil { + return err + } + + // Check if comments are provided + if len(comments) == 0 { + return errors.New("could not add merge request review comments, no comments provided") + } + + projectID := getProjectID(owner, repository) + + // Get merge request diff versions + versions, err := client.getMergeRequestDiffVersions(ctx, projectID, pullRequestID) + if err != nil { + return fmt.Errorf("could not get merge request diff versions: %w", err) + } + + // Get merge request details + mergeRequestChanges, err := client.getMergeRequestChanges(ctx, projectID, pullRequestID) + if err != nil { + return fmt.Errorf("could not get merge request changes: %w", err) + } + + for _, comment := range comments { + if err = client.addPullRequestReviewComment(ctx, projectID, pullRequestID, comment, versions, mergeRequestChanges); err != nil { + return err + } + } + + return nil +} + +func (client *GitLabClient) getMergeRequestDiffVersions(ctx context.Context, projectID string, pullRequestID int) ([]*gitlab.MergeRequestDiffVersion, error) { + versions, _, err := client.glClient.MergeRequests.GetMergeRequestDiffVersions(projectID, pullRequestID, &gitlab.GetMergeRequestDiffVersionsOptions{}, gitlab.WithContext(ctx)) + return versions, err +} + +func (client *GitLabClient) getMergeRequestChanges(ctx context.Context, projectID string, pullRequestID int) (*gitlab.MergeRequest, error) { + mergeRequestChanges, _, err := client.glClient.MergeRequests.GetMergeRequestChanges(projectID, pullRequestID, &gitlab.GetMergeRequestChangesOptions{}, gitlab.WithContext(ctx)) + return mergeRequestChanges, err +} + +func (client *GitLabClient) addPullRequestReviewComment(ctx context.Context, projectID string, pullRequestID int, comment PullRequestComment, versions []*gitlab.MergeRequestDiffVersion, mergeRequestChanges *gitlab.MergeRequest) error { + // Find the corresponding change in merge request + var newPath, oldPath string + var newLine int + var diffFound bool + + for _, diff := range mergeRequestChanges.Changes { + if diff.NewPath != comment.NewFilePath { + continue + } + + diffFound = true + newLine = comment.NewStartLine + newPath = diff.NewPath + + // New files don't have old data + if !diff.NewFile { + oldPath = diff.OldPath + } + break + } + + // If no matching change is found, return an error + if !diffFound { + return fmt.Errorf("could not find changes to %s in the current merge request", comment.NewFilePath) + } + + // Create a NotePosition for the comment + latestVersion := versions[0] + diffPosition := &gitlab.NotePosition{ + StartSHA: latestVersion.StartCommitSHA, + HeadSHA: latestVersion.HeadCommitSHA, + BaseSHA: latestVersion.BaseCommitSHA, + PositionType: "text", + NewLine: newLine, + NewPath: newPath, + OldLine: newLine, + OldPath: oldPath, + } + + // The GitLab REST API for creating a merge request discussion has strange behavior: + // If the API call is not constructed precisely according to these rules, it may fail with an unclear error. + // In all cases, 'new_path' and 'new_line' parameters are required. + // - When commenting on a new file, do not include 'old_path' and 'old_line' parameters. + // - When commenting on an existing file that has changed in the diff, omit 'old_path' and 'old_line' parameters. + // - When commenting on an existing file that hasn't changed in the diff, include 'old_path' and 'old_line' parameters. + + client.logger.Debug(fmt.Sprintf("Create merge request discussion sent. newPath: %v newLine: %v oldPath: %v, oldLine: %v", + newPath, newLine, oldPath, newLine)) + // Attempt to create a merge request discussion thread + _, _, err := client.createMergeRequestDiscussion(ctx, projectID, comment.Content, pullRequestID, diffPosition) + + // Retry without oldLine and oldPath if the GitLab API call fails + if err != nil { + diffPosition.OldLine = 0 + diffPosition.OldPath = "" + client.logger.Debug(fmt.Sprintf("Create merge request discussion second attempt sent. newPath: %v newLine: %v oldPath: %v, oldLine: %v", + newPath, newLine, oldPath, newLine)) + _, _, err = client.createMergeRequestDiscussion(ctx, projectID, comment.Content, pullRequestID, diffPosition) + } + + // If the comment creation still fails, return an error + if err != nil { + return fmt.Errorf("could not create a merge request discussion thread: %w", err) + } + + return nil +} + +func (client *GitLabClient) createMergeRequestDiscussion(ctx context.Context, projectID, content string, pullRequestID int, position *gitlab.NotePosition) (*gitlab.Discussion, *gitlab.Response, error) { + return client.glClient.Discussions.CreateMergeRequestDiscussion(projectID, pullRequestID, &gitlab.CreateMergeRequestDiscussionOptions{ + Body: &content, + Position: position, + }, gitlab.WithContext(ctx)) +} + +// ListPullRequestReviewComments on GitLab +func (client *GitLabClient) ListPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) { + // Validate parameters + if err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository, "pullRequestID": strconv.Itoa(pullRequestID)}); err != nil { + return nil, err + } + + projectID := getProjectID(owner, repository) + + discussions, _, err := client.glClient.Discussions.ListMergeRequestDiscussions(projectID, pullRequestID, &gitlab.ListMergeRequestDiscussionsOptions{}, gitlab.WithContext(ctx)) + if err != nil { + return nil, fmt.Errorf("failed fetching the list of merge requests discussions: %w", err) + } + + var commentsInfo []CommentInfo + for _, discussion := range discussions { + commentsInfo = append(commentsInfo, mapGitLabNotesToCommentInfoList(discussion.Notes, discussion.ID)...) + } + + return commentsInfo, nil +} + // ListPullRequestComments on GitLab func (client *GitLabClient) ListPullRequestComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) { - err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository}) - if err != nil { - return []CommentInfo{}, err + if err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository, "pullRequestID": strconv.Itoa(pullRequestID)}); err != nil { + return nil, err } commentsList, _, err := client.glClient.Notes.ListMergeRequestNotes(getProjectID(owner, repository), pullRequestID, &gitlab.ListMergeRequestNotesOptions{}, gitlab.WithContext(ctx)) if err != nil { return []CommentInfo{}, err } - return mapGitLabNotesToCommentInfoList(commentsList), nil + return mapGitLabNotesToCommentInfoList(commentsList, ""), nil +} + +// DeletePullRequestReviewComment on GitLab +func (client *GitLabClient) DeletePullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...CommentInfo) error { + if err := validateParametersNotBlank(map[string]string{"owner": owner, "repository": repository, "pullRequestID": strconv.Itoa(pullRequestID)}); err != nil { + return err + } + for _, comment := range comments { + var commentID int64 + if err := validateParametersNotBlank(map[string]string{"commentID": strconv.FormatInt(commentID, 10), "discussionID": comment.ThreadID}); err != nil { + return err + } + if _, err := client.glClient.Discussions.DeleteMergeRequestDiscussionNote(getProjectID(owner, repository), pullRequestID, comment.ThreadID, int(commentID), gitlab.WithContext(ctx)); err != nil { + return fmt.Errorf("an error occurred while deleting pull request review comment: %w", err) + } + } + return nil } // DeletePullRequestComment on GitLab @@ -599,12 +759,13 @@ func mapGitLabCommitToCommitInfo(commit *gitlab.Commit) CommitInfo { } } -func mapGitLabNotesToCommentInfoList(notes []*gitlab.Note) (res []CommentInfo) { +func mapGitLabNotesToCommentInfoList(notes []*gitlab.Note, discussionId string) (res []CommentInfo) { for _, note := range notes { res = append(res, CommentInfo{ - ID: int64(note.ID), - Content: note.Body, - Created: *note.CreatedAt, + ID: int64(note.ID), + ThreadID: discussionId, + Content: note.Body, + Created: *note.CreatedAt, }) } return diff --git a/vcsclient/gitlab_test.go b/vcsclient/gitlab_test.go index 7e2a12e5..b324fcb1 100644 --- a/vcsclient/gitlab_test.go +++ b/vcsclient/gitlab_test.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "testing" "time" @@ -187,6 +188,64 @@ func TestGitLabClient_AddPullRequestComment(t *testing.T) { assert.NoError(t, err) } +func TestGitLabClient_AddPullRequestReviewComment(t *testing.T) { + ctx := context.Background() + client, cleanUp := createServerAndClient(t, vcsutils.GitLab, false, "", + fmt.Sprintf("/api/v4/projects/%s/merge_requests/1/notes", url.PathEscape(owner+"/"+repo1)), createAddPullRequestReviewCommentGitLabHandler) + defer cleanUp() + comments := []PullRequestComment{ + { + CommentInfo: CommentInfo{Content: "test1"}, + PullRequestDiff: PullRequestDiff{ + OriginalFilePath: "oldPath", + OriginalStartLine: 1, + NewFilePath: "newPath", + NewStartLine: 2, + }, + }, + } + err := client.AddPullRequestReviewComments(ctx, owner, repo1, 7, comments...) + // No diff found + assert.Error(t, err) + comments = []PullRequestComment{ + { + CommentInfo: CommentInfo{Content: "test1"}, + PullRequestDiff: PullRequestDiff{ + OriginalFilePath: "VERSION", + OriginalStartLine: 1, + NewFilePath: "VERSION", + NewStartLine: 2, + }, + }, + } + err = client.AddPullRequestReviewComments(ctx, owner, repo1, 7, comments...) + assert.NoError(t, err) +} + +func TestGitLabClient_ListPullRequestReviewComments(t *testing.T) { + ctx := context.Background() + response, err := os.ReadFile(filepath.Join("testdata", "gitlab", "merge_request_discussion_items.json")) + assert.NoError(t, err) + + client, cleanUp := createServerAndClient(t, vcsutils.GitLab, false, response, + fmt.Sprintf("/api/v4/projects/%s/merge_requests/1/discussions", url.PathEscape(owner+"/"+repo1)), createGitLabHandler) + defer cleanUp() + + result, err := client.ListPullRequestReviewComments(ctx, owner, repo1, 1) + assert.NoError(t, err) + assert.NoError(t, err) + assert.Len(t, result, 3) + assert.Equal(t, int64(1126), result[0].ID) + assert.Equal(t, "discussion text", result[0].Content) + assert.Equal(t, "2018-03-03 21:54:39.668 +0000 UTC", result[0].Created.String()) + assert.Equal(t, int64(1129), result[1].ID) + assert.Equal(t, "reply to the discussion", result[1].Content) + assert.Equal(t, "2018-03-04 13:38:02.127 +0000 UTC", result[1].Created.String()) + assert.Equal(t, int64(1128), result[2].ID) + assert.Equal(t, "a single comment", result[2].Content) + assert.Equal(t, "2018-03-04 09:17:22.52 +0000 UTC", result[2].Created.String()) +} + func TestGitLabClient_ListPullRequestComments(t *testing.T) { ctx := context.Background() response, err := os.ReadFile(filepath.Join("testdata", "gitlab", "pull_request_comments_list_response.json")) @@ -556,6 +615,19 @@ func TestGitlabClient_GetRepositoryEnvironmentInfo(t *testing.T) { assert.ErrorIs(t, err, errGitLabGetRepoEnvironmentInfoNotSupported) } +func TestGitLabClient_DeletePullRequestReviewComment(t *testing.T) { + ctx := context.Background() + client, cleanUp := createServerAndClient(t, vcsutils.GitLab, false, "", + "", createGitLabHandlerWithoutExpectedURI) + defer cleanUp() + err := client.DeletePullRequestReviewComments(ctx, owner, "", 1, CommentInfo{}) + assert.Error(t, err) + err = client.DeletePullRequestReviewComments(ctx, owner, "test", 1, CommentInfo{}) + assert.Error(t, err) + err = client.DeletePullRequestReviewComments(ctx, owner, repo1, 1, []CommentInfo{{ThreadID: "ab22", ID: 2}, {ThreadID: "ba22", ID: 3}}...) + assert.NoError(t, err) +} + func TestGitLabClient_DeletePullRequestComment(t *testing.T) { ctx := context.Background() client, cleanUp := createServerAndClient(t, vcsutils.GitLab, false, "", @@ -634,6 +706,49 @@ func createGitLabHandler(t *testing.T, expectedURI string, response []byte, expe } } +func createGitLabHandlerWithoutExpectedURI(t *testing.T, _ string, response []byte, expectedStatusCode int) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if r.RequestURI == "/api/v4/" { + w.WriteHeader(http.StatusOK) + return + } + w.WriteHeader(expectedStatusCode) + _, err := w.Write(response) + assert.NoError(t, err) + assert.Equal(t, token, r.Header.Get("Private-Token")) + } +} + +func createAddPullRequestReviewCommentGitLabHandler(t *testing.T, _ string, _ []byte, _ int) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v4/projects/jfrog%2Frepo-1/merge_requests/7/versions": + versionsDiff, err := os.ReadFile(filepath.Join("testdata", "gitlab", "merge_request_diff_versions.json")) + assert.NoError(t, err) + _, err = w.Write(versionsDiff) + assert.NoError(t, err) + assert.Equal(t, token, r.Header.Get("Private-Token")) + case "/api/v4/projects/jfrog%2Frepo-1/merge_requests/7/changes": + mergeRequestChanges, err := os.ReadFile(filepath.Join("testdata", "gitlab", "merge_request_changes.json")) + assert.NoError(t, err) + _, err = w.Write(mergeRequestChanges) + assert.NoError(t, err) + assert.Equal(t, token, r.Header.Get("Private-Token")) + case "/api/v4/projects/jfrog%2Frepo-1/merge_requests/7/discussions": + body, err := io.ReadAll(r.Body) + assert.NoError(t, err) + if strings.Contains(string(body), "old_path") { + w.WriteHeader(http.StatusNotFound) + } + newMergeRequestThreadResponse, err := os.ReadFile(filepath.Join("testdata", "gitlab", "new_merge_request_thread.json")) + assert.NoError(t, err) + w.WriteHeader(http.StatusOK) + _, err = w.Write(newMergeRequestThreadResponse) + assert.NoError(t, err) + } + } +} + func createDownloadRepositoryGitLabHandler(t *testing.T, expectedURI string, response []byte, expectedStatusCode int) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { if r.RequestURI == "/api/v4/" { diff --git a/vcsclient/testdata/gitlab/merge_request_changes.json b/vcsclient/testdata/gitlab/merge_request_changes.json new file mode 100644 index 00000000..29e12b20 --- /dev/null +++ b/vcsclient/testdata/gitlab/merge_request_changes.json @@ -0,0 +1,108 @@ +{ + "id": 21, + "iid": 1, + "project_id": 4, + "title": "Blanditiis beatae suscipit hic assumenda et molestias nisi asperiores repellat et.", + "state": "reopened", + "created_at": "2015-02-02T19:49:39.159Z", + "updated_at": "2015-02-02T20:08:49.959Z", + "target_branch": "secret_token", + "source_branch": "version-1-9", + "upvotes": 0, + "downvotes": 0, + "author": { + "name": "Chad Hamill", + "username": "jarrett", + "id": 5, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/b95567800f828948baf5f4160ebb2473?s=40&d=identicon", + "web_url": "https://gitlab.example.com/jarrett" + }, + "assignee": { + "name": "Administrator", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40&d=identicon", + "web_url": "https://gitlab.example.com/root" + }, + "assignees": [ + { + "name": "Miss Monserrate Beier", + "username": "axel.block", + "id": 12, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/46f6f7dc858ada7be1853f7fb96e81da?s=80&d=identicon", + "web_url": "https://gitlab.example.com/axel.block" + } + ], + "reviewers": [ + { + "name": "Miss Monserrate Beier", + "username": "axel.block", + "id": 12, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/46f6f7dc858ada7be1853f7fb96e81da?s=80&d=identicon", + "web_url": "https://gitlab.example.com/axel.block" + } + ], + "source_project_id": 4, + "target_project_id": 4, + "labels": [], + "description": "Qui voluptatibus placeat ipsa alias quasi. Deleniti rem ut sint. Optio velit qui distinctio.", + "draft": false, + "work_in_progress": false, + "milestone": { + "id": 5, + "iid": 1, + "project_id": 4, + "title": "v2.0", + "description": "Assumenda aut placeat expedita exercitationem labore sunt enim earum.", + "state": "closed", + "created_at": "2015-02-02T19:49:26.013Z", + "updated_at": "2015-02-02T19:49:26.013Z", + "due_date": null + }, + "merge_when_pipeline_succeeds": true, + "merge_status": "can_be_merged", + "detailed_merge_status": "can_be_merged", + "subscribed": true, + "sha": "8888888888888888888888888888888888888888", + "merge_commit_sha": null, + "squash_commit_sha": null, + "user_notes_count": 1, + "changes_count": "1", + "should_remove_source_branch": true, + "force_remove_source_branch": false, + "squash": false, + "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, + "discussion_locked": false, + "time_stats": { + "time_estimate": 0, + "total_time_spent": 0, + "human_time_estimate": null, + "human_total_time_spent": null + }, + "task_completion_status": { + "count": 0, + "completed_count": 0 + }, + "changes": [ + { + "old_path": "VERSION", + "new_path": "VERSION", + "a_mode": "100644", + "b_mode": "100644", + "diff": "@@ -1 +1 @@\\ -1.9.7\\ +1.9.8", + "new_file": false, + "renamed_file": false, + "deleted_file": false + } + ], + "overflow": false +} \ No newline at end of file diff --git a/vcsclient/testdata/gitlab/merge_request_diff_versions.json b/vcsclient/testdata/gitlab/merge_request_diff_versions.json new file mode 100644 index 00000000..ac675ab8 --- /dev/null +++ b/vcsclient/testdata/gitlab/merge_request_diff_versions.json @@ -0,0 +1,24 @@ +[ + { + "id": 110, + "head_commit_sha": "33e2ee8579fda5bc36accc9c6fbd0b4fefda9e30", + "base_commit_sha": "eeb57dffe83deb686a60a71c16c32f71046868fd", + "start_commit_sha": "eeb57dffe83deb686a60a71c16c32f71046868fd", + "created_at": "2016-07-26T14:44:48.926Z", + "merge_request_id": 105, + "state": "collected", + "real_size": "1", + "patch_id_sha": "d504412d5b6e6739647e752aff8e468dde093f2f" + }, + { + "id": 108, + "head_commit_sha": "3eed087b29835c48015768f839d76e5ea8f07a24", + "base_commit_sha": "eeb57dffe83deb686a60a71c16c32f71046868fd", + "start_commit_sha": "eeb57dffe83deb686a60a71c16c32f71046868fd", + "created_at": "2016-07-25T14:21:33.028Z", + "merge_request_id": 105, + "state": "collected", + "real_size": "1", + "patch_id_sha": "72c30d1f0115fc1d2bb0b29b24dc2982cbcdfd32" + } +] \ No newline at end of file diff --git a/vcsclient/testdata/gitlab/merge_request_discussion_items.json b/vcsclient/testdata/gitlab/merge_request_discussion_items.json new file mode 100644 index 00000000..dc5c5d9a --- /dev/null +++ b/vcsclient/testdata/gitlab/merge_request_discussion_items.json @@ -0,0 +1,87 @@ +[ + { + "id": "6a9c1750b37d513a43987b574953fceb50b03ce7", + "individual_note": false, + "notes": [ + { + "id": 1126, + "type": "DiscussionNote", + "body": "discussion text", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-03T21:54:39.668Z", + "updated_at": "2018-03-03T21:54:39.668Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Merge request", + "project_id": 5, + "noteable_iid": null, + "resolved": false, + "resolvable": true, + "resolved_by": null, + "resolved_at": null + }, + { + "id": 1129, + "type": "DiscussionNote", + "body": "reply to the discussion", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T13:38:02.127Z", + "updated_at": "2018-03-04T13:38:02.127Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Merge request", + "project_id": 5, + "noteable_iid": null, + "resolved": false, + "resolvable": true, + "resolved_by": null + } + ] + }, + { + "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6", + "individual_note": true, + "notes": [ + { + "id": 1128, + "type": null, + "body": "a single comment", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T09:17:22.520Z", + "updated_at": "2018-03-04T09:17:22.520Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Merge request", + "project_id": 5, + "noteable_iid": null, + "resolved": false, + "resolvable": true, + "resolved_by": null + } + ] + } +] \ No newline at end of file diff --git a/vcsclient/testdata/gitlab/new_merge_request_thread.json b/vcsclient/testdata/gitlab/new_merge_request_thread.json new file mode 100644 index 00000000..c8dc6fe2 --- /dev/null +++ b/vcsclient/testdata/gitlab/new_merge_request_thread.json @@ -0,0 +1,46 @@ +{ + "id": "ec358c38875d5133b171208e64415b8583acaed1", + "individual_note": false, + "notes": [ + { + "id": 1544943809, + "type": "DiffNote", + "body": "test comment body", + "attachment": null, + "author": { + "id": 13377328, + "username": "jfrog", + "name": "JFrog", + "state": "active", + "avatar_url": "https://secure.gravatar.com/avatar/005f2d268f06e85fcae3e48c96217dd3?s=80\u0026d=identicon", + "web_url": "https://gitlab.com/omerz1" + }, + "created_at": "2023-09-06T07:48:45.081Z", + "updated_at": "2023-09-06T07:48:45.081Z", + "system": false, + "noteable_id": 247767421, + "noteable_type": "MergeRequest", + "project_id": 47457684, + "commit_id": null, + "position": { + "base_sha": "b840235eecc4d6bca96839a94ad141fc6e9e9170", + "start_sha": "b840235eecc4d6bca96839a94ad141fc6e9e9170", + "head_sha": "204c25f7d0a81b9163eb4bdda18a8ed279a32fa6", + "old_path": null, + "new_path": ".gitlab-ci.yml", + "position_type": "text", + "old_line": null, + "new_line": 14, + "line_range": null + }, + "resolvable": true, + "resolved": false, + "resolved_by": null, + "resolved_at": null, + "confidential": false, + "internal": false, + "noteable_iid": 12, + "commands_changes": {} + } + ] +} \ No newline at end of file diff --git a/vcsclient/vcsclient.go b/vcsclient/vcsclient.go index 22f7346f..6c3d4366 100644 --- a/vcsclient/vcsclient.go +++ b/vcsclient/vcsclient.go @@ -164,6 +164,26 @@ type VcsClient interface { // pullRequestID - Pull request ID AddPullRequestComment(ctx context.Context, owner, repository, content string, pullRequestID int) error + // AddPullRequestReviewComments Adds a new review comment on the requested pull request + // owner - User or organization + // repository - VCS repository name + // pullRequestID - Pull request ID + // comment - The new comment details defined in PullRequestComment + AddPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...PullRequestComment) error + + // ListPullRequestReviewComments Gets all pull request review comments + // owner - User or organization + // repository - VCS repository name + // pullRequestID - Pull request ID + ListPullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int) ([]CommentInfo, error) + + // DeletePullRequestReviewComments Gets all comments assigned to a pull request. + // owner - User or organization + // repository - VCS repository name + // pullRequestID - Pull request ID + // commentID - The ID of the comment + DeletePullRequestReviewComments(ctx context.Context, owner, repository string, pullRequestID int, comments ...CommentInfo) error + // ListPullRequestComments Gets all comments assigned to a pull request. // owner - User or organization // repository - VCS repository name @@ -175,7 +195,6 @@ type VcsClient interface { // repository - VCS repository name // pullRequestID - Pull request ID // commentID - The ID of the comment - // commentVersion - The version of the comment DeletePullRequestComment(ctx context.Context, owner, repository string, pullRequestID, commentID int) error // ListOpenPullRequestsWithBody Gets all open pull requests ids and the pull request body. @@ -299,10 +318,11 @@ type CommitInfo struct { } type CommentInfo struct { - ID int64 - Content string - Created time.Time - Version int + ID int64 + ThreadID string + Content string + Created time.Time + Version int } type PullRequestInfo struct { @@ -319,7 +339,39 @@ type BranchInfo struct { Owner string } -// RepositoryInfo contains general information about repository. +// PullRequestInfo contains the details of a pull request comment +// content - the content of the pull request comment +// PullRequestDiff - the content of the pull request diff +type PullRequestComment struct { + CommentInfo + PullRequestDiff +} + +// PullRequestDiff contains the details of the pull request diff +// OriginalFilePath - the original file path +// OriginalStartLine - the original start line number +// OriginalEndLine - the original end line number +// originalStartColum - the original start column number +// OriginalEndColumn - the original end column number +// NewFilePath - the new file path +// NewStartLine - the new start line number +// NewEndLine - the new end line number +// NewStartColumn - the new start column number +// NewEndColumn - the new end column number +type PullRequestDiff struct { + OriginalFilePath string + OriginalStartLine int + OriginalEndLine int + OriginalStartColumn int + OriginalEndColumn int + NewFilePath string + NewStartLine int + NewEndLine int + NewStartColumn int + NewEndColumn int +} + +// RepositoryInfo contains general information about the repository. type RepositoryInfo struct { CloneInfo CloneInfo RepositoryVisibility RepositoryVisibility diff --git a/vcsutils/consts.go b/vcsutils/consts.go index 89dc2ddb..0a04518b 100644 --- a/vcsutils/consts.go +++ b/vcsutils/consts.go @@ -4,6 +4,7 @@ const ( branchPrefix = "refs/heads/" TagPrefix = "refs/tags/" NumberOfCommitsToFetch = 50 + ErrNoCommentsProvided = "could not add a pull request review comment, no comments were provided" ) // VcsProvider is an enum represents the VCS provider type diff --git a/vcsutils/utils.go b/vcsutils/utils.go index 73390575..c2f52e5e 100644 --- a/vcsutils/utils.go +++ b/vcsutils/utils.go @@ -353,3 +353,10 @@ func RemoveDirContents(dirPath string) (err error) { } return } + +func GetPullRequestFilePath(filePath string) string { + if filePath == "" { + return "" + } + return fmt.Sprintf("/%s", strings.TrimPrefix(filePath, "/")) +} diff --git a/vcsutils/utils_test.go b/vcsutils/utils_test.go index 63fae584..644dcb2d 100644 --- a/vcsutils/utils_test.go +++ b/vcsutils/utils_test.go @@ -239,3 +239,20 @@ func TestMapPullRequestState(t *testing.T) { }) } } + +func TestGetPullRequestFilePath(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"", ""}, + {"file.txt", "/file.txt"}, + {"/path/to/file.txt", "/path/to/file.txt"}, + {"dir/file.txt", "/dir/file.txt"}, + } + + for _, test := range tests { + result := GetPullRequestFilePath(test.input) + assert.Equal(t, test.expected, result) + } +}