Apply comment action only on modified issues

5 files changed, 83 insertions, 6 deletions

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 6f20133a3c3..b1e5133d90a 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
@@ -46,6 +46,7 @@ public class IssueBulkChangeQuery {
 
   private List<String> issues;
   private List<String> actions;
+  private boolean hasComment;
 
   Map<String, Map<String, Object>> propertiesByActions = new HashMap<String, Map<String, Object>>();
 
@@ -72,7 +73,7 @@ public class IssueBulkChangeQuery {
       propertiesByActions.put(action, actionProperties);
     }
     if (!Strings.isNullOrEmpty(comment)) {
-      actions.add(CommentAction.KEY);
+      hasComment = true;
       Map<String, Object> commentMap = newHashMap();
       commentMap.put(CommentAction.COMMENT_PROPERTY, comment);
       propertiesByActions.put(CommentAction.KEY, commentMap);
@@ -95,10 +96,18 @@ public class IssueBulkChangeQuery {
     return issues;
   }
 
+  /**
+   * The list of actions to apply
+   * Note that even if a comment has been added, this list will NOT contains the comment action
+   */
   public List<String> actions() {
     return actions;
   }
 
+  public boolean hasComment(){
+    return hasComment;
+  }
+
   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 7793ad6630d..b6a0cff6ba2 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
@@ -82,7 +82,7 @@ public class IssueBulkChangeService {
       ActionContext actionContext = new ActionContext(issue, issueChangeContext);
       for (Action action : bulkActions) {
         try {
-          if (action.supports(issue) && action.execute(issueBulkChangeQuery.properties(action.key()), actionContext)) {
+          if (applyAction(action, actionContext, issueBulkChangeQuery)) {
             result.addIssueChanged(issue);
           } else {
             result.addIssueNotChanged(issue);
@@ -93,6 +93,10 @@ public class IssueBulkChangeService {
         }
       }
       if (result.issuesChanged().contains(issue)) {
+        // Apply comment action only on changed issues
+        if (issueBulkChangeQuery.hasComment()) {
+          applyAction(getAction(CommentAction.KEY), actionContext, issueBulkChangeQuery);
+        }
         issueStorage.save((DefaultIssue) issue);
         issueNotifications.sendChanges((DefaultIssue) issue, issueChangeContext, issueQueryResult);
       }
@@ -101,6 +105,10 @@ public class IssueBulkChangeService {
     return result;
   }
 
+  private boolean applyAction(Action action, ActionContext actionContext, IssueBulkChangeQuery issueBulkChangeQuery){
+    return action.supports(actionContext.issue()) && action.execute(issueBulkChangeQuery.properties(action.key()), actionContext);
+  }
+
   @CheckForNull
   private Action getAction(final String actionKey) {
     return Iterables.find(actions, new Predicate<Action>() {
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 f6287c13517..8063e1d6464 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
@@ -90,7 +90,8 @@
 
       <div class="modal-field">
         <label>
-          <%= message('issue.comment.formlink') -%>
+         <%= message('issue.comment.formlink') -%>
+          <span style="cursor: help;"><%= image_tag 'help.png', :title => h(message('issue_bulk_change.comment.help')) -%></span>
         </label>
         <div style="padding: 0 10px 10px 0;">
           <div>
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 0176a342d86..e58a3fedb96 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
@@ -80,7 +80,8 @@ public class IssueBulkChangeQueryTest {
     params.put("assign.assignee", "arthur");
 
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, "My comment for bulk change");
-    assertThat(issueBulkChangeQuery.actions()).containsOnly("assign", "comment");
+    assertThat(issueBulkChangeQuery.hasComment()).isTrue();
+    assertThat(issueBulkChangeQuery.actions()).containsOnly("assign");
     assertThat(issueBulkChangeQuery.properties("comment").get("comment")).isEqualTo("My comment for bulk change");
   }
 
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 4ebb6c998c3..70281632224 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
@@ -92,7 +92,65 @@ public class IssueBulkChangeServiceTest {
   }
 
   @Test
-  public void should_do_sve_once_per_issue() {
+  public void should_execute_bulk_change_with_comment() {
+    Map<String, Object> properties = newHashMap();
+    properties.put("issues", "ABCD");
+    properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
+
+    Action commentAction = mock(Action.class);
+    when(commentAction.key()).thenReturn("comment");
+    when(commentAction.supports(any(Issue.class))).thenReturn(true);
+    when(commentAction.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true);
+    when(commentAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
+    actions.add(commentAction);
+    actions.add(new MockAction("assign"));
+
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment");
+    IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
+    assertThat(result.issuesChanged()).hasSize(1);
+    assertThat(result.issuesNotChanged()).isEmpty();
+
+    verify(commentAction).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class));
+    verify(issueStorage).save(eq(issue));
+  }
+
+  @Test
+  public void should_execute_bulk_change_with_comment_only_on_changed_issues() {
+    when(issueQueryResult.issues()).thenReturn(newArrayList((Issue) new DefaultIssue().setKey("ABCD"), new DefaultIssue().setKey("EFGH")));
+
+    Map<String, Object> properties = newHashMap();
+    properties.put("issues", "ABCD,EFGH");
+    properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
+
+    Action commentAction = mock(Action.class);
+    when(commentAction.key()).thenReturn("comment");
+    when(commentAction.supports(any(Issue.class))).thenReturn(true);
+    when(commentAction.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true);
+    when(commentAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
+    actions.add(commentAction);
+
+    // This action will only be executed on the first issue, not the second
+    Action assignAction = mock(Action.class);
+    when(assignAction.key()).thenReturn("assign");
+    when(assignAction.supports(any(Issue.class))).thenReturn(true).thenReturn(false);
+    when(assignAction.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true);
+    when(assignAction.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true).thenReturn(false);
+    actions.add(assignAction);
+
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties, "my comment");
+    IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);
+    assertThat(result.issuesChanged()).hasSize(1);
+    assertThat(result.issuesNotChanged()).hasSize(1);
+
+    // Only one issue will receive the comment
+    verify(assignAction, times(1)).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class));
+    verify(issueStorage).save(eq(issue));
+  }
+
+  @Test
+  public void should_save_once_per_issue() {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
     properties.put("actions", "assign,set_severity");
@@ -174,11 +232,11 @@ public class IssueBulkChangeServiceTest {
     properties.put("assign.assignee", "fred");
 
     Action action = mock(Action.class);
-    actions.add(action);
     when(action.key()).thenReturn("assign");
     when(action.supports(any(Issue.class))).thenReturn(true);
     when(action.verify(anyMap(), anyListOf(Issue.class), any(UserSession.class))).thenReturn(true);
     doThrow(new RuntimeException("Error")).when(action).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class));
+    actions.add(action);
 
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     IssueBulkChangeResult result = service.execute(issueBulkChangeQuery, userSession);