Fixed that copying an issue as a child of itself creates an extra issue (#13328).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11513 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 5ec33a3dfe898a887ce6655e28247b3b8703dc97..96e0326e27e39f48945ce59ab0cc8a97e2b8132c 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -1147,20 +1147,27 @@ class Issue < ActiveRecord::Base
     end
 
     unless @copied_from.leaf? || @copy_options[:subtasks] == false
-      @copied_from.children.each do |child|
+      copy_options = (@copy_options || {}).merge(:subtasks => false)
+      copied_issue_ids = {@copied_from.id => self.id}
+      @copied_from.reload.descendants.reorder("#{Issue.table_name}.lft").each do |child|
+        # Do not copy self when copying an issue as a descendant of the copied issue
+        next if child == self
+        # Do not copy subtasks of issues that were not copied
+        next unless copied_issue_ids[child.parent_id]
+        # Do not copy subtasks that are not visible to avoid potential disclosure of private data
         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 = Issue.new.copy_from(child, copy_options)
         copy.author = author
         copy.project = project
-        copy.parent_issue_id = id
-        # Children subtasks are copied recursively
+        copy.parent_issue_id = copied_issue_ids[child.parent_id]
         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
+          next
         end
+        copied_issue_ids[child.id] = copy.id
       end
     end
     @after_create_from_copy_handled = true

diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 7f6d7819ceca9b6161048ba309a81493ad1b83a6..b7ace0d56f9f378213002af14f5c6c74b942143d 100644 (file)
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -849,6 +849,49 @@ class IssueTest < ActiveSupport::TestCase
     assert_equal copy.author, child_copy.author
   end
 
+  def test_copy_as_a_child_of_copied_issue_should_not_copy_itself
+    parent = Issue.generate!
+    child1 = Issue.generate!(:parent_issue_id => parent.id, :subject => 'Child 1')
+    child2 = Issue.generate!(:parent_issue_id => parent.id, :subject => 'Child 2')
+
+    copy = parent.reload.copy
+    copy.parent_issue_id = parent.id
+    copy.author = User.find(7)
+    assert_difference 'Issue.count', 3 do
+      assert copy.save
+    end
+    parent.reload
+    copy.reload
+    assert_equal parent, copy.parent
+    assert_equal 3, parent.children.count
+    assert_equal 5, parent.descendants.count
+    assert_equal 2, copy.children.count
+    assert_equal 2, copy.descendants.count
+  end
+
+  def test_copy_as_a_descendant_of_copied_issue_should_not_copy_itself
+    parent = Issue.generate!
+    child1 = Issue.generate!(:parent_issue_id => parent.id, :subject => 'Child 1')
+    child2 = Issue.generate!(:parent_issue_id => parent.id, :subject => 'Child 2')
+
+    copy = parent.reload.copy
+    copy.parent_issue_id = child1.id
+    copy.author = User.find(7)
+    assert_difference 'Issue.count', 3 do
+      assert copy.save
+    end
+    parent.reload
+    child1.reload
+    copy.reload
+    assert_equal child1, copy.parent
+    assert_equal 2, parent.children.count
+    assert_equal 5, parent.descendants.count
+    assert_equal 1, child1.children.count
+    assert_equal 3, child1.descendants.count
+    assert_equal 2, copy.children.count
+    assert_equal 2, copy.descendants.count
+  end
+
   def test_copy_should_copy_subtasks_to_target_project
     issue = Issue.generate_with_descendants!