diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 37b4c0f4b..d396af6f3 100644 --- a/app/controllers/workflows_controller.rb +++ b/app/controllers/workflows_controller.rb @@ -20,6 +20,7 @@ class WorkflowsController < ApplicationController layout 'admin' self.main_menu = false + before_action :find_trackers_roles_and_statuses_for_edit, only: [:edit, :update, :permissions, :update_permissions] before_action :require_admin @@ -30,21 +31,6 @@ class WorkflowsController < ApplicationController end def edit - find_trackers_roles_and_statuses_for_edit - - if request.post? && @roles && @trackers && params[:transitions] - transitions = params[:transitions].deep_dup - transitions.each do |old_status_id, transitions_by_new_status| - transitions_by_new_status.each do |new_status_id, transition_by_rule| - transition_by_rule.reject! {|rule, transition| transition == 'no_change'} - end - end - WorkflowTransition.replace_transitions(@trackers, @roles, transitions) - flash[:notice] = l(:notice_successful_update) - redirect_to_referer_or workflows_edit_path - return - end - if @trackers && @roles && @statuses.any? workflows = WorkflowTransition. where(:role_id => @roles.map(&:id), :tracker_id => @trackers.map(&:id)). @@ -56,20 +42,21 @@ class WorkflowsController < ApplicationController end end - def permissions - find_trackers_roles_and_statuses_for_edit - - if request.post? && @roles && @trackers && params[:permissions] - permissions = params[:permissions].deep_dup - permissions.each do |field, rule_by_status_id| - rule_by_status_id.reject! {|status_id, rule| rule == 'no_change'} + def update + if @roles && @trackers && params[:transitions] + transitions = params[:transitions].deep_dup + transitions.each do |old_status_id, transitions_by_new_status| + transitions_by_new_status.each do |new_status_id, transition_by_rule| + transition_by_rule.reject! {|rule, transition| transition == 'no_change'} + end end - WorkflowPermission.replace_permissions(@trackers, @roles, permissions) + WorkflowTransition.replace_transitions(@trackers, @roles, transitions) flash[:notice] = l(:notice_successful_update) - redirect_to_referer_or workflows_permissions_path - return end + redirect_to_referer_or edit_workflows_path + end + def permissions if @roles && @trackers @fields = (Tracker::CORE_FIELDS_ALL - @trackers.map(&:disabled_core_fields).reduce(:&)).map {|field| [field, l("field_"+field.sub(/_id$/, ''))]} @custom_fields = @trackers.map(&:custom_fields).flatten.uniq.sort @@ -78,10 +65,46 @@ class WorkflowsController < ApplicationController end end + def update_permissions + if @roles && @trackers && params[:permissions] + permissions = params[:permissions].deep_dup + permissions.each do |field, rule_by_status_id| + rule_by_status_id.reject! {|status_id, rule| rule == 'no_change'} + end + WorkflowPermission.replace_permissions(@trackers, @roles, permissions) + flash[:notice] = l(:notice_successful_update) + end + redirect_to_referer_or permissions_workflows_path + end + def copy + find_sources_and_targets + end + + def duplicate + find_sources_and_targets + if params[:source_tracker_id].blank? || params[:source_role_id].blank? || + (@source_tracker.nil? && @source_role.nil?) + flash.now[:error] = l(:error_workflow_copy_source) + render :copy + elsif @target_trackers.blank? || @target_roles.blank? + flash.now[:error] = l(:error_workflow_copy_target) + render :copy + else + WorkflowRule.copy(@source_tracker, @source_role, @target_trackers, @target_roles) + flash[:notice] = l(:notice_successful_update) + redirect_to copy_workflows_path( + :source_tracker_id => @source_tracker, + :source_role_id => @source_role + ) + end + end + + private + + def find_sources_and_targets @roles = Role.sorted.select(&:consider_workflow?) @trackers = Tracker.sorted - if params[:source_tracker_id].blank? || params[:source_tracker_id] == 'any' @source_tracker = nil else @@ -104,25 +127,8 @@ class WorkflowsController < ApplicationController else Role.where(:id => params[:target_role_ids]).to_a end - if request.post? - if params[:source_tracker_id].blank? || params[:source_role_id].blank? || - (@source_tracker.nil? && @source_role.nil?) - flash.now[:error] = l(:error_workflow_copy_source) - elsif @target_trackers.blank? || @target_roles.blank? - flash.now[:error] = l(:error_workflow_copy_target) - else - WorkflowRule.copy(@source_tracker, @source_role, @target_trackers, @target_roles) - flash[:notice] = l(:notice_successful_update) - redirect_to( - workflows_copy_path(:source_tracker_id => @source_tracker, - :source_role_id => @source_role) - ) - end - end end - private - def find_trackers_roles_and_statuses_for_edit find_roles find_trackers diff --git a/app/views/issue_statuses/index.html.erb b/app/views/issue_statuses/index.html.erb index b4802c241..c9f120cbd 100644 --- a/app/views/issue_statuses/index.html.erb +++ b/app/views/issue_statuses/index.html.erb @@ -26,7 +26,7 @@ <% unless WorkflowTransition.where('old_status_id = ? OR new_status_id = ?', status.id, status.id).exists? %> - <%= l(:text_status_no_workflow) %> (<%= link_to l(:button_edit), workflows_edit_path(:used_statuses_only => 0) %>) + <%= l(:text_status_no_workflow) %> (<%= link_to l(:button_edit), edit_workflows_path(:used_statuses_only => 0) %>) <% end %> diff --git a/app/views/roles/index.html.erb b/app/views/roles/index.html.erb index bc8cc9cc7..f76e90466 100644 --- a/app/views/roles/index.html.erb +++ b/app/views/roles/index.html.erb @@ -18,7 +18,7 @@ <% unless role.builtin? || role.workflow_rules.exists? %> - <%= l(:text_role_no_workflow) %> (<%= link_to l(:button_edit), workflows_edit_path(:role_id => role) %>) + <%= l(:text_role_no_workflow) %> (<%= link_to l(:button_edit), edit_workflows_path(:role_id => role) %>) <% end %> @@ -36,4 +36,4 @@ <%= javascript_tag do %> $(function() { $("table.roles tbody").positionedItems({items: ".givable"}); }); -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/trackers/index.html.erb b/app/views/trackers/index.html.erb index 85d3249fc..84093e943 100644 --- a/app/views/trackers/index.html.erb +++ b/app/views/trackers/index.html.erb @@ -22,7 +22,7 @@ <% unless tracker.workflow_rules.exists? %> - <%= l(:text_tracker_no_workflow) %> (<%= link_to l(:button_edit), workflows_edit_path(:tracker_id => tracker) %>) + <%= l(:text_tracker_no_workflow) %> (<%= link_to l(:button_edit), edit_workflows_path(:tracker_id => tracker) %>) <% end %> diff --git a/app/views/workflows/copy.html.erb b/app/views/workflows/copy.html.erb index 78997caf5..cd270a8db 100644 --- a/app/views/workflows/copy.html.erb +++ b/app/views/workflows/copy.html.erb @@ -1,6 +1,6 @@ -<%= title [l(:label_workflow), workflows_edit_path], l(:button_copy) %> +<%= title [l(:label_workflow), edit_workflows_path], l(:button_copy) %> -<%= form_tag({}, :id => 'workflow_copy_form') do %> +<%= form_tag duplicate_workflows_path, method: :post, id: 'workflow_copy_form' do %>
<%= l(:label_copy_source) %>

diff --git a/app/views/workflows/edit.html.erb b/app/views/workflows/edit.html.erb index c247097a4..df4507be2 100644 --- a/app/views/workflows/edit.html.erb +++ b/app/views/workflows/edit.html.erb @@ -4,8 +4,8 @@

@@ -32,7 +32,7 @@ <% end %> <% if @trackers && @roles && @statuses.any? %> - <%= form_tag({}, :id => 'workflow_form' ) do %> + <%= form_tag workflows_path, method: :patch, id: 'workflow_form' do %> <%= @trackers.map {|tracker| hidden_field_tag 'tracker_id[]', tracker.id, :id => nil}.join.html_safe %> <%= @roles.map {|role| hidden_field_tag 'role_id[]', role.id, :id => nil}.join.html_safe %> <%= hidden_field_tag 'used_statuses_only', params[:used_statuses_only], :id => nil %> diff --git a/app/views/workflows/index.html.erb b/app/views/workflows/index.html.erb index 66785a40c..659b55c25 100644 --- a/app/views/workflows/index.html.erb +++ b/app/views/workflows/index.html.erb @@ -1,4 +1,4 @@ -<%= title [l(:label_workflow), workflows_edit_path], l(:field_summary) %> +<%= title [l(:label_workflow), edit_workflows_path], l(:field_summary) %> <% if @roles.empty? || @trackers.empty? %>

<%= l(:label_no_data) %>

diff --git a/app/views/workflows/permissions.html.erb b/app/views/workflows/permissions.html.erb index 28a671052..806d557dc 100644 --- a/app/views/workflows/permissions.html.erb +++ b/app/views/workflows/permissions.html.erb @@ -4,8 +4,8 @@
@@ -30,7 +30,7 @@ <% end %> <% if @trackers && @roles && @statuses.any? %> - <%= form_tag({}, :id => 'workflow_form' ) do %> + <%= form_tag update_permissions_workflows_path, method: :patch, id: 'workflow_form' do %> <%= @trackers.map {|tracker| hidden_field_tag 'tracker_id[]', tracker.id, :id => nil}.join.html_safe %> <%= @roles.map {|role| hidden_field_tag 'role_id[]', role.id, :id => nil}.join.html_safe %> <%= hidden_field_tag 'used_statuses_only', params[:used_statuses_only], :id => nil %> diff --git a/config/routes.rb b/config/routes.rb index a01456dba..be12e504e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -372,10 +372,17 @@ Rails.application.routes.draw do end end - match 'workflows', :controller => 'workflows', :action => 'index', :via => :get - match 'workflows/edit', :controller => 'workflows', :action => 'edit', :via => [:get, :post] - match 'workflows/permissions', :controller => 'workflows', :action => 'permissions', :via => [:get, :post] - match 'workflows/copy', :controller => 'workflows', :action => 'copy', :via => [:get, :post] + resources :workflows, only: [:index] do + collection do + get 'edit' + patch 'update' + get 'permissions' + patch 'update_permissions' + get 'copy' + post 'duplicate' + end + end + match 'settings', :controller => 'settings', :action => 'index', :via => :get match 'settings/edit', :controller => 'settings', :action => 'edit', :via => [:get, :post] match 'settings/plugin/:id', :controller => 'settings', :action => 'plugin', :via => [:get, :post], :as => 'plugin_settings' diff --git a/test/functional/workflows_controller_test.rb b/test/functional/workflows_controller_test.rb index cdb1f320f..e4bdd7084 100644 --- a/test/functional/workflows_controller_test.rb +++ b/test/functional/workflows_controller_test.rb @@ -134,7 +134,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_edit WorkflowTransition.delete_all - post :edit, :params => { + patch :update, :params => { :role_id => 2, :tracker_id => 1, :transitions => { @@ -152,7 +152,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_edit_with_allowed_statuses_for_new_issues WorkflowTransition.delete_all - post :edit, :params => { + patch :update, :params => { :role_id => 2, :tracker_id => 1, :transitions => { @@ -169,7 +169,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_edit_with_additional_transitions WorkflowTransition.delete_all - post :edit, :params => { + patch :update, :params => { :role_id => 2, :tracker_id => 1, :transitions => { @@ -346,7 +346,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_permissions WorkflowPermission.delete_all - post :permissions, :params => { + patch :update_permissions, :params => { :role_id => 1, :tracker_id => 2, :permissions => { @@ -389,7 +389,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_copy_one_to_one source_transitions = status_transitions(:tracker_id => 1, :role_id => 2) - post :copy, :params => { + post :duplicate, :params => { :source_tracker_id => '1', :source_role_id => '2', :target_tracker_ids => ['3'], :target_role_ids => ['1'] } @@ -400,7 +400,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_copy_one_to_many source_transitions = status_transitions(:tracker_id => 1, :role_id => 2) - post :copy, :params => { + post :duplicate, :params => { :source_tracker_id => '1', :source_role_id => '2', :target_tracker_ids => ['2', '3'], :target_role_ids => ['1', '3'] } @@ -415,7 +415,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest source_t2 = status_transitions(:tracker_id => 2, :role_id => 2) source_t3 = status_transitions(:tracker_id => 3, :role_id => 2) - post :copy, :params => { + post :duplicate, :params => { :source_tracker_id => 'any', :source_role_id => '2', :target_tracker_ids => ['2', '3'], :target_role_ids => ['1', '3'] } @@ -428,7 +428,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_copy_with_incomplete_source_specification_should_fail assert_no_difference 'WorkflowRule.count' do - post :copy, :params => { + post :duplicate, :params => { :source_tracker_id => '', :source_role_id => '2', :target_tracker_ids => ['2', '3'], :target_role_ids => ['1', '3'] } @@ -439,7 +439,7 @@ class WorkflowsControllerTest < Redmine::ControllerTest def test_post_copy_with_incomplete_target_specification_should_fail assert_no_difference 'WorkflowRule.count' do - post :copy, :params => { + post :duplicate, :params => { :source_tracker_id => '1', :source_role_id => '2', :target_tracker_ids => ['2', '3'] } diff --git a/test/integration/routing/workflows_test.rb b/test/integration/routing/workflows_test.rb index ca6b16e48..de0e7d2a6 100644 --- a/test/integration/routing/workflows_test.rb +++ b/test/integration/routing/workflows_test.rb @@ -23,12 +23,12 @@ class RoutingWorkflowsTest < Redmine::RoutingTest def test_workflows should_route 'GET /workflows' => 'workflows#index' should_route 'GET /workflows/edit' => 'workflows#edit' - should_route 'POST /workflows/edit' => 'workflows#edit' + should_route 'PATCH /workflows/update' => 'workflows#update' should_route 'GET /workflows/permissions' => 'workflows#permissions' - should_route 'POST /workflows/permissions' => 'workflows#permissions' + should_route 'PATCH /workflows/update_permissions' => 'workflows#update_permissions' should_route 'GET /workflows/copy' => 'workflows#copy' - should_route 'POST /workflows/copy' => 'workflows#copy' + should_route 'POST /workflows/duplicate' => 'workflows#duplicate' end end