From 8d64596363ee3240147b30af70f6a0c2220ad7be Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 28 Jun 2013 19:27:42 +0200 Subject: [PATCH] SONAR-4421 Remove empty actions and issues from bulk change parameters --- .../org/sonar/api/issue/IssueQueryTest.java | 41 +++++++++++++------ .../server/issue/IssueBulkChangeQuery.java | 21 +++++++++- .../views/issues/_bulk_change_form.html.erb | 2 +- .../issue/IssueBulkChangeQueryTest.java | 24 +++++++++++ 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java index feb57e3c984..adac4a108bd 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java @@ -19,7 +19,6 @@ */ package org.sonar.api.issue; -import com.google.common.collect.Lists; import org.junit.Test; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; @@ -27,7 +26,9 @@ import org.sonar.api.web.UserRole; import java.util.Arrays; import java.util.Date; +import java.util.List; +import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; @@ -36,16 +37,16 @@ public class IssueQueryTest { @Test public void should_build_query() throws Exception { IssueQuery query = IssueQuery.builder() - .issueKeys(Lists.newArrayList("ABCDE")) - .severities(Lists.newArrayList(Severity.BLOCKER)) - .statuses(Lists.newArrayList(Issue.STATUS_RESOLVED)) - .resolutions(Lists.newArrayList(Issue.RESOLUTION_FALSE_POSITIVE)) - .components(Lists.newArrayList("org/struts/Action.java")) - .componentRoots(Lists.newArrayList("org.struts:core")) - .rules(Lists.newArrayList(RuleKey.of("squid", "AvoidCycle"))) - .actionPlans(Lists.newArrayList("AP1", "AP2")) - .reporters(Lists.newArrayList("crunky")) - .assignees(Lists.newArrayList("gargantua")) + .issueKeys(newArrayList("ABCDE")) + .severities(newArrayList(Severity.BLOCKER)) + .statuses(newArrayList(Issue.STATUS_RESOLVED)) + .resolutions(newArrayList(Issue.RESOLUTION_FALSE_POSITIVE)) + .components(newArrayList("org/struts/Action.java")) + .componentRoots(newArrayList("org.struts:core")) + .rules(newArrayList(RuleKey.of("squid", "AvoidCycle"))) + .actionPlans(newArrayList("AP1", "AP2")) + .reporters(newArrayList("crunky")) + .assignees(newArrayList("gargantua")) .assigned(true) .createdAfter(new Date()) .createdBefore(new Date()) @@ -82,7 +83,7 @@ public class IssueQueryTest { @Test public void should_build_query_without_dates() throws Exception { IssueQuery query = IssueQuery.builder() - .issueKeys(Lists.newArrayList("ABCDE")) + .issueKeys(newArrayList("ABCDE")) .build(); assertThat(query.issueKeys()).containsOnly("ABCDE"); @@ -195,6 +196,22 @@ public class IssueQueryTest { } } + @Test + public void number_of_issue_keys_should_be_limited() throws Exception { + List issueKeys = newArrayList(); + for (int i=0; i props, String comment) { - this.issues = RubyUtils.toStrings(props.get("issues")); + this.issues = sanitizeList(RubyUtils.toStrings(props.get("issues"))); if (issues == null || issues.isEmpty()) { throw new IllegalArgumentException("Issues must not be empty"); } - actions = RubyUtils.toStrings(props.get("actions")); + actions = sanitizeList(RubyUtils.toStrings(props.get("actions"))); if (actions == null || actions.isEmpty()) { throw new IllegalArgumentException("At least one action must be provided"); } @@ -72,6 +77,18 @@ public class IssueBulkChangeQuery { } } + private List sanitizeList(List list){ + if (list == null || list.isEmpty()){ + return Collections.emptyList(); + } + return newArrayList(Iterables.filter(list, new Predicate() { + @Override + public boolean apply(@Nullable String input) { + return !Strings.isNullOrEmpty(input); + } + })); + } + public List issues() { return issues; } 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 0baa99b8a03..0fcfa5b1c41 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 @@ -1,7 +1,7 @@
- +

<%= message('issue_filter.bulk_change.form.title', {:params => @issues.size.to_s}) -%>

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 08cf08e1216..4fa46bf44b7 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 @@ -47,6 +47,30 @@ public class IssueBulkChangeQueryTest { assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH"); } + @Test + public void should_remove_empty_actions() { + Map params = newHashMap(); + params.put("issues", newArrayList("ABCD", "EFGH")); + params.put("actions", newArrayList("do_transition", "", null)); + params.put("do_transition.transition", "confirm"); + + IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params); + assertThat(issueBulkChangeQuery.actions()).containsOnly("do_transition"); + assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH"); + } + + @Test + public void should_remove_empty_issues() { + Map params = newHashMap(); + params.put("issues", newArrayList("ABCD", "EFGH", "", null)); + params.put("actions", newArrayList("do_transition")); + params.put("do_transition.transition", "confirm"); + + IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(params); + assertThat(issueBulkChangeQuery.actions()).containsOnly("do_transition"); + assertThat(issueBulkChangeQuery.issues()).containsOnly("ABCD", "EFGH"); + } + @Test public void should_create_query_with_comment() { Map params = newHashMap(); -- 2.39.5