#276: Use correct link on Pull Request annotations for Hostspots

Sonarqube uses a different URL format for issues compared to Security Hotspots, but the plugin was using the same format for both during Pull Request decoration, so linking to invalid URLS for hotspots. The URL generation has therefore been altered to take the issue type into account when generating a link to each issue.

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/AnalysisDetails.java

@@ -116,8 +116,12 @@ public String getDashboardUrl() {
         return publicRootURL + "/dashboard?id=" + encode(project.getKey()) + "&pullRequest=" + branchDetails.getBranchName();
     }
 
-    public String getIssueUrl(String issueKey) {
-        return publicRootURL + "/project/issues?id=" + encode(project.getKey()) + "&pullRequest=" + branchDetails.getBranchName() + "&issues=" + issueKey + "&open=" + issueKey;
+    public String getIssueUrl(PostAnalysisIssueVisitor.LightIssue issue) {
+        if (issue.type() == RuleType.SECURITY_HOTSPOT) {
+            return String.format("%s/security_hotspots?id=%s&pullRequest=%s&hotspots=%s", publicRootURL, encode(project.getKey()), branchDetails.getBranchName(), issue.key());
+        } else {
+            return String.format("%s/project/issues?id=%s&pullRequest=%s&issues=%s&open=%s", publicRootURL, encode(project.getKey()), branchDetails.getBranchName(), issue.key(), issue.key());
+        }
     }
 
     public Optional<String> getPullRequestBase() {
@@ -234,7 +238,7 @@ public String createAnalysisIssueSummary(PostAnalysisIssueVisitor.ComponentIssue
                 new Paragraph(new Text(String.format("**Message:** %s", issue.getMessage()))),
                 effortNode,
                 resolutionNode,
-                new Link(getIssueUrl(issue.key()), new Text("View in SonarQube"))
+                new Link(getIssueUrl(issue), new Text("View in SonarQube"))
         );
         return formatterFactory.documentFormatter().format(document, formatterFactory);
     }

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/azuredevops/AzureDevOpsServerPullRequestDecorator.java

@@ -190,7 +190,7 @@ private void handleIssue(AnalysisDetails analysisDetails, AzurePullRequestDetail
                         issue.getIssue().type().name(),
                         issue.getIssue().getMessage(),
                         analysisDetails.getRuleUrlWithRuleKey(issue.getIssue().getRuleKey().toString()),
-                        analysisDetails.getIssueUrl(issue.getIssue().key())
+                        analysisDetails.getIssueUrl(issue.getIssue())
                 );
 
                 CommentThread thread = new CommentThread(filePath, locate, message);

src/main/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecorator.java

@@ -147,7 +147,7 @@ private void updateAnnotations(BitbucketClient client, String project, String re
                     String path = componentIssue.getComponent().getReportAttributes().getScmPath().get();
                     return client.createCodeInsightsAnnotation(componentIssue.getIssue().key(),
                             Optional.ofNullable(componentIssue.getIssue().getLine()).orElse(0),
-                            analysisDetails.getIssueUrl(componentIssue.getIssue().key()),
+                            analysisDetails.getIssueUrl(componentIssue.getIssue()),
                             componentIssue.getIssue().getMessage(),
                             path,
                             toBitbucketSeverity(componentIssue.getIssue().severity()),

src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/AnalysisDetailsTest.java

@@ -63,6 +63,7 @@
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class AnalysisDetailsTest {
 
@@ -757,7 +758,7 @@ public void testGetBaseImageUrlFromRootUrl() {
     }
 
     @Test
-    public void testGetIssueUrl() {
+    public void testGetIssueUrlBug() {
         Project project = mock(Project.class);
         doReturn("projectKey").when(project).getKey();
 
@@ -769,7 +770,31 @@ public void testGetIssueUrl() {
                         mock(QualityGate.class), mock(AnalysisDetails.MeasuresHolder.class),
                         mock(Analysis.class), project, mock(Configuration.class), "http://localhost:9000", mock(ScannerContext.class));
 
-        assertEquals("http://localhost:9000/project/issues?id=projectKey&pullRequest=123&issues=issueKey&open=issueKey", analysisDetails.getIssueUrl("issueKey"));
+        PostAnalysisIssueVisitor.LightIssue lightIssue = mock(PostAnalysisIssueVisitor.LightIssue.class);
+        when(lightIssue.key()).thenReturn("issueKey");
+        when(lightIssue.type()).thenReturn(RuleType.BUG);
+
+        assertEquals("http://localhost:9000/project/issues?id=projectKey&pullRequest=123&issues=issueKey&open=issueKey", analysisDetails.getIssueUrl(lightIssue));
+    }
+
+    @Test
+    public void testGetIssueUrlSecurityHotspot() {
+        Project project = mock(Project.class);
+        doReturn("projectKey").when(project).getKey();
+
+        AnalysisDetails.BranchDetails branchDetails = mock(AnalysisDetails.BranchDetails.class);
+        doReturn("123").when(branchDetails).getBranchName();
+
+        AnalysisDetails analysisDetails =
+                new AnalysisDetails(branchDetails, mock(PostAnalysisIssueVisitor.class),
+                        mock(QualityGate.class), mock(AnalysisDetails.MeasuresHolder.class),
+                        mock(Analysis.class), project, mock(Configuration.class), "http://localhost:9000", mock(ScannerContext.class));
+
+        PostAnalysisIssueVisitor.LightIssue lightIssue = mock(PostAnalysisIssueVisitor.LightIssue.class);
+        when(lightIssue.key()).thenReturn("secondIssueKey");
+        when(lightIssue.type()).thenReturn(RuleType.SECURITY_HOTSPOT);
+
+        assertEquals("http://localhost:9000/security_hotspots?id=projectKey&pullRequest=123&hotspots=secondIssueKey", analysisDetails.getIssueUrl(lightIssue));
     }
 
     @Test
@@ -781,4 +806,53 @@ public void testGetRuleUrlWithRuleKey() {
 
         assertEquals("http://localhost:9000/coding_rules?open=ruleKey&rule_key=ruleKey", analysisDetails.getRuleUrlWithRuleKey("ruleKey"));
     }
+
+    @Test
+    public void testCreateAnalysisIssueSummary() {
+        FormatterFactory formatterFactory = mock(FormatterFactory.class);
+        PostAnalysisIssueVisitor.ComponentIssue componentIssue = mock(PostAnalysisIssueVisitor.ComponentIssue.class);
+
+        AnalysisDetails.BranchDetails branchDetails = mock(AnalysisDetails.BranchDetails.class);
+        when(branchDetails.getBranchName()).thenReturn("branchName");
+
+        PostAnalysisIssueVisitor.LightIssue lightIssue = mock(PostAnalysisIssueVisitor.LightIssue.class);
+        when(lightIssue.type()).thenReturn(RuleType.BUG);
+        when(lightIssue.getMessage()).thenReturn("message");
+        when(lightIssue.severity()).thenReturn("severity");
+        when(lightIssue.key()).thenReturn("issueKey");
+        when(lightIssue.effortInMinutes()).thenReturn(123L);
+        when(componentIssue.getIssue()).thenReturn(lightIssue);
+
+        Project project = mock(Project.class);
+        when(project.getKey()).thenReturn("projectKey");
+
+        Formatter<Document> documentFormatter = mock(Formatter.class);
+        when(formatterFactory.documentFormatter()).thenReturn(documentFormatter);
+
+        AnalysisDetails analysisDetails =
+                new AnalysisDetails(branchDetails, mock(PostAnalysisIssueVisitor.class),
+                        mock(QualityGate.class), mock(AnalysisDetails.MeasuresHolder.class),
+                        mock(Analysis.class), project, mock(Configuration.class), "http://localhost:9000", mock(ScannerContext.class));
+
+        ArgumentCaptor<Document> documentArgumentCaptor = ArgumentCaptor.forClass(Document.class);
+        analysisDetails.createAnalysisIssueSummary(componentIssue, formatterFactory);
+        verify(documentFormatter).format(documentArgumentCaptor.capture(), any());
+
+        assertThat(documentArgumentCaptor.getValue()).usingRecursiveComparison().isEqualTo(
+                new Document(
+                        new Paragraph(
+                                new Text("**Type:** BUG "),
+                                new Image("BUG", "http://localhost:9000/static/communityBranchPlugin/checks/IssueType/bug.svg?sanitize=true")
+                        ),
+                        new Paragraph(
+                                new Text("**Severity:** severity "),
+                                new Image("severity", "http://localhost:9000/static/communityBranchPlugin/checks/Severity/severity.svg?sanitize=true")
+                        ),
+                        new Paragraph(new Text("**Message:** message")),
+                        new Paragraph(new Text("**Duration (min):** 123")),
+                        new Text(""),
+                        new Link("http://localhost:9000/project/issues?id=projectKey&pullRequest=branchName&issues=issueKey&open=issueKey", new Text("View in SonarQube"))
+                )
+        );
+    }
 }

src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/azuredevops/AzureDevOpsServerPullRequestDecoratorTest.java

@@ -97,7 +97,7 @@ private void configureTestDefaults() {
         when(analysisDetails.getBranchName()).thenReturn(pullRequestId);
         when(analysisDetails.getPostAnalysisIssueVisitor()).thenReturn(issueVisitor);
         when(analysisDetails.getRuleUrlWithRuleKey(ruleKeyVal)).thenReturn(ruleUrl);
-        when(analysisDetails.getIssueUrl(issueKeyVal)).thenReturn(issueUrl);
+        when(analysisDetails.getIssueUrl(defaultIssue)).thenReturn(issueUrl);
         when(analysisDetails.getSCMPathForIssue(componentIssue)).thenReturn(Optional.of(filePath));
         when(issueVisitor.getIssues()).thenReturn(Collections.singletonList(componentIssue));
 

src/test/java/com/github/mc1arke/sonarqube/plugin/ce/pullrequest/bitbucket/BitbucketPullRequestDecoratorTest.java

@@ -148,7 +148,7 @@ private void mockValidAnalysis() {
         when(defaultIssue.key()).thenReturn(ISSUE_KEY);
         when(defaultIssue.type()).thenReturn(RuleType.BUG);
         when(defaultIssue.getMessage()).thenReturn(ISSUE_MESSAGE);
-        when(analysisDetails.getIssueUrl(ISSUE_KEY)).thenReturn(ISSUE_LINK);
+        when(analysisDetails.getIssueUrl(defaultIssue)).thenReturn(ISSUE_LINK);
         when(analysisDetails.getBaseImageUrl()).thenReturn(IMAGE_URL);
 
         PostAnalysisIssueVisitor.ComponentIssue componentIssue = mock(PostAnalysisIssueVisitor.ComponentIssue.class);