diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java index be1822654..a001377ba 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/CommunityBranchPlugin.java @@ -73,6 +73,7 @@ public class CommunityBranchPlugin implements Plugin, CoreExtension { public static final String IMAGE_URL_BASE = "com.github.mc1arke.sonarqube.plugin.branch.image-url-base"; + public static final String PR_SUMMARY_NOTE_EDIT = "com.github.mc1arke.sonarqube.plugin.branch.pullrequest.summary.edit"; @Override public String getName() { @@ -157,6 +158,15 @@ public void load(CoreExtension.Context context) { .build(), MonoRepoFeature.class); + context.addExtensions(PropertyDefinition.builder(PR_SUMMARY_NOTE_EDIT) + .category(getName()) + .subCategory("Merge Request Decoration") + .onQualifiers(Qualifiers.PROJECT) + .name("Edit summary note") + .description("Edit summary discussion thread instead of resolving it and creating a new one (Gitlab only).") + .type(PropertyType.BOOLEAN) + .defaultValue(String.valueOf(false)) + .build()); } } diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabClient.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabClient.java index dd2937c06..8b60d9cb9 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabClient.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabClient.java @@ -43,6 +43,8 @@ public interface GitlabClient { void addMergeRequestDiscussionNote(long projectId, long mergeRequestIid, String discussionId, String noteContent) throws IOException; + void editMergeRequestDisscussionNote(long projectId, long mergeRequestIid, String discussionId, long noteId, String noteContent) throws IOException; + void resolveMergeRequestDiscussion(long projectId, long mergeRequestIid, String discussionId) throws IOException; void setMergeRequestPipelineStatus(long projectId, String commitRevision, PipelineStatus status) throws IOException; diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabRestClient.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabRestClient.java index 27ed9dada..9bcf1b198 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabRestClient.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/almclient/gitlab/GitlabRestClient.java @@ -126,6 +126,15 @@ public void addMergeRequestDiscussionNote(long projectId, long mergeRequestIid, entity(httpPost, null, httpResponse -> validateResponse(httpResponse, 201, "Commit discussions note added")); } + @Override + public void editMergeRequestDisscussionNote(long projectId, long mergeRequestIid, String discussionId, long noteId, String noteContent) throws IOException { + String targetUrl = String.format("%s/projects/%s/merge_requests/%s/discussions/%s/notes/%s", baseGitlabApiUrl, projectId, mergeRequestIid, discussionId, noteId); + + HttpPut httpPut = new HttpPut(targetUrl); + httpPut.setEntity(new UrlEncodedFormEntity(Collections.singletonList(new BasicNameValuePair("body", noteContent)), StandardCharsets.UTF_8)); + entity(httpPut, null); + } + @Override public void resolveMergeRequestDiscussion(long projectId, long mergeRequestIid, String discussionId) throws IOException { String discussionIdUrl = String.format("%s/projects/%s/merge_requests/%s/discussions/%s?resolved=true", baseGitlabApiUrl, projectId, mergeRequestIid, discussionId); diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/DiscussionAwarePullRequestDecorator.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/DiscussionAwarePullRequestDecorator.java index 40687e623..847467cfc 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/DiscussionAwarePullRequestDecorator.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/DiscussionAwarePullRequestDecorator.java @@ -71,7 +71,7 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS U user = getCurrentUser(client); List openSonarqubeIssues = analysis.getScmReportableIssues(); - List>> currentProjectSonarqubeComments = findOpenSonarqubeComments(client, + List>> currentProjectSonarqubeComments = findSonarqubeComments(client, pullRequest, user) .stream() @@ -82,7 +82,8 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS user, currentProjectSonarqubeComments, openSonarqubeIssues, - pullRequest); + pullRequest, + isEditSummaryNoteEnabled(analysis)); List commitIds = getCommitIdsForPullRequest(client, pullRequest); List> uncommentedIssues = findIssuesWithoutComments(openSonarqubeIssues, @@ -102,7 +103,7 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS reportGenerator.createAnalysisIssueSummary(issue.getLeft(), analysis))); AnalysisSummary analysisSummary = reportGenerator.createAnalysisSummary(analysis); - submitSummaryNote(client, pullRequest, analysis, analysisSummary); + editExistingOrSubmitSummaryNote(client, pullRequest, currentProjectSonarqubeComments, analysis, analysisSummary); submitPipelineStatus(client, pullRequest, analysis, analysisSummary); DecorationResult.Builder builder = DecorationResult.builder(); @@ -110,6 +111,26 @@ public DecorationResult decorateQualityGateStatus(AnalysisDetails analysis, AlmS return builder.build(); } + private void editExistingOrSubmitSummaryNote(C client, P pullRequest, + List>> currentProjectSonarqubeComments, + AnalysisDetails analysis, AnalysisSummary analysisSummary) { + Optional>> existingSummaryNote = currentProjectSonarqubeComments.stream() + .filter(discussion -> isSummaryNote(discussion.getRight())) + .findFirst(); + + if (isEditSummaryNoteEnabled(analysis) && existingSummaryNote.isPresent()) { + editSummaryNote(client, pullRequest, existingSummaryNote.get().getLeft(), analysis, analysisSummary); + } else { + submitSummaryNote(client, pullRequest, analysis, analysisSummary); + } + } + + private boolean isSummaryNote(Optional projectIssueIdentifier) { + return projectIssueIdentifier + .map(ProjectIssueIdentifier::isSummary) + .orElse(false); + } + protected abstract C createClient(AlmSettingDto almSettingDto, ProjectAlmSettingDto projectAlmSettingDto); protected abstract Optional createFrontEndUrl(P pullRequest, AnalysisDetails analysisDetails); @@ -139,6 +160,8 @@ protected abstract void submitCommitNoteForIssue(C client, P pullRequest, PostAn protected abstract void submitSummaryNote(C client, P pullRequest, AnalysisDetails analysis, AnalysisSummary analysisSummary); + protected abstract void editSummaryNote(C client, P pullRequest, D discussion, AnalysisDetails analysis, AnalysisSummary analysisSummary); + protected abstract List getDiscussions(C client, P pullRequest); protected abstract boolean isNoteFromCurrentUser(N note, U user); @@ -171,15 +194,14 @@ private static boolean isIssueFromCommitInCurrentRequest(PostAnalysisIssueVisito .isPresent(); } - private List>> findOpenSonarqubeComments(C client, P pullRequest, - U currentUser) { + private List>> findSonarqubeComments(C client, P pullRequest, + U currentUser) { return getDiscussions(client, pullRequest).stream() .map(discussion -> { List commentsForDiscussion = getNotesForDiscussion(client, discussion); return commentsForDiscussion.stream() .findFirst() .filter(note -> isNoteFromCurrentUser(note, currentUser)) - .filter(note -> !isResolved(client, discussion, commentsForDiscussion, currentUser)) .map(note -> new ImmutableTriple<>(discussion, note, parseIssueDetails(client, note))); }) .filter(Optional::isPresent) @@ -188,22 +210,31 @@ private List>> findOpenSonarqubeCo } private List closeOldDiscussionsAndExtractRemainingKeys(C client, U currentUser, - List>> openSonarqubeComments, + List>> sonarqubeComments, List openIssues, - P pullRequest) { + P pullRequest, + boolean skipSummaryNote) { List openIssueKeys = openIssues.stream() .map(issue -> issue.getIssue().key()) .collect(Collectors.toList()); List remainingCommentKeys = new ArrayList<>(); - for (Triple> openSonarqubeComment : openSonarqubeComments) { - Optional noteIdentifier = openSonarqubeComment.getRight(); - D discussion = openSonarqubeComment.getLeft(); + for (Triple> sonarqubeComment : sonarqubeComments) { + Optional noteIdentifier = sonarqubeComment.getRight(); + D discussion = sonarqubeComment.getLeft(); if (noteIdentifier.isEmpty()) { continue; } + if (skipSummaryNote && isSummaryNote(noteIdentifier)) { + continue; + } + + if (isResolved(client, discussion, getNotesForDiscussion(client, discussion), currentUser)) { + continue; + } + String issueKey = noteIdentifier.get().getIssueKey(); if (!openIssueKeys.contains(issueKey)) { resolveOrPlaceFinalCommentOnDiscussion(client, currentUser, discussion, pullRequest); @@ -274,7 +305,7 @@ private static Optional parseIssueIdFromUrl(String issue String projectId = optionalProjectId.get(); if (url.getPath().endsWith("/dashboard")) { - return Optional.of(new ProjectIssueIdentifier(projectId, "decorator-summary-comment")); + return Optional.of(new ProjectIssueIdentifier(projectId, ProjectIssueIdentifier.SUMMARY_NOTE_ISSUE_KEY)); } else if (url.getPath().endsWith("security_hotspots")) { return parameters.stream() .filter(parameter -> "hotspots".equals(parameter.getName())) @@ -296,6 +327,8 @@ private static boolean isCommentFromCurrentProject(Triple alm(); + + default boolean isEditSummaryNoteEnabled(AnalysisDetails analysisDetails) { + return false; + } } diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/azuredevops/AzureDevOpsPullRequestDecorator.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/azuredevops/AzureDevOpsPullRequestDecorator.java index 2b37c7516..df7c66f42 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/azuredevops/AzureDevOpsPullRequestDecorator.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/azuredevops/AzureDevOpsPullRequestDecorator.java @@ -185,6 +185,11 @@ protected void submitSummaryNote(AzureDevopsClient client, PullRequest pullReque } } + @Override + protected void editSummaryNote(AzureDevopsClient client, PullRequest pullRequest, CommentThread discussion, AnalysisDetails analysis, AnalysisSummary analysisSummary) { + // not implemented + } + protected List getDiscussions(AzureDevopsClient client, PullRequest pullRequest) { try { return client.retrieveThreads(pullRequest.getRepository().getProject().getName(), pullRequest.getRepository().getName(), pullRequest.getId()); diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecorator.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecorator.java index 37a22f6fc..6b5f3f0a1 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecorator.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecorator.java @@ -18,6 +18,7 @@ */ package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab; +import com.github.mc1arke.sonarqube.plugin.CommunityBranchPlugin; import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.GitlabClient; import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.GitlabClientFactory; import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Commit; @@ -62,6 +63,13 @@ public GitlabMergeRequestDecorator(ScmInfoRepository scmInfoRepository, GitlabCl this.formatterFactory = formatterFactory; } + @Override + public boolean isEditSummaryNoteEnabled(AnalysisDetails analysis) { + return analysis.getScannerProperty(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT) + .map(Boolean::parseBoolean) + .orElse(false); + } + @Override public List alm() { return Collections.singletonList(ALM.GITLAB); @@ -169,6 +177,22 @@ protected void submitSummaryNote(GitlabClient client, MergeRequest mergeRequest, } + @Override + protected void editSummaryNote(GitlabClient client, MergeRequest mergeRequest, Discussion discussion, AnalysisDetails analysis, AnalysisSummary analysisSummary) { + try { + client.editMergeRequestDisscussionNote(mergeRequest.getTargetProjectId(), + mergeRequest.getIid(), + discussion.getId(), + discussion.getNotes().get(0).getId(), + analysisSummary.format(formatterFactory)); + if (!isClosed(discussion, discussion.getNotes()) && analysis.getQualityGateStatus() == QualityGate.Status.OK) { + client.resolveMergeRequestDiscussion(mergeRequest.getTargetProjectId(), mergeRequest.getIid(), discussion.getId()); + } + } catch (IOException ex) { + throw new IllegalStateException("Could not edit summary comment to Gitlab", ex); + } + } + @Override protected List getDiscussions(GitlabClient client, MergeRequest pullRequest) { try { diff --git a/src/main/java/com/github/mc1arke/sonarqube/plugin/scanner/ScannerPullRequestPropertySensor.java b/src/main/java/com/github/mc1arke/sonarqube/plugin/scanner/ScannerPullRequestPropertySensor.java index ccaffd806..e7c796b6f 100644 --- a/src/main/java/com/github/mc1arke/sonarqube/plugin/scanner/ScannerPullRequestPropertySensor.java +++ b/src/main/java/com/github/mc1arke/sonarqube/plugin/scanner/ScannerPullRequestPropertySensor.java @@ -18,6 +18,7 @@ */ package com.github.mc1arke.sonarqube.plugin.scanner; +import com.github.mc1arke.sonarqube.plugin.CommunityBranchPlugin; import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.GitlabMergeRequestDecorator; import org.sonar.api.batch.sensor.Sensor; import org.sonar.api.batch.sensor.SensorContext; @@ -54,6 +55,8 @@ public void execute(SensorContext sensorContext) { Optional.ofNullable(system2.property(GitlabMergeRequestDecorator.PULLREQUEST_GITLAB_PIPELINE_ID)).ifPresent( v -> sensorContext.addContextProperty(GitlabMergeRequestDecorator.PULLREQUEST_GITLAB_PIPELINE_ID, v)); + sensorContext.config().get(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT) + .ifPresent(p -> sensorContext.addContextProperty(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT, p)); } } diff --git a/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/DiscussionMock.java b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/DiscussionMock.java new file mode 100644 index 000000000..c94caf9c6 --- /dev/null +++ b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/DiscussionMock.java @@ -0,0 +1,86 @@ +package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab; + +import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Discussion; +import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Note; +import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.User; + +import java.util.*; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +class DiscussionMock { + private static final String PROJECT_KEY = "projectKey"; + private static final String SONARQUBE_USERNAME = "sonarqube@gitlab.dummy"; + private static final String OLD_SONARQUBE_ISSUE_COMMENT = "This issue no longer exists in SonarQube, " + + "but due to other comments being present in this discussion, " + + "the discussion is not being being closed automatically. " + + "Please manually resolve this discussion once the other comments have been reviewed."; + private static final User sonarqubeUser = new User(SONARQUBE_USERNAME); + + private static Discussion createIssueComment() { + Note note = createUnresolvedSonarQubeNote( + "Reported issue\n[View in SonarQube](http://domain.url/sonar/issue?issues=issueKey1&id=" + + PROJECT_KEY + ")"); + return new Discussion("issue-comment-id", Collections.singletonList(note)); + } + + private static Discussion createResolvedIssueComment() { + Note note = createResolvedSonarQubeNote( + "Reported issue\n[View in SonarQube](http://domain.url/sonar/issue?issues=issueKey1&id=" + + PROJECT_KEY + ")"); + return new Discussion("resolved-issue-comment-id", Collections.singletonList(note)); + } + + private static Discussion createResolvedByCommentIssueComment() { + Note note = createUnresolvedSonarQubeNote( + "Reported issue\n[View in SonarQube](http://domain.url/sonar/issue?issues=issueKey1&id=" + + PROJECT_KEY + ")"); + Note note2 = createUnresolvedSonarQubeNote(OLD_SONARQUBE_ISSUE_COMMENT); + Note note3 = createUnresolvedSonarQubeNote("Some additional comment"); + return new Discussion("issue-with-resolved-comment-id", Arrays.asList(note, note2, note3)); + } + + private static Discussion createUnresolvedSummaryNote() { + Note note = createUnresolvedSonarQubeNote( + "Analysis Details\n[View in SonarQube](http://domain.url/sonar/dashboard?id=" + PROJECT_KEY + ")"); + return new Discussion("summary-note-id", Collections.singletonList(note)); + } + + private static Discussion createResolvedSummaryNote() { + Note note = createResolvedSonarQubeNote( + "Analysis Details\n[View in SonarQube](http://domain.url/sonar/dashboard?id=" + PROJECT_KEY + ")"); + return new Discussion("summary-note-id", Collections.singletonList(note)); + } + + private static Note createUnresolvedSonarQubeNote(String body) { + return new Note(new Random().nextLong(), false, sonarqubeUser, body, false, true); + } + + private static Note createResolvedSonarQubeNote(String body) { + return new Note(new Random().nextLong(), false, sonarqubeUser, body, true, true); + } + + public static Map getDiscussionsMap(DiscussionType... discussions) { + return Arrays.stream(discussions) + .collect(Collectors.toMap(k -> k, DiscussionType::create, (e1, e2) -> e1, LinkedHashMap::new)); + } + + enum DiscussionType { + RESOLVED_SUMMARY_NOTE(DiscussionMock::createResolvedSummaryNote), + UNRESOLVED_SUMMARY_NOTE(DiscussionMock::createUnresolvedSummaryNote), + ISSUE_COMMENT(DiscussionMock::createIssueComment), + RESOLVED_ISSUE_COMMENT(DiscussionMock::createResolvedIssueComment), + RESOLVED_BY_COMMENT_ISSUE_COMMENT(DiscussionMock::createResolvedByCommentIssueComment); + + private final Supplier creationMethod; + + DiscussionType(Supplier creationMethod) { + this.creationMethod = creationMethod; + } + + public Discussion create() { + return creationMethod.get(); + } + } + +} diff --git a/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecoratorTest.java b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecoratorTest.java index 009e7dab8..e792ef063 100644 --- a/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecoratorTest.java +++ b/src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/gitlab/GitlabMergeRequestDecoratorTest.java @@ -18,6 +18,7 @@ */ package com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab; +import com.github.mc1arke.sonarqube.plugin.CommunityBranchPlugin; import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.GitlabClient; import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.GitlabClientFactory; import com.github.mc1arke.sonarqube.plugin.almclient.gitlab.model.Commit; @@ -36,8 +37,8 @@ import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.report.AnalysisIssueSummary; import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.report.AnalysisSummary; import com.github.mc1arke.sonarqube.plugin.ce.pullrequest.report.ReportGenerator; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.sonar.api.ce.posttask.QualityGate; import org.sonar.api.issue.Issue; @@ -54,9 +55,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; +import static com.github.mc1arke.sonarqube.plugin.ce.pullrequest.gitlab.DiscussionMock.DiscussionType.*; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; @@ -101,7 +104,7 @@ public class GitlabMergeRequestDecoratorTest { private final GitlabMergeRequestDecorator underTest = new GitlabMergeRequestDecorator(scmInfoRepository, gitlabClientFactory, reportGenerator, markdownFormatterFactory); - @Before + @BeforeEach public void setUp() throws IOException { when(analysisSummary.format(any())).thenReturn("Summary Comment"); when(reportGenerator.createAnalysisSummary(any())).thenReturn(analysisSummary); @@ -589,6 +592,58 @@ public void shouldNotStartNewDiscussionForIssueWithExistingCommentFromCommitInMe assertThat(mergeRequestNoteArgumentCaptor.getValue()).isNotInstanceOf(CommitNote.class); } + @Test + public void shouldStartNewDiscussionForIssueIfExistingCommentResolvedInMergeRequest() throws IOException { + testStartNewDiscussionForIssueWhenCommentExists(RESOLVED_ISSUE_COMMENT); + } + + @Test + public void shouldStartNewDiscussionForIssueIfResolvedBySonarQubeCommentInMergeRequest() throws IOException { + testStartNewDiscussionForIssueWhenCommentExists(RESOLVED_BY_COMMENT_ISSUE_COMMENT); + } + + private void testStartNewDiscussionForIssueWhenCommentExists(DiscussionMock.DiscussionType discussionType) throws IOException { + reportIssue(); + existingMergeRequestDiscussions(discussionType); + + underTest.decorateQualityGateStatus(analysisDetails, almSettingDto, projectAlmSettingDto); + + verify(gitlabClient, never()).resolveMergeRequestDiscussion(anyLong(), anyLong(), any()); + verify(gitlabClient, never()).addMergeRequestDiscussionNote(anyLong(), anyLong(), any(), any()); + + ArgumentCaptor mergeRequestNoteArgumentCaptor = ArgumentCaptor.forClass(MergeRequestNote.class); + verify(gitlabClient, times(2)).addMergeRequestDiscussion(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), mergeRequestNoteArgumentCaptor.capture()); + + assertThat(mergeRequestNoteArgumentCaptor.getAllValues().get(0)) + .usingRecursiveComparison() + .isEqualTo(new CommitNote("Issue Summary", BASE_SHA, START_SHA, HEAD_SHA, "path-to-file", "path-to-file", 999)); + assertThat(mergeRequestNoteArgumentCaptor.getAllValues().get(1)).isNotInstanceOf(CommitNote.class); + } + + private void reportIssue() { + PostAnalysisIssueVisitor.LightIssue lightIssue = mock(PostAnalysisIssueVisitor.LightIssue.class); + when(lightIssue.key()).thenReturn("issueKey1"); + when(lightIssue.getStatus()).thenReturn(Issue.STATUS_OPEN); + when(lightIssue.getLine()).thenReturn(999); + + Component component = mock(Component.class); + + PostAnalysisIssueVisitor.ComponentIssue componentIssue = mock(PostAnalysisIssueVisitor.ComponentIssue.class); + when(componentIssue.getIssue()).thenReturn(lightIssue); + when(componentIssue.getComponent()).thenReturn(component); + when(componentIssue.getScmPath()).thenReturn(Optional.of("path-to-file")); + + when(analysisDetails.getScmReportableIssues()).thenReturn(Collections.singletonList(componentIssue)); + + Changeset changeset = mock(Changeset.class); + when(changeset.getRevision()).thenReturn("DEF"); + + ScmInfo scmInfo = mock(ScmInfo.class); + when(scmInfo.hasChangesetForLine(999)).thenReturn(true); + when(scmInfo.getChangesetForLine(999)).thenReturn(changeset); + when(scmInfoRepository.getScmInfo(component)).thenReturn(Optional.of(scmInfo)); + } + @Test public void shouldNotCreateCommentsForIssuesWithNoLineNumbers() throws IOException { PostAnalysisIssueVisitor.LightIssue lightIssue = mock(PostAnalysisIssueVisitor.LightIssue.class); @@ -617,6 +672,80 @@ public void shouldNotCreateCommentsForIssuesWithNoLineNumbers() throws IOExcepti assertThat(mergeRequestNoteArgumentCaptor.getValue()).isNotInstanceOf(CommitNote.class); } + @Test + public void shouldAddSummaryNoteIfNoExistsAndEditingSummaryNoteEnabled() throws IOException { + enableScannerProperty(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT); + noDiscussionsExist(); + when(analysisSummary.format(any())).thenReturn("Summary comment"); + + underTest.decorateQualityGateStatus(analysisDetails, almSettingDto, projectAlmSettingDto); + + ArgumentCaptor mergeRequestNoteArgumentCaptor = ArgumentCaptor.forClass(MergeRequestNote.class); + verify(gitlabClient, never()).editMergeRequestDisscussionNote(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), any(), anyLong(), any()); + verify(gitlabClient).addMergeRequestDiscussion(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), mergeRequestNoteArgumentCaptor.capture()); + + assertThat(mergeRequestNoteArgumentCaptor.getValue()).isNotInstanceOf(CommitNote.class); + } + + private void noDiscussionsExist() throws IOException { + when(gitlabClient.getMergeRequestDiscussions(anyLong(), anyLong())).thenReturn(new ArrayList<>()); + } + + @Test + public void shouldEditExistingSummaryNoteWhenEditingSummaryNoteEnabled() throws IOException { + enableScannerProperty(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT); + when(analysisDetails.getQualityGateStatus()).thenReturn(QualityGate.Status.OK); + + var discussions = existingMergeRequestDiscussions(ISSUE_COMMENT, UNRESOLVED_SUMMARY_NOTE); + var summaryNote = discussions.get(UNRESOLVED_SUMMARY_NOTE); + + when(analysisSummary.format(any())).thenReturn("Edited Summary comment"); + + underTest.decorateQualityGateStatus(analysisDetails, almSettingDto, projectAlmSettingDto); + + ArgumentCaptor noteContentArgumentCaptor = ArgumentCaptor.forClass(String.class); + verify(gitlabClient, never()).addMergeRequestDiscussion(anyLong(), anyLong(), any()); + verify(gitlabClient).editMergeRequestDisscussionNote(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), + eq(summaryNote.getId()), eq(summaryNote.getNotes().get(0).getId()), noteContentArgumentCaptor.capture()); + verify(gitlabClient, times(1)).resolveMergeRequestDiscussion(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), eq(summaryNote.getId())); + + assertThat(noteContentArgumentCaptor.getValue()) + .isEqualTo("Edited Summary comment"); + } + + @Test + public void shouldEditExistingSummaryNoteEvenIfItIsResolvedWhenEditingSummaryNoteEnabled() throws IOException { + enableScannerProperty(CommunityBranchPlugin.PR_SUMMARY_NOTE_EDIT); + when(analysisDetails.getQualityGateStatus()).thenReturn(QualityGate.Status.OK); + + var discussions = existingMergeRequestDiscussions(RESOLVED_SUMMARY_NOTE, ISSUE_COMMENT); + var summaryNote = discussions.get(RESOLVED_SUMMARY_NOTE); + + when(analysisSummary.format(any())).thenReturn("Edited Summary comment"); + + underTest.decorateQualityGateStatus(analysisDetails, almSettingDto, projectAlmSettingDto); + + ArgumentCaptor noteContentArgumentCaptor = ArgumentCaptor.forClass(String.class); + verify(gitlabClient, never()).addMergeRequestDiscussion(anyLong(), anyLong(), any()); + verify(gitlabClient).editMergeRequestDisscussionNote(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), + eq(summaryNote.getId()), eq(summaryNote.getNotes().get(0).getId()), noteContentArgumentCaptor.capture()); + verify(gitlabClient, never()).resolveMergeRequestDiscussion(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), eq(summaryNote.getId())); + + assertThat(noteContentArgumentCaptor.getValue()) + .isEqualTo("Edited Summary comment"); + } + + private void enableScannerProperty(String property) { + when(analysisDetails.getScannerProperty(property)).thenReturn(Optional.of(String.valueOf(true))); + } + + private Map existingMergeRequestDiscussions(DiscussionMock.DiscussionType... discussions) throws IOException { + var discussionsMap = DiscussionMock.getDiscussionsMap(discussions); + when(gitlabClient.getMergeRequestDiscussions(anyLong(), anyLong())) + .thenReturn(new ArrayList<>(discussionsMap.values())); + return discussionsMap; + } + @Test public void shouldSubmitSuccessfulPipelineStatusAndResolvedSummaryCommentOnSuccessAnalysis() throws IOException { when(analysisDetails.getQualityGateStatus()).thenReturn(QualityGate.Status.OK); @@ -625,6 +754,9 @@ public void shouldSubmitSuccessfulPipelineStatusAndResolvedSummaryCommentOnSucce when(analysisSummary.format(any())).thenReturn("Summary comment"); when(analysisSummary.getDashboardUrl()).thenReturn("https://sonarqube.dummy/dashboard?id=projectKey&pullRequest=123"); + var discussions = existingMergeRequestDiscussions(ISSUE_COMMENT, UNRESOLVED_SUMMARY_NOTE); + var summaryNote = discussions.get(UNRESOLVED_SUMMARY_NOTE); + Discussion discussion = mock(Discussion.class); when(discussion.getId()).thenReturn("dicussion id"); when(gitlabClient.addMergeRequestDiscussion(anyLong(), anyLong(), any())).thenReturn(discussion); @@ -632,7 +764,9 @@ public void shouldSubmitSuccessfulPipelineStatusAndResolvedSummaryCommentOnSucce underTest.decorateQualityGateStatus(analysisDetails, almSettingDto, projectAlmSettingDto); ArgumentCaptor mergeRequestNoteArgumentCaptor = ArgumentCaptor.forClass(MergeRequestNote.class); + verify(gitlabClient, never()).editMergeRequestDisscussionNote(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), any(), anyLong(), any()); verify(gitlabClient).addMergeRequestDiscussion(eq(PROJECT_ID), eq(MERGE_REQUEST_IID), mergeRequestNoteArgumentCaptor.capture()); + verify(gitlabClient).resolveMergeRequestDiscussion(PROJECT_ID, MERGE_REQUEST_IID, summaryNote.getId()); verify(gitlabClient).resolveMergeRequestDiscussion(PROJECT_ID, MERGE_REQUEST_IID, discussion.getId()); ArgumentCaptor pipelineStatusArgumentCaptor = ArgumentCaptor.forClass(PipelineStatus.class); verify(gitlabClient).setMergeRequestPipelineStatus(eq(PROJECT_ID), eq("commitsha"), pipelineStatusArgumentCaptor.capture());