From 80256cf298e669e5b63b86594f6d9607e830cc80 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Tue, 24 Aug 2010 15:27:12 +0000 Subject: [PATCH] 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 | 45 ++++++++++++----------- app/views/issues/bulk_edit.rhtml | 2 +- config/routes.rb | 1 + lib/redmine.rb | 2 +- test/functional/issues_controller_test.rb | 32 ++++++++-------- test/integration/routing_test.rb | 3 ++ 6 files changed, 46 insertions(+), 39 deletions(-) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index dc1cef492dd..29b13136050 100644 --- a/app/controllers/issues_controller.rb +++ b/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 @@ def update # 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 diff --git a/app/views/issues/bulk_edit.rhtml b/app/views/issues/bulk_edit.rhtml index b01128840ba..5fdfd58a66b 100644 --- a/app/views/issues/bulk_edit.rhtml +++ b/app/views/issues/bulk_edit.rhtml @@ -2,7 +2,7 @@ -<% form_tag() do %> +<% form_tag(:action => 'bulk_update') do %> <%= @issues.collect {|i| hidden_field_tag('ids[]', i.id)}.join %>
diff --git a/config/routes.rb b/config/routes.rb index b480a490000..4e1f9d7d509 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -132,6 +132,7 @@ 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+/ diff --git a/lib/redmine.rb b/lib/redmine.rb index b0a5a9f09e4..e1f1e2e16d2 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -66,7 +66,7 @@ :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]} diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 7c00db70849..7d69db8ea5a 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -910,10 +910,10 @@ def test_get_bulk_edit 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 @@ def test_bulk_edit 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 @@ def test_bullk_edit_should_send_a_notification 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 @@ def test_bulk_edit_status 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 @@ def test_bulk_edit_custom_field 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 @@ def test_post_bulk_edit_should_allow_fixed_version_to_be_set_to_a_subproject 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 diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index b05a1285d1f..119b506ae68 100644 --- a/test/integration/routing_test.rb +++ b/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