From b34a13ffe49c95dae3b365428744aa06623c5db5 Mon Sep 17 00:00:00 2001 From: Walther Lalk <83255+dakota@users.noreply.github.com> Date: Fri, 5 Jul 2019 16:10:16 +0200 Subject: [PATCH 1/5] Only attempt to decrement orders after delete --- src/Model/Behavior/SequenceBehavior.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Model/Behavior/SequenceBehavior.php b/src/Model/Behavior/SequenceBehavior.php index f0d6d48..f81dda3 100644 --- a/src/Model/Behavior/SequenceBehavior.php +++ b/src/Model/Behavior/SequenceBehavior.php @@ -228,12 +228,12 @@ public function beforeSave(Event $event, Entity $entity, ArrayObject $options) * When you delete a record from a set, you need to decrement the order of all * records that were after it in the set. * - * @param \Cake\Event\Event $event The beforeDelete event that was fired. + * @param \Cake\Event\Event $event The afterDelete event that was fired. * @param \Cake\ORM\Entity $entity The entity that is going to be saved. * * @return void */ - public function beforeDelete(Event $event, Entity $entity) + public function afterDelete(Event $event, Entity $entity) { $orderField = $this->_config['order']; list($order, $scope) = $this->_getOldValues($entity); From 3fd64f9de8b77af9868bdffcf3ef57e45adde079 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 8 Jul 2019 11:19:28 +0200 Subject: [PATCH 2/5] Add a test for unique sequences --- tests/Fixture/UniqueItemsFixture.php | 35 +++++++++++++++++++ .../Model/Behavior/SequenceBehaviorTest.php | 19 ++++++++++ 2 files changed, 54 insertions(+) create mode 100644 tests/Fixture/UniqueItemsFixture.php diff --git a/tests/Fixture/UniqueItemsFixture.php b/tests/Fixture/UniqueItemsFixture.php new file mode 100644 index 0000000..4f25ba8 --- /dev/null +++ b/tests/Fixture/UniqueItemsFixture.php @@ -0,0 +1,35 @@ + ['type' => 'integer'], + 'name' => ['type' => 'string', 'null' => true], + 'position' => ['type' => 'integer', 'null' => true], + '_constraints' => [ + 'primary' => ['type' => 'primary', 'columns' => ['id']], + 'position' => ['type' => 'unique', 'columns' => ['position']], + ], + ]; + + /** + * records property. + * + * @var array + */ + public $records = [ + ['name' => 'Item A', 'position' => 0], + ['name' => 'Item B', 'position' => 1], + ['name' => 'Item C', 'position' => 2], + ['name' => 'Item D', 'position' => 3], + ['name' => 'Item E', 'position' => 4], + ]; +} diff --git a/tests/TestCase/Model/Behavior/SequenceBehaviorTest.php b/tests/TestCase/Model/Behavior/SequenceBehaviorTest.php index f2e2386..180af97 100644 --- a/tests/TestCase/Model/Behavior/SequenceBehaviorTest.php +++ b/tests/TestCase/Model/Behavior/SequenceBehaviorTest.php @@ -44,12 +44,21 @@ public function initialize(array $config) } } +class UniqueItems extends Table +{ + public function initialize(array $config) + { + $this->addBehavior('ADmad/Sequence.Sequence', ['start' => 0]); + } +} + class SequenceTest extends TestCase { public $fixtures = [ 'plugin.ADmad/Sequence.Items', 'plugin.ADmad/Sequence.GroupedItems', 'plugin.ADmad/Sequence.KeywordItems', + 'plugin.ADmad/Sequence.UniqueItems' ]; /** @@ -241,6 +250,16 @@ public function testDelete() $this->assertOrder([1, 2, 4, 5], $GroupedItems, ['group_field' => 1]); $this->assertOrder([6, 7, 8, 9, 10], $GroupedItems, ['group_field' => 2]); $this->assertOrder([11, 12, 13, 14, 15], $GroupedItems, ['group_field' => 3]); + + $UniqueItems = TableRegistry::get('UniqueItems', [ + 'table' => 'unique_items', + 'alias' => 'UniqueItems', + 'className' => 'ADmad\Sequence\Test\TestCase\Model\Behavior\UniqueItems', + ]); + + $entity = $UniqueItems->get(3); + $UniqueItems->delete($entity); + $this->assertOrder([1, 2, 4, 5], $UniqueItems); } /** From 22999e899a929d1a9f161db0d45a0ddcb1563deb Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 8 Jul 2019 11:20:18 +0200 Subject: [PATCH 3/5] Read the old values before deleting the entity --- src/Model/Behavior/SequenceBehavior.php | 28 ++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/Model/Behavior/SequenceBehavior.php b/src/Model/Behavior/SequenceBehavior.php index f81dda3..53087d9 100644 --- a/src/Model/Behavior/SequenceBehavior.php +++ b/src/Model/Behavior/SequenceBehavior.php @@ -73,6 +73,13 @@ class SequenceBehavior extends Behavior 'start' => 1, ]; + /** + * Old values for the entity being deleted + * + * @var array|null + */ + protected $_oldValues; + /** * Normalize config options. * @@ -224,6 +231,20 @@ public function beforeSave(Event $event, Entity $entity, ArrayObject $options) } } + /** + * When you delete a record from a set, you need to decrement the order of all + * records that were after it in the set. + * + * @param \Cake\Event\Event $event The afterDelete event that was fired. + * @param \Cake\ORM\Entity $entity The entity that is going to be saved. + * + * @return void + */ + public function beforeDelete(Event $event, Entity $entity) + { + $this->_oldValues = $this->_getOldValues($entity); + } + /** * When you delete a record from a set, you need to decrement the order of all * records that were after it in the set. @@ -235,14 +256,19 @@ public function beforeSave(Event $event, Entity $entity, ArrayObject $options) */ public function afterDelete(Event $event, Entity $entity) { + if (!$this->_oldValues) { + return null; + } + $orderField = $this->_config['order']; - list($order, $scope) = $this->_getOldValues($entity); + list($order, $scope) = $this->_oldValues; $this->_sync( [$orderField => $this->_getUpdateExpression('-')], [$orderField . ' >' => $order], $scope ); + $this->_oldValues = null; } /** From 8dd2f5b4f89f96428eca8e0c0114274a67e79d95 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Mon, 8 Jul 2019 11:36:48 +0200 Subject: [PATCH 4/5] Correct return value --- src/Model/Behavior/SequenceBehavior.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/Behavior/SequenceBehavior.php b/src/Model/Behavior/SequenceBehavior.php index 53087d9..d8ef865 100644 --- a/src/Model/Behavior/SequenceBehavior.php +++ b/src/Model/Behavior/SequenceBehavior.php @@ -257,7 +257,7 @@ public function beforeDelete(Event $event, Entity $entity) public function afterDelete(Event $event, Entity $entity) { if (!$this->_oldValues) { - return null; + return; } $orderField = $this->_config['order']; From ddbe0ccf8cbca1548fe9e2f1e1ec24c289b44d08 Mon Sep 17 00:00:00 2001 From: Walther Lalk Date: Tue, 9 Jul 2019 15:44:28 +0200 Subject: [PATCH 5/5] Correct docblocks --- src/Model/Behavior/SequenceBehavior.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Model/Behavior/SequenceBehavior.php b/src/Model/Behavior/SequenceBehavior.php index d8ef865..da4d601 100644 --- a/src/Model/Behavior/SequenceBehavior.php +++ b/src/Model/Behavior/SequenceBehavior.php @@ -235,7 +235,7 @@ public function beforeSave(Event $event, Entity $entity, ArrayObject $options) * When you delete a record from a set, you need to decrement the order of all * records that were after it in the set. * - * @param \Cake\Event\Event $event The afterDelete event that was fired. + * @param \Cake\Event\Event $event The beforeDelete event that was fired. * @param \Cake\ORM\Entity $entity The entity that is going to be saved. * * @return void @@ -250,7 +250,7 @@ public function beforeDelete(Event $event, Entity $entity) * records that were after it in the set. * * @param \Cake\Event\Event $event The afterDelete event that was fired. - * @param \Cake\ORM\Entity $entity The entity that is going to be saved. + * @param \Cake\ORM\Entity $entity The entity that has been deleted. * * @return void */