SONAR-7024 global report failure notif only on administrated projects

3 files changed, 40 insertions, 19 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java b/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java
index 2481d9937ae..ff3ad8346c2 100644
--- a/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java
+++ b/server/sonar-server/src/main/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcher.java
@@ -24,13 +24,16 @@ import java.util.Collection;
 import java.util.Map;
 import org.sonar.api.notifications.Notification;
 import org.sonar.api.notifications.NotificationChannel;
+import org.sonar.api.web.UserRole;
 import org.sonar.server.notification.NotificationDispatcher;
 import org.sonar.server.notification.NotificationDispatcherMetadata;
 import org.sonar.server.notification.NotificationManager;
+import org.sonar.server.notification.NotificationManager.SubscriberPermissionsOnProject;
 
 public class ReportAnalysisFailureNotificationDispatcher extends NotificationDispatcher {
 
   public static final String KEY = "CeReportTaskFailure";
+  private static final SubscriberPermissionsOnProject REQUIRED_SUBSCRIBER_PERMISSIONS = new SubscriberPermissionsOnProject(UserRole.ADMIN, UserRole.USER);
 
   private final NotificationManager manager;
 
@@ -54,7 +57,7 @@ public class ReportAnalysisFailureNotificationDispatcher extends NotificationDis
   public void dispatch(Notification notification, Context context) {
     String projectUuid = notification.getFieldValue("project.uuid");
     Multimap<String, NotificationChannel> subscribedRecipients = manager.findSubscribedRecipientsForDispatcher(
-      this, projectUuid, NotificationManager.SubscriberPermissionsOnProject.ALL_MUST_HAVE_ROLE_USER);
+      this, projectUuid, REQUIRED_SUBSCRIBER_PERMISSIONS);
 
     for (Map.Entry<String, Collection<NotificationChannel>> channelsByRecipients : subscribedRecipients.asMap().entrySet()) {
       String userLogin = channelsByRecipients.getKey();
diff --git a/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java b/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java
index 92c233a8fc8..82cad44378c 100644
--- a/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/ce/notification/ReportAnalysisFailureNotificationDispatcherTest.java
@@ -27,6 +27,7 @@ import org.sonar.api.web.UserRole;
 import org.sonar.server.notification.NotificationDispatcher;
 import org.sonar.server.notification.NotificationDispatcherMetadata;
 import org.sonar.server.notification.NotificationManager;
+import org.sonar.server.notification.NotificationManager.SubscriberPermissionsOnProject;
 
 import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -89,7 +90,7 @@ public class ReportAnalysisFailureNotificationDispatcherTest {
     multimap.put(login1, channel2);
     multimap.put(login2, channel2);
     multimap.put(login2, channel3);
-    when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new NotificationManager.SubscriberPermissionsOnProject(UserRole.USER)))
+    when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new SubscriberPermissionsOnProject(UserRole.ADMIN, UserRole.USER)))
       .thenReturn(multimap);
 
     underTest.performDispatch(notificationMock, contextMock);
@@ -107,7 +108,7 @@ public class ReportAnalysisFailureNotificationDispatcherTest {
     String projectUuid = randomAlphanumeric(9);
     when(notificationMock.getFieldValue("project.uuid")).thenReturn(projectUuid);
     HashMultimap<String, NotificationChannel> multimap = HashMultimap.create();
-    when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new NotificationManager.SubscriberPermissionsOnProject(UserRole.USER)))
+    when(notificationManager.findSubscribedRecipientsForDispatcher(underTest, projectUuid, new SubscriberPermissionsOnProject(UserRole.ADMIN, UserRole.USER)))
       .thenReturn(multimap);
 
     underTest.performDispatch(notificationMock, contextMock);
diff --git a/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java b/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java
index 877b1334b0a..46727f2c375 100644
--- a/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java
+++ b/tests/src/test/java/org/sonarqube/tests/ce/ReportFailureNotificationTest.java
@@ -43,10 +43,12 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.sonarqube.tests.Category6Suite;
 import org.sonarqube.tests.Tester;
+import org.sonarqube.ws.Organizations;
 import org.sonarqube.ws.WsProjects;
 import org.sonarqube.ws.WsUsers;
 import org.sonarqube.ws.client.PostRequest;
 import org.sonarqube.ws.client.WsClient;
+import org.sonarqube.ws.client.permission.AddUserWsRequest;
 import org.subethamail.wiser.Wiser;
 import org.subethamail.wiser.WiserMessage;
 
@@ -85,7 +87,10 @@ public class ReportFailureNotificationTest {
       "email.smtp_port.secured", valueOf(smtpServer.getServer().getPort()));
 
     smtpServer.getMessages().clear();
+  }
 
+  @After
+  public void tearDown() throws Exception {
     tester.elasticsearch().unlockWrites("components");
   }
 
@@ -96,12 +101,24 @@ public class ReportFailureNotificationTest {
 
   @Test
   public void send_notification_on_report_processing_failures_to_global_and_project_subscribers() throws Exception {
-    WsUsers.CreateWsResponse.User user1 = tester.users().generate(t -> t.setPassword("user1").setEmail("user1@bar.com"));
-    WsUsers.CreateWsResponse.User user2 = tester.users().generate(t -> t.setPassword("user2").setEmail("user2@bar.com"));
-    WsUsers.CreateWsResponse.User user3 = tester.users().generate(t -> t.setPassword("user3").setEmail("user3@bar.com"));
-    WsProjects.CreateWsResponse.Project project1 = tester.projects().generate(null, t -> t.setName("Project1"));
-    WsProjects.CreateWsResponse.Project project2 = tester.projects().generate(null, t -> t.setName("Project2"));
-    WsProjects.CreateWsResponse.Project project3 = tester.projects().generate(null, t -> t.setName("Project3"));
+    Organizations.Organization organization = tester.organizations().getDefaultOrganization();
+    WsUsers.CreateWsResponse.User user1 = tester.users().generateMember(organization, t -> t.setPassword("user1").setEmail("user1@bar.com"));
+    WsUsers.CreateWsResponse.User user2 = tester.users().generateMember(organization, t -> t.setPassword("user2").setEmail("user2@bar.com"));
+    WsUsers.CreateWsResponse.User user3 = tester.users().generateMember(organization, t -> t.setPassword("user3").setEmail("user3@bar.com"));
+    WsProjects.CreateWsResponse.Project project1 = tester.projects().generate(organization, t -> t.setName("Project1"));
+    WsProjects.CreateWsResponse.Project project2 = tester.projects().generate(organization, t -> t.setName("Project2"));
+    WsProjects.CreateWsResponse.Project project3 = tester.projects().generate(organization, t -> t.setName("Project3"));
+    // user 1 is admin of project 1 and will subscribe to global notifications
+    tester.wsClient().permissions().addUser(new AddUserWsRequest()
+      .setLogin(user1.getLogin())
+      .setPermission("admin")
+      .setProjectKey(project1.getKey()));
+    // user 2 is admin of project 2 but won't subscribe to global notifications
+    tester.wsClient().permissions().addUser(new AddUserWsRequest()
+      .setLogin(user2.getLogin())
+      .setPermission("admin")
+      .setProjectKey(project2.getKey()));
+    // user 3 is no admin of any project and will subscribe to global notifications
 
     // analyses successful and no-one subscribed => no email
     executeAnalysis(project1);
@@ -116,29 +133,28 @@ public class ReportFailureNotificationTest {
     executeAnalysis(project3);
     assertThat(waitForEmails()).isEmpty();
 
-    // Add notifications to users: user1 globally, user2 for project1, user3 for project2
+    // Add notifications to users: user1 and user3 globally, user2 for project1, user3 for project2
     subscribeToReportFailures(user1, "user1", null);
     subscribeToReportFailures(user2, "user2", project1);
     subscribeToReportFailures(user3, "user3", project2);
+    subscribeToReportFailures(user3, "user3", null);
 
-    // analysis failed and 1 global subscriber + 1 project subscriber => 2 emails
+    // analysis failed and 1 global subscriber with admin permission + 1 project subscriber => 2 emails
     executeAnalysis(project1);
     List<MimeMessage> messages = waitForEmails();
     assertThat(messages.stream().flatMap(toToRecipients()).collect(Collectors.toSet()))
       .containsOnly("user1@bar.com", "user2@bar.com");
     assertSubjectAndContent(project1, messages);
-    // analysis failed and 1 global subscriber + 1 project subscriber => 2 emails
+    // analysis failed and no global subscriber with admin permission + 1 project subscriber => 1 emails
     executeAnalysis(project2);
     messages = waitForEmails();
     assertThat(messages.stream().flatMap(toToRecipients()).collect(Collectors.toSet()))
-      .containsOnly("user1@bar.com", "user3@bar.com");
+      .containsOnly("user3@bar.com");
     assertSubjectAndContent(project2, messages);
-    // analysis fails and 1 global subscriber => 1 email
+    // analysis fails and no global subscriber with admin permission => 0 email
     executeAnalysis(project3);
     messages = waitForEmails();
-    assertThat(messages.stream().flatMap(toToRecipients()).collect(Collectors.toSet()))
-      .containsOnly("user1@bar.com");
-    assertSubjectAndContent(project3, messages);
+    assertThat(messages).isEmpty();
 
     // global and project subscribed but analysis is successful => no email
     tester.elasticsearch().unlockWrites("components");
@@ -147,12 +163,13 @@ public class ReportFailureNotificationTest {
     executeAnalysis(project3);
     assertThat(waitForEmails()).isEmpty();
 
-    // Add notifications to users: user1 globally, user2 for project1, user3 for project2
+    // remove notifications from users: user1 globally, user2 for project1, user3 for project2
+    //  => no email anyway
     unsubscribeFromReportFailures(user1, "user1", null);
     unsubscribeFromReportFailures(user2, "user2", project1);
     unsubscribeFromReportFailures(user3, "user3", project2);
 
-    // analyses fail and no-one subscribed => no email
+    // analyses fail and no subscriber but user3 globally but is not admin of any project => no email
     tester.elasticsearch().lockWrites("components");
     executeAnalysis(project1);
     executeAnalysis(project2);