SONAR-4418 Fix transition action

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 1b3b819d34f9d9b178b276a517ccc83dabcd743c..cb12690cb26dffb0e6a3c7baf04af5942cc777fd 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
@@ -20,6 +20,7 @@
 
 package org.sonar.server.issue;
 
+import com.google.common.base.Strings;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.internal.DefaultIssue;
@@ -44,7 +45,7 @@ public class CommentAction extends Action implements ServerComponent {
 
   @Override
   public boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
-    return true;
+    return comment(properties) != null;
   }
 
   @Override
@@ -54,6 +55,10 @@ public class CommentAction extends Action implements ServerComponent {
   }
 
   private String comment(Map<String, Object> properties) {
-    return (String) properties.get(COMMENT_PROPERTY);
+    String param = (String) properties.get(COMMENT_PROPERTY);
+    if (Strings.isNullOrEmpty(param)) {
+      throw new IllegalArgumentException("Missing parameter : '"+ COMMENT_PROPERTY +"'");
+    }
+    return param;
   }
 }
\ No newline at end of file

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java b/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java
index 15e00f97849064a74c596a63e3343f7fedb8ef1a..4a5ea4c82f1addebd47bb7af202e76575d758ce0 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/SetSeverityAction.java
@@ -20,6 +20,7 @@
 
 package org.sonar.server.issue;
 
+import com.google.common.base.Strings;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.condition.IsUnResolved;
@@ -45,7 +46,7 @@ public class SetSeverityAction extends Action implements ServerComponent {
 
   @Override
   public boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
-    return true;
+    return severity(properties) != null;
   }
 
   @Override
@@ -54,6 +55,10 @@ public class SetSeverityAction extends Action implements ServerComponent {
   }
 
   private String severity(Map<String, Object> properties) {
-    return (String) properties.get("severity");
+    String param = (String) properties.get("severity");
+    if (Strings.isNullOrEmpty(param)) {
+      throw new IllegalArgumentException("Missing parameter : 'severity'");
+    }
+    return param;
   }
 }
\ No newline at end of file

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
index 55b79598fb771ba468b7295e38a65da50d3a2d00..cc949f993dcc9ab0bcefdeeb08f46fb34ae1a026 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/TransitionAction.java
@@ -20,6 +20,7 @@
 
 package org.sonar.server.issue;
 
+import com.google.common.base.Strings;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.internal.DefaultIssue;
@@ -42,7 +43,7 @@ public class TransitionAction extends Action implements ServerComponent {
 
   @Override
   public boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession) {
-    return true;
+    return transition(properties) != null;
   }
 
   @Override
@@ -51,7 +52,11 @@ public class TransitionAction extends Action implements ServerComponent {
   }
 
   private String transition(Map<String, Object> properties) {
-    return (String) properties.get("transition");
+    String param = (String) properties.get("transition");
+    if (Strings.isNullOrEmpty(param)) {
+      throw new IllegalArgumentException("Missing parameter : 'transition'");
+    }
+    return param;
   }
 
 }
\ No newline at end of file

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 bd43fb8fa429dd0d38bd7d342717ff4ae23a974e..8dafa130e3986c07dee5d5e18312d91951f16d16 100644 (file)
--- 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
@@ -91,7 +91,7 @@
           <%= message('issue.transition') -%>
         </label>
         <% transitions_by_issues.keys.each do |transition| %>
-          <input type="radio" name="transition.transition" value="<%= transition -%>"
+          <input type="radio" name="do_transition.transition" value="<%= transition -%>"
                  onClick="addTransitionAction();">&nbsp;<%= message("issue.transition.#{transition}") -%>
           <span style="float:right;">(<%= message('issue_bulk_change.x_issues', :params => transitions_by_issues[transition].to_s) %>)</span><br/>
       <% end %>

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 9d06ad206475fe89b70890436a31714ea1877f8f..29aa053aea3ba00adc5c8633fa7d2cfeaf15a2b2 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
@@ -20,17 +20,20 @@
 
 package org.sonar.server.issue;
 
+import com.google.common.collect.Lists;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.issue.internal.IssueChangeContext;
 import org.sonar.core.issue.IssueUpdater;
+import org.sonar.server.user.MockUserSession;
 
 import java.util.Map;
 
 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.eq;
 import static org.mockito.Mockito.*;
@@ -60,6 +63,19 @@ public class CommentActionTest {
     verify(issueUpdater).addComment(eq(issue), eq(comment), any(IssueChangeContext.class));
   }
 
+  @Test
+  public void should_verify_fail_if_parameter_not_found() {
+    Map<String, Object> properties = newHashMap();
+    properties.put("unknwown", "unknown value");
+    try {
+      action.verify(properties, Lists.<Issue>newArrayList(), MockUserSession.create());
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Missing parameter : 'comment'");
+    }
+    verifyZeroInteractions(issueUpdater);
+  }
+
   @Test
   public void should_support_all_issues(){
     assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();

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 487f9bb13892df76c01faadfde9bd16b7488f2b1..c2272d40bbd0179b27f4bcb1b27c49f03962727e 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
@@ -20,17 +20,20 @@
 
 package org.sonar.server.issue;
 
+import com.google.common.collect.Lists;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.issue.internal.IssueChangeContext;
 import org.sonar.core.issue.IssueUpdater;
+import org.sonar.server.user.MockUserSession;
 
 import java.util.Map;
 
 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.eq;
 import static org.mockito.Mockito.*;
@@ -60,6 +63,19 @@ public class SetSeverityActionTest {
     verify(issueUpdater).setManualSeverity(eq(issue), eq(severity), any(IssueChangeContext.class));
   }
 
+  @Test
+  public void should_verify_fail_if_parameter_not_found() {
+    Map<String, Object> properties = newHashMap();
+    properties.put("unknwown", "unknown value");
+    try {
+      action.verify(properties, Lists.<Issue>newArrayList(), MockUserSession.create());
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Missing parameter : 'severity'");
+    }
+    verifyZeroInteractions(issueUpdater);
+  }
+
   @Test
   public void should_support_only_unresolved_issues(){
     assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();

diff --git a/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java
index ffda6ea947071f1eac13819d9bf629073bf80578..6ff1a129c414f7eb91b59b0cb2934040453442fc 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/TransitionActionTest.java
@@ -20,17 +20,20 @@
 
 package org.sonar.server.issue;
 
+import com.google.common.collect.Lists;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.issue.internal.IssueChangeContext;
 import org.sonar.core.issue.workflow.IssueWorkflow;
+import org.sonar.server.user.MockUserSession;
 
 import java.util.Map;
 
 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.eq;
 import static org.mockito.Mockito.*;
@@ -60,6 +63,20 @@ public class TransitionActionTest {
     verify(workflow).doTransition(eq(issue), eq(transition), any(IssueChangeContext.class));
   }
 
+  @Test
+  public void should_verify_fail_if_parameter_not_found() {
+    String transition = "reopen";
+    Map<String, Object> properties = newHashMap();
+    properties.put("unknwown", transition);
+    try {
+      action.verify(properties, Lists.<Issue>newArrayList(), MockUserSession.create());
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Missing parameter : 'transition'");
+    }
+    verifyZeroInteractions(workflow);
+  }
+
   @Test
   public void should_support_all_issues() {
     assertThat(action.supports(new DefaultIssue().setResolution(null))).isTrue();