fix merge

jfrog · attiasas · Oct 16, 2023 · Sep 21, 2023 · Sep 21, 2023 · Sep 21, 2023
commit 2d45ff219c56c4e52390faef970150a118d75f13
diff --git a/scanpullrequest/scanpullrequest.go b/scanpullrequest/scanpullrequest.go
@@ -7,7 +7,6 @@ import (
 	"os"
 
 	"golang.org/x/exp/slices"
-	"os"
 
 	"github.com/jfrog/frogbot/utils"
 	"github.com/jfrog/froggit-go/vcsclient"

diff --git a/utils/comment.go b/utils/comment.go
@@ -27,38 +27,69 @@ const (
 	SastComment       ReviewCommentType = "Sast"
 
 	RescanRequestComment = "rescan"
-
-	frogbotCommentNotFound = -1
 )
 
 func HandlePullRequestCommentsAfterScan(issues *IssuesCollection, repo *Repository, client vcsclient.VcsClient, pullRequestID int) (err error) {
-	// Delete previous Frogbot pull request message if exists
-	if err = DeleteExistingPullRequestComment(repo, client); err != nil {
-		return
+	if !repo.Params.AvoidPreviousPrCommentsDeletion {
+		log.Debug("Looking for an existing Frogbot pull request comment. Deleting it if it exists...")
+		// Delete previous PR regular comments, if exists (not related to location of a change)
+		if err = DeleteExistingPullRequestComments(repo, client); err != nil {
+			err = errors.New("couldn't delete pull request comment: " + err.Error())
+			return
+		}
+		// Delete previous PR review comments, if exists (related to location of a change)
+		if err = DeleteExistingPullRequestReviewComments(repo, pullRequestID, client); err != nil {
+			err = errors.New("couldn't delete pull request review comment: " + err.Error())
+			return
+		}
 	}
 
-	// Create a pull request message
-	message := GeneratePullRequestSummaryComment(issues, repo.OutputWriter)
-
-	// Add SCA scan comment
-	if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, message, pullRequestID); err != nil {
+	// Add summary (SCA, license) scan comment
+	if err = client.AddPullRequestComment(context.Background(), repo.RepoOwner, repo.RepoName, generatePullRequestSummaryComment(issues, repo.OutputWriter), pullRequestID); err != nil {
 		err = errors.New("couldn't add pull request comment: " + err.Error())
 		return
 	}
-
 	// Handle review comments at the pull request
-	if err = AddReviewComments(repo, pullRequestID, client, issues); err != nil {
+	if err = addReviewComments(repo, pullRequestID, client, issues); err != nil {
 		err = errors.New("couldn't add review comments: " + err.Error())
 		return
 	}
 	return
 }
 
+// Delete existing pull request regular comments (Summary, Fallback review comments)
+func DeleteExistingPullRequestComments(repository *Repository, client vcsclient.VcsClient) error {
+	prDetails := repository.PullRequestDetails
+	comments, err := GetSortedPullRequestComments(client, prDetails.Target.Owner, prDetails.Target.Repository, int(prDetails.ID))
+	if err != nil {
+		return fmt.Errorf(
+			"failed to get comments. the following details were used in order to fetch the comments: <%s/%s> pull request #%d. the error received: %s",
+			repository.RepoOwner, repository.RepoName, int(repository.PullRequestDetails.ID), err.Error())
+	}
+	// Previous Fallback review comments
+	commentsToDelete := getFrogbotReviewComments(comments)
+	// Previous Summary comments
+	for _, comment := range comments {
+		if outputwriter.IsFrogbotSummaryComment(repository.OutputWriter, comment.Content) {
+			commentsToDelete = append(commentsToDelete, comment)
+		}
+	}
+	// Delete
+	if len(commentsToDelete) > 0 {
+		for _, commentToDelete := range commentsToDelete {
+			if err = client.DeletePullRequestComment(context.Background(), prDetails.Target.Owner, prDetails.Target.Repository, int(prDetails.ID), int(commentToDelete.ID)); err != nil {
+				return err
+			}
+		}
+	}
+	return err
+}
+
 func GenerateFixPullRequestDetails(vulnerabilities []formats.VulnerabilityOrViolationRow, writer outputwriter.OutputWriter) string {
 	return outputwriter.GetPRSummaryContent(outputwriter.VulnerabilitiesContent(vulnerabilities, writer), true, false, writer)
 }
 
-func GeneratePullRequestSummaryComment(issuesCollection *IssuesCollection, writer outputwriter.OutputWriter) string {
+func generatePullRequestSummaryComment(issuesCollection *IssuesCollection, writer outputwriter.OutputWriter) string {
 	issuesExists := false
 	content := strings.Builder{}
 
@@ -86,41 +117,7 @@ func GetSortedPullRequestComments(client vcsclient.VcsClient, repoOwner, repoNam
 	return pullRequestsComments, nil
 }
 
-func DeleteExistingPullRequestComment(repository *Repository, client vcsclient.VcsClient) error {
-	log.Debug("Looking for an existing Frogbot pull request comment. Deleting it if it exists...")
-	prDetails := repository.PullRequestDetails
-	comments, err := GetSortedPullRequestComments(client, prDetails.Target.Owner, prDetails.Target.Repository, int(prDetails.ID))
-	if err != nil {
-		return fmt.Errorf(
-			"failed to get comments. the following details were used in order to fetch the comments: <%s/%s> pull request #%d. the error received: %s",
-			repository.RepoOwner, repository.RepoName, int(repository.PullRequestDetails.ID), err.Error())
-	}
-
-	commentID := frogbotCommentNotFound
-	for _, comment := range comments {
-		if outputwriter.IsFrogbotSummaryComment(repository.OutputWriter, comment.Content) {
-			log.Debug("Found previous Frogbot comment with the id:", comment.ID)
-			commentID = int(comment.ID)
-			break
-		}
-	}
-
-	if commentID != frogbotCommentNotFound {
-		err = client.DeletePullRequestComment(context.Background(), prDetails.Target.Owner, prDetails.Target.Repository, int(prDetails.ID), commentID)
-	}
-
-	return err
-}
-
-func AddReviewComments(repo *Repository, pullRequestID int, client vcsclient.VcsClient, issues *IssuesCollection) (err error) {
-	if err = deleteOldReviewComments(repo, pullRequestID, client); err != nil {
-		err = errors.New("couldn't delete pull request review comment: " + err.Error())
-		return
-	}
-	if err = deleteOldFallbackComments(repo, pullRequestID, client); err != nil {
-		err = errors.New("couldn't delete pull request comment: " + err.Error())
-		return
-	}
+func addReviewComments(repo *Repository, pullRequestID int, client vcsclient.VcsClient, issues *IssuesCollection) (err error) {
 	commentsToAdd := getNewReviewComments(repo, issues)
 	if len(commentsToAdd) == 0 {
 		return
@@ -139,8 +136,9 @@ func AddReviewComments(repo *Repository, pullRequestID int, client vcsclient.Vcs
 	return
 }
 
-func deleteOldReviewComments(repo *Repository, pullRequestID int, client vcsclient.VcsClient) (err error) {
-	// Get all comments in PR
+// Delete existing pull request review comments (Applicable, Sast, Iac)
+func DeleteExistingPullRequestReviewComments(repo *Repository, pullRequestID int, client vcsclient.VcsClient) (err error) {
+	// Get all review comments in PR
 	var existingComments []vcsclient.CommentInfo
 	if existingComments, err = client.ListPullRequestReviewComments(context.Background(), repo.RepoOwner, repo.RepoName, pullRequestID); err != nil {
 		err = errors.New("couldn't list existing review comments: " + err.Error())

diff --git a/utils/comment_test.go b/utils/comment_test.go
@@ -1,126 +1,15 @@
 package utils
 
 import (
-	"os"
-	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/jfrog/frogbot/utils/outputwriter"
 	"github.com/jfrog/froggit-go/vcsclient"
-	"github.com/jfrog/froggit-go/vcsutils"
 	"github.com/jfrog/jfrog-cli-core/v2/utils/coreutils"
 	"github.com/jfrog/jfrog-cli-core/v2/xray/formats"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestCreatePullRequestMessageNoVulnerabilities(t *testing.T) {
-	vulnerabilities := []formats.VulnerabilityOrViolationRow{}
-	message := createPullRequestComment(&IssuesCollection{Vulnerabilities: vulnerabilities}, &outputwriter.StandardOutput{})
-
-	expectedMessageByte, err := os.ReadFile(filepath.Join("..", "testdata", "messages", "novulnerabilities.md"))
-	assert.NoError(t, err)
-	expectedMessage := strings.ReplaceAll(string(expectedMessageByte), "\r\n", "\n")
-	assert.Equal(t, expectedMessage, message)
-
-	outputWriter := &outputwriter.StandardOutput{}
-	outputWriter.SetVcsProvider(vcsutils.GitLab)
-	message = createPullRequestComment(&IssuesCollection{Vulnerabilities: vulnerabilities}, outputWriter)
-
-	expectedMessageByte, err = os.ReadFile(filepath.Join("..", "testdata", "messages", "novulnerabilitiesMR.md"))
-	assert.NoError(t, err)
-	expectedMessage = strings.ReplaceAll(string(expectedMessageByte), "\r\n", "\n")
-	assert.Equal(t, expectedMessage, message)
-}
-
-func TestCreatePullRequestComment(t *testing.T) {
-	vulnerabilities := []formats.VulnerabilityOrViolationRow{
-		{
-			Summary: "Summary XRAY-122345",
-			ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
-				SeverityDetails:           formats.SeverityDetails{Severity: "High"},
-				ImpactedDependencyName:    "github.com/nats-io/nats-streaming-server",
-				ImpactedDependencyVersion: "v0.21.0",
-				Components: []formats.ComponentRow{
-					{
-						Name:    "github.com/nats-io/nats-streaming-server",
-						Version: "v0.21.0",
-					},
-				},
-			},
-			Applicable:    "Undetermined",
-			FixedVersions: []string{"[0.24.1]"},
-			IssueId:       "XRAY-122345",
-			Cves:          []formats.CveRow{{}},
-		},
-		{
-			Summary: "Summary",
-			ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
-				SeverityDetails:           formats.SeverityDetails{Severity: "High"},
-				ImpactedDependencyName:    "github.com/mholt/archiver/v3",
-				ImpactedDependencyVersion: "v3.5.1",
-				Components: []formats.ComponentRow{
-					{
-						Name:    "github.com/mholt/archiver/v3",
-						Version: "v3.5.1",
-					},
-				},
-			},
-			Applicable: "Undetermined",
-			Cves:       []formats.CveRow{},
-		},
-		{
-			Summary: "Summary CVE-2022-26652",
-			ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
-				SeverityDetails:           formats.SeverityDetails{Severity: "Medium"},
-				ImpactedDependencyName:    "github.com/nats-io/nats-streaming-server",
-				ImpactedDependencyVersion: "v0.21.0",
-				Components: []formats.ComponentRow{
-					{
-						Name:    "github.com/nats-io/nats-streaming-server",
-						Version: "v0.21.0",
-					},
-				},
-			},
-			Applicable:    "Undetermined",
-			FixedVersions: []string{"[0.24.3]"},
-			Cves:          []formats.CveRow{{Id: "CVE-2022-26652"}},
-		},
-	}
-	licenses := []formats.LicenseRow{
-		{
-			LicenseKey: "Apache-2.0",
-			ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
-				SeverityDetails:           formats.SeverityDetails{Severity: "High", SeverityNumValue: 13},
-				ImpactedDependencyName:    "minimatch",
-				ImpactedDependencyVersion: "1.2.3",
-				Components: []formats.ComponentRow{
-					{
-						Name:    "root",
-						Version: "1.0.0",
-					},
-					{
-						Name:    "minimatch",
-						Version: "1.2.3",
-					},
-				},
-			},
-		},
-	}
-
-	writerOutput := &outputwriter.StandardOutput{}
-	writerOutput.SetJasOutputFlags(true, true)
-	message := createPullRequestComment(&IssuesCollection{Vulnerabilities: vulnerabilities, Licenses: licenses}, writerOutput)
-
-	expectedMessage := "<div align='center'>\n\n[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesBannerPR.png)](https://github.com/jfrog/frogbot#readme)\n\n</div>\n\n\n## 📦 Vulnerable Dependencies\n\n### ✍️ Summary\n\n<div align=\"center\">\n\n| SEVERITY                | CONTEXTUAL ANALYSIS                  | DIRECT DEPENDENCIES                  | IMPACTED DEPENDENCY                   | FIXED VERSIONS                       | CVES                       |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br>    High | Undetermined | github.com/nats-io/nats-streaming-server:v0.21.0 | github.com/nats-io/nats-streaming-server:v0.21.0 | [0.24.1] |  -  |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br>    High | Undetermined | github.com/mholt/archiver/v3:v3.5.1 | github.com/mholt/archiver/v3:v3.5.1 |  -  |  -  |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableMediumSeverity.png)<br>  Medium | Undetermined | github.com/nats-io/nats-streaming-server:v0.21.0 | github.com/nats-io/nats-streaming-server:v0.21.0 | [0.24.3] | CVE-2022-26652 |\n\n</div>\n\n## 🔬 Research Details\n\n<details>\n<summary> <b>[ XRAY-122345 ] github.com/nats-io/nats-streaming-server v0.21.0</b> </summary>\n<br>\n\n**Description:**\nSummary XRAY-122345\n\n\n</details>\n\n\n<details>\n<summary> <b>github.com/mholt/archiver/v3 v3.5.1</b> </summary>\n<br>\n\n**Description:**\nSummary\n\n\n</details>\n\n\n<details>\n<summary> <b>[ CVE-2022-26652 ] github.com/nats-io/nats-streaming-server v0.21.0</b> </summary>\n<br>\n\n**Description:**\nSummary CVE-2022-26652\n\n\n</details>\n\n\n## ⚖️ Violated Licenses \n\n<div align=\"center\">\n\n\n| LICENSE                | DIRECT DEPENDENCIES                  | IMPACTED DEPENDENCY                   | \n| :---------------------: | :----------------------------------: | :-----------------------------------: | \n| Apache-2.0 | root 1.0.0<br>minimatch 1.2.3 | minimatch 1.2.3 |\n\n</div>\n\n\n---\n<div align=\"center\">\n\n[🐸 JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n</div>"
-	assert.Equal(t, expectedMessage, message)
-
-	writerOutput.SetVcsProvider(vcsutils.GitLab)
-	message = createPullRequestComment(&IssuesCollection{Vulnerabilities: vulnerabilities}, writerOutput)
-	expectedMessage = "<div align='center'>\n\n[![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/vulnerabilitiesBannerMR.png)](https://github.com/jfrog/frogbot#readme)\n\n</div>\n\n\n## 📦 Vulnerable Dependencies\n\n### ✍️ Summary\n\n<div align=\"center\">\n\n| SEVERITY                | CONTEXTUAL ANALYSIS                  | DIRECT DEPENDENCIES                  | IMPACTED DEPENDENCY                   | FIXED VERSIONS                       | CVES                       |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | :---------------------------------: | \n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br>    High | Undetermined | github.com/nats-io/nats-streaming-server:v0.21.0 | github.com/nats-io/nats-streaming-server:v0.21.0 | [0.24.1] |  -  |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableHighSeverity.png)<br>    High | Undetermined | github.com/mholt/archiver/v3:v3.5.1 | github.com/mholt/archiver/v3:v3.5.1 |  -  |  -  |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableMediumSeverity.png)<br>  Medium | Undetermined | github.com/nats-io/nats-streaming-server:v0.21.0 | github.com/nats-io/nats-streaming-server:v0.21.0 | [0.24.3] | CVE-2022-26652 |\n\n</div>\n\n## 🔬 Research Details\n\n<details>\n<summary> <b>[ XRAY-122345 ] github.com/nats-io/nats-streaming-server v0.21.0</b> </summary>\n<br>\n\n**Description:**\nSummary XRAY-122345\n\n\n</details>\n\n\n<details>\n<summary> <b>github.com/mholt/archiver/v3 v3.5.1</b> </summary>\n<br>\n\n**Description:**\nSummary\n\n\n</details>\n\n\n<details>\n<summary> <b>[ CVE-2022-26652 ] github.com/nats-io/nats-streaming-server v0.21.0</b> </summary>\n<br>\n\n**Description:**\nSummary CVE-2022-26652\n\n\n</details>\n\n\n---\n<div align=\"center\">\n\n[🐸 JFrog Frogbot](https://github.com/jfrog/frogbot#readme)\n\n</div>"
-	assert.Equal(t, expectedMessage, message)
-}
-
 func TestGetFrogbotReviewComments(t *testing.T) {
 	testCases := []struct {
 		name             string
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,7 +7,6 @@ import ( @@
     	"os"
     	"golang.org/x/exp/slices"
-    	"os"
     	"github.com/jfrog/frogbot/utils"
     	"github.com/jfrog/froggit-go/vcsclient"
@@ Expand Down @@