From c951b6eca893310f86a9da7594bd832f577e8e08 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 19:40:59 -0400 Subject: [PATCH 1/2] refactor: Simplify `deleteWithChildren` method by removing transaction handling. --- src/NestedSetsBehavior.php | 19 +------ tests/NestedSetsBehaviorTest.php | 98 -------------------------------- 2 files changed, 1 insertion(+), 116 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index a488be1..6d4aebe 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -534,24 +534,7 @@ public function deleteWithChildren(): bool|int { $this->operation = self::OPERATION_DELETE_WITH_CHILDREN; - if ($this->getOwner()->isTransactional(ActiveRecord::OP_DELETE) !== false) { - return $this->deleteWithChildrenInternal(); - } - - $transaction = $this->getDb()->beginTransaction(); - - try { - match ($result = $this->deleteWithChildrenInternal()) { - false => $transaction->rollBack(), - default => $transaction->commit(), - }; - - return $result; - } catch (Exception $e) { - $transaction->rollBack(); - - throw $e; - } + return $this->deleteWithChildrenInternal(); } /** diff --git a/tests/NestedSetsBehaviorTest.php b/tests/NestedSetsBehaviorTest.php index bb8d3f6..2cf733e 100644 --- a/tests/NestedSetsBehaviorTest.php +++ b/tests/NestedSetsBehaviorTest.php @@ -1429,71 +1429,6 @@ public function testThrowLogicExceptionWhenBehaviorIsDetachedFromOwner(): void $behavior->parents(); } - public function testReturnAffectedRowsAndUpdateTreeAfterDeleteWithChildrenWhenManualTransactionIsUsed(): void - { - $this->generateFixtureTree(); - - $node = Tree::findOne(10); - - self::assertNotNull( - $node, - "Node with ID '10' should exist before attempting to delete with children using manual transaction.", - ); - self::assertEquals( - 'Node 2.1', - $node->getAttribute('name'), - "Node with ID '10' should have the name 'Node 2.1' before deletion.", - ); - self::assertFalse( - $node->isTransactional(ActiveRecord::OP_DELETE), - "Node with ID '10' should not use transactional delete (manual transaction expected).", - ); - - $initialCount = (int) Tree::find()->count(); - $toDeleteCount = (int) Tree::find() - ->andWhere(['>=', 'lft', $node->getAttribute('lft')]) - ->andWhere(['<=', 'rgt', $node->getAttribute('rgt')]) - ->count(); - - self::assertEquals( - 3, - $toDeleteCount, - "Node '2.1' should have itself and '2' children (total '3' nodes to delete).", - ); - - $result = (int) $node->deleteWithChildren(); - - self::assertEquals( - $toDeleteCount, - $result, - "'deleteWithChildren()' should return the number of affected rows equal to the nodes deleted.", - ); - - $finalCount = (int) Tree::find()->count(); - - self::assertEquals( - $initialCount - $toDeleteCount, - $finalCount, - 'Tree node count after deletion should decrease by the number of deleted nodes.', - ); - self::assertNull( - Tree::findOne(10), - "Node with ID '10' should not exist after deletion.", - ); - self::assertNull( - Tree::findOne(11), - "Node with ID '11' should not exist after deletion.", - ); - self::assertNull( - Tree::findOne(12), - "Node with ID '12' should not exist after deletion.", - ); - self::assertNotNull( - Tree::findOne(1), - "Root node with ID '1' should still exist after deleting node '10' and its children.", - ); - } - public function testReturnFalseWhenDeleteWithChildrenIsAbortedByBeforeDelete(): void { $this->createDatabase(); @@ -1529,39 +1464,6 @@ public function testReturnFalseWhenDeleteWithChildrenIsAbortedByBeforeDelete(): ); } - public function testThrowExceptionWhenDeleteWithChildrenThrowsExceptionInTransaction(): void - { - $this->createDatabase(); - - $node = new Tree(['name' => 'Root']); - - $node->detachBehavior('nestedSetsBehavior'); - - self::assertNull( - $node->getBehavior('nestedSetsBehavior'), - 'Behavior must be detached before testing exception handling.', - ); - - $nestedSetsBehavior = $this->createMock(NestedSetsBehavior::class); - $nestedSetsBehavior->expects(self::once()) - ->method('deleteWithChildren') - ->willThrowException(new Exception('Simulated database error during deletion')); - - $node->attachBehavior('nestedSetsBehavior', $nestedSetsBehavior); - $behavior = $node->getBehavior('nestedSetsBehavior'); - - self::assertInstanceOf( - NestedSetsBehavior::class, - $behavior, - 'Behavior must be attached to the node before testing exception handling.', - ); - - $this->expectException(Exception::class); - $this->expectExceptionMessage('Simulated database error during deletion'); - - $behavior->deleteWithChildren(); - } - public function testThrowExceptionWhenMakeRootIsCalledOnModelWithoutPrimaryKey(): void { $this->createDatabase(); From 6bcf1e322b829bd039264456dcb0531a4acfe24f Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 19:54:06 -0400 Subject: [PATCH 2/2] docs: Add notes on transaction handling in `appendTo`, `insertAfter`, `insertBefore`, `makeRoot`, and `prependTo` methods. --- src/NestedSetsBehavior.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 6d4aebe..ea1bdf4 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -300,6 +300,10 @@ public function afterUpdate(): void * * @return bool Whether the operation was successful and the node was appended or moved. * + * **Note:** This method uses {@see ActiveRecord::save()} internally, which means Yii2 will automatically handle + * database transactions if the model {@see ActiveRecord::isTransactional()} method returns `true` for the current + * scenario and {@see ActiveRecord::OP_INSERT} or {@see ActiveRecord::OP_UPDATE} operations. + * * Usage example: * ```php * $category->appendTo($parentCategory); @@ -593,6 +597,10 @@ public function events(): array * * @return bool Whether the operation was successful and the node was inserted or moved. * + * **Note:** This method uses {@see ActiveRecord::save()} internally, which means Yii2 will automatically handle + * database transactions if the model's {@see ActiveRecord::isTransactional()} method returns `true` for the + * current scenario and {@see ActiveRecord::OP_INSERT} or {@see ActiveRecord::OP_UPDATE} operations. + * * Usage example: * ```php * $category->insertAfter($siblingCategory); @@ -634,6 +642,10 @@ public function insertAfter(ActiveRecord $node, bool $runValidation = true, arra * * @return bool Whether the operation was successful and the node was inserted or moved. * + * **Note:** This method uses {@see ActiveRecord::save()} internally, which means Yii2 will automatically handle + * database transactions if the model {@see ActiveRecord::isTransactional()} method returns `true` for the current + * scenario and {@see ActiveRecord::OP_INSERT} or {@see ActiveRecord::OP_UPDATE} operations. + * * Usage example: * ```php * $category->insertBefore($siblingCategory); @@ -804,6 +816,10 @@ public function leaves(): ActiveQuery * * @return bool Whether the operation was successful and the node was created or moved as root. * + * **Note:** This method uses {@see ActiveRecord::save()} internally, which means Yii2 will automatically handle + * database transactions if the model {@see ActiveRecord::isTransactional()} method returns `true` for the current + * scenario and {@see ActiveRecord::OP_INSERT} or {@see ActiveRecord::OP_UPDATE} operations. + * * Usage example: * ```php * // Create a new root node @@ -940,6 +956,10 @@ public function parents(int|null $depth = null): ActiveQuery * * @return bool Whether the operation was successful and the node was prepended or moved. * + * **Note:** This method uses {@see ActiveRecord::save()} internally, which means Yii2 will automatically handle + * database transactions if the model {@see ActiveRecord::isTransactional()} method returns `true` for the current + * scenario and {@see ActiveRecord::OP_INSERT} or {@see ActiveRecord::OP_UPDATE} operations. + * * Usage example: * ```php * $category->prependTo($parentCategory);