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

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 4f6f975cdd75c992e070511607e8754183157551..7fb4148ed9e461a5a0b94850946a9dad7a872c35 100644 (file)
--- 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 47388c36bf894f0bae53172d8c56eb6d095bbd5c..c216cba7c8263cbfdc76d53227dce39b0369c884 100644 (file)
--- a/app/views/issues/move.rhtml
+++ b/app/views/issues/move.rhtml
@@ -6,7 +6,7 @@
 <% end -%>
 </ul>
 
-<% 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 %>
 
 <div class="box tabular">

diff --git a/config/routes.rb b/config/routes.rb
index 58e620eabb6636f8fdcaf12ae643a70b1aa98bc8..2e8a145cbcf6022649650c003c06673bf87ede6f 100644 (file)
--- 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 b95166e767f91e7477ad8ce32a40a328b517701c..4196a5d7a22c5f73c8ffeb2acb04a8a3c26d3cd6 100644 (file)
--- 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 670fb2d7eb3a1e0f2847b050c457fbaadc8d4f3b..92e6fd30a18b18041292123ca071cf93f2ffb169 100644 (file)
--- 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 f8476df567cecd4dbc624c66848db9abc8824313..51f8e71f6e822aa2882b85292a73edc2b51eb17a 100644 (file)
--- 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'