SONAR-3714 Add the comment in a separated parameter

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/CommentAction.java b/sonar-server/src/main/java/org/sonar/server/issue/CommentAction.java
index da4c244d1cb3580dbddfb4facc7e90d9492ff302..72d16aa966235ef4b1a34ba064b3b1c30e4c33ac 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/CommentAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/CommentAction.java
@@ -22,7 +22,6 @@ package org.sonar.server.issue;
 
 import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.Issue;
-import org.sonar.api.issue.condition.IsUnResolved;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.server.user.UserSession;
 
@@ -36,7 +35,6 @@ public class CommentAction extends Action implements ServerComponent {
 
   public CommentAction() {
     super(COMMENT_ACTION_KEY);
-    super.setConditions(new IsUnResolved());
   }
 
   @Override

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 f25fbd45238b2c4d5ea6c046e3e9321ce80250e1..858c061f1cc401a4a2c50ac9736d3e2bb4bd7eb5 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
@@ -289,7 +289,7 @@ public class InternalRubyIssueService implements ServerComponent {
     String projectParam = parameters.get("project");
 
     checkMandatorySizeParameter(name, "name", 200, result);
-    checkOptionnalSizeParameter(description, "description", 1000, result);
+    checkOptionalSizeParameter(description, "description", 1000, result);
 
     // Can only set project on creation
     if (existingActionPlan == null) {
@@ -532,7 +532,7 @@ public class InternalRubyIssueService implements ServerComponent {
       checkMandatoryParameter(id, "id", result);
     }
     checkMandatorySizeParameter(name, "name", 100, result);
-    checkOptionnalSizeParameter(description, "description", 4000, result);
+    checkOptionalSizeParameter(description, "description", 4000, result);
 
     if (result.ok()) {
       DefaultIssueFilter defaultIssueFilter = DefaultIssueFilter.create(name)
@@ -566,6 +566,20 @@ public class InternalRubyIssueService implements ServerComponent {
     return result;
   }
 
+  /**
+   * Execute a bulk change
+   */
+  public Result<IssueBulkChangeResult> bulkChange(Map<String, Object> props, String comment) {
+    Result<IssueBulkChangeResult> result = Result.of();
+    try {
+      IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(props, comment);
+      result.set(issueBulkChangeService.execute(issueBulkChangeQuery, UserSession.get()));
+    } catch (Exception e) {
+      result.addError(e.getMessage());
+    }
+    return result;
+  }
+
   private void checkMandatoryParameter(String value, String paramName, Result result) {
     if (Strings.isNullOrEmpty(value)) {
       result.addError(Result.Message.ofL10n("errors.cant_be_empty", paramName));
@@ -579,24 +593,10 @@ public class InternalRubyIssueService implements ServerComponent {
     }
   }
 
-  private void checkOptionnalSizeParameter(String value, String paramName, Integer size, Result result) {
+  private void checkOptionalSizeParameter(String value, String paramName, Integer size, Result result) {
     if (!Strings.isNullOrEmpty(value) && value.length() > size) {
       result.addError(Result.Message.ofL10n("errors.is_too_long", paramName, size));
     }
   }
 
-  /**
-   * Execute a bulk change
-   */
-  public Result<IssueBulkChangeResult> executeBulkChange(Map<String, Object> props) {
-    Result<IssueBulkChangeResult> result = Result.of();
-    try {
-      IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(props);
-      result.set(issueBulkChangeService.execute(issueBulkChangeQuery, UserSession.get()));
-    } catch (Exception e) {
-      result.addError(e.getMessage());
-    }
-    return result;
-  }
-
 }
\ No newline at end of file

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 42324b1315dc6576ce4fbc1b8b177fe285ea5999..3b50f532007b72b1d2cebc641cc1761069c9efc2 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
@@ -20,6 +20,8 @@
 
 package org.sonar.server.issue;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.server.util.RubyUtils;
 
@@ -40,14 +42,15 @@ public class IssueBulkChangeQuery {
 
   Map<String, Map<String, Object>> propertiesByActions = new HashMap<String, Map<String, Object>>();
 
-  public IssueBulkChangeQuery(Map<String, Object> props) {
-    parse(props, null);
-  }
-
   public IssueBulkChangeQuery(Map<String, Object> props, String comment) {
     parse(props, comment);
   }
 
+  @VisibleForTesting
+  IssueBulkChangeQuery(Map<String, Object> props) {
+    parse(props, null);
+  }
+
   private void parse(Map<String, Object> props, String comment) {
     this.comment = comment;
     this.issues = RubyUtils.toStrings(props.get("issues"));
@@ -59,7 +62,17 @@ public class IssueBulkChangeQuery {
       throw new IllegalArgumentException("At least one action must be provided");
     }
     for (String action : actions) {
-      propertiesByActions.put(action, getActionProps(action, props));
+      Map<String, Object> actionProperties = getActionProps(action, props);
+      if (actionProperties.isEmpty()) {
+        throw new IllegalArgumentException("Missing properties for action: "+ action);
+      }
+      propertiesByActions.put(action, actionProperties);
+    }
+    if (!Strings.isNullOrEmpty(comment)) {
+      actions.add(CommentAction.COMMENT_ACTION_KEY);
+      Map<String, Object> commentMap = newHashMap();
+      commentMap.put(CommentAction.COMMENT_ACTION_KEY, comment);
+      propertiesByActions.put(CommentAction.COMMENT_ACTION_KEY, commentMap);
     }
   }
 
@@ -75,10 +88,6 @@ public class IssueBulkChangeQuery {
     return propertiesByActions.get(action);
   }
 
-  public String getComment() {
-    return comment;
-  }
-
   private static Map<String, Object> getActionProps(String action, Map<String, Object> props) {
     Map<String, Object> actionProps = newHashMap();
     for (Map.Entry<String, Object> propsEntry : props.entrySet()) {

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java b/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java
deleted file mode 100644 (file)
index 0379011..0000000
--- a/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * SonarQube, open source software quality management tool.
- * Copyright (C) 2008-2013 SonarSource
- * mailto:contact AT sonarsource DOT com
- *
- * SonarQube is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or (at your option) any later version.
- *
- * SonarQube is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
- */
-
-package org.sonar.server.issue;
-
-import org.sonar.api.ServerComponent;
-import org.sonar.api.issue.Issue;
-import org.sonar.api.issue.condition.IsUnResolved;
-import org.sonar.api.issue.internal.DefaultIssue;
-import org.sonar.server.user.UserSession;
-
-import java.util.List;
-import java.util.Map;
-
-
-public class TransitionAction extends Action implements ServerComponent {
-
-  public static final String TRANSITION_ACTION_KEY = "transition";
-
-  public TransitionAction() {
-    super(TRANSITION_ACTION_KEY);
-    super.setConditions(new IsUnResolved());
-  }
-
-  @Override
-  public boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
-    return true;
-  }
-
-  @Override
-  public boolean execute(Map<String, Object> properties, Context context) {
-    context.issueUpdater().addComment((DefaultIssue) context.issue(), transition(properties), context.issueChangeContext());
-    return true;
-  }
-
-  private String transition(Map<String, Object> properties) {
-    return (String) properties.get("transition");
-  }
-}
\ No newline at end of file

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java
index 98c3d68e476ff90be8f5435dd90160256f443612..0dd3866e508c513e49d567c2482cc329a083a61f 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java
@@ -96,7 +96,7 @@ public class AssignActionTest {
 
   @Test
   public void should_support_only_unresolved_issues(){
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_OPEN))).isTrue();
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_CLOSED))).isTrue();
+    assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();
+    assertThat(action.supports(new DefaultIssue().setResolution(Issue.RESOLUTION_FIXED))).isFalse();
   }
 }
\ No newline at end of file

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/CommentActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/CommentActionTest.java
index 247fe8825882a3d7555bbbd2670fc865464851a8..120a1c2b6493b12756b43c02dd7ccefc843f9580 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/CommentActionTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/CommentActionTest.java
@@ -61,9 +61,9 @@ public class CommentActionTest {
   }
 
   @Test
-  public void should_support_only_unresolved_issues(){
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_OPEN))).isTrue();
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_CLOSED))).isTrue();
+  public void should_support_all_issues(){
+    assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();
+    assertThat(action.supports(new DefaultIssue().setResolution(Issue.RESOLUTION_FIXED))).isTrue();
   }
 
 }

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 badffa93a4ede8310eb30d7be202b629241ed880..a8e9e8ea8b3ce0cc724ec9ca47f67913a41d6865 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
@@ -542,7 +542,9 @@ public class InternalRubyIssueServiceTest {
     params.put("assign.assignee", "arthur");
     params.put("set_severity.severity", "MINOR");
     params.put("plan.plan", "3.7");
-    service.executeBulkChange(params);
+    Result<IssueBulkChangeResult> result = service.bulkChange(params, "My comment");
+    assertThat(result.errors()).isEmpty();
+    assertThat(result.ok()).isTrue();
     verify(issueBulkChangeService).execute(any(IssueBulkChangeQuery.class), any(UserSession.class));
   }
 
@@ -554,7 +556,7 @@ public class InternalRubyIssueServiceTest {
     params.put("issues", newArrayList("ABCD", "EFGH"));
     params.put("actions", newArrayList("assign"));
     params.put("assign.assignee", "arthur");
-    Result result = service.executeBulkChange(params);
+    Result result = service.bulkChange(params, "A comment");
     assertThat(result.ok()).isFalse();
     assertThat(((Result.Message) result.errors().get(0)).text()).contains("Error");
   }

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 3c9a94bb49ff89ea2d303534d4ba52995839ae52..15745c525f63897e20e25b6d41df2adbdf17de28 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeQueryTest.java
@@ -46,6 +46,18 @@ public class IssueBulkChangeQueryTest {
     assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH");
   }
 
+  @Test
+  public void should_create_query_with_comment(){
+    Map<String, Object> params = newHashMap();
+    params.put("issues", newArrayList("ABCD", "EFGH"));
+    params.put("actions", newArrayList("assign"));
+    params.put("assign.assignee", "arthur");
+
+    IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params, "My comment for bulk change");
+    assertThat(issueBulkChangeQuery.actions()).containsOnly("assign", "comment");
+    assertThat(issueBulkChangeQuery.properties("comment").get("comment")).isEqualTo("My comment for bulk change");
+  }
+
   @Test
   public void should_get_properties_action(){
     Map<String, Object> params = newHashMap();
@@ -61,7 +73,8 @@ public class IssueBulkChangeQueryTest {
   @Test(expected = IllegalArgumentException.class)
   public void fail_to_build_if_no_issue(){
     Map<String, Object> params = newHashMap();
-    params.put("actions", newArrayList("do_transition", "assign", "set_severity", "plan"));
+    params.put("actions", newArrayList("assign"));
+    params.put("assign.assignee", "arthur");
     new IssueBulkChangeQuery(params);
   }
 
@@ -69,7 +82,8 @@ public class IssueBulkChangeQueryTest {
   public void fail_to_build_if_issues_are_empty(){
     Map<String, Object> params = newHashMap();
     params.put("issues", Collections.emptyList());
-    params.put("actions", newArrayList("do_transition", "assign", "set_severity", "plan"));
+    params.put("actions", newArrayList("assign"));
+    params.put("assign.assignee", "arthur");
     new IssueBulkChangeQuery(params);
   }
 
@@ -88,4 +102,12 @@ public class IssueBulkChangeQueryTest {
     new IssueBulkChangeQuery(params);
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void fail_to_build_if_action_properties_are_empty(){
+    Map<String, Object> params = newHashMap();
+    params.put("issues", Collections.emptyList());
+    params.put("actions", newArrayList("assign"));
+    new IssueBulkChangeQuery(params);
+  }
+
 }

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 7d6041daacf544db4f13ab314bd598fc39d69068..b2a9551d30db22dd804252918a9b9e857e39d7e3 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueBulkChangeServiceTest.java
@@ -81,6 +81,7 @@ public class IssueBulkChangeServiceTest {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
 
     when(action.supports(any(Issue.class))).thenReturn(true);
     when(action.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(true);
@@ -103,6 +104,7 @@ public class IssueBulkChangeServiceTest {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
 
     when(action.supports(any(Issue.class))).thenReturn(false);
 
@@ -121,6 +123,7 @@ public class IssueBulkChangeServiceTest {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
 
     when(action.supports(any(Issue.class))).thenReturn(true);
     when(action.execute(anyMap(), any(IssueBulkChangeService.ActionContext.class))).thenReturn(false);
@@ -140,6 +143,7 @@ public class IssueBulkChangeServiceTest {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
 
     when(action.supports(any(Issue.class))).thenReturn(true);
     doThrow(new RuntimeException("Error")).when(action).execute(anyMap(), any(IssueBulkChangeService.ActionContext.class));
@@ -161,6 +165,7 @@ public class IssueBulkChangeServiceTest {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
     properties.put("actions", "assign");
+    properties.put("assign.assignee", "fred");
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     try {
       service.execute(issueBulkChangeQuery, userSession);
@@ -178,6 +183,7 @@ public class IssueBulkChangeServiceTest {
     Map<String, Object> properties = newHashMap();
     properties.put("issues", "ABCD");
     properties.put("actions", "unknown");
+    properties.put("unknown.unknown", "unknown");
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(properties);
     try {
       service.execute(issueBulkChangeQuery, userSession);

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java
index d610146a2210520f44fac0bc987eafa2d6cd6b50..f6187dec0a3300db99f875d10f563919417d8990 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java
@@ -109,7 +109,7 @@ public class PlanActionTest {
 
   @Test
   public void should_support_only_unresolved_issues(){
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_OPEN))).isTrue();
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_CLOSED))).isTrue();
+    assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();
+    assertThat(action.supports(new DefaultIssue().setResolution(Issue.RESOLUTION_FIXED))).isFalse();
   }
 }
\ No newline at end of file

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/SetSeverityActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/SetSeverityActionTest.java
index 82aeb4fd48e4c975674c2cf8be6f8fc473559081..2255c2fa9b7e3927e15a8ef85d8aa6ad7159fa2c 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/SetSeverityActionTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/SetSeverityActionTest.java
@@ -62,8 +62,8 @@ public class SetSeverityActionTest {
 
   @Test
   public void should_support_only_unresolved_issues(){
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_OPEN))).isTrue();
-    assertThat(action.supports(new DefaultIssue().setStatus(Issue.STATUS_CLOSED))).isTrue();
+    assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();
+    assertThat(action.supports(new DefaultIssue().setResolution(Issue.RESOLUTION_FIXED))).isFalse();
   }
 
 }