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

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

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

app/views/issues/bulk_edit.html.erb

@@ -77,6 +77,13 @@
 </p>
 <% end %>
 
+<% if @copy && @subtasks_present %>
+<p>
+  <label for='copy_subtasks'><%= l(:label_copy_subtasks) %></label>
+  <%= check_box_tag 'copy_subtasks', '1', true %>
+</p>
+<% end %>
+
 <%= call_hook(:view_issues_bulk_edit_details_bottom, { :issues => @issues }) %>
 </div>
 

app/views/issues/new.html.erb

@@ -17,6 +17,12 @@
       <%= check_box_tag 'copy_attachments', '1', @copy_attachments %>
     </p>
     <% end %>
+    <% if @copy_from && !@copy_from.leaf? %>
+    <p>
+      <label for="copy_subtasks"><%= l(:label_copy_subtasks) %></label>
+      <%= check_box_tag 'copy_subtasks', '1', @copy_subtasks %>
+    </p>
+    <% end %>
 
     <p id="attachments_form"><label><%= l(:label_attachment_plural) %></label><%= render :partial => 'attachments/form', :locals => {:container => @issue} %></p>
 

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

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

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'

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!

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