Fixed that issue nested set update is triggered even if parent is not changed (#15135).

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

diff --git a/app/models/issue.rb b/app/models/issue.rb
index fc25e66206fd913b62bfc6ea4b8609a9269b4693..68381f65f4f406c00d4ee0d364bfce8446293ae8 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -1105,6 +1105,10 @@ class Issue < ActiveRecord::Base
     s = arg.to_s.strip.presence
     if s && (m = s.match(%r{\A#?(\d+)\z})) && (@parent_issue = Issue.find_by_id(m[1]))
       @parent_issue.id
+      @invalid_parent_issue_id = nil
+    elsif s.blank?
+      @parent_issue = nil
+      @invalid_parent_issue_id = nil
     else
       @parent_issue = nil
       @invalid_parent_issue_id = arg
@@ -1280,38 +1284,43 @@ class Issue < ActiveRecord::Base
         move_to_child_of(@parent_issue)
       end
     elsif parent_issue_id != parent_id
-      former_parent_id = parent_id
-      # moving an existing issue
-      if @parent_issue && @parent_issue.root_id == root_id
-        # inside the same tree
+      update_nested_set_attributes_on_parent_change
+    end
+    remove_instance_variable(:@parent_issue) if instance_variable_defined?(:@parent_issue)
+  end
+
+  # Updates the nested set for when an existing issue is moved
+  def update_nested_set_attributes_on_parent_change
+    former_parent_id = parent_id
+    # moving an existing issue
+    if @parent_issue && @parent_issue.root_id == root_id
+      # inside the same tree
+      move_to_child_of(@parent_issue)
+    else
+      # to another tree
+      unless root?
+        move_to_right_of(root)
+      end
+      old_root_id = root_id
+      self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id )
+      target_maxright = nested_set_scope.maximum(right_column_name) || 0
+      offset = target_maxright + 1 - lft
+      Issue.update_all(["root_id = ?, lft = lft + ?, rgt = rgt + ?", root_id, offset, offset],
+                        ["root_id = ? AND lft >= ? AND rgt <= ? ", old_root_id, lft, rgt])
+      self[left_column_name] = lft + offset
+      self[right_column_name] = rgt + offset
+      if @parent_issue
         move_to_child_of(@parent_issue)
-      else
-        # to another tree
-        unless root?
-          move_to_right_of(root)
-        end
-        old_root_id = root_id
-        self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id )
-        target_maxright = nested_set_scope.maximum(right_column_name) || 0
-        offset = target_maxright + 1 - lft
-        Issue.update_all(["root_id = ?, lft = lft + ?, rgt = rgt + ?", root_id, offset, offset],
-                          ["root_id = ? AND lft >= ? AND rgt <= ? ", old_root_id, lft, rgt])
-        self[left_column_name] = lft + offset
-        self[right_column_name] = rgt + offset
-        if @parent_issue
-          move_to_child_of(@parent_issue)
-        end
       end
-      # delete invalid relations of all descendants
-      self_and_descendants.each do |issue|
-        issue.relations.each do |relation|
-          relation.destroy unless relation.valid?
-        end
+    end
+    # delete invalid relations of all descendants
+    self_and_descendants.each do |issue|
+      issue.relations.each do |relation|
+        relation.destroy unless relation.valid?
       end
-      # update former parent
-      recalculate_attributes_for(former_parent_id) if former_parent_id
     end
-    remove_instance_variable(:@parent_issue) if instance_variable_defined?(:@parent_issue)
+    # update former parent
+    recalculate_attributes_for(former_parent_id) if former_parent_id
   end
 
   def update_parent_attributes

diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb
index 3a53eb3af6c4d5ffca52a4f4f85ac4c1dcf54ab8..3c611be281140a4f7dbcf9a38672a12138d78028 100644 (file)
--- a/test/unit/issue_nested_set_test.rb
+++ b/test/unit/issue_nested_set_test.rb
@@ -166,6 +166,41 @@ class IssueNestedSetTest < ActiveSupport::TestCase
     assert_not_equal [], child.errors[:parent_issue_id]
   end
 
+  def test_updating_a_root_issue_should_not_trigger_update_nested_set_attributes_on_parent_change
+    issue = Issue.find(Issue.generate!.id)
+    issue.parent_issue_id = ""
+    issue.expects(:update_nested_set_attributes_on_parent_change).never
+    issue.save!
+  end
+
+  def test_updating_a_child_issue_should_not_trigger_update_nested_set_attributes_on_parent_change
+    issue = Issue.find(Issue.generate!(:parent_issue_id => 1).id)
+    issue.parent_issue_id = "1"
+    issue.expects(:update_nested_set_attributes_on_parent_change).never
+    issue.save!
+  end
+
+  def test_moving_a_root_issue_should_trigger_update_nested_set_attributes_on_parent_change
+    issue = Issue.find(Issue.generate!.id)
+    issue.parent_issue_id = "1"
+    issue.expects(:update_nested_set_attributes_on_parent_change).once
+    issue.save!
+  end
+
+  def test_moving_a_child_issue_to_another_parent_should_trigger_update_nested_set_attributes_on_parent_change
+    issue = Issue.find(Issue.generate!(:parent_issue_id => 1).id)
+    issue.parent_issue_id = "2"
+    issue.expects(:update_nested_set_attributes_on_parent_change).once
+    issue.save!
+  end
+
+  def test_moving_a_child_issue_to_root_should_trigger_update_nested_set_attributes_on_parent_change
+    issue = Issue.find(Issue.generate!(:parent_issue_id => 1).id)
+    issue.parent_issue_id = ""
+    issue.expects(:update_nested_set_attributes_on_parent_change).once
+    issue.save!
+  end
+
   def test_destroy_should_destroy_children
     issue1 = Issue.generate!
     issue2 = Issue.generate!