Correctly implementing node reparenting

src/Model/Behavior/TreeBehavior.php

@@ -120,8 +120,8 @@ class TreeBehavior extends Behavior {
 	protected function _setParent($entity, $parent) {
 		$config = $this->config();
 		$parentNode = $this->_getParent($parent);
-		$parentNodeLeft = $parentNode->get($config['left']);
-		$parentNoderight = $parentNode->get($config['right']);
+		$parentLeft = $parentNode->get($config['left']);
+		$parentRight = $parentNode->get($config['right']);
 
 		$right = $entity->get($config['right']);
 		$left = $entity->get($config['left']);
@@ -141,13 +141,19 @@ class TreeBehavior extends Behavior {
 			$diff *= -1;
 		}
 
-		$this->_sync($diff, '+', "BETWEEN {$min} AND {$max}");
 
 		if ($right - $left > 1) {
 			//Correcting internal subtree
 			$internalLeft = $left + 1;
 			$internalRight = $right - 1;
-			$this->_sync($targetLeft - $left, '+', "BETWEEN {$internalLeft} AND {$internalRight}");
+			$this->_sync($targetLeft - $left, '+', "BETWEEN {$internalLeft} AND {$internalRight}", true);
+		}
+
+		$this->_sync($diff, '+', "BETWEEN {$min} AND {$max}");
+
+		if ($right - $left > 1) {
+			//Inverting sign again
+			$this->_sync('-1', '*', "< 0");
 		}
 
 		//Allocating new position
@@ -459,17 +465,19 @@ class TreeBehavior extends Behavior {
 		return $edge->{$LorR};
 	}
 
-	protected function _sync($shift, $dir = '+', $conditions = null, $field = 'both') {
+	protected function _sync($shift, $dir = '+', $conditions = null, $invert = false, $field = 'both') {
 		extract($this->config());
 
 		if ($field === 'both') {
-			$this->_sync($shift, $dir, $conditions, $left);
+			$this->_sync($shift, $dir, $conditions, $invert, $left);
 			$field = $right;
 		}
 
 		// updateAll + scope
 		$exp = new QueryExpression();
-		$exp->add("{$field} = ({$field} {$dir} {$shift})");
+		$invert = $invert ? '*-1' : '';
+		$template = sprintf('%s = (%s %s %s)%s', $field, $field, $dir, $shift, $invert);
+		$exp->add($template);
 
 		$query = $this->_scope($this->_table->query());
 		$query->update()

tests/TestCase/Model/Behavior/TreeBehaviorTest.php

@@ -321,7 +321,7 @@ class TreeBehaviorTest extends TestCase {
 
 		$expected[] = $entity->toArray();
 		$results = $table->find()->order('lft')->hydrate(false)->toArray();
-		$this->assertEquals($expected, $results);
+		$this->assertEquals($expected, $result);
 	}
 
 /**
@@ -337,14 +337,13 @@ class TreeBehaviorTest extends TestCase {
 			['markNew' => true]
 		);
 		$this->assertSame($entity, $table->save($entity));
-		$results = $table->find()->order('lft')->hydrate(false)->toArray();
 		$this->assertEquals(20, $entity->lft);
 		$this->assertEquals(21, $entity->rght);
 
-		$expected = $table->find()->order('lft')->hydrate(false)->toArray();
-		$table->recover();
 		$result = $table->find()->order('lft')->hydrate(false)->toArray();
-		$this->assertEquals($expected, $results);
+		$table->recover();
+		$expected = $table->find()->order('lft')->hydrate(false)->toArray();
+		$this->assertEquals($expected, $result);
 	}
 
 /**
@@ -360,13 +359,39 @@ class TreeBehaviorTest extends TestCase {
 			['markNew' => true]
 		);
 		$this->assertSame($entity, $table->save($entity));
-		$results = $table->find()->order('lft')->hydrate(false)->toArray();
 		$this->assertEquals(9, $entity->lft);
 		$this->assertEquals(10, $entity->rght);
 
-		$expected = $table->find()->order('lft')->hydrate(false)->toArray();
-		$table->recover();
 		$result = $table->find()->order('lft')->hydrate(false)->toArray();
+		$table->recover();
+		$expected = $table->find()->order('lft')->hydrate(false)->toArray();
 		$this->assertEquals($expected, $results);
 	}
+
+/**
+ * Tests moving a subtree to the right
+ *
+ * @return void
+ */
+	public function testReParentSubTreeRight() {
+		$table = TableRegistry::get('NumberTrees');
+		$table->addBehavior('Tree');
+		$entity = $table->get(2);
+		$entity->parent_id = 6;
+		$this->assertSame($entity, $table->save($entity));
+		$this->assertEquals(11, $entity->lft);
+		$this->assertEquals(18, $entity->rght);
+
+		$result = $table->find()->order('lft')->hydrate(false);
+		$expected = [1, 6, 7, 8, 9, 10, 2, 3, 4, 5, 11];
+		$this->assertEquals($expected, $result->extract('id')->toArray());
+		$numbers = [];
+		$result->each(function($v) use (&$numbers) {
+			$numbers[] = $v['lft'];
+			$numbers[] = $v['rght'];
+		});
+		sort($numbers);
+		$this->assertEquals(range(1, 22), $numbers);
+	}
+
 }