SONAR-4606 Allow to skip notifications during issue bulk change

9 files changed, 82 insertions, 40 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
index de33721f1bb..d2703e1a078 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
@@ -83,10 +83,10 @@ public class InternalRubyIssueService implements ServerComponent {
   private final IssueBulkChangeService issueBulkChangeService;
 
   public InternalRubyIssueService(IssueService issueService,
-                                  IssueCommentService commentService,
-                                  IssueChangelogService changelogService, ActionPlanService actionPlanService,
-                                  IssueStatsFinder issueStatsFinder, ResourceDao resourceDao, ActionService actionService,
-                                  IssueFilterService issueFilterService, IssueBulkChangeService issueBulkChangeService) {
+    IssueCommentService commentService,
+    IssueChangelogService changelogService, ActionPlanService actionPlanService,
+    IssueStatsFinder issueStatsFinder, ResourceDao resourceDao, ActionService actionService,
+    IssueFilterService issueFilterService, IssueBulkChangeService issueBulkChangeService) {
     this.issueService = issueService;
     this.commentService = commentService;
     this.changelogService = changelogService;
@@ -396,7 +396,7 @@ public class InternalRubyIssueService implements ServerComponent {
   }
 
   public IssueQuery emptyIssueQuery() {
-    return PublicRubyIssueService.toQuery(Maps.<String, Object>newHashMap());
+    return PublicRubyIssueService.toQuery(Maps.<String, Object> newHashMap());
   }
 
   @CheckForNull
@@ -578,8 +578,8 @@ public class InternalRubyIssueService implements ServerComponent {
   /**
    * Execute a bulk change
    */
-  public IssueBulkChangeResult bulkChange(Map<String, Object> props, String comment) {
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(props, comment);
+  public IssueBulkChangeResult bulkChange(Map<String, Object> props, String comment, boolean sendNotifications) {
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(props, comment, sendNotifications);
     return issueBulkChangeService.execute(issueBulkChangeQuery, UserSession.get());
   }
 
diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
index b1e5133d90a..c7d31cecd42 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
@@ -47,15 +47,18 @@ public class IssueBulkChangeQuery {
   private List<String> issues;
   private List<String> actions;
   private boolean hasComment;
+  private boolean sendNotifications;
 
   Map<String, Map<String, Object>> propertiesByActions = new HashMap<String, Map<String, Object>>();
 
-  public IssueBulkChangeQuery(Map<String, Object> props, String comment) {
+  public IssueBulkChangeQuery(Map<String, Object> props, String comment, boolean sendNotifications) {
+    this.sendNotifications = sendNotifications;
     parse(props, comment);
   }
 
   @VisibleForTesting
-  IssueBulkChangeQuery(Map<String, Object> props) {
+  IssueBulkChangeQuery(Map<String, Object> props, boolean sendNotifications) {
+    this.sendNotifications = sendNotifications;
     parse(props, null);
   }
 
@@ -80,8 +83,8 @@ public class IssueBulkChangeQuery {
     }
   }
 
-  private List<String> sanitizeList(List<String> list){
-    if (list == null || list.isEmpty()){
+  private List<String> sanitizeList(List<String> list) {
+    if (list == null || list.isEmpty()) {
       return Collections.emptyList();
     }
     return newArrayList(Iterables.filter(list, new Predicate<String>() {
@@ -104,10 +107,14 @@ public class IssueBulkChangeQuery {
     return actions;
   }
 
-  public boolean hasComment(){
+  public boolean hasComment() {
     return hasComment;
   }
 
+  public boolean sendNotifications() {
+    return sendNotifications;
+  }
+
   public Map<String, Object> properties(String action) {
     return propertiesByActions.get(action);
   }
diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java
index 94cbc26300d..d978d5278c2 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java
@@ -84,7 +84,9 @@ public class IssueBulkChangeService {
           applyAction(getAction(CommentAction.KEY), actionContext, issueBulkChangeQuery, result);
         }
         issueStorage.save((DefaultIssue) issue);
-        issueNotifications.sendChanges((DefaultIssue) issue, issueChangeContext, issueQueryResult);
+        if (issueBulkChangeQuery.sendNotifications()) {
+          issueNotifications.sendChanges((DefaultIssue) issue, issueChangeContext, issueQueryResult);
+        }
         concernedProjects.add(((DefaultIssue) issue).projectKey());
       }
     }
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb
index 4bad8bd681c..e36fcd7d231 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb
@@ -273,7 +273,8 @@ class Api::IssuesController < Api::ApiController
     verify_post_request
 
     comment = Api::Utils.read_post_request_param(params[:comment])
-    result = Internal.issues.bulkChange(params, comment)
+    sendNotifications = params[:sendNotifications] || 'true'
+    result = Internal.issues.bulkChange(params, comment, sendNotifications == 'true')
     hash = {}
     hash[:issuesChanged] = {
         :total => result.issuesChanged().size,
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb
index 7396185a46b..2da67f5e802 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb
@@ -174,7 +174,7 @@ class IssuesController < ApplicationController
   # POST /issues/bulk_change?criteria
   def bulk_change
     verify_post_request
-    Internal.issues.bulkChange(params, params[:comment])
+    Internal.issues.bulkChange(params, params[:comment], params[:sendNotifications] == 'true')
     render :text => '', :status => 200
   end
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb
index 6571bf8d22e..bdebf0a3ea7 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_bulk_change_form.html.erb
@@ -102,6 +102,13 @@
           </div>
         </div>
       </div>
+
+      <div class="modal-field">
+        <label for="send-notifications">
+          <%= message('issue.send_notifications') -%>
+        </label>
+        <input id="send-notifications" name="sendNotifications" type="checkbox" value="true" checked="checked" />
+      </div>
     </div>
     <div class="modal-foot">
       <span id="bulk-change-loading-image" class="loading-image hidden"><%= image_tag 'loading.gif' %></span>
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
index eb3fc3c561d..09f0dc91543 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
@@ -43,8 +43,13 @@ import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class InternalRubyIssueServiceTest {
 
@@ -286,7 +291,7 @@ public class InternalRubyIssueServiceTest {
 
   @Test
   public void test_changelog() throws Exception {
-    IssueChangelog changelog = new IssueChangelog(Collections.<FieldDiffs>emptyList(), Collections.<User>emptyList());
+    IssueChangelog changelog = new IssueChangelog(Collections.<FieldDiffs> emptyList(), Collections.<User> emptyList());
     when(changelogService.changelog(eq("ABCDE"), any(UserSession.class))).thenReturn(changelog);
 
     IssueChangelog result = service.changelog("ABCDE");
@@ -451,7 +456,7 @@ public class InternalRubyIssueServiceTest {
 
   @Test
   public void should_execute_issue_filter_from_issue_query() {
-    service.execute(Maps.<String, Object>newHashMap());
+    service.execute(Maps.<String, Object> newHashMap());
     verify(issueFilterService).execute(any(IssueQuery.class));
   }
 
@@ -538,7 +543,7 @@ public class InternalRubyIssueServiceTest {
   }
 
   @Test
-  public void should_check_if_user_can_share_issue_filter(){
+  public void should_check_if_user_can_share_issue_filter() {
     service.canUserShareIssueFilter();
     verify(issueFilterService).canShareFilter(any(UserSession.class));
   }
@@ -552,7 +557,7 @@ public class InternalRubyIssueServiceTest {
     params.put("assign.assignee", "arthur");
     params.put("set_severity.severity", "MINOR");
     params.put("plan.plan", "3.7");
-    service.bulkChange(params, "My comment");
+    service.bulkChange(params, "My comment", true);
     verify(issueBulkChangeService).execute(any(IssueBulkChangeQuery.class), any(UserSession.class));
   }
 
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
index e58a3fedb96..707ad0f8f36 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
@@ -43,7 +43,7 @@ public class IssueBulkChangeQueryTest {
     params.put("set_severity.severity", "MINOR");
     params.put("plan.plan", "3.7");
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, true);
     assertThat(issueBulkChangeQuery.actions()).containsOnly("do_transition", "assign", "set_severity", "plan");
     assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH");
   }
@@ -55,7 +55,7 @@ public class IssueBulkChangeQueryTest {
     params.put("actions", newArrayList("do_transition", "", null));
     params.put("do_transition.transition", "confirm");
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, true);
     assertThat(issueBulkChangeQuery.actions()).containsOnly("do_transition");
     assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH");
   }
@@ -67,7 +67,7 @@ public class IssueBulkChangeQueryTest {
     params.put("actions", newArrayList("do_transition"));
     params.put("do_transition.transition", "confirm");
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, true);
     assertThat(issueBulkChangeQuery.actions()).containsOnly("do_transition");
     assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH");
   }
@@ -79,7 +79,7 @@ public class IssueBulkChangeQueryTest {
     params.put("actions", newArrayList("assign"));
     params.put("assign.assignee", "arthur");
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, "My comment for bulk change");
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, "My comment for bulk change", true);
     assertThat(issueBulkChangeQuery.hasComment()).isTrue();
     assertThat(issueBulkChangeQuery.actions()).containsOnly("assign");
     assertThat(issueBulkChangeQuery.properties("comment").get("comment")).isEqualTo("My comment for bulk change");
@@ -92,7 +92,7 @@ public class IssueBulkChangeQueryTest {
     params.put("actions", newArrayList("assign"));
     params.put("assign.assignee", "arthur");
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, true);
     assertThat(issueBulkChangeQuery.properties("assign")).hasSize(1);
     assertThat(issueBulkChangeQuery.properties("assign").get("assignee")).isEqualTo("arthur");
   }
@@ -103,7 +103,7 @@ public class IssueBulkChangeQueryTest {
     params.put("actions", newArrayList("assign"));
     params.put("assign.assignee", "arthur");
     try {
-      new IssueBulkChangeQuery(params);
+      new IssueBulkChangeQuery(params, true);
       fail();
     } catch (Exception e) {
       assertThat(e).isInstanceOf(BadRequestException.class);
@@ -118,7 +118,7 @@ public class IssueBulkChangeQueryTest {
     params.put("actions", newArrayList("assign"));
     params.put("assign.assignee", "arthur");
     try {
-      new IssueBulkChangeQuery(params);
+      new IssueBulkChangeQuery(params, true);
       fail();
     } catch (Exception e) {
       assertThat(e).isInstanceOf(BadRequestException.class);
@@ -131,7 +131,7 @@ public class IssueBulkChangeQueryTest {
     Map<String, Object> params = newHashMap();
     params.put("issues", newArrayList("ABCD", "EFGH"));
     try {
-      new IssueBulkChangeQuery(params);
+      new IssueBulkChangeQuery(params, true);
       fail();
     } catch (Exception e) {
       assertThat(e).isInstanceOf(BadRequestException.class);
@@ -145,7 +145,7 @@ public class IssueBulkChangeQueryTest {
     params.put("issues", newArrayList("ABCD", "EFGH"));
     params.put("actions", Collections.emptyList());
     try {
-      new IssueBulkChangeQuery(params);
+      new IssueBulkChangeQuery(params, true);
       fail();
     } catch (Exception e) {
       assertThat(e).isInstanceOf(BadRequestException.class);
diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
index 331a623bc66..9461440a6ff 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
@@ -51,6 +51,7 @@ import static org.mockito.Matchers.anyMap;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -89,7 +90,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("assign.assignee", "fred");
     actions.add(new MockAction("assign"));
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).hasSize(1);
     assertThat(result.issuesNotChanged()).isEmpty();
@@ -101,6 +102,25 @@ public class IssueBulkChangeServiceTest {
   }
 
   @Test
+  public void should_skip_send_notifications() {
+    Map<String, Object> properties = newHashMap();
+    properties.put("issues", "ABCD");
+    properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
+    actions.add(new MockAction("assign"));
+
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, false);
+    IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
+    assertThat(result.issuesChanged()).hasSize(1);
+    assertThat(result.issuesNotChanged()).isEmpty();
+
+    verify(issueStorage).save(eq(issue));
+    verifyNoMoreInteractions(issueStorage);
+    verify(issueNotifications, never()).sendChanges(eq(issue), any(IssueChangeContext.class), eq(issueQueryResult));
+    verifyNoMoreInteractions(issueNotifications);
+  }
+
+  @Test
   public void should_execute_bulk_change_with_comment() {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
@@ -115,7 +135,7 @@ public class IssueBulkChangeServiceTest {
     actions.add(commentAction);
     actions.add(new MockAction("assign"));
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment");
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment", true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).hasSize(1);
     assertThat(result.issuesNotChanged()).isEmpty();
@@ -148,7 +168,7 @@ public class IssueBulkChangeServiceTest {
     when(assignAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true).thenReturn(false);
     actions.add(assignAction);
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment");
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment", true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).hasSize(1);
     assertThat(result.issuesNotChanged()).hasSize(1);
@@ -169,7 +189,7 @@ public class IssueBulkChangeServiceTest {
     actions.add(new MockAction("set_severity"));
     actions.add(new MockAction("assign"));
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).hasSize(1);
     assertThat(result.issuesNotChanged()).isEmpty();
@@ -188,7 +208,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("assign.assignee", "fred");
     actions.add(new MockAction("assign"));
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     service.execute(issueBulkChangeQuery, userSession);
 
     ArgumentCaptor<IssueQuery> captor = ArgumentCaptor.forClass(IssueQuery.class);
@@ -207,7 +227,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("assign.assignee", "fred");
     actions.add(new MockAction("assign", true, true, false));
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).isEmpty();
     assertThat(result.issuesNotChanged()).hasSize(1);
@@ -224,7 +244,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("assign.assignee", "fred");
     actions.add(new MockAction("assign", false, true, true));
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).isEmpty();
     assertThat(result.issuesNotChanged()).isEmpty();
@@ -241,7 +261,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("assign.assignee", "fred");
     actions.add(new MockAction("assign", true, false, true));
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).isEmpty();
     assertThat(result.issuesNotChanged()).hasSize(1);
@@ -264,7 +284,7 @@ public class IssueBulkChangeServiceTest {
     doThrow(new RuntimeException("Error")).when(action).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class));
     actions.add(action);
 
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
     assertThat(result.issuesChanged()).isEmpty();
     assertThat(result.issuesNotChanged()).hasSize(1);
@@ -281,7 +301,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
     properties.put("assign.assignee", "fred");
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     try {
       service.execute(issueBulkChangeQuery, userSession);
       fail();
@@ -298,7 +318,7 @@ public class IssueBulkChangeServiceTest {
     properties.put("issues", "ABCD");
     properties.put("actions", "unknown");
     properties.put("unknown.unknown", "unknown");
-    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, true);
     try {
       service.execute(issueBulkChangeQuery, userSession);
       fail();