SONAR-3714 Improve bulk change ui and move issue updater from Action.context to Actio...

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/Action.java b/sonar-server/src/main/java/org/sonar/server/issue/Action.java
index ec56d59ec9602462a6a414c9a1b566990b9f4338..b110cdab2a2b62eb2756248f85dcacbeb607c33d 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/Action.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/Action.java
@@ -27,7 +27,6 @@ import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.condition.Condition;
 import org.sonar.api.issue.internal.IssueChangeContext;
-import org.sonar.core.issue.IssueUpdater;
 import org.sonar.server.user.UserSession;
 
 import java.util.List;
@@ -103,8 +102,6 @@ public abstract class Action implements ServerComponent {
   interface Context {
     Issue issue();
 
-    IssueUpdater issueUpdater();
-
     IssueChangeContext issueChangeContext();
   }
 

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java b/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java
index 5fa96a2a37aa55d3034db1f1c6c47c3b199b1927..14b41c13bad899eab6d6c64b43bac4dcdfae269f 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java
@@ -20,11 +20,13 @@
 
 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;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.user.UserFinder;
+import org.sonar.core.issue.IssueUpdater;
 import org.sonar.server.user.UserSession;
 
 import java.util.List;
@@ -36,17 +38,19 @@ public class AssignAction extends Action implements ServerComponent {
   public static final String ASSIGN_ACTION_KEY = "assign";
 
   private final UserFinder userFinder;
+  private final IssueUpdater issueUpdater;
 
-  public AssignAction(UserFinder userFinder) {
+  public AssignAction(UserFinder userFinder, IssueUpdater issueUpdater) {
     super(ASSIGN_ACTION_KEY);
     this.userFinder = userFinder;
+    this.issueUpdater = issueUpdater;
     super.setConditions(new IsUnResolved());
   }
 
   @Override
   public boolean verify(Map<String, Object> properties, List<Issue> issues, UserSession userSession){
     String assignee = assignee(properties);
-    if (assignee != null && userFinder.findByLogin(assignee) == null) {
+    if (!Strings.isNullOrEmpty(assignee) && userFinder.findByLogin(assignee) == null) {
       throw new IllegalArgumentException("Unknown user: " + assignee);
     }
     return true;
@@ -54,7 +58,7 @@ public class AssignAction extends Action implements ServerComponent {
 
   @Override
   public boolean execute(Map<String, Object> properties, Context context) {
-    return context.issueUpdater().assign((DefaultIssue) context.issue(), assignee(properties), context.issueChangeContext());
+    return issueUpdater.assign((DefaultIssue) context.issue(), assignee(properties), context.issueChangeContext());
   }
 
   private String assignee(Map<String, Object> properties){

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 d926fd543cf45d1d52e19ba18f8fb0165a9321aa..de6ed273edf2e2d64ce641eafba35605e71d35d2 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
@@ -23,6 +23,7 @@ package org.sonar.server.issue;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.internal.DefaultIssue;
+import org.sonar.core.issue.IssueUpdater;
 import org.sonar.server.user.UserSession;
 
 import java.util.List;
@@ -34,8 +35,11 @@ public class CommentAction extends Action implements ServerComponent {
   public static final String COMMENT_ACTION_KEY = "comment";
   public static final String COMMENT_PROPERTY = "comment";
 
-  public CommentAction() {
+  private final IssueUpdater issueUpdater;
+
+  public CommentAction(IssueUpdater issueUpdater) {
     super(COMMENT_ACTION_KEY);
+    this.issueUpdater = issueUpdater;
   }
 
   @Override
@@ -45,7 +49,7 @@ public class CommentAction extends Action implements ServerComponent {
 
   @Override
   public boolean execute(Map<String, Object> properties, Context context) {
-    context.issueUpdater().addComment((DefaultIssue) context.issue(), comment(properties), context.issueChangeContext());
+    issueUpdater.addComment((DefaultIssue) context.issue(), comment(properties), context.issueChangeContext());
     return true;
   }
 

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 0398ed0ce5e8c51fbc2313fb70a086d4d03972b5..501683771d677a700246f563b84272044ae9f9d0 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeService.java
@@ -36,7 +36,6 @@ import org.sonar.core.issue.db.IssueStorage;
 import org.sonar.server.user.UserSession;
 
 import javax.annotation.CheckForNull;
-
 import java.util.Date;
 import java.util.List;
 
@@ -78,7 +77,7 @@ public class IssueBulkChangeService {
       for (String actionName : issueBulkChangeQuery.actions()) {
         try {
           Action action = getAction(actionName);
-          ActionContext actionContext = new ActionContext(issue, issueUpdater, issueChangeContext);
+          ActionContext actionContext = new ActionContext(issue, issueChangeContext);
           if (action.supports(issue) && action.execute(issueBulkChangeQuery.properties(actionName), actionContext)) {
             issueStorage.save((DefaultIssue) issue);
             issueNotifications.sendChanges((DefaultIssue) issue, issueChangeContext, issueQueryResult);
@@ -114,11 +113,9 @@ public class IssueBulkChangeService {
 
   static class ActionContext implements Action.Context {
     private final Issue issue;
-    private final IssueUpdater updater;
     private final IssueChangeContext changeContext;
 
-    ActionContext(Issue issue, IssueUpdater updater, IssueChangeContext changeContext) {
-      this.updater = updater;
+    ActionContext(Issue issue, IssueChangeContext changeContext) {
       this.issue = issue;
       this.changeContext = changeContext;
     }
@@ -128,11 +125,6 @@ public class IssueBulkChangeService {
       return issue;
     }
 
-    @Override
-    public IssueUpdater issueUpdater() {
-      return updater;
-    }
-
     @Override
     public IssueChangeContext issueChangeContext() {
       return changeContext;

diff --git a/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java b/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java
index bb5363dfce1bee635ca9645811e92019c504c4d0..59fe7f240ddf6978065e06bb4806eb199666a1e5 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java
@@ -26,6 +26,7 @@ import org.sonar.api.issue.ActionPlan;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.condition.IsUnResolved;
 import org.sonar.api.issue.internal.DefaultIssue;
+import org.sonar.core.issue.IssueUpdater;
 import org.sonar.server.user.UserSession;
 
 import java.util.List;
@@ -37,10 +38,12 @@ public class PlanAction extends Action implements ServerComponent {
   public static final String PLAN_ACTION_KEY = "plan";
 
   private final ActionPlanService actionPlanService;
+  private final IssueUpdater issueUpdater;
 
-  public PlanAction(ActionPlanService actionPlanService) {
+  public PlanAction(ActionPlanService actionPlanService, IssueUpdater issueUpdater) {
     super(PLAN_ACTION_KEY);
     this.actionPlanService = actionPlanService;
+    this.issueUpdater = issueUpdater;
     super.setConditions(new IsUnResolved());
   }
 
@@ -61,7 +64,7 @@ public class PlanAction extends Action implements ServerComponent {
 
   @Override
   public boolean execute(Map<String, Object> properties, Context context) {
-    return context.issueUpdater().plan((DefaultIssue) context.issue(), planKey(properties), context.issueChangeContext());
+    return issueUpdater.plan((DefaultIssue) context.issue(), planKey(properties), context.issueChangeContext());
   }
 
   private String planKey(Map<String, Object> properties) {

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 1933d055d49917f84333c403303bead1215a1986..363d963e57662fdd1a5926e5e22a895c06c66c0e 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
@@ -24,6 +24,7 @@ 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.core.issue.IssueUpdater;
 import org.sonar.server.user.UserSession;
 
 import java.util.List;
@@ -34,8 +35,11 @@ public class SetSeverityAction extends Action implements ServerComponent {
 
   public static final String SET_SEVERITY_ACTION_KEY = "set_severity";
 
-  public SetSeverityAction() {
+  private final IssueUpdater issueUpdater;
+
+  public SetSeverityAction(IssueUpdater issueUpdater) {
     super(SET_SEVERITY_ACTION_KEY);
+    this.issueUpdater = issueUpdater;
     super.setConditions(new IsUnResolved());
   }
 
@@ -46,7 +50,7 @@ public class SetSeverityAction extends Action implements ServerComponent {
 
   @Override
   public boolean execute(Map<String, Object> properties, Context context) {
-    return context.issueUpdater().setSeverity((DefaultIssue) context.issue(), severity(properties), context.issueChangeContext());
+    return issueUpdater.setSeverity((DefaultIssue) context.issue(), severity(properties), context.issueChangeContext());
   }
 
   private String severity(Map<String, Object> properties) {

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb
index ee7789ab664c56a42817858b315b8bc2a56dd21a..f5f2afca06969d136995ea57508768b2a6558575 100644 (file)
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb
@@ -186,10 +186,12 @@ class IssuesController < ApplicationController
     @criteria_params = criteria_params_to_save
     @criteria_params['pageSize'] = -1
     issue_filter_result = Internal.issues.execute(@criteria_params)
-    @issue_query = issue_filter_result.query
-    @issues_result = issue_filter_result.result
+    issue_query = issue_filter_result.query
+    issues_result = issue_filter_result.result
+
+    @issues = issues_result.issues.map {|issue| issue.key()}
+    @project = issue_query.componentRoots.to_a.first if issue_query.componentRoots and issue_query.componentRoots.size == 1
 
-    @issue_keys = @issues_result.issues.map {|issue| issue.key()}.join(',') unless @issues_result.issues.empty?
     render :partial => 'issues/bulk_change_form'
   end
 

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 3a80795c07b0bf332021b5c681b42e5c59ba67a0..e4076fe0a6821964f2a40f906f550b7269428c81 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
@@ -1,9 +1,15 @@
+<%
+   project = params[:project] || @project
+   issues = params[:issues].split(',') if params[:issues]
+   issues = @issues unless issues
+%>
 <form id="bulk-change-form" method="post" action="<%= ApplicationController.root_context -%>/issues/bulk_change">
-  <input type="hidden" name="issues" value="<%= @issue_keys -%>">
-  <input type="hidden" name="criteria_params" value="<%= @criteria_params.to_query -%>">
+  <input type="hidden" name="issues" value="<%= params[:issues] || @issues.join(',') -%>">
+  <input type="hidden" name="criteria_params" value="<%= params[:criteria_params] ||  @criteria_params.to_query -%>">
+  <input type="hidden" name="project" value="<%= project -%>">
   <fieldset>
     <div class="modal-head">
-      <h2><%= message('issue_filter.bulk_change.form.title', {:params => @issues_result.issues.size.to_s}) -%></h2>
+      <h2><%= message('issue_filter.bulk_change.form.title', {:params => issues.size.to_s}) -%></h2>
     </div>
     <div class="modal-body">
       <%= render :partial => 'display_errors' %>
@@ -14,10 +20,9 @@
         </label>
         <%= user_select_tag('assign.assignee', :html_id => 'assignee', :open => false) -%>
       </div>
-      <% if @issue_query.componentRoots and @issue_query.componentRoots.size == 1 %>
-        <%
-           componentRoot = @issue_query.componentRoots.to_a.first
-           plans = Internal.issues.findOpenActionPlans(componentRoot)
+      <%
+         if project && !project.blank?
+           plans = Internal.issues.findOpenActionPlans(project)
            first_plan = plans[0]
            plan_options = ""
            unless plans.empty?

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 0dd3866e508c513e49d567c2482cc329a083a61f..3a9cec4ac86c268a3f6d7daf4e81af9786e147fd 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
@@ -45,10 +45,11 @@ public class AssignActionTest {
   private AssignAction action;
 
   private final UserFinder userFinder = mock(UserFinder.class);
+  private IssueUpdater issueUpdater = mock(IssueUpdater.class);
 
   @Before
   public void before(){
-    action = new AssignAction(userFinder);
+    action = new AssignAction(userFinder, issueUpdater);
   }
 
   @Test
@@ -58,10 +59,8 @@ public class AssignActionTest {
     Map<String, Object> properties = newHashMap();
     properties.put("assignee", assignee);
     DefaultIssue issue = mock(DefaultIssue.class);
-    IssueUpdater issueUpdater = mock(IssueUpdater.class);
 
     Action.Context context = mock(Action.Context.class);
-    when(context.issueUpdater()).thenReturn(issueUpdater);
     when(context.issue()).thenReturn(issue);
 
     action.execute(properties, context);
@@ -94,6 +93,21 @@ public class AssignActionTest {
     }
   }
 
+  @Test
+  public void should_fail_if_assignee_is_empty(){
+    String assignee = "";
+    Map<String, Object> properties = newHashMap();
+    properties.put("assignee", assignee);
+
+    List<Issue> issues = newArrayList((Issue) new DefaultIssue().setKey("ABC"));
+    when(userFinder.findByLogin(assignee)).thenReturn(null);
+    try {
+      action.verify(properties, issues, mock(UserSession.class));
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Unknown user: ");
+    }
+  }
+
   @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/CommentActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/CommentActionTest.java
index 120a1c2b6493b12756b43c02dd7ccefc843f9580..9d06ad206475fe89b70890436a31714ea1877f8f 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
@@ -39,9 +39,11 @@ public class CommentActionTest {
 
   private CommentAction action;
 
+  private IssueUpdater issueUpdater = mock(IssueUpdater.class);
+
   @Before
   public void before(){
-    action = new CommentAction();
+    action = new CommentAction(issueUpdater);
   }
 
   @Test
@@ -50,10 +52,8 @@ public class CommentActionTest {
     Map<String, Object> properties = newHashMap();
     properties.put("comment", comment);
     DefaultIssue issue = mock(DefaultIssue.class);
-    IssueUpdater issueUpdater = mock(IssueUpdater.class);
 
     Action.Context context = mock(Action.Context.class);
-    when(context.issueUpdater()).thenReturn(issueUpdater);
     when(context.issue()).thenReturn(issue);
 
     action.execute(properties, context);

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 f6187dec0a3300db99f875d10f563919417d8990..5b56097781fcb85dc260250df0735b12e3282b41 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
@@ -44,10 +44,11 @@ public class PlanActionTest {
   private PlanAction action;
 
   private ActionPlanService actionPlanService = mock(ActionPlanService.class);
+  private IssueUpdater issueUpdater = mock(IssueUpdater.class);
 
   @Before
   public void before(){
-    action = new PlanAction(actionPlanService);
+    action = new PlanAction(actionPlanService, issueUpdater);
   }
 
   @Test
@@ -56,10 +57,8 @@ public class PlanActionTest {
     Map<String, Object> properties = newHashMap();
     properties.put("plan", planKey);
     DefaultIssue issue = mock(DefaultIssue.class);
-    IssueUpdater issueUpdater = mock(IssueUpdater.class);
 
     Action.Context context = mock(Action.Context.class);
-    when(context.issueUpdater()).thenReturn(issueUpdater);
     when(context.issue()).thenReturn(issue);
 
     action.execute(properties, context);
@@ -92,6 +91,21 @@ public class PlanActionTest {
     }
   }
 
+  @Test
+  public void should_fail_if_action_plan_is_empty(){
+    String planKey = "";
+    Map<String, Object> properties = newHashMap();
+    properties.put("plan", planKey);
+
+    List<Issue> issues = newArrayList((Issue) new DefaultIssue().setKey("ABC").setProjectKey("struts"));
+    when(actionPlanService.findByKey(eq(planKey), any(UserSession.class))).thenReturn(null);
+    try {
+      action.verify(properties, issues, mock(UserSession.class));
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Unknown action plan: ");
+    }
+  }
+
   @Test
   public void should_verify_issues_are_on_the_same_project(){
     String planKey = "ABCD";

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 2255c2fa9b7e3927e15a8ef85d8aa6ad7159fa2c..0148ab2ccc916495ba024b00554b18a1f8b838ed 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
@@ -39,9 +39,11 @@ public class SetSeverityActionTest {
 
   private SetSeverityAction action;
 
+  private IssueUpdater issueUpdater = mock(IssueUpdater.class);
+
   @Before
   public void before(){
-    action = new SetSeverityAction();
+    action = new SetSeverityAction(issueUpdater);
   }
 
   @Test
@@ -50,10 +52,8 @@ public class SetSeverityActionTest {
     Map<String, Object> properties = newHashMap();
     properties.put("severity", severity);
     DefaultIssue issue = mock(DefaultIssue.class);
-    IssueUpdater issueUpdater = mock(IssueUpdater.class);
 
     Action.Context context = mock(Action.Context.class);
-    when(context.issueUpdater()).thenReturn(issueUpdater);
     when(context.issue()).thenReturn(issue);
 
     action.execute(properties, context);