From 7de1d4bf7acdc5db6be5eef04d2e08278d1fb8a0 Mon Sep 17 00:00:00 2001 From: Aran Keyhan-Rad Date: Fri, 30 Oct 2026 14:02:32 +0000 Subject: [PATCH] Re-read left sibling position after removing the moved subtree move_after/move_before captured the left sibling's lft/rgt before mptt_before_delete shifted it, so a node moved to the right within the same tree landed one slot too far and corrupted the tree. Fixes #67. --- sqlalchemy_mptt/events.py | 17 +++++++- sqlalchemy_mptt/tests/cases/move_node.py | 54 ++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/sqlalchemy_mptt/events.py b/sqlalchemy_mptt/events.py index 718c315..28a6c90 100644 --- a/sqlalchemy_mptt/events.py +++ b/sqlalchemy_mptt/events.py @@ -222,6 +222,7 @@ def mptt_before_update(mapper, connection, instance): ).fetchone() current_lvl_nodes = connection.execute( compat_layer.select( + table_pk, table.c.lft, table.c.rgt, table.c.parent_id, @@ -236,6 +237,7 @@ def mptt_before_update(mapper, connection, instance): ).fetchall() if current_lvl_nodes: ( + left_sibling_pk, left_sibling_left, left_sibling_right, left_sibling_parent, @@ -243,6 +245,7 @@ def mptt_before_update(mapper, connection, instance): ) = current_lvl_nodes[-1] instance.parent_id = left_sibling_parent left_sibling = { + 'pk': left_sibling_pk, 'lft': left_sibling_left, 'rgt': left_sibling_right, 'is_parent': False @@ -270,6 +273,7 @@ def mptt_before_update(mapper, connection, instance): ).fetchone() instance.parent_id = left_sibling_parent left_sibling = { + 'pk': instance.mptt_move_after, 'lft': left_sibling_left, 'rgt': left_sibling_right, 'is_parent': False @@ -376,7 +380,18 @@ def mptt_before_update(mapper, connection, instance): node_size = node_pos_right - node_pos_left + 1 # left sibling node - if not left_sibling: + if left_sibling: + # Removing the moved subtree above may have shifted the left + # sibling, so re-read its current position (#67). + left_sibling['lft'], left_sibling['rgt'] = connection.execute( + compat_layer.select( + table.c.lft, + table.c.rgt + ).where( + table_pk == left_sibling['pk'] + ) + ).fetchone() + else: left_sibling = { 'lft': parent_pos_left, 'rgt': parent_pos_right, diff --git a/sqlalchemy_mptt/tests/cases/move_node.py b/sqlalchemy_mptt/tests/cases/move_node.py index 1c1a20e..f59c9d6 100644 --- a/sqlalchemy_mptt/tests/cases/move_node.py +++ b/sqlalchemy_mptt/tests/cases/move_node.py @@ -864,6 +864,60 @@ def test_move_after_between_tree(self): ) + def test_move_after_right_sibling(self): + """ Move node(2) after its right sibling node(4) + + Regression test for #67: the left sibling's lft/rgt were read + before node(2) was removed, so node(2) landed one slot too far + right. + + .. code:: + + level Move 2 after 4 + 1 1(1)22 + _______________|___________________ + | | | + 2 2(4)7 8(2)11 12(7)21 + ^ | ^ + 3 3(5)4 5(6)6 9(3)10 13(8)16 17(10)20 + | | + 4 14(9)15 18(11)19 + """ + node = self.session.query(self.model).\ + filter(self.model.get_pk_column() == 2).one() + node.move_after("4") + _level = node.get_default_level() + self.assertEqual( + [ + # id lft rgt lvl parent tree + (1, 1, 22, _level + 0, None, 1), + (2, 8, 11, _level + 1, 1, 1), + (3, 9, 10, _level + 2, 2, 1), + (4, 2, 7, _level + 1, 1, 1), + (5, 3, 4, _level + 2, 4, 1), + (6, 5, 6, _level + 2, 4, 1), + (7, 12, 21, _level + 1, 1, 1), + (8, 13, 16, _level + 2, 7, 1), + (9, 14, 15, _level + 3, 8, 1), + (10, 17, 20, _level + 2, 7, 1), + (11, 18, 19, _level + 3, 10, 1), + + (12, 1, 22, _level + 0, None, 2), + (13, 2, 5, _level + 1, 12, 2), + (14, 3, 4, _level + 2, 13, 2), + (15, 6, 11, _level + 1, 12, 2), + (16, 7, 8, _level + 2, 15, 2), + (17, 9, 10, _level + 2, 15, 2), + (18, 12, 21, _level + 1, 12, 2), + (19, 13, 16, _level + 2, 18, 2), + (20, 14, 15, _level + 3, 19, 2), + (21, 17, 20, _level + 2, 18, 2), + (22, 18, 19, _level + 3, 21, 2) + ], + self.result.all() + ) + + class MoveInside(object): def test_move_between_tree(self):