From d9244f27b13da4409797b477399060ef1c050d90 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 5 Jul 2013 17:11:11 +0200 Subject: [PATCH] SONAR-4418 Fix transition action --- .../org/sonar/server/issue/CommentAction.java | 9 +++++++-- .../sonar/server/issue/SetSeverityAction.java | 9 +++++++-- .../sonar/server/issue/TransitionAction.java | 9 +++++++-- .../app/views/issues/_bulk_change_form.html.erb | 2 +- .../sonar/server/issue/CommentActionTest.java | 16 ++++++++++++++++ .../server/issue/SetSeverityActionTest.java | 16 ++++++++++++++++ .../server/issue/TransitionActionTest.java | 17 +++++++++++++++++ 7 files changed, 71 insertions(+), 7 deletions(-) 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 1b3b819d34f..cb12690cb26 100644 --- 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 properties, List 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 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 15e00f97849..4a5ea4c82f1 100644 --- 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 properties, List 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 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 55b79598fb7..cc949f993dc 100644 --- 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 properties, List 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 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 bd43fb8fa42..8dafa130e39 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 @@ -91,7 +91,7 @@ <%= message('issue.transition') -%> <% transitions_by_issues.keys.each do |transition| %> -  <%= message("issue.transition.#{transition}") -%> (<%= message('issue_bulk_change.x_issues', :params => transitions_by_issues[transition].to_s) %>)
<% 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 9d06ad20647..29aa053aea3 100644 --- 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 properties = newHashMap(); + properties.put("unknwown", "unknown value"); + try { + action.verify(properties, Lists.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 487f9bb1389..c2272d40bbd 100644 --- 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 properties = newHashMap(); + properties.put("unknwown", "unknown value"); + try { + action.verify(properties, Lists.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 ffda6ea9470..6ff1a129c41 100644 --- 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 properties = newHashMap(); + properties.put("unknwown", transition); + try { + action.verify(properties, Lists.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(); -- 2.39.5