From 99bf8c95aba29aa2cde30ba8ff39d4d8a37ae9e5 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 20 Oct 2013 09:25:14 +0000 Subject: [PATCH] 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 --- app/models/issue.rb | 69 +++++++++++++++++------------- test/unit/issue_nested_set_test.rb | 35 +++++++++++++++ 2 files changed, 74 insertions(+), 30 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index fc25e6620..68381f65f 100644 --- 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,40 +1284,45 @@ 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 - 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 - # update former parent - recalculate_attributes_for(former_parent_id) if former_parent_id + 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) + 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 + # update former parent + recalculate_attributes_for(former_parent_id) if former_parent_id + end + def update_parent_attributes recalculate_attributes_for(parent_id) if parent_id end diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb index 3a53eb3af..3c611be28 100644 --- 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!