Use a subquery to delete child records.

Picking off the where clause is risky as the default find() method may contain associations or joins.
cakephp · Oct 10, 2017 · e60ae26 · e60ae26
1 parent 66bfde9
commit e60ae26
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 4 deletions.
diff --git a/src/ORM/Behavior/TreeBehavior.php b/src/ORM/Behavior/TreeBehavior.php
@@ -222,18 +222,19 @@ public function beforeDelete(Event $event, EntityInterface $entity)
         $left = $entity->get($config['left']);
         $right = $entity->get($config['right']);
         $diff = $right - $left + 1;
+        $primaryKey = $this->_getPrimaryKey();
 
         if ($diff > 2) {
             /* @var \Cake\Database\Expression\QueryExpression $expression */
-            $expression = $this->_scope($this->_table->find())
+            $finder = $this->_scope($this->_table->find())
+                ->select([$primaryKey], true)
                 ->where(function ($exp) use ($config, $left, $right) {
                     return $exp
                         ->gte($config['leftField'], $left + 1)
                         ->lte($config['leftField'], $right - 1);
-                })
-                ->clause('where');
+                });
 
-            $this->_table->deleteAll($expression);
+            $this->_table->deleteAll([$primaryKey . ' IN' => $finder]);
         }
 
         $this->_sync($diff, '-', "> {$right}");

diff --git a/tests/TestCase/ORM/Behavior/TreeBehaviorTest.php b/tests/TestCase/ORM/Behavior/TreeBehaviorTest.php
@@ -1202,6 +1202,43 @@ public function testDeleteSubTree()
         $this->assertMpttValues($expected, $this->table);
     }
 
+    /**
+     * Tests deleting a subtree in a scoped tree
+     *
+     * @return void
+     */
+    public function testDeleteSubTreeScopedTree()
+    {
+        $table = TableRegistry::get('MenuLinkTrees');
+        $table->addBehavior('Tree', ['scope' => ['menu' => 'main-menu']]);
+        $entity = $table->get(3);
+        $this->assertTrue($table->delete($entity));
+
+        $expected = [
+            ' 1: 4 -  1:Link 1',
+            '_ 2: 3 -  2:Link 2',
+            ' 5: 8 -  6:Link 6',
+            '_ 6: 7 -  7:Link 7',
+            ' 9:10 -  8:Link 8',
+        ];
+        $this->assertMpttValues($expected, $table);
+
+        $table->behaviors()->get('Tree')->config('scope', ['menu' => 'categories']);
+        $expected = [
+            ' 1:10 -  9:electronics',
+            '_ 2: 9 - 10:televisions',
+            '__ 3: 4 - 11:tube',
+            '__ 5: 8 - 12:lcd',
+            '___ 6: 7 - 13:plasma',
+            '11:20 - 14:portable',
+            '_12:15 - 15:mp3',
+            '__13:14 - 16:flash',
+            '_16:17 - 17:cd',
+            '_18:19 - 18:radios',
+        ];
+        $this->assertMpttValues($expected, $table);
+    }
+
     /**
      * Test deleting a root node
      *