From aec989707edc47161192eba74f1a88123b12360a Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 15 Mar 2008 08:27:38 +0000 Subject: [PATCH] Workflow copy: * added the ability the copy an existing workflow when creating a new role (closes #841) * use a single raw insert statement to copy tracker/role workflow rather than instantiating hundreds/thousands of objects git-svn-id: http://redmine.rubyforge.org/svn/trunk@1252 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/roles_controller.rb | 5 ++ app/controllers/trackers_controller.rb | 8 +-- app/models/role.rb | 13 +++- app/models/tracker.rb | 13 +++- app/views/roles/_form.rhtml | 4 ++ app/views/trackers/_form.rhtml | 5 +- test/functional/roles_controller_test.rb | 92 ++++++++++++++++++++++++ test/unit/role_test.rb | 33 +++++++++ test/unit/tracker_test.rb | 33 +++++++++ 9 files changed, 196 insertions(+), 10 deletions(-) create mode 100644 test/functional/roles_controller_test.rb create mode 100644 test/unit/role_test.rb create mode 100644 test/unit/tracker_test.rb diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 4796bb7f..9fdd9701 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -36,10 +36,15 @@ class RolesController < ApplicationController # Prefills the form with 'Non member' role permissions @role = Role.new(params[:role] || {:permissions => Role.non_member.permissions}) if request.post? && @role.save + # workflow copy + if !params[:copy_workflow_from].blank? && (copy_from = Role.find_by_id(params[:copy_workflow_from])) + @role.workflows.copy(copy_from) + end flash[:notice] = l(:notice_successful_create) redirect_to :action => 'list' end @permissions = @role.setable_permissions + @roles = Role.find :all, :order => 'builtin, position' end def edit diff --git a/app/controllers/trackers_controller.rb b/app/controllers/trackers_controller.rb index 46edea54..3d7dbd5c 100644 --- a/app/controllers/trackers_controller.rb +++ b/app/controllers/trackers_controller.rb @@ -37,16 +37,12 @@ class TrackersController < ApplicationController if request.post? and @tracker.save # workflow copy if !params[:copy_workflow_from].blank? && (copy_from = Tracker.find_by_id(params[:copy_workflow_from])) - Workflow.transaction do - copy_from.workflows.find(:all, :include => [:role, :old_status, :new_status]).each do |w| - Workflow.create(:tracker_id => @tracker.id, :role => w.role, :old_status => w.old_status, :new_status => w.new_status) - end - end + @tracker.workflows.copy(copy_from) end flash[:notice] = l(:notice_successful_create) redirect_to :action => 'list' end - @trackers = Tracker.find :all + @trackers = Tracker.find :all, :order => 'position' end def edit diff --git a/app/models/role.rb b/app/models/role.rb index 015146dc..6f1fb476 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -21,7 +21,18 @@ class Role < ActiveRecord::Base BUILTIN_ANONYMOUS = 2 before_destroy :check_deletable - has_many :workflows, :dependent => :delete_all + has_many :workflows, :dependent => :delete_all do + def copy(role) + raise "Can not copy workflow from a #{role.class}" unless role.is_a?(Role) + raise "Can not copy workflow from/to an unsaved role" if proxy_owner.new_record? || role.new_record? + clear + connection.insert "INSERT INTO workflows (tracker_id, old_status_id, new_status_id, role_id)" + + " SELECT tracker_id, old_status_id, new_status_id, #{proxy_owner.id}" + + " FROM workflows" + + " WHERE role_id = #{role.id}" + end + end + has_many :members acts_as_list diff --git a/app/models/tracker.rb b/app/models/tracker.rb index 07253a22..ecee908e 100644 --- a/app/models/tracker.rb +++ b/app/models/tracker.rb @@ -18,7 +18,18 @@ class Tracker < ActiveRecord::Base before_destroy :check_integrity has_many :issues - has_many :workflows, :dependent => :delete_all + has_many :workflows, :dependent => :delete_all do + def copy(tracker) + raise "Can not copy workflow from a #{tracker.class}" unless tracker.is_a?(Tracker) + raise "Can not copy workflow from/to an unsaved tracker" if proxy_owner.new_record? || tracker.new_record? + clear + connection.insert "INSERT INTO workflows (tracker_id, old_status_id, new_status_id, role_id)" + + " SELECT #{proxy_owner.id}, old_status_id, new_status_id, role_id" + + " FROM workflows" + + " WHERE tracker_id = #{tracker.id}" + end + end + has_and_belongs_to_many :projects has_and_belongs_to_many :custom_fields, :class_name => 'IssueCustomField', :join_table => "#{table_name_prefix}custom_fields_trackers#{table_name_suffix}", :association_foreign_key => 'custom_field_id' acts_as_list diff --git a/app/views/roles/_form.rhtml b/app/views/roles/_form.rhtml index b77cbacd..58dc2af4 100644 --- a/app/views/roles/_form.rhtml +++ b/app/views/roles/_form.rhtml @@ -4,6 +4,10 @@

<%= f.text_field :name, :required => true %>

<%= f.check_box :assignable %>

+<% if @role.new_record? && @roles.any? %> +

+<%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name)) %>

+<% end %>
<% end %> diff --git a/app/views/trackers/_form.rhtml b/app/views/trackers/_form.rhtml index 26a34443..856b70bb 100644 --- a/app/views/trackers/_form.rhtml +++ b/app/views/trackers/_form.rhtml @@ -4,8 +4,9 @@

<%= f.text_field :name, :required => true %>

<%= f.check_box :is_in_chlog %>

<%= f.check_box :is_in_roadmap %>

-<% if @tracker.new_record? %> -

<%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@trackers, :id, :name)) %>

+<% if @tracker.new_record? && @trackers.any? %> +

+<%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@trackers, :id, :name)) %>

<% end %> diff --git a/test/functional/roles_controller_test.rb b/test/functional/roles_controller_test.rb new file mode 100644 index 00000000..3c245edf --- /dev/null +++ b/test/functional/roles_controller_test.rb @@ -0,0 +1,92 @@ +# redMine - project management software +# Copyright (C) 2006-2007 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.dirname(__FILE__) + '/../test_helper' +require 'roles_controller' + +# Re-raise errors caught by the controller. +class RolesController; def rescue_action(e) raise e end; end + +class RolesControllerTest < Test::Unit::TestCase + fixtures :roles, :users, :members, :workflows + + def setup + @controller = RolesController.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + User.current = nil + @request.session[:user_id] = 1 # admin + end + + def test_get_new + get :new + assert_response :success + assert_template 'new' + end + + def test_post_new_with_validaton_failure + post :new, :role => {:name => '', + :permissions => ['add_issues', 'edit_issues', 'log_time', ''], + :assignable => '0'} + + assert_response :success + assert_template 'new' + assert_tag :tag => 'div', :attributes => { :id => 'errorExplanation' } + end + + def test_post_new_without_workflow_copy + post :new, :role => {:name => 'RoleWithoutWorkflowCopy', + :permissions => ['add_issues', 'edit_issues', 'log_time', ''], + :assignable => '0'} + + assert_redirected_to 'roles/list' + role = Role.find_by_name('RoleWithoutWorkflowCopy') + assert_not_nil role + assert_equal [:add_issues, :edit_issues, :log_time], role.permissions + assert !role.assignable? + end + + def test_post_new_with_workflow_copy + post :new, :role => {:name => 'RoleWithWorkflowCopy', + :permissions => ['add_issues', 'edit_issues', 'log_time', ''], + :assignable => '0'}, + :copy_workflow_from => '1' + + assert_redirected_to 'roles/list' + role = Role.find_by_name('RoleWithWorkflowCopy') + assert_not_nil role + assert_equal Role.find(1).workflows.size, role.workflows.size + end + + def test_get_edit + get :edit, :id => 1 + assert_response :success + assert_template 'edit' + assert_equal Role.find(1), assigns(:role) + end + + def test_post_edit + post :edit, :id => 1, + :role => {:name => 'Manager', + :permissions => ['edit_project', ''], + :assignable => '0'} + + assert_redirected_to 'roles/list' + role = Role.find(1) + assert_equal [:edit_project], role.permissions + end +end diff --git a/test/unit/role_test.rb b/test/unit/role_test.rb new file mode 100644 index 00000000..5e0d1675 --- /dev/null +++ b/test/unit/role_test.rb @@ -0,0 +1,33 @@ +# redMine - project management software +# Copyright (C) 2006-2008 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.dirname(__FILE__) + '/../test_helper' + +class RoleTest < Test::Unit::TestCase + fixtures :roles, :workflows + + def test_copy_workflows + source = Role.find(1) + assert_equal 90, source.workflows.size + + target = Role.new(:name => 'Target') + assert target.save + assert target.workflows.copy(source) + target.reload + assert_equal 90, target.workflows.size + end +end diff --git a/test/unit/tracker_test.rb b/test/unit/tracker_test.rb new file mode 100644 index 00000000..7adacef3 --- /dev/null +++ b/test/unit/tracker_test.rb @@ -0,0 +1,33 @@ +# redMine - project management software +# Copyright (C) 2006-2008 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.dirname(__FILE__) + '/../test_helper' + +class TrackerTest < Test::Unit::TestCase + fixtures :trackers, :workflows + + def test_copy_workflows + source = Tracker.find(1) + assert_equal 90, source.workflows.size + + target = Tracker.new(:name => 'Target') + assert target.save + assert target.workflows.copy(source) + target.reload + assert_equal 90, target.workflows.size + end +end