Approve PR only when all issues are below threshold severity

README.md

@@ -60,6 +60,8 @@ Go to Stash general settings screen on SonarQube server to fill:
 
 **Stash reviewer approval** (sonar.stash.reviewer.approval): SonarQube is able to approve the pull-request if there is no new issue introduced by the change. By default, this feature is deactivated: if activated, **Stash base user must have REPO_WRITE permission for the repositories.** 
 
+**Stash reviewer reject severity threshold** (sonar.stash.reviewer.reject.severity.threshold): Do not approve pull-request when there are issues with severity equal or higher than the threshold. By default, threshold is INFO.
+
 **Include Analysis Overview Comment** (sonar.stash.include.overview): Toggles whether a comment with overview information should be created.
 
 ![Screenshot SonarQube plugin](resources/Sonar-plugin-approver.PNG)

src/main/java/org/sonar/plugins/stash/StashIssueReportingPostJob.java

@@ -117,8 +117,6 @@ private void postInfoAndPRsActions(PullRequestRef pr, List<Issue> issueReport, i
                                      StashClient stashClient) {
 
     // Some local definitions
-    boolean canApprovePullrequest = config.canApprovePullRequest();
-
     int issueTotal = issueReport.size();
 
     // if threshold exceeded, do not push issue list to Stash
@@ -136,11 +134,13 @@ private void postInfoAndPRsActions(PullRequestRef pr, List<Issue> issueReport, i
 
     // if no new issues and coverage is improved,
     // plugin approves the pull-request
-    if (canApprovePullrequest) {
-      if (issueTotal == 0) {
-        stashRequestFacade.approvePullRequest(pr, stashClient);
-      } else {
+    if (config.canApprovePullRequest()) {
+      List<String> rejectSeverities = stashRequestFacade.getRejectSeverities();
+      boolean reject = issueReport.stream().anyMatch(issue -> rejectSeverities.contains(issue.severity()));
+      if (reject) {
         stashRequestFacade.resetPullRequestApproval(pr, stashClient);
+      } else {
+        stashRequestFacade.approvePullRequest(pr, stashClient);
       }
     }
   }

src/main/java/org/sonar/plugins/stash/StashPlugin.java

@@ -62,6 +62,7 @@ public class StashPlugin extends SonarPlugin {
   public static final String STASH_PASSWORD = "sonar.stash.password";
   public static final String STASH_PASSWORD_ENVIRONMENT_VARIABLE = "sonar.stash.password.variable";
   public static final String STASH_REVIEWER_APPROVAL = "sonar.stash.reviewer.approval";
+  public static final String STASH_REVIEWER_REJECT_SEVERITY_THRESHOLD = "sonar.stash.reviewer.reject.severity.threshold";
   public static final String STASH_ISSUE_THRESHOLD = "sonar.stash.issue.threshold";
   public static final String STASH_TIMEOUT = "sonar.stash.timeout";
   public static final String SONARQUBE_URL = "sonar.host.url";
@@ -111,11 +112,19 @@ public List getExtensions() {
                           .defaultValue(DEFAULT_STASH_TIMEOUT_VALUE).build(),
         PropertyDefinition.builder(STASH_REVIEWER_APPROVAL)
                           .name("Stash reviewer approval")
-                          .description("Does SonarQube approve the pull-request if there is no new issues?")
+                          .description("SonarQube approves pull-request if there is no new issues above approval threshold")
                           .subCategory(CONFIG_PAGE_SUB_CATEGORY_STASH)
                           .onQualifiers(Qualifiers.PROJECT)
                           .type(PropertyType.BOOLEAN)
                           .defaultValue("false").build(),
+        PropertyDefinition.builder(STASH_REVIEWER_REJECT_SEVERITY_THRESHOLD)
+                          .name("Stash reviewer reject severity threshold")
+                          .description("Don't approve pull request when there are issues with this or higher severity")
+                          .type(PropertyType.SINGLE_SELECT_LIST)
+                          .subCategory(CONFIG_PAGE_SUB_CATEGORY_STASH)
+                          .onQualifiers(Qualifiers.PROJECT)
+                          .defaultValue(Severity.INFO)
+                          .options(SEVERITY_LIST).build(),
         PropertyDefinition.builder(STASH_ISSUE_THRESHOLD)
                           .name("Stash issue Threshold")
                           .description("Threshold to limit the number of issues pushed to Stash server")

src/main/java/org/sonar/plugins/stash/StashPluginConfiguration.java

@@ -77,6 +77,10 @@ public boolean canApprovePullRequest() {
     return settings.getBoolean(StashPlugin.STASH_REVIEWER_APPROVAL);
   }
 
+  public String getRejectSeverityThreshold() {
+    return settings.getString(StashPlugin.STASH_REVIEWER_REJECT_SEVERITY_THRESHOLD);
+  }
+
   public boolean resetComments() {
     return settings.getBoolean(StashPlugin.STASH_RESET_COMMENTS);
   }

src/main/java/org/sonar/plugins/stash/StashRequestFacade.java

@@ -26,6 +26,7 @@
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -438,20 +439,29 @@ public void resetComments(PullRequestRef pr,
    * Get reported severities to create a task.
    */
   public List<String> getReportedSeverities() {
-    List<String> result = new ArrayList<>();
     String threshold = config.getTaskIssueSeverityThreshold();
 
     // threshold == NONE, no severities reported
-    if (!StringUtils.equals(threshold, StashPlugin.SEVERITY_NONE)) {
+    if (StringUtils.equals(threshold, StashPlugin.SEVERITY_NONE)) {
+      return Collections.emptyList();
+    }
 
-      // INFO, MINOR, MAJOR, CRITICAL, BLOCKER
-      boolean hit = false;
-      for (String severity : StashPlugin.SEVERITY_LIST) {
+    return getGreatherThanOrEqualSeverities(threshold);
+  }
 
-        if (hit || StringUtils.equals(severity, threshold)) {
-          result.add(severity);
-          hit = true;
-        }
+  List<String> getRejectSeverities() {
+    return getGreatherThanOrEqualSeverities(config.getRejectSeverityThreshold());
+  }
+
+  private List<String> getGreatherThanOrEqualSeverities(String threshold) {
+    List<String> result = new ArrayList<>();
+    // INFO, MINOR, MAJOR, CRITICAL, BLOCKER
+    boolean hit = false;
+    for (String severity : StashPlugin.SEVERITY_LIST) {
+
+      if (hit || StringUtils.equals(severity, threshold)) {
+        result.add(severity);
+        hit = true;
       }
     }
 

src/test/java/org/sonar/plugins/stash/StashIssueReportingPostJobTest.java

@@ -17,13 +17,20 @@
 import org.sonar.plugins.stash.issue.StashUser;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.sonar.api.rule.Severity.BLOCKER;
+import static org.sonar.api.rule.Severity.CRITICAL;
+import static org.sonar.api.rule.Severity.INFO;
+import static org.sonar.api.rule.Severity.MAJOR;
+import static org.sonar.api.rule.Severity.MINOR;
 
 
 @RunWith(MockitoJUnitRunner.class)
@@ -412,4 +419,55 @@ public void testExecuteOnWithoutAnalysisComment() throws Exception {
     verify(stashRequestFacade, times(0)).postAnalysisOverview(eq(pr), eq(STASH_ISSUE_THRESHOLD), eq(report), (StashClient) Mockito.anyObject());
   }
   */
+
+  @Test
+  public void whenNoIssues_approve() throws Exception {
+    when(config.canApprovePullRequest()).thenReturn(true);
+
+    List<Issue> report = Collections.emptyList();
+    when(stashRequestFacade.extractIssueReport(projectIssues)).thenReturn(report);
+
+    myJob = new StashIssueReportingPostJob(config, projectIssues, stashRequestFacade);
+    myJob.executeOn(project, context);
+
+    verify(stashRequestFacade, times(1)).approvePullRequest(eq(pr), Mockito.anyObject());
+  }
+
+  @Test
+  public void whenAllIssuesBelowThreshold_approve() throws Exception {
+    when(config.canApprovePullRequest()).thenReturn(true);
+
+    List<Issue> report = report(INFO, MINOR, MAJOR, CRITICAL);
+    when(stashRequestFacade.extractIssueReport(projectIssues)).thenReturn(report);
+    when(stashRequestFacade.getRejectSeverities()).thenReturn(Collections.singletonList(BLOCKER));
+
+    myJob = new StashIssueReportingPostJob(config, projectIssues, stashRequestFacade);
+    myJob.executeOn(project, context);
+
+    verify(stashRequestFacade, times(1)).approvePullRequest(eq(pr), Mockito.anyObject());
+  }
+
+  @Test
+  public void whenGotIssuesAboveThreshold_resetApproval() throws Exception {
+    when(config.canApprovePullRequest()).thenReturn(true);
+
+    List<Issue> report = report(INFO, MINOR, MAJOR, CRITICAL, BLOCKER);
+    when(stashRequestFacade.extractIssueReport(projectIssues)).thenReturn(report);
+    when(stashRequestFacade.getRejectSeverities()).thenReturn(Collections.singletonList(BLOCKER));
+
+    myJob = new StashIssueReportingPostJob(config, projectIssues, stashRequestFacade);
+    myJob.executeOn(project, context);
+
+    verify(stashRequestFacade, times(1)).resetPullRequestApproval(eq(pr), Mockito.anyObject());
+  }
+
+  private List<Issue> report(String... severities) {
+    List<Issue> result = new ArrayList<>();
+    for (String s : severities) {
+      Issue issue = mock(Issue.class);
+      when(issue.severity()).thenReturn(s);
+      result.add(issue);
+    }
+    return result;
+  }
 }

src/test/java/org/sonar/plugins/stash/StashPluginConfigurationTest.java

@@ -31,6 +31,7 @@ public void testStashPluginConfiguration_ConstructorAndAccessors() {
     settings.setProperty(StashPlugin.STASH_ISSUE_THRESHOLD, 42000);
     settings.setProperty(StashPlugin.STASH_TIMEOUT, 42);
     settings.setProperty(StashPlugin.STASH_REVIEWER_APPROVAL, true);
+    settings.setProperty(StashPlugin.STASH_REVIEWER_REJECT_SEVERITY_THRESHOLD, "Info");
     settings.setProperty(StashPlugin.STASH_RESET_COMMENTS, false);
     settings.setProperty(StashPlugin.STASH_TASK_SEVERITY_THRESHOLD, "Minor");
     settings.setProperty(CoreProperties.SERVER_VERSION, "5.6.3");
@@ -57,6 +58,7 @@ public void testStashPluginConfiguration_ConstructorAndAccessors() {
     assertEquals(42000, SPC.getIssueThreshold());
     assertEquals(42, SPC.getStashTimeout());
     assertEquals(true, SPC.canApprovePullRequest());
+    assertEquals("Info", SPC.getRejectSeverityThreshold());
     assertEquals(false, SPC.resetComments());
     assertEquals("Minor", SPC.getTaskIssueSeverityThreshold());
     assertEquals("5.6.3", SPC.getSonarQubeVersion());

src/test/java/org/sonar/plugins/stash/StashPluginTest.java

@@ -35,13 +35,16 @@ public void testGetExtensions() {
     PropertyDefinition SP_SRA = (PropertyDefinition)ext.get(14);
     assertEquals(StashPlugin.STASH_REVIEWER_APPROVAL, SP_SRA.key());
 
-    PropertyDefinition SP_SIT = (PropertyDefinition)ext.get(15);
+    PropertyDefinition SP_SAST = (PropertyDefinition)ext.get(15);
+    assertEquals(StashPlugin.STASH_REVIEWER_REJECT_SEVERITY_THRESHOLD, SP_SAST.key());
+
+    PropertyDefinition SP_SIT = (PropertyDefinition)ext.get(16);
     assertEquals(StashPlugin.STASH_ISSUE_THRESHOLD, SP_SIT.key());
 
-    PropertyDefinition SP_STST = (PropertyDefinition)ext.get(16);
+    PropertyDefinition SP_STST = (PropertyDefinition)ext.get(17);
     assertEquals(StashPlugin.STASH_TASK_SEVERITY_THRESHOLD, SP_STST.key());
 
-    PropertyDefinition SP_SIAO = (PropertyDefinition)ext.get(17);
+    PropertyDefinition SP_SIAO = (PropertyDefinition)ext.get(18);
     assertEquals(StashPlugin.STASH_INCLUDE_ANALYSIS_OVERVIEW, SP_SIAO.key());
   }
 }