Refactor: extract #bulk_update method from IssuesController#bulk_edit.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4037 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/issues_controller.rb

@@ -20,7 +20,7 @@ class IssuesController < ApplicationController
   default_search_scope :issues
   
   before_filter :find_issue, :only => [:show, :edit, :update]
-  before_filter :find_issues, :only => [:bulk_edit, :move, :perform_move, :destroy]
+  before_filter :find_issues, :only => [:bulk_edit, :bulk_update, :move, :perform_move, :destroy]
   before_filter :find_project, :only => [:new, :create]
   before_filter :authorize, :except => [:index]
   before_filter :find_optional_project, :only => [:index]
@@ -54,6 +54,7 @@ class IssuesController < ApplicationController
          :render => { :nothing => true, :status => :method_not_allowed }
 
   verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed }
+  verify :method => :post, :only => :bulk_update, :render => {:nothing => true, :status => :method_not_allowed }
   verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
   
   def index
@@ -191,29 +192,31 @@ class IssuesController < ApplicationController
   # Bulk edit a set of issues
   def bulk_edit
     @issues.sort!
-    if request.post?
-      attributes = (params[:issue] || {}).reject {|k,v| v.blank?}
-      attributes.keys.each {|k| attributes[k] = '' if attributes[k] == 'none'}
-      attributes[:custom_field_values].reject! {|k,v| v.blank?} if attributes[:custom_field_values]
-      
-      unsaved_issue_ids = []
-      @issues.each do |issue|
-        issue.reload
-        journal = issue.init_journal(User.current, params[:notes])
-        issue.safe_attributes = attributes
-        call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue })
-        unless issue.save
-          # Keep unsaved issue ids to display them in flash error
-          unsaved_issue_ids << issue.id
-        end
-      end
-      set_flash_from_bulk_issue_save(@issues, unsaved_issue_ids)
-      redirect_back_or_default({:controller => 'issues', :action => 'index', :project_id => @project})
-      return
-    end
     @available_statuses = Workflow.available_statuses(@project)
     @custom_fields = @project.all_issue_custom_fields
   end
+
+  def bulk_update
+    @issues.sort!
+
+    attributes = (params[:issue] || {}).reject {|k,v| v.blank?}
+    attributes.keys.each {|k| attributes[k] = '' if attributes[k] == 'none'}
+    attributes[:custom_field_values].reject! {|k,v| v.blank?} if attributes[:custom_field_values]
+    
+    unsaved_issue_ids = []
+    @issues.each do |issue|
+      issue.reload
+      journal = issue.init_journal(User.current, params[:notes])
+      issue.safe_attributes = attributes
+      call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue })
+      unless issue.save
+        # Keep unsaved issue ids to display them in flash error
+        unsaved_issue_ids << issue.id
+      end
+    end
+    set_flash_from_bulk_issue_save(@issues, unsaved_issue_ids)
+    redirect_back_or_default({:controller => 'issues', :action => 'index', :project_id => @project})
+  end
   
   def destroy
     @hours = TimeEntry.sum(:hours, :conditions => ['issue_id IN (?)', @issues]).to_f

app/views/issues/bulk_edit.rhtml

@@ -2,7 +2,7 @@
 
 <ul><%= @issues.collect {|i| content_tag('li', link_to(h("#{i.tracker} ##{i.id}"), { :action => 'show', :id => i }) + h(": #{i.subject}")) }.join("\n") %></ul>
 
-<% form_tag() do %>
+<% form_tag(:action => 'bulk_update') do %>
 <%= @issues.collect {|i| hidden_field_tag('ids[]', i.id)}.join %>
 <div class="box tabular">
 <fieldset class="attributes">

config/routes.rb

@@ -132,6 +132,7 @@ ActionController::Routing::Routes.draw do |map|
       issues_actions.connect 'issues/:id/quoted', :controller => 'journals', :action => 'new', :id => /\d+/
       issues_actions.connect 'issues/:id/:action', :action => /edit|destroy/, :id => /\d+/
       issues_actions.connect 'issues.:format', :action => 'create', :format => /xml/
+      issues_actions.connect 'issues/bulk_edit', :action => 'bulk_update'
     end
     issues_routes.with_options :conditions => {:method => :put} do |issues_actions|
       issues_actions.connect 'issues/:id/edit', :action => 'update', :id => /\d+/

lib/redmine.rb

@@ -66,7 +66,7 @@ Redmine::AccessControl.map do |map|
                                   :queries => :index,
                                   :reports => [:issue_report, :issue_report_details]}
     map.permission :add_issues, {:issues => [:new, :create, :update_form]}
-    map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :update_form], :journals => [:new]}
+    map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update, :update_form], :journals => [:new]}
     map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
     map.permission :manage_subtasks, {}
     map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new]}

test/functional/issues_controller_test.rb

@@ -910,10 +910,10 @@ class IssuesControllerTest < ActionController::TestCase
     assert_tag :select, :attributes => {:name => 'issue[custom_field_values][1]'}
   end
 
-  def test_bulk_edit
+  def test_bulk_update
     @request.session[:user_id] = 2
     # update issues priority
-    post :bulk_edit, :ids => [1, 2], :notes => 'Bulk editing',
+    post :bulk_update, :ids => [1, 2], :notes => 'Bulk editing',
                                      :issue => {:priority_id => 7,
                                                 :assigned_to_id => '',
                                                 :custom_field_values => {'2' => ''}}
@@ -929,10 +929,10 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal 1, journal.details.size
   end
 
-  def test_bullk_edit_should_send_a_notification
+  def test_bullk_update_should_send_a_notification
     @request.session[:user_id] = 2
     ActionMailer::Base.deliveries.clear
-    post(:bulk_edit,
+    post(:bulk_update,
          {
            :ids => [1, 2],
            :notes => 'Bulk editing',
@@ -947,10 +947,10 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
-  def test_bulk_edit_status
+  def test_bulk_update_status
     @request.session[:user_id] = 2
     # update issues priority
-    post :bulk_edit, :ids => [1, 2], :notes => 'Bulk editing status',
+    post :bulk_update, :ids => [1, 2], :notes => 'Bulk editing status',
                                      :issue => {:priority_id => '',
                                                 :assigned_to_id => '',
                                                 :status_id => '5'}
@@ -960,10 +960,10 @@ class IssuesControllerTest < ActionController::TestCase
     assert issue.closed?
   end
 
-  def test_bulk_edit_custom_field
+  def test_bulk_update_custom_field
     @request.session[:user_id] = 2
     # update issues priority
-    post :bulk_edit, :ids => [1, 2], :notes => 'Bulk editing custom field',
+    post :bulk_update, :ids => [1, 2], :notes => 'Bulk editing custom field',
                                      :issue => {:priority_id => '',
                                                 :assigned_to_id => '',
                                                 :custom_field_values => {'2' => '777'}}
@@ -978,20 +978,20 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal '777', journal.details.first.value
   end
 
-  def test_bulk_unassign
+  def test_bulk_update_unassign
     assert_not_nil Issue.find(2).assigned_to
     @request.session[:user_id] = 2
     # unassign issues
-  post :bulk_edit, :ids => [1, 2], :notes => 'Bulk unassigning', :issue => {:assigned_to_id => 'none'}
+    post :bulk_update, :ids => [1, 2], :notes => 'Bulk unassigning', :issue => {:assigned_to_id => 'none'}
     assert_response 302
     # check that the issues were updated
     assert_nil Issue.find(2).assigned_to
   end
   
-  def test_post_bulk_edit_should_allow_fixed_version_to_be_set_to_a_subproject
+  def test_post_bulk_update_should_allow_fixed_version_to_be_set_to_a_subproject
     @request.session[:user_id] = 2
 
-    post :bulk_edit, :ids => [1,2], :issue => {:fixed_version_id => 4}
+    post :bulk_update, :ids => [1,2], :issue => {:fixed_version_id => 4}
 
     assert_response :redirect
     issues = Issue.find([1,2])
@@ -1001,17 +1001,17 @@ class IssuesControllerTest < ActionController::TestCase
     end
   end
 
-  def test_post_bulk_edit_should_redirect_back_using_the_back_url_parameter
+  def test_post_bulk_update_should_redirect_back_using_the_back_url_parameter
     @request.session[:user_id] = 2
-    post :bulk_edit, :ids => [1,2], :back_url => '/issues'
+    post :bulk_update, :ids => [1,2], :back_url => '/issues'
 
     assert_response :redirect
     assert_redirected_to '/issues'
   end
 
-  def test_post_bulk_edit_should_not_redirect_back_using_the_back_url_parameter_off_the_host
+  def test_post_bulk_update_should_not_redirect_back_using_the_back_url_parameter_off_the_host
     @request.session[:user_id] = 2
-    post :bulk_edit, :ids => [1,2], :back_url => 'http://google.com'
+    post :bulk_update, :ids => [1,2], :back_url => 'http://google.com'
 
     assert_response :redirect
     assert_redirected_to :controller => 'issues', :action => 'index', :project_id => Project.find(1).identifier

test/integration/routing_test.rb

@@ -108,6 +108,9 @@ class RoutingTest < ActionController::IntegrationTest
     should_route :post, "/issues/context_menu", :controller => 'context_menus', :action => 'issues'
 
     should_route :get, "/issues/changes", :controller => 'journals', :action => 'index'
+
+    should_route :get, "/issues/bulk_edit", :controller => 'issues', :action => 'bulk_edit'
+    should_route :post, "/issues/bulk_edit", :controller => 'issues', :action => 'bulk_update'
   end
 
   context "issue categories" do