From 041bca6ac90d0d53c6e29a729e84b66b4d87b040 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Fri, 6 Aug 2010 15:47:27 +0000 Subject: [PATCH] Refactor: start to split IssuesController#move into two separate actions. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3918 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 7 +++++- app/views/issues/move.rhtml | 2 +- config/routes.rb | 2 +- lib/redmine.rb | 2 +- test/functional/issues_controller_test.rb | 26 +++++++++++------------ test/integration/routing_test.rb | 2 +- 6 files changed, 23 insertions(+), 18 deletions(-) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 4f6f975cd..7fb4148ed 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, :reply] - before_filter :find_issues, :only => [:bulk_edit, :move, :destroy] + before_filter :find_issues, :only => [:bulk_edit, :move, :perform_move, :destroy] before_filter :find_project, :only => [:new, :create, :update_form, :preview, :auto_complete] before_filter :authorize, :except => [:index, :changes, :preview, :context_menu] before_filter :find_optional_project, :only => [:index, :changes] @@ -287,6 +287,11 @@ class IssuesController < ApplicationController end render :layout => false if request.xhr? end + + # TODO: more descriptive name? move to separate controller like IssueMovesController? + def perform_move + move + end def destroy @hours = TimeEntry.sum(:hours, :conditions => ['issue_id IN (?)', @issues]).to_f diff --git a/app/views/issues/move.rhtml b/app/views/issues/move.rhtml index 47388c36b..c216cba7c 100644 --- a/app/views/issues/move.rhtml +++ b/app/views/issues/move.rhtml @@ -6,7 +6,7 @@ <% end -%> -<% form_tag({}, :id => 'move_form') do %> +<% form_tag({:action => 'perform_move'}, :id => 'move_form') do %> <%= @issues.collect {|i| hidden_field_tag('ids[]', i.id)}.join %>
diff --git a/config/routes.rb b/config/routes.rb index 58e620eab..2e8a145cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -124,7 +124,7 @@ ActionController::Routing::Routes.draw do |map| issues_actions.connect 'projects/:project_id/issues/gantt', :controller => 'gantts', :action => 'show' issues_actions.connect 'projects/:project_id/issues/calendar', :controller => 'calendars', :action => 'show' issues_actions.connect 'issues/:id/quoted', :action => 'reply', :id => /\d+/ - issues_actions.connect 'issues/:id/:action', :action => /edit|move|destroy/, :id => /\d+/ + issues_actions.connect 'issues/:id/:action', :action => /edit|perform_move|destroy/, :id => /\d+/ issues_actions.connect 'issues.:format', :action => 'create', :format => /xml/ end issues_routes.with_options :conditions => {:method => :put} do |issues_actions| diff --git a/lib/redmine.rb b/lib/redmine.rb index b95166e76..4196a5d7a 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -69,7 +69,7 @@ Redmine::AccessControl.map do |map| map.permission :add_issue_notes, {:issues => [:edit, :update, :reply]} map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin - map.permission :move_issues, {:issues => :move}, :require => :loggedin + map.permission :move_issues, {:issues => [:move, :perform_move]}, :require => :loggedin map.permission :delete_issues, {:issues => :destroy}, :require => :member # Queries map.permission :manage_public_queries, {:queries => [:new, :edit, :destroy]}, :require => :member diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 670fb2d7e..92e6fd30a 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1038,22 +1038,22 @@ class IssuesControllerTest < ActionController::TestCase assert_redirected_to :controller => 'issues', :action => 'index', :project_id => Project.find(1).identifier end - def test_move_one_issue_to_another_project + def test_perform_move_one_issue_to_another_project @request.session[:user_id] = 2 - post :move, :id => 1, :new_project_id => 2, :tracker_id => '', :assigned_to_id => '', :status_id => '', :start_date => '', :due_date => '' + post :perform_move, :id => 1, :new_project_id => 2, :tracker_id => '', :assigned_to_id => '', :status_id => '', :start_date => '', :due_date => '' assert_redirected_to :action => 'index', :project_id => 'ecookbook' assert_equal 2, Issue.find(1).project_id end - def test_move_one_issue_to_another_project_should_follow_when_needed + def test_perform_move_one_issue_to_another_project_should_follow_when_needed @request.session[:user_id] = 2 - post :move, :id => 1, :new_project_id => 2, :follow => '1' + post :perform_move, :id => 1, :new_project_id => 2, :follow => '1' assert_redirected_to '/issues/1' end - def test_bulk_move_to_another_project + def test_bulk_perform_move_to_another_project @request.session[:user_id] = 2 - post :move, :ids => [1, 2], :new_project_id => 2 + post :perform_move, :ids => [1, 2], :new_project_id => 2 assert_redirected_to :action => 'index', :project_id => 'ecookbook' # Issues moved to project 2 assert_equal 2, Issue.find(1).project_id @@ -1063,9 +1063,9 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 2, Issue.find(2).tracker_id end - def test_bulk_move_to_another_tracker + def test_bulk_perform_move_to_another_tracker @request.session[:user_id] = 2 - post :move, :ids => [1, 2], :new_tracker_id => 2 + post :perform_move, :ids => [1, 2], :new_tracker_id => 2 assert_redirected_to :action => 'index', :project_id => 'ecookbook' assert_equal 2, Issue.find(1).tracker_id assert_equal 2, Issue.find(2).tracker_id @@ -1075,19 +1075,19 @@ class IssuesControllerTest < ActionController::TestCase @request.session[:user_id] = 2 assert_difference 'Issue.count', 2 do assert_no_difference 'Project.find(1).issues.count' do - post :move, :ids => [1, 2], :new_project_id => 2, :copy_options => {:copy => '1'} + post :perform_move, :ids => [1, 2], :new_project_id => 2, :copy_options => {:copy => '1'} end end assert_redirected_to 'projects/ecookbook/issues' end - context "#move via bulk copy" do + context "#perform_move via bulk copy" do should "allow not changing the issue's attributes" do @request.session[:user_id] = 2 issue_before_move = Issue.find(1) assert_difference 'Issue.count', 1 do assert_no_difference 'Project.find(1).issues.count' do - post :move, :ids => [1], :new_project_id => 2, :copy_options => {:copy => '1'}, :new_tracker_id => '', :assigned_to_id => '', :status_id => '', :start_date => '', :due_date => '' + post :perform_move, :ids => [1], :new_project_id => 2, :copy_options => {:copy => '1'}, :new_tracker_id => '', :assigned_to_id => '', :status_id => '', :start_date => '', :due_date => '' end end issue_after_move = Issue.first(:order => 'id desc', :conditions => {:project_id => 2}) @@ -1104,7 +1104,7 @@ class IssuesControllerTest < ActionController::TestCase @request.session[:user_id] = 2 assert_difference 'Issue.count', 2 do assert_no_difference 'Project.find(1).issues.count' do - post :move, :ids => [1, 2], :new_project_id => 2, :copy_options => {:copy => '1'}, :new_tracker_id => '', :assigned_to_id => 4, :status_id => 3, :start_date => '2009-12-01', :due_date => '2009-12-31' + post :perform_move, :ids => [1, 2], :new_project_id => 2, :copy_options => {:copy => '1'}, :new_tracker_id => '', :assigned_to_id => 4, :status_id => 3, :start_date => '2009-12-01', :due_date => '2009-12-31' end end @@ -1122,7 +1122,7 @@ class IssuesControllerTest < ActionController::TestCase def test_copy_to_another_project_should_follow_when_needed @request.session[:user_id] = 2 - post :move, :ids => [1], :new_project_id => 2, :copy_options => {:copy => '1'}, :follow => '1' + post :perform_move, :ids => [1], :new_project_id => 2, :copy_options => {:copy => '1'}, :follow => '1' issue = Issue.first(:order => 'id DESC') assert_redirected_to :controller => 'issues', :action => 'show', :id => issue end diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index f8476df56..51f8e71f6 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -86,7 +86,7 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/projects/23/issues/64/copy", :controller => 'issues', :action => 'new', :project_id => '23', :copy_from => '64' should_route :get, "/issues/1/move", :controller => 'issues', :action => 'move', :id => '1' - should_route :post, "/issues/1/move", :controller => 'issues', :action => 'move', :id => '1' + should_route :post, "/issues/1/perform_move", :controller => 'issues', :action => 'perform_move', :id => '1' should_route :post, "/issues/1/quoted", :controller => 'issues', :action => 'reply', :id => '1'