Fix sibling reordering when node changes parent or scope#484
Merged
seuros merged 6 commits intoClosureTree:masterfrom Feb 16, 2026
Merged
Conversation
Updates prepend_child to reorder remaining root nodes when a root node becomes a child, ensuring order_values remain sequential. Adds tests to verify correct reordering for both prepend_child and append_child operations.
Ensure numeric ordering maintenance runs when the order column changes and add cross-database tests. - hierarchy_maintenance.rb: run rebuild or sibling reordering from _ct_after_save when the order column changes; remove single-use skip-sort flag behavior. - numeric_deterministic_ordering.rb: stop skipping sort-order maintenance in prepend/sibling operations so reordering happens correctly and avoid double reorders. - tests: add order_value_cross_database_test to validate reorder behavior across PostgreSQL (Label), SQLite (MemoryTag) and MySQL (SecondaryTag); update label tests to persist order with update_columns. - dummy/schema: enable numeric ordering by adding a sort_order column to test schemas and configuring MemoryTag and SecondaryTag with has_closure_tree order: :sort_order, numeric_order: true.
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.
seuros
requested review from
Copilot and
seuros
and removed request for
Copilot
seuros
requested changes
Feb 15, 2026
seuros
approved these changes
Feb 16, 2026
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where sibling nodes were not properly reordered when a node changed its parent (via prepend_child, append_child, or add_sibling) or when a scoped node moved to a different scope. The fix removes the @_ct_skip_sort_order_maintenance mechanism and replaces it with automatic callback-based reordering that correctly handles both parent and scope changes.
Changes:
- Removed manual skip flags from
prepend_childandadd_siblingmethods, enabling automatic reordering via callbacks - Added scope change detection and reordering support for scoped hierarchies
- Added comprehensive cross-database tests for PostgreSQL, MySQL, and SQLite
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/closure_tree/hierarchy_maintenance.rb | Deprecated _ct_skip_sort_order_maintenance! method; added scope change handling in _ct_after_save callback; removed skip flag check in rebuild! |
| lib/closure_tree/numeric_deterministic_ordering.rb | Updated _ct_reorder_prior_siblings_if_parent_changed to handle scope changes; removed manual reordering logic from prepend_child and add_sibling |
| lib/closure_tree/support.rb | Added previous_scope_values_from_instance and scope_changed? helper methods for scope change detection |
| test/closure_tree/order_value_cross_database_test.rb | Added comprehensive cross-database tests for PostgreSQL, MySQL, and SQLite covering all three node movement methods |
| test/closure_tree/scope_test.rb | Added tests for scope change reordering with single and multiple scope attributes |
| test/closure_tree/label_test.rb | Fixed variable shadowing bug in create_preorder_tree helper; changed from save! to update_columns to bypass callbacks in test setup |
| test/dummy/app/models/secondary_tag.rb | Added order: :sort_order, numeric_order: true configuration |
| test/dummy/app/models/memory_tag.rb | Added order: :sort_order, numeric_order: true configuration |
| test/dummy/db/secondary_schema.rb | Added sort_order column to secondary_tags table |
| test/closure_tree/multi_database_test.rb | Added sort_order column to memory_tags table setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add a guard to check _ct.order_option? before inspecting saved_changes for the order column. This prevents attempting to read order-column changes when ordering is not enabled, avoiding unintended reordering or errors when the order column is absent.
seuros
reviewed
Feb 16, 2026
vsmay98
approved these changes
Feb 16, 2026
Remove per-test setup/teardown that created/dropped in-memory memory_tags and memory_tag_hierarchies tables and instead add those tables and indexes to test/dummy/db/sqlite_schema.rb. Update test/dummy/lib/tasks/db.rake to explicitly establish the :lite connection and load the SQLite schema so the in-memory tables are created centrally for tests. This centralizes SQLite test DB setup and simplifies the test files.
seuros
approved these changes
Feb 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
prepend_child,append_child, oradd_sibling)Problem
When a root node was moved to become a child of another node, the remaining root nodes were left with gaps in their
order_valuesequence (e.g., 0, 2, 3 instead of 0, 1, 2).When a scoped record moved to a different scope (e.g.,
user_idchanged from 1 to 2), siblings in the old scope were not reordered, leaving gaps.Solution
attribute_before_last_save) when reordering old parent's siblingsprevious_scope_values_from_instanceandscope_changed?helper methodsChanges
lib/closure_tree/hierarchy_maintenance.rb- Handle scope changes in_ct_after_savelib/closure_tree/numeric_deterministic_ordering.rb- Use previous scope values in_ct_reorder_prior_siblings_if_parent_changedlib/closure_tree/support.rb- Addprevious_scope_values_from_instanceandscope_changed?methodstest/closure_tree/order_value_cross_database_test.rb- New cross-database teststest/closure_tree/scope_test.rb- Add scope change reordering testsFixes Root node siblings not reordered after prepend_child/add_sibling moves a root to child position