Handle scope changes when reordering siblings

Detect and handle changes to scoped attributes during saves so siblings are reordered correctly when an object changes scope. _ct_after_save now checks for scope changes and triggers reordering of the prior and current siblings. _ct_reorder_prior_siblings_if_parent_changed was updated to consider scope changes (and early-return for new records), use the previous scope values, and only act when the parent or scope actually changed. Added support methods previous_scope_values_from_instance and scope_changed? to build previous-scope conditions and detect scope changes for Symbol or Array scope configurations. Tests were added to verify reordering behavior when moving items between scopes and with multiple scope attributes.

Author: sampatbadhe

lib/closure_tree/hierarchy_maintenance.rb

@@ -34,8 +34,14 @@ def _ct_before_save
     end
 
     def _ct_after_save
+      scope_changed = _ct.order_is_numeric? && _ct.scope_changed?(self)
+
       if saved_changes[_ct.parent_column_name] || @was_new_record
         rebuild!
+      elsif scope_changed
+        # Scope changed without parent change - reorder old scope's siblings
+        _ct_reorder_prior_siblings_if_parent_changed
+        _ct_reorder_siblings
       elsif saved_changes[_ct.order_column_sym]
         _ct_reorder_siblings(saved_changes[_ct.order_column_sym].min)
       end

lib/closure_tree/numeric_deterministic_ordering.rb

@@ -12,11 +12,16 @@ module NumericDeterministicOrdering
     end
 
     def _ct_reorder_prior_siblings_if_parent_changed
-      return unless saved_change_to_attribute?(_ct.parent_column_name) && !@was_new_record
+      return if @was_new_record
+
+      parent_changed = saved_change_to_attribute?(_ct.parent_column_name)
+      scope_changed = _ct.scope_changed?(self)
+
+      return unless parent_changed || scope_changed
 
       was_parent_id = attribute_before_last_save(_ct.parent_column_name)
-      scope_conditions = _ct.scope_values_from_instance(self)
-      _ct.reorder_with_parent_id(was_parent_id, nil, scope_conditions)
+      previous_scope_conditions = _ct.previous_scope_values_from_instance(self)
+      _ct.reorder_with_parent_id(was_parent_id, nil, previous_scope_conditions)
     end
 
     def _ct_reorder_siblings(minimum_sort_order_value = nil)

lib/closure_tree/support.rb

@@ -257,6 +257,45 @@ def scope_values_from_instance(instance)
       scope_hash
     end
 
+    def previous_scope_values_from_instance(instance)
+      return {} unless options[:scope] && instance
+
+      scope_option = options[:scope]
+      scope_hash = {}
+
+      case scope_option
+      when Symbol
+        value = instance.attribute_before_last_save(scope_option)
+        scope_hash[scope_option] = value
+      when Array
+        scope_option.each do |item|
+          if item.is_a?(Symbol)
+            value = instance.attribute_before_last_save(item)
+            scope_hash[item] = value
+          end
+        end
+      end
+
+      scope_hash
+    end
+
+    def scope_changed?(instance)
+      return false unless options[:scope] && instance
+
+      scope_option = options[:scope]
+
+      case scope_option
+      when Symbol
+        instance.saved_change_to_attribute?(scope_option)
+      when Array
+        scope_option.any? do |item|
+          item.is_a?(Symbol) && instance.saved_change_to_attribute?(item)
+        end
+      else
+        false
+      end
+    end
+
     def apply_scope_conditions(scope, instance = nil)
       return scope unless options[:scope] && instance
 

test/closure_tree/scope_test.rb

@@ -234,4 +234,101 @@ def test_build_scope_where_clause_with_nil_value_sqlite
     assert_includes clause, 'IS NULL'
     assert_includes clause, '456'
   end
+
+  def test_reorder_previous_scope_siblings_when_scope_changes
+    # Create items in scope user_id: 1
+    item1 = ScopedItem.create!(name: 'item1', user_id: 1)
+    item2 = ScopedItem.create!(name: 'item2', user_id: 1)
+    item3 = ScopedItem.create!(name: 'item3', user_id: 1)
+
+    # Verify initial order values
+    assert_equal 0, item1.order_value
+    assert_equal 1, item2.order_value
+    assert_equal 2, item3.order_value
+
+    # Move item2 to a different scope (user_id: 2)
+    item2.update!(user_id: 2)
+
+    # Reload all items
+    item1.reload
+    item2.reload
+    item3.reload
+
+    # item2 should be first in its new scope
+    assert_equal 0, item2.order_value
+
+    # item1 and item3 should be reordered without gaps in old scope
+    assert_equal 0, item1.order_value
+    assert_equal 1, item3.order_value
+  end
+
+  def test_reorder_previous_scope_siblings_when_multiple_scope_changes
+    # Create items in scope user_id: 1, group_id: 10
+    item1 = MultiScopedItem.create!(name: 'item1', user_id: 1, group_id: 10)
+    item2 = MultiScopedItem.create!(name: 'item2', user_id: 1, group_id: 10)
+    item3 = MultiScopedItem.create!(name: 'item3', user_id: 1, group_id: 10)
+
+    # Verify initial order values
+    assert_equal 0, item1.order_value
+    assert_equal 1, item2.order_value
+    assert_equal 2, item3.order_value
+
+    # Move item2 to a different scope (user_id: 2, group_id: 10)
+    item2.update!(user_id: 2)
+
+    # Reload all items
+    item1.reload
+    item2.reload
+    item3.reload
+
+    # item2 should be first in its new scope
+    assert_equal 0, item2.order_value
+
+    # item1 and item3 should be reordered without gaps in old scope
+    assert_equal 0, item1.order_value
+    assert_equal 1, item3.order_value
+  end
+
+  def test_scope_changed_detection
+    # Test scope_changed? by checking the actual reordering behavior
+    # which implicitly tests that scope_changed? works during callbacks
+    item1 = ScopedItem.create!(name: 'item1', user_id: 1)
+    item2 = ScopedItem.create!(name: 'item2', user_id: 1)
+
+    assert_equal 0, item1.order_value
+    assert_equal 1, item2.order_value
+
+    # Change scope - if scope_changed? works, item1 will be reordered in new scope
+    item2.update!(user_id: 2)
+
+    item1.reload
+    item2.reload
+
+    # item2 should be 0 in new scope (proves scope change was detected)
+    assert_equal 0, item2.order_value
+    # item1 should remain 0 (was already 0, reordering removes gap from item2)
+    assert_equal 0, item1.order_value
+  end
+
+  def test_previous_scope_values_from_instance
+    # Test previous_scope_values by checking that OLD scope siblings are reordered
+    # which implicitly tests that previous scope values are correctly retrieved
+    item1 = ScopedItem.create!(name: 'item1', user_id: 1)
+    item2 = ScopedItem.create!(name: 'item2', user_id: 1)
+    item3 = ScopedItem.create!(name: 'item3', user_id: 1)
+
+    assert_equal 0, item1.order_value
+    assert_equal 1, item2.order_value
+    assert_equal 2, item3.order_value
+
+    # Move middle item to different scope
+    item2.update!(user_id: 2)
+
+    item1.reload
+    item3.reload
+
+    # If previous_scope_values works, item3 should now be 1 (gap filled)
+    assert_equal 0, item1.order_value
+    assert_equal 1, item3.order_value
+  end
 end