-
Notifications
You must be signed in to change notification settings - Fork 248
Open
Description
When using prepend_child or add_sibling to move a root-level node to become a child of another node, the remaining root nodes' order_value positions are not reordered, leaving gaps in the sequence.
Steps to Reproduce
Example 1: Using prepend_child
node_1 = Label.create(name: 'node_1') # order_value: 0
node_2 = Label.create(name: 'node_2') # order_value: 1
node_3 = Label.create(name: 'node_3') # order_value: 2
node_4 = Label.create(name: 'node_4') # order_value: 3
# Move node_2 as a child of node_3
node_3.prepend_child(node_2)
# Reload nodes
[node_1, node_2, node_3, node_4].each(&:reload)Example 2: Using add_sibling
node_1 = Label.create(name: 'node_1') # order_value: 0
node_2 = Label.create(name: 'node_2') # order_value: 1
node_3 = Label.create(name: 'node_3') # order_value: 2
node_4 = Label.create(name: 'node_4') # order_value: 3
# Create a child under node_3
child = Label.create(name: 'child', parent: node_3) # order_value: 0
# Move node_2 as a sibling of child (making it a child of node_3)
child.add_sibling(node_2)
# Reload nodes
[node_1, node_2, node_3, node_4].each(&:reload)Expected Behavior
Remaining root nodes should have sequential order_value without gaps:
- node_1.order_value → 0
- node_3.order_value → 1
- node_4.order_value → 2
Actual Behavior
Root nodes retain their original positions, creating a gap:
- node_1.order_value → 0
- node_3.order_value → 2
- node_4.order_value → 3
Root Cause
Both prepend_child and add_sibling method calls _ct_skip_sort_order_maintenance! which prevents the normal callback _ct_reorder_prior_siblings_if_parent_changed from running. This flag is necessary to avoid double-reordering when setting up the child's position, but it has the side effect of not reordering the old parent's remaining children.
Test Cases
# test/closure_tree/label_order_value_test.rb
test 'should reorder remaining root nodes when a root node becomes a child via prepend_child' do
node_1 = Label.create(name: 'node_1')
node_2 = Label.create(name: 'node_2')
node_3 = Label.create(name: 'node_3')
node_4 = Label.create(name: 'node_4')
# Verify initial positions
assert_equal 0, node_1.order_value
assert_equal 1, node_2.order_value
assert_equal 2, node_3.order_value
assert_equal 3, node_4.order_value
# Move node_2 as a child of node_3 using prepend_child
node_3.prepend_child(node_2)
# Reload all nodes to get updated positions
node_1.reload
node_2.reload
node_3.reload
node_4.reload
# Verify node_2 is now a child of node_3 with position 0
assert_equal node_3.id, node_2._ct_parent_id
assert_equal 0, node_2.order_value
# Verify remaining root nodes have sequential positions without gaps
assert_equal 0, node_1.order_value
assert_equal 1, node_3.order_value
assert_equal 2, node_4.order_value
end
test 'should reorder remaining root nodes when a root node becomes a child via add_sibling' do
node_1 = Label.create(name: 'node_1')
node_2 = Label.create(name: 'node_2')
node_3 = Label.create(name: 'node_3')
node_4 = Label.create(name: 'node_4')
# Create a child under node_3
child = Label.create(name: 'child', parent: node_3)
# Verify initial positions
assert_equal 0, node_1.order_value
assert_equal 1, node_2.order_value
assert_equal 2, node_3.order_value
assert_equal 3, node_4.order_value
assert_equal 0, child.order_value
# Move node_2 as a sibling of child (making it a child of node_3)
child.add_sibling(node_2)
# Reload all nodes to get updated positions
node_1.reload
node_2.reload
node_3.reload
node_4.reload
# Verify node_2 is now a child of node_3
assert_equal node_3.id, node_2._ct_parent_id
# Verify remaining root nodes have sequential positions without gaps
assert_equal 0, node_1.order_value
assert_equal 1, node_3.order_value
assert_equal 2, node_4.order_value
endvjain98Copilot