SONAR-4418 Add a way to bulk change issues from a resource viewer

9 files changed, 102 insertions, 60 deletions

diff --git a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties
index f19b6367e9d..891659203fc 100644
--- a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties
+++ b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties
@@ -549,9 +549,21 @@ issue_filter.private=Private
 issue_filter.shared_with_all_users=Shared with all users
 issue_filter.sharing=Sharing
 issue_filter.manage.shared_filters=Shared Filters
-issue_filter.bulk_change.form.title=Change {0} issues
-issue_filter.bulk_change.max_issues_reached=As too many issues have been selected, only the first {0} issues will be updated.
-issue_filter.bulk_change.x_issues={0} issues
+
+
+#------------------------------------------------------------------------------
+#
+# ISSUE BULK CHANGE
+#
+#------------------------------------------------------------------------------
+
+issue_bulk_change.form.title=Change {0} issues
+issue_bulk_change.max_issues_reached=As too many issues have been selected, only the first {0} issues will be updated.
+issue_bulk_change.x_issues={0} issues
+issue_bulk_change.error.empty_issues=Issues must not be empty
+issue_bulk_change.error.need_one_action=At least one action must be provided
+
+
 
 #------------------------------------------------------------------------------
 #
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 7d677cbbe82..1432fddeab4 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java
@@ -556,11 +556,9 @@ public class InternalRubyIssueService implements ServerComponent {
   /**
    * Execute a bulk change
    */
-  public Result<IssueBulkChangeResult> bulkChange(Map<String, Object> props, String comment) {
-    Result<IssueBulkChangeResult> result = Result.of();
+  public IssueBulkChangeResult bulkChange(Map<String, Object> props, String comment) {
     IssueBulkChangeQuery issueBulkChangeQuery = new IssueBulkChangeQuery(props, comment);
-    result.set(issueBulkChangeService.execute(issueBulkChangeQuery, UserSession.get()));
-    return result;
+    return issueBulkChangeService.execute(issueBulkChangeQuery, UserSession.get());
   }
 
   private void checkMandatoryParameter(String value, String paramName, Result result) {
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 4654e23ae2f..172f6f34b74 100644
--- a/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
+++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueBulkChangeQuery.java
@@ -26,9 +26,11 @@ import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.ReflectionToStringBuilder;
+import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.util.RubyUtils;
 
 import javax.annotation.Nullable;
+
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -59,11 +61,11 @@ public class IssueBulkChangeQuery {
   private void parse(Map<String, Object> props, String comment) {
     this.issues = sanitizeList(RubyUtils.toStrings(props.get("issues")));
     if (issues == null || issues.isEmpty()) {
-      throw new IllegalArgumentException("Issues must not be empty");
+      throw BadRequestException.ofL10n("issue_bulk_change.error.empty_issues");
     }
     actions = sanitizeList(RubyUtils.toStrings(props.get("actions")));
     if (actions == null || actions.isEmpty()) {
-      throw new IllegalArgumentException("At least one action must be provided");
+      throw BadRequestException.ofL10n("issue_bulk_change.error.need_one_action");
     }
     for (String action : actions) {
       Map<String, Object> actionProperties = getActionProps(action, props);
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 08dd52ff669..dc520aaa140 100644
--- 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
@@ -144,33 +144,13 @@ class IssuesController < ApplicationController
 
   # GET /issues/bulk_change_form?[&criteria]
   def bulk_change_form
-    params_to_execute = criteria_params.clone
-    params_to_execute['pageSize'] = -1
+    params[:from] ||= 'issue_filters'
+    params_for_query = params.clone.merge({'pageSize' => -1})
     if params[:id]
-      issue_filter_result = Internal.issues.execute(params[:id].to_i, params_to_execute)
+      @issue_filter_result = Internal.issues.execute(params[:id].to_i, params_for_query)
     else
-      issue_filter_result = Internal.issues.execute(params_to_execute)
+      @issue_filter_result = Internal.issues.execute(params_for_query)
     end
-
-    issue_query = issue_filter_result.query
-    issues_result = issue_filter_result.result
-
-    @transitions_by_issues = {}
-    @unresolved_issues = 0
-    issues_result.issues.each do |issue|
-      transitions = Internal.issues.listTransitions(issue)
-      transitions.each do |transition|
-        issues_for_transition = @transitions_by_issues[transition.key] || 0
-        issues_for_transition += 1
-        @transitions_by_issues[transition.key] = issues_for_transition
-      end
-      @unresolved_issues += 1 unless issue.resolution()
-    end
-    @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
-    @criteria_params = criteria_params
-    @max_page_size_reached = issues_result.issues.size >= issues_result.paging.pageSize()
-
     render :partial => 'issues/bulk_change_form'
   end
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
index 28d71e7ccd0..1e0b00bfdcc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/resource_controller.rb
@@ -25,7 +25,7 @@ class ResourceController < ApplicationController
 
   SECTION=Navigation::SECTION_RESOURCE
   helper :dashboard
-  helper SourceHelper, UsersHelper
+  helper SourceHelper, UsersHelper, IssuesHelper
 
   def index
     if request.xhr?
@@ -336,6 +336,7 @@ class ResourceController < ApplicationController
         @global_issues<<issue
       end
     end
+    @issues_params = options
 
     if !@expanded && @lines
       filter_lines { |line| line.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 3d2bfbf19ab..dbfa1807183 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,20 +1,40 @@
+<%
+   issue_query = @issue_filter_result.query
+   issues_result = @issue_filter_result.result
+
+   issues = issues_result.issues
+   max_page_size_reached = issues_result.issues.size >= issues_result.paging.pageSize()
+   project_key = issue_query.componentRoots.to_a.first if issue_query.componentRoots and issue_query.componentRoots.size == 1
+
+   transitions_by_issues = {}
+   unresolved_issues = 0
+   issues.each do |issue|
+     transitions = Internal.issues.listTransitions(issue)
+     transitions.each do |transition|
+       issues_for_transition = transitions_by_issues[transition.key] || 0
+       issues_for_transition += 1
+       transitions_by_issues[transition.key] = issues_for_transition
+     end
+     unresolved_issues += 1 unless issue.resolution()
+   end
+%>
 <form id="bulk-change-form" method="post" action="<%= ApplicationController.root_context -%>/issues/bulk_change">
-  <input type="hidden" name="issues" value="<%= @issues.join(',') -%>">
-  <input type="hidden" name="criteria_params" value="<%= @criteria_params.to_query -%>">
+  <input type="hidden" name="issues" value="<%= issues.map { |issue| issue.key() }.join(',') -%>">
+  <input type="hidden" name="criteria_params" value="<%= params.to_query -%>">
   <input type="hidden" name="actions[]" id="bulk-change-transition-action">
   <fieldset>
     <div class="modal-head">
-      <h2><%= message('issue_filter.bulk_change.form.title', {:params => @issues.size.to_s}) -%></h2>
+      <h2><%= message('issue_bulk_change.form.title', {:params => issues.size.to_s}) -%></h2>
     </div>
     <div class="modal-body">
       <div>
-      <% if @max_page_size_reached %>
-        <p class="notes"><%= message('issue_filter.bulk_change.max_issues_reached', :params => @issues.size) -%></p>
+      <% if max_page_size_reached %>
+        <p class="notes"><%= message('issue_bulk_change.max_issues_reached', :params => issues.size) -%></p>
       <% end %>
       </div>
-      <div class="bulk-change errors" style="display:none;"/>
+      <div class="bulk-change errors error" style="display:none;"/>
 
-      <% if @unresolved_issues > 0 %>
+      <% if unresolved_issues > 0 %>
       <div class="modal-field">
         <label for="assignee">
           <%= message('issue.assign.formlink') -%>
@@ -22,11 +42,11 @@
         <input id="assign-action" name="actions[]" type="checkbox" value="assign"/>
         <%= user_select_tag('assign.assignee', :html_id => 'assignee', :open => false, :selected_user => current_user,
                             :include_choices => {'' => escape_javascript(message('unassigned')), current_user.login => escape_javascript(message('assigned_to_me'))}) -%>
-        <span style="float:right;">(<%= message('issue_filter.bulk_change.x_issues', :params => @unresolved_issues.to_s) -%>)</span>
+        <span style="float:right;">(<%= message('issue_bulk_change.x_issues', :params => unresolved_issues.to_s) -%>)</span>
       </div>
       <%
-         if @project && !@project.blank?
-           plans = Internal.issues.findOpenActionPlans(@project)
+         if project_key && !project_key.blank?
+           plans = Internal.issues.findOpenActionPlans(project_key)
            unless plans.empty?
              first_plan = plans[0]
              options = plans.map { |plan|
@@ -46,7 +66,7 @@
           </label>
           <input id="plan-action" name="actions[]" type="checkbox" value="plan"/>
           <%= dropdown_tag('plan.plan', plan_options, {:show_search_box => false}, {:id => 'plan'}) -%>
-          <span style="float:right;">(<%= message('issue_filter.bulk_change.x_issues', :params => @unresolved_issues.to_s) -%>)</span>
+          <span style="float:right;">(<%= message('issue_bulk_change.x_issues', :params => unresolved_issues.to_s) -%>)</span>
         </div>
         <% end %>
       <% end %>
@@ -57,19 +77,19 @@
         <input id="set-severity-action" name="actions[]" type="checkbox" value="set_severity"/>
         <%= severity_dropdown_tag('set_severity.severity', severitiy_select_option_tags, {:show_search_box => false},
                          {:id => 'severity'}) -%>
-        <span style="float:right;">(<%= message('issue_filter.bulk_change.x_issues', :params => @unresolved_issues.to_s) -%>)</span>
+        <span style="float:right;">(<%= message('issue_bulk_change.x_issues', :params => unresolved_issues.to_s) -%>)</span>
       </div>
       <% end %>
 
-      <% if @transitions_by_issues.size > 0 %>
+      <% if transitions_by_issues.size > 0 %>
       <div class="modal-field">
         <label>
           <%= message('issue.transition') -%>
         </label>
-        <% @transitions_by_issues.keys.each do |transition| %>
+        <% transitions_by_issues.keys.each do |transition| %>
           <input type="radio" name="transition.transition" value="<%= transition -%>"
                  onClick="addTransitionAction();">&nbsp;<%= message("issue.transition.#{transition}") -%>
-          <span style="float:right;">(<%= message('issue_filter.bulk_change.x_issues', :params => @transitions_by_issues[transition].to_s) %>)</span><br/>
+          <span style="float:right;">(<%= message('issue_bulk_change.x_issues', :params => transitions_by_issues[transition].to_s) %>)</span><br/>
       <% end %>
       </div>
       <% end %>
@@ -98,7 +118,16 @@
 <script>
   $j("#bulk-change-form").modalForm({
     success: function (data) {
-      window.location = baseUrl + '/issues/search?' + data;
+      <% if params[:from] == 'issue_filters' %>
+        window.location = baseUrl + '/issues/search?' + data;
+      <% elsif params[:from] == 'drilldown' %>
+        <% component = issue_query.components.to_a.first if issue_query.components and issue_query.components.size == 1
+        component_id = Project.by_key(component).id %>
+        d('<%= component_id %>');
+        closeModalWindow();
+      <% else %>
+        alert('Unknown origin : <%= params[:from] %>');
+      <% end %>
     },
     error: function (xhr, textStatus, errorThrown) {
       $j('#bulk-change-loading-image').addClass("hidden");
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb
index dec63d2d542..5007355a5cc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/resource/_tabs.html.erb
@@ -21,6 +21,14 @@
 
   <div class="source_tabs">
     <ul class="tablinks">
+
+      <% if @display_issues %>
+        <li>
+          <a href="<%= url_for @issues_params.merge({:controller => 'issues', :action => 'bulk_change_form', :from => 'drilldown'}) -%>"
+             class="bulk-change-link open-modal"><%= message('bulk_change') -%></a>
+        </li>
+      <% end %>
+
       <%
          first=true
          if @snapshot.has_source && has_role?(:codeviewer, @snapshot)
@@ -32,15 +40,18 @@
          end %>
       <% unless @popup_mode %>
       <li class="<%= 'first' if first -%>">
-        <a href="<%= url_for :controller => 'resource', :action => 'index', :id => @resource.key, :period => params[:period], :metric => params[:metric], :rule => params[:rule] ? params[:rule] : params[:rule_sev], :display_title => 'true' -%>"
-           onclick="window.open(this.href,'resource','height=800,width=900,scrollbars=1,resizable=1');return false;" id="new-window-<%= @resource.key.parameterize -%>"><%= image_tag 'new-window-16.gif', :alt => message('new_window') -%></a>
+        <a href="<%= url_for :controller => 'resource', :action => 'index', :id => @resource.key, :period => params[:period], :metric => params[:metric],
+                             :rule => params[:rule] ? params[:rule] : params[:rule_sev], :display_title => 'true' -%>"
+           onclick="window.open(this.href,'resource','height=800,width=900,scrollbars=1,resizable=1');return false;"
+              id="new-window-<%= @resource.key.parameterize -%>"><%= image_tag 'new-window-16.gif', :alt => message('new_window') -%></a>
       </li>
       <% end %>
     </ul>
     <ul class="tabs2">
       <% @extensions.each do |extension| %>
         <li>
-          <a href="#" onclick="return loadResourceViewer('<%= @resource.id -%>','<%= extension.getId() -%>',<%= display_title -%>, this)" class="<%= 'selected' if @extension && @extension.getId()==extension.getId() -%>"><%= message(extension.getId() + '.page', :default => extension.getTitle()) %></a>
+          <a href="#" onclick="return loadResourceViewer('<%= @resource.id -%>','<%= extension.getId() -%>',<%= display_title -%>, this)"
+             class="<%= 'selected' if @extension && @extension.getId()==extension.getId() -%>"><%= message(extension.getId() + '.page', :default => extension.getTitle()) %></a>
         </li>
       <% end %>
       <li>
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 1ec3b68d872..95dcdb4aac2 100644
--- a/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java
@@ -547,9 +547,7 @@ public class InternalRubyIssueServiceTest {
     params.put("assign.assignee", "arthur");
     params.put("set_severity.severity", "MINOR");
     params.put("plan.plan", "3.7");
-    Result<IssueBulkChangeResult> result = service.bulkChange(params, "My comment");
-    assertThat(result.errors()).isEmpty();
-    assertThat(result.ok()).isTrue();
+    service.bulkChange(params, "My comment");
     verify(issueBulkChangeService).execute(any(IssueBulkChangeQuery.class), any(UserSession.class));
   }
 
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 4fa46bf44b7..0176a342d86 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
@@ -21,6 +21,7 @@
 package org.sonar.server.issue;
 
 import org.junit.Test;
+import org.sonar.server.exceptions.BadRequestException;
 
 import java.util.Collections;
 import java.util.Map;
@@ -104,7 +105,8 @@ public class IssueBulkChangeQueryTest {
       new IssueBulkChangeQuery(params);
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Issues must not be empty");
+      assertThat(e).isInstanceOf(BadRequestException.class);
+      checkBadRequestException(e, "issue_bulk_change.error.empty_issues");
     }
   }
 
@@ -118,7 +120,8 @@ public class IssueBulkChangeQueryTest {
       new IssueBulkChangeQuery(params);
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Issues must not be empty");
+      assertThat(e).isInstanceOf(BadRequestException.class);
+      checkBadRequestException(e, "issue_bulk_change.error.empty_issues");
     }
   }
 
@@ -130,7 +133,8 @@ public class IssueBulkChangeQueryTest {
       new IssueBulkChangeQuery(params);
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("At least one action must be provided");
+      assertThat(e).isInstanceOf(BadRequestException.class);
+      checkBadRequestException(e, "issue_bulk_change.error.need_one_action");
     }
   }
 
@@ -143,8 +147,15 @@ public class IssueBulkChangeQueryTest {
       new IssueBulkChangeQuery(params);
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("At least one action must be provided");
+      assertThat(e).isInstanceOf(BadRequestException.class);
+      checkBadRequestException(e, "issue_bulk_change.error.need_one_action");
     }
   }
 
+  private void checkBadRequestException(Exception e, String key, Object... params) {
+    BadRequestException exception = (BadRequestException) e;
+    assertThat(exception.l10nKey()).isEqualTo(key);
+    assertThat(exception.l10nParams()).containsOnly(params);
+  }
+
 }