From: Jean-Philippe Lang Date: Sat, 8 Sep 2012 05:34:07 +0000 (+0000) Subject: Option to copy subtasks when copying issue(s) (#6965). X-Git-Tag: 2.1.0~45 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5003927f13f54850ca9eeac48e353df5e4e325a1;p=redmine.git Option to copy subtasks when copying issue(s) (#6965). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10327 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index e443fac55..c2ab10e61 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -221,6 +221,7 @@ class IssuesController < ApplicationController @categories = target_projects.map {|p| p.issue_categories}.reduce(:&) if @copy @attachments_present = @issues.detect {|i| i.attachments.any?}.present? + @subtasks_present = @issues.detect {|i| !i.leaf?}.present? end @safe_attributes = @issues.map(&:safe_attribute_names).reduce(:&) @@ -237,7 +238,10 @@ class IssuesController < ApplicationController @issues.each do |issue| issue.reload if @copy - issue = issue.copy({}, :attachments => params[:copy_attachments].present?) + issue = issue.copy({}, + :attachments => params[:copy_attachments].present?, + :subtasks => params[:copy_subtasks].present? + ) end journal = issue.init_journal(User.current, params[:notes]) issue.safe_attributes = attributes @@ -374,7 +378,8 @@ private begin @copy_from = Issue.visible.find(params[:copy_from]) @copy_attachments = params[:copy_attachments].present? || request.get? - @issue.copy_from(@copy_from, :attachments => @copy_attachments) + @copy_subtasks = params[:copy_subtasks].present? || request.get? + @issue.copy_from(@copy_from, :attachments => @copy_attachments, :subtasks => @copy_subtasks) rescue ActiveRecord::RecordNotFound render_404 return diff --git a/app/models/issue.rb b/app/models/issue.rb index c7358c0e5..f3851927e 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -77,6 +77,8 @@ class Issue < ActiveRecord::Base before_save :close_duplicates, :update_done_ratio_from_issue_status, :force_updated_on_change after_save {|issue| issue.send :after_project_change if !issue.id_changed? && issue.project_id_changed?} after_save :reschedule_following_issues, :update_nested_set_attributes, :update_parent_attributes, :create_journal + # Should be after_create but would be called before previous after_save callbacks + after_save :after_create_from_copy after_destroy :update_parent_attributes # Returns a SQL conditions string used to find all issues visible by the specified user @@ -169,6 +171,7 @@ class Issue < ActiveRecord::Base end end @copied_from = issue + @copy_options = options self end @@ -1000,6 +1003,30 @@ class Issue < ActiveRecord::Base end end + # Copies subtasks from the copied issue + def after_create_from_copy + return unless copy? + + unless @copied_from.leaf? || @copy_options[:subtasks] == false || @subtasks_copied + @copied_from.children.each do |child| + unless child.visible? + # Do not copy subtasks that are not visible to avoid potential disclosure of private data + logger.error "Subtask ##{child.id} was not copied during ##{@copied_from.id} copy because it is not visible to the current user" if logger + next + end + copy = Issue.new.copy_from(child, @copy_options) + copy.author = author + copy.project = project + copy.parent_issue_id = id + # Children subtasks are copied recursively + unless copy.save + logger.error "Could not copy subtask ##{child.id} while copying ##{@copied_from.id} to ##{id} due to validation errors: #{copy.errors.full_messages.join(', ')}" if logger + end + end + @subtasks_copied = true + end + end + def update_nested_set_attributes if root_id.nil? # issue was just created diff --git a/app/views/issues/bulk_edit.html.erb b/app/views/issues/bulk_edit.html.erb index 1312aa303..bec8e2987 100644 --- a/app/views/issues/bulk_edit.html.erb +++ b/app/views/issues/bulk_edit.html.erb @@ -77,6 +77,13 @@

<% end %> +<% if @copy && @subtasks_present %> +

+ + <%= check_box_tag 'copy_subtasks', '1', true %> +

+<% end %> + <%= call_hook(:view_issues_bulk_edit_details_bottom, { :issues => @issues }) %> diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb index 63a6e2496..45a258249 100644 --- a/app/views/issues/new.html.erb +++ b/app/views/issues/new.html.erb @@ -17,6 +17,12 @@ <%= check_box_tag 'copy_attachments', '1', @copy_attachments %>

<% end %> + <% if @copy_from && !@copy_from.leaf? %> +

+ + <%= check_box_tag 'copy_subtasks', '1', @copy_subtasks %> +

+ <% end %>

<%= render :partial => 'attachments/form', :locals => {:container => @issue} %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 8038b92c0..0133ce247 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -857,6 +857,7 @@ en: label_child_revision: Child label_export_options: "%{export_format} export options" label_copy_attachments: Copy attachments + label_copy_subtasks: Copy subtasks label_item_position: "%{position} of %{count}" label_completed_versions: Completed versions label_search_for_watchers: Search for watchers to add diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 4802d7605..7bc0d3a02 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -833,6 +833,7 @@ fr: label_issues_visibility_own: Demandes créées par ou assignées à l'utilisateur label_export_options: Options d'exportation %{export_format} label_copy_attachments: Copier les fichiers + label_copy_subtasks: Copier les sous-tâches label_item_position: "%{position} sur %{count}" label_completed_versions: Versions passées label_session_expiration: Expiration des sessions diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index f17402ec4..9488b5331 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -2268,6 +2268,14 @@ class IssuesControllerTest < ActionController::TestCase assert_no_tag 'input', :attributes => {:name => 'copy_attachments', :type => 'checkbox', :checked => 'checked', :value => '1'} end + def test_new_as_copy_with_subtasks_should_show_copy_subtasks_checkbox + @request.session[:user_id] = 2 + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + get :new, :project_id => 1, :copy_from => issue.id + + assert_select 'input[type=checkbox][name=copy_subtasks][checked=checked][value=1]' + end + def test_new_as_copy_with_invalid_issue_should_respond_with_404 @request.session[:user_id] = 2 get :new, :project_id => 1, :copy_from => 99999 @@ -2349,6 +2357,37 @@ class IssuesControllerTest < ActionController::TestCase assert_equal count + 1, copy.attachments.count end + def test_create_as_copy_should_copy_subtasks + @request.session[:user_id] = 2 + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + count = issue.descendants.count + + assert_difference 'Issue.count', count+1 do + assert_no_difference 'Journal.count' do + post :create, :project_id => 1, :copy_from => issue.id, + :issue => {:project_id => '1', :tracker_id => '3', :status_id => '1', :subject => 'Copy with subtasks'}, + :copy_subtasks => '1' + end + end + copy = Issue.where(:parent_id => nil).first(:order => 'id DESC') + assert_equal count, copy.descendants.count + assert_equal issue.descendants.map(&:subject).sort, copy.descendants.map(&:subject).sort + end + + def test_create_as_copy_without_copy_subtasks_option_should_not_copy_subtasks + @request.session[:user_id] = 2 + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + + assert_difference 'Issue.count', 1 do + assert_no_difference 'Journal.count' do + post :create, :project_id => 1, :copy_from => 3, + :issue => {:project_id => '1', :tracker_id => '3', :status_id => '1', :subject => 'Copy with subtasks'} + end + end + copy = Issue.where(:parent_id => nil).first(:order => 'id DESC') + assert_equal 0, copy.descendants.count + end + def test_create_as_copy_with_failure @request.session[:user_id] = 2 post :create, :project_id => 1, :copy_from => 1, @@ -3473,6 +3512,33 @@ class IssuesControllerTest < ActionController::TestCase end end + def test_bulk_copy_should_allow_not_copying_the_subtasks + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + @request.session[:user_id] = 2 + + assert_difference 'Issue.count', 1 do + post :bulk_update, :ids => [issue.id], :copy => '1', + :issue => { + :project_id => '' + } + end + end + + def test_bulk_copy_should_allow_copying_the_subtasks + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + count = issue.descendants.count + @request.session[:user_id] = 2 + + assert_difference 'Issue.count', count+1 do + post :bulk_update, :ids => [issue.id], :copy => '1', :copy_subtasks => '1', + :issue => { + :project_id => '' + } + end + copy = Issue.where(:parent_id => nil).order("id DESC").first + assert_equal count, copy.descendants.count + end + def test_bulk_copy_to_another_project_should_follow_when_needed @request.session[:user_id] = 2 post :bulk_update, :ids => [1], :copy => '1', :issue => {:project_id => 2}, :follow => '1' diff --git a/test/object_helpers.rb b/test/object_helpers.rb index 42dfdecda..0100a8bed 100644 --- a/test/object_helpers.rb +++ b/test/object_helpers.rb @@ -81,6 +81,15 @@ module ObjectHelpers issue end + # Generates an issue with some children and a grandchild + def Issue.generate_with_descendants!(project, attributes={}) + issue = Issue.generate_for_project!(project, attributes) + child = Issue.generate_for_project!(project, :subject => 'Child1', :parent_issue_id => issue.id) + Issue.generate_for_project!(project, :subject => 'Child2', :parent_issue_id => issue.id) + Issue.generate_for_project!(project, :subject => 'Child11', :parent_issue_id => child.id) + issue.reload + end + def Version.generate!(attributes={}) @generated_version_name ||= 'Version 0' @generated_version_name.succ! diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index a3df951a6..59f38dbd8 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -633,6 +633,41 @@ class IssueTest < ActiveSupport::TestCase assert_equal orig.status, issue.status end + def test_copy_should_copy_subtasks + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + + copy = issue.reload.copy + copy.author = User.find(7) + assert_difference 'Issue.count', 1+issue.descendants.count do + assert copy.save + end + copy.reload + assert_equal %w(Child1 Child2), copy.children.map(&:subject).sort + child_copy = copy.children.detect {|c| c.subject == 'Child1'} + assert_equal %w(Child11), child_copy.children.map(&:subject).sort + assert_equal copy.author, child_copy.author + end + + def test_copy_should_copy_subtasks_to_target_project + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + + copy = issue.copy(:project_id => 3) + assert_difference 'Issue.count', 1+issue.descendants.count do + assert copy.save + end + assert_equal [3], copy.reload.descendants.map(&:project_id).uniq + end + + def test_copy_should_not_copy_subtasks_twice_when_saving_twice + issue = Issue.generate_with_descendants!(Project.find(1), :subject => 'Parent') + + copy = issue.reload.copy + assert_difference 'Issue.count', 1+issue.descendants.count do + assert copy.save + assert copy.save + end + end + def test_should_not_call_after_project_change_on_creation issue = Issue.new(:project_id => 1, :tracker_id => 1, :status_id => 1, :subject => 'Test', :author_id => 1) issue.expects(:after_project_change).never