From b616a82906a847913037edd6c7a2103ce9a310ae Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 13:11:46 +0200 Subject: [PATCH 01/23] Shift models to end of sequence when model is created --- src/HasPosition.php | 16 +++++++++++++++- tests/Support/Factories/Factory.php | 14 ++++++++++++++ tests/Unit/CreateTest.php | 16 ++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index 7468c2e..1cebf3b 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -12,7 +12,7 @@ trait HasPosition { /** - * Boot the position trait. + * Boot the trait. */ public static function bootHasPosition(): void { @@ -22,6 +22,10 @@ public static function bootHasPosition(): void $model->assignPositionIfMissing(); }); + static::created(static function (self $model) { + $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); + }); + static::updating(static function (self $model) { if ($model->isDirty($model->getPositionColumn())) { $model->shiftBeforeMove($model->getPosition(), $model->getOriginal($model->getPositionColumn())); @@ -33,6 +37,16 @@ public static function bootHasPosition(): void }); } + /** + * Initialize the trait. + */ + public function initializeHasPosition(): void + { + $this->mergeCasts([ + $this->getPositionColumn() => 'int', + ]); + } + /** * Get the name of the "position" column. */ diff --git a/tests/Support/Factories/Factory.php b/tests/Support/Factories/Factory.php index 0e50b7c..4b0c29b 100644 --- a/tests/Support/Factories/Factory.php +++ b/tests/Support/Factories/Factory.php @@ -44,6 +44,20 @@ final public function create(array $attributes = []): Model return $model; } + /** + * Create many model instances and save them into the database. + */ + final public function createMany(int $count, array $attributes = []): array + { + $models = []; + + for ($i = 0; $i < $count; $i++) { + $models[] = $this->create($attributes); + } + + return $models; + } + /** * Make a new model instance. */ diff --git a/tests/Unit/CreateTest.php b/tests/Unit/CreateTest.php index 060e6e2..ae80f0b 100644 --- a/tests/Unit/CreateTest.php +++ b/tests/Unit/CreateTest.php @@ -31,6 +31,8 @@ public function it_sets_next_position_value_in_model_sequence(): void static::assertSame(2, $category2->position); } + // @todo ensure it produces single database query when model is created at the end of the sequence + /** * @test */ @@ -55,4 +57,18 @@ public function it_can_configure_start_position_value(): void static::assertSame(23, $category->position); } + + /** + * @test + */ + public function it_can_create_model_in_the_middle_of_the_sequence(): void + { + $categories = CategoryFactory::new()->createMany(2); + + $category = CategoryFactory::new()->create(['position' => 1]); + + static::assertSame(1, $category->position); + static::assertSame($categories[0]->fresh()->position, 0); + static::assertSame($categories[1]->fresh()->position, 2); + } } From b5edd6c0da23cead11078a7aeaa30eeddebac6bc Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 13:12:54 +0200 Subject: [PATCH 02/23] Update CHANGELOG.md --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13775e7..41dce3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,19 +8,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.4.1] - 2023-02-08 + +### Added + +- Possibility to update `position` attribute along with other attributes + ## [0.4.0] - 2022-05-08 ### Added + - Laravel 9 support ## [0.3.0] - 2021-06-24 ### Fixed + - Fix position query scoping for relations ## [0.2.0] - 2021-06-19 ### Added + - Documentation - `OrderByPosition` global scope - Support for models delete @@ -28,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add PHP 8 support ### Changed + - Rename `arrangeByIds` into `arrangeByKeys` - Extract `arrangeByKeys` method into query builder - Extract shift methods into query builder @@ -35,4 +45,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.1.0] - 2021-06-13 ### Added + - Base ordering features From c49d46f7a7de85ea25f5bdad416a6385e3db9598 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 13:13:33 +0200 Subject: [PATCH 03/23] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41dce3c..8e45a9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Possibility to create model in the middle of the sequence + ## [0.4.1] - 2023-02-08 ### Added From 4629d71bfa22150d89431f5c15808de32070828a Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 13:15:26 +0200 Subject: [PATCH 04/23] Refactor --- tests/Unit/CreateTest.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/Unit/CreateTest.php b/tests/Unit/CreateTest.php index ae80f0b..749c00c 100644 --- a/tests/Unit/CreateTest.php +++ b/tests/Unit/CreateTest.php @@ -24,11 +24,9 @@ public function it_sets_position_value_on_model_create(): void */ public function it_sets_next_position_value_in_model_sequence(): void { - $category0 = CategoryFactory::new()->create(); - $category1 = CategoryFactory::new()->create(); - $category2 = CategoryFactory::new()->create(); + $categories = CategoryFactory::new()->createMany(3); - static::assertSame(2, $category2->position); + static::assertSame(2, $categories[1]->position); } // @todo ensure it produces single database query when model is created at the end of the sequence @@ -38,7 +36,9 @@ public function it_sets_next_position_value_in_model_sequence(): void */ public function it_does_not_override_position_value_if_it_is_set_already(): void { - $category = CategoryFactory::new()->position(15)->create(); + $category = CategoryFactory::new() + ->position(15) + ->create(); static::assertSame(15, $category->position); } @@ -65,7 +65,9 @@ public function it_can_create_model_in_the_middle_of_the_sequence(): void { $categories = CategoryFactory::new()->createMany(2); - $category = CategoryFactory::new()->create(['position' => 1]); + $category = CategoryFactory::new() + ->position(1) + ->create(); static::assertSame(1, $category->position); static::assertSame($categories[0]->fresh()->position, 0); From d5b1b56f717df11625d508d674dcfb783067d120 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 13:33:39 +0200 Subject: [PATCH 05/23] WIP --- src/HasPosition.php | 54 ++++++++++++++++++++++++++++++--------- tests/Unit/CreateTest.php | 26 +++++++++++++++---- 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index 1cebf3b..a32c795 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -11,6 +11,13 @@ */ trait HasPosition { + /** + * Indicates if the model should shift position of other models in the sequence. + * + * @var bool + */ + protected $shiftPosition = false; + /** * Boot the trait. */ @@ -19,11 +26,19 @@ public static function bootHasPosition(): void static::addGlobalScope(new PositioningScope()); static::creating(static function (self $model) { - $model->assignPositionIfMissing(); + if ($model->getPosition() === null) { + $model->assignPosition(); + } else { + $model->withShiftPosition(); + } }); + // @todo consider using "saved" event for this. static::created(static function (self $model) { - $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); + if ($model->shouldShiftPosition()) { + // @todo consider extracting into "others" method. + $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); + } }); static::updating(static function (self $model) { @@ -71,6 +86,21 @@ public function alwaysOrderByPosition(): bool return false; } + /** + * Determine if the model should shift position of other models in the sequence. + */ + public function shouldShiftPosition(): bool + { + return $this->shiftPosition; + } + + public function withShiftPosition(bool $value = true): self + { + $this->shiftPosition = $value; + + return $this; + } + /** * Get the position value of the model. */ @@ -142,20 +172,20 @@ protected function newPositionQuery(): Builder return $this->newQuery(); } - /** - * Assign the next position value to the model if it is missing. - */ - protected function assignPositionIfMissing(): void - { - if (null === $this->getPosition()) { - $this->assignNextPosition(); - } - } +// /** +// * Assign the next position value to the model if it is missing. +// */ +// protected function assignPositionIfMissing(): void +// { +// if (null === $this->getPosition()) { +// $this->assignPosition(); +// } +// } /** * Assign the next position value to the model. */ - protected function assignNextPosition(): Model + protected function assignPosition(): Model { return $this->setPosition($this->getNextPosition()); } diff --git a/tests/Unit/CreateTest.php b/tests/Unit/CreateTest.php index 749c00c..d806f0e 100644 --- a/tests/Unit/CreateTest.php +++ b/tests/Unit/CreateTest.php @@ -12,7 +12,7 @@ class CreateTest extends TestCase /** * @test */ - public function it_sets_position_value_on_model_create(): void + public function it_assigns_position_value_when_model_is_creating(): void { $category = CategoryFactory::new()->create(); @@ -22,14 +22,30 @@ public function it_sets_position_value_on_model_create(): void /** * @test */ - public function it_sets_next_position_value_in_model_sequence(): void + public function it_creates_model_at_end_of_sequence(): void { - $categories = CategoryFactory::new()->createMany(3); + CategoryFactory::new()->createMany(2); - static::assertSame(2, $categories[1]->position); + $category = CategoryFactory::new()->create(); + + static::assertSame(2, $category->position); + + // @todo what if we start to update position after that? previous "shift" value is still working. } - // @todo ensure it produces single database query when model is created at the end of the sequence + /** + * @test + */ + public function it_executes_2_queries_to_create_model_at_end_of_sequence(): void + { + CategoryFactory::new()->createMany(2); + + Category::query()->getConnection()->enableQueryLog(); + + CategoryFactory::new()->create(); + + self::assertCount(2, Category::query()->getConnection()->getQueryLog()); + } /** * @test From 61a249ea7b4e23b8274afb50fe5ed9921d6d64b8 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 14:11:03 +0200 Subject: [PATCH 06/23] Add possibility to create model in beginning of sequence --- CHANGELOG.md | 5 ++++ README.md | 17 ++++++++--- src/HasPosition.php | 53 +++++++++++++++++++-------------- src/Scopes/PositioningScope.php | 2 +- tests/Unit/ArrangeTest.php | 16 +++++----- tests/Unit/CreateTest.php | 45 ++++++++++++++++++++++++++-- 6 files changed, 99 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e45a9c..74744e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Possibility to create model in the middle of the sequence +- Possibility to create model in the beginning of the sequence + +### Changed + +- Rename method `getInitPosition` to `startPosition` ## [0.4.1] - 2023-02-08 diff --git a/README.md b/README.md index ae29635..7e88b10 100644 --- a/README.md +++ b/README.md @@ -58,17 +58,26 @@ Models simply have an integer `position' attribute corresponding to the model's The `position' attribute is a kind of array index and is automatically inserted when a new model is created. -By default, the model takes a position at the very end of the sequence. - -The initial position gets a `0` value by default. To change that, override the `getInitPosition` method in the model. +The starting position gets a `0` value by default. To change that, override the `startPosition` method in the model: ```php -public function getInitPosition(): int +public function startPosition(): int { return 0; } ``` +By default, the created model takes a position at the very end of the sequence. If you need to customize that behaviour, you can override the `nextPosition` method: + +```php +public function nextPosition(): ?int +{ + return $this->startPosition(); +} +``` + +In that example, a new model will be created in the beginning of the sequence. + ### Deleting models When a model is deleted, the positions of other records in the sequence are updated automatically. diff --git a/src/HasPosition.php b/src/HasPosition.php index a32c795..97b6d9c 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -26,11 +26,7 @@ public static function bootHasPosition(): void static::addGlobalScope(new PositioningScope()); static::creating(static function (self $model) { - if ($model->getPosition() === null) { - $model->assignPosition(); - } else { - $model->withShiftPosition(); - } + $model->assignPosition(); }); // @todo consider using "saved" event for this. @@ -73,7 +69,7 @@ public function getPositionColumn(): string /** * Get a value of the starting position. */ - public function getInitPosition(): int + public function startPosition(): int { return 0; } @@ -94,9 +90,12 @@ public function shouldShiftPosition(): bool return $this->shiftPosition; } - public function withShiftPosition(bool $value = true): self + /** + * Specify that the model should shift position of other models in the sequence. + */ + public function withShiftPosition(): self { - $this->shiftPosition = $value; + $this->shiftPosition = true; return $this; } @@ -112,7 +111,7 @@ public function getPosition(): ?int /** * Set the position to the given value. */ - public function setPosition(int $position): Model + public function setPosition(?int $position): Model { return $this->setAttribute($this->getPositionColumn(), $position); } @@ -172,33 +171,39 @@ protected function newPositionQuery(): Builder return $this->newQuery(); } -// /** -// * Assign the next position value to the model if it is missing. -// */ -// protected function assignPositionIfMissing(): void -// { -// if (null === $this->getPosition()) { -// $this->assignPosition(); -// } -// } - /** * Assign the next position value to the model. */ - protected function assignPosition(): Model + protected function assignPosition(): void { - return $this->setPosition($this->getNextPosition()); + if ($this->getPosition() === null) { + $this->setPosition($this->nextPosition()); + } + + if ($this->getPosition() !== null) { + $this->withShiftPosition(); + } else { + $this->setPosition($this->getEndPosition()); + } + } + + /** + * Get the next position in the sequence for the model. + */ + protected function nextPosition(): ?int + { + return null; } /** * Determine the next position value in the model sequence. */ - protected function getNextPosition(): int + protected function getEndPosition(): int { $maxPosition = $this->getMaxPosition(); if (null === $maxPosition) { - return $this->getInitPosition(); + return $this->startPosition(); } return $maxPosition + 1; @@ -214,6 +219,8 @@ protected function getMaxPosition(): ?int /** * Shift models in a sequence before the move to a new position. + * + * @todo rework to shift after move. */ protected function shiftBeforeMove(int $newPosition, int $oldPosition): void { diff --git a/src/Scopes/PositioningScope.php b/src/Scopes/PositioningScope.php index fb2e993..92fd5ef 100644 --- a/src/Scopes/PositioningScope.php +++ b/src/Scopes/PositioningScope.php @@ -36,7 +36,7 @@ public function apply(Builder $query, Model $model): void */ public function arrangeByKeys(Builder $builder, array $keys, int $startPosition = null): void { - $startPosition = $startPosition ?: $builder->getModel()->getInitPosition(); + $startPosition = $startPosition ?: $builder->getModel()->startPosition(); foreach ($keys as $position => $key) { (clone $builder)->whereKey($key) diff --git a/tests/Unit/ArrangeTest.php b/tests/Unit/ArrangeTest.php index 231470e..1489467 100644 --- a/tests/Unit/ArrangeTest.php +++ b/tests/Unit/ArrangeTest.php @@ -37,9 +37,9 @@ public function it_can_arrange_models_by_keys(): void /** * @test */ - public function it_can_arrange_models_with_init_positions_by_keys(): void + public function it_can_arrange_models_with_start_positions_by_keys(): void { - $initPosition = 5; + $startPosition = 5; $category0 = CategoryFactory::new()->create(); $category1 = CategoryFactory::new()->create(); @@ -53,12 +53,12 @@ public function it_can_arrange_models_with_init_positions_by_keys(): void $category0->getKey(), $category4->getKey(), $category1->getKey(), - ], $initPosition); + ], $startPosition); - static::assertSame(0 + $initPosition, $category2->fresh()->getPosition()); - static::assertSame(1 + $initPosition, $category3->fresh()->getPosition()); - static::assertSame(2 + $initPosition, $category0->fresh()->getPosition()); - static::assertSame(3 + $initPosition, $category4->fresh()->getPosition()); - static::assertSame(4 + $initPosition, $category1->fresh()->getPosition()); + static::assertSame(0 + $startPosition, $category2->fresh()->getPosition()); + static::assertSame(1 + $startPosition, $category3->fresh()->getPosition()); + static::assertSame(2 + $startPosition, $category0->fresh()->getPosition()); + static::assertSame(3 + $startPosition, $category4->fresh()->getPosition()); + static::assertSame(4 + $startPosition, $category1->fresh()->getPosition()); } } diff --git a/tests/Unit/CreateTest.php b/tests/Unit/CreateTest.php index d806f0e..fb7043d 100644 --- a/tests/Unit/CreateTest.php +++ b/tests/Unit/CreateTest.php @@ -64,9 +64,10 @@ public function it_does_not_override_position_value_if_it_is_set_already(): void */ public function it_can_configure_start_position_value(): void { - $fakeCategory = Mockery::mock(Category::class)->makePartial(); + $fakeCategory = Mockery::mock(Category::class); + $fakeCategory->makePartial(); $fakeCategory->shouldReceive('newInstance')->andReturnSelf(); - $fakeCategory->shouldReceive('getInitPosition')->andReturn(23); + $fakeCategory->shouldReceive('startPosition')->andReturn(23); $fakeCategory->__construct(); $category = Category::query()->setModel($fakeCategory)->create(); @@ -77,7 +78,7 @@ public function it_can_configure_start_position_value(): void /** * @test */ - public function it_can_create_model_in_the_middle_of_the_sequence(): void + public function it_can_create_model_in_middle_of_sequence(): void { $categories = CategoryFactory::new()->createMany(2); @@ -89,4 +90,42 @@ public function it_can_create_model_in_the_middle_of_the_sequence(): void static::assertSame($categories[0]->fresh()->position, 0); static::assertSame($categories[1]->fresh()->position, 2); } + + /** + * @test + */ + public function it_can_create_model_at_start_of_sequence(): void + { + $categories = CategoryFactory::new()->createMany(2); + + $category = CategoryFactory::new() + ->position(0) + ->create(); + + static::assertSame(0, $category->position); + static::assertSame($categories[0]->fresh()->position, 1); + static::assertSame($categories[1]->fresh()->position, 2); + } + + /** + * @test + */ + public function it_can_automatically_create_model_at_start_of_sequence(): void + { + $categories = CategoryFactory::new()->createMany(2); + + $fakeCategory = Mockery::mock(Category::class); + $fakeCategory->makePartial(); + $fakeCategory->shouldAllowMockingProtectedMethods(); + $fakeCategory->shouldReceive('newInstance')->andReturnSelf(); + $fakeCategory->shouldReceive('startPosition')->andReturn(0); + $fakeCategory->shouldReceive('nextPosition')->andReturn(0); + $fakeCategory->__construct(); + + $category = Category::query()->setModel($fakeCategory)->create(); + + static::assertSame(0, $category->position); + static::assertSame($categories[0]->fresh()->position, 1); + static::assertSame($categories[1]->fresh()->position, 2); + } } From 0d8ad466d23a7c66c573c5129de3ba6bf981fdaa Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 14:11:23 +0200 Subject: [PATCH 07/23] Update test structure --- tests/{Unit => }/ArrangeTest.php | 3 +-- tests/{Unit => }/CreateTest.php | 3 +-- tests/{Unit => }/DeleteTest.php | 3 +-- tests/{Unit => }/MoveTest.php | 3 +-- tests/{Unit => }/OrderTest.php | 3 +-- tests/{Unit => }/ScopePositionQueryTest.php | 3 +-- tests/{Unit => }/SwapTest.php | 3 +-- tests/{Unit => }/UpdateTest.php | 3 +-- 8 files changed, 8 insertions(+), 16 deletions(-) rename tests/{Unit => }/ArrangeTest.php (96%) rename tests/{Unit => }/CreateTest.php (97%) rename tests/{Unit => }/DeleteTest.php (93%) rename tests/{Unit => }/MoveTest.php (95%) rename tests/{Unit => }/OrderTest.php (96%) rename tests/{Unit => }/ScopePositionQueryTest.php (96%) rename tests/{Unit => }/SwapTest.php (91%) rename tests/{Unit => }/UpdateTest.php (94%) diff --git a/tests/Unit/ArrangeTest.php b/tests/ArrangeTest.php similarity index 96% rename from tests/Unit/ArrangeTest.php rename to tests/ArrangeTest.php index 1489467..226f2db 100644 --- a/tests/Unit/ArrangeTest.php +++ b/tests/ArrangeTest.php @@ -1,10 +1,9 @@ Date: Thu, 9 Feb 2023 14:23:04 +0200 Subject: [PATCH 08/23] Refactor --- src/HasPosition.php | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index 97b6d9c..3c88f7c 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -29,20 +29,24 @@ public static function bootHasPosition(): void $model->assignPosition(); }); - // @todo consider using "saved" event for this. static::created(static function (self $model) { if ($model->shouldShiftPosition()) { - // @todo consider extracting into "others" method. $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); } }); static::updating(static function (self $model) { - if ($model->isDirty($model->getPositionColumn())) { + if ($model->isMoving()) { $model->shiftBeforeMove($model->getPosition(), $model->getOriginal($model->getPositionColumn())); } }); +// static::updated(static function (self $model) { +// if ($model->shouldShiftPosition()) { +// $model->shiftBeforeMove($model->getPosition(), $model->getOriginal($model->getPositionColumn())); +// } +// }); + static::deleted(static function (self $model) { $model->newPositionQuery()->shiftToStart($model->getPosition()); }); @@ -217,6 +221,14 @@ protected function getMaxPosition(): ?int return $this->newPositionQuery()->max($this->getPositionColumn()); } + /** + * Determine if the model is moving to the new position. + */ + public function isMoving(): bool + { + return $this->isDirty($this->getPositionColumn()); + } + /** * Shift models in a sequence before the move to a new position. * From 4d12e1b8ee8885409f3bd4a8b50cf8fab95eb8fa Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 14:39:47 +0200 Subject: [PATCH 09/23] Refactor --- src/HasPosition.php | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index 3c88f7c..a102560 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -32,11 +32,12 @@ public static function bootHasPosition(): void static::created(static function (self $model) { if ($model->shouldShiftPosition()) { $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); + $model->disableShiftingPosition(); } }); static::updating(static function (self $model) { - if ($model->isMoving()) { + if ($model->isDirty($model->getPositionColumn())()) { $model->shiftBeforeMove($model->getPosition(), $model->getOriginal($model->getPositionColumn())); } }); @@ -44,6 +45,7 @@ public static function bootHasPosition(): void // static::updated(static function (self $model) { // if ($model->shouldShiftPosition()) { // $model->shiftBeforeMove($model->getPosition(), $model->getOriginal($model->getPositionColumn())); +// $model->disableShiftingPosition(); // } // }); @@ -97,7 +99,17 @@ public function shouldShiftPosition(): bool /** * Specify that the model should shift position of other models in the sequence. */ - public function withShiftPosition(): self + public function enableShiftingPosition(): self + { + $this->shiftPosition = true; + + return $this; + } + + /** + * Specify that the model should not shift position of other models in the sequence. + */ + public function disableShiftingPosition(): self { $this->shiftPosition = true; @@ -185,7 +197,7 @@ protected function assignPosition(): void } if ($this->getPosition() !== null) { - $this->withShiftPosition(); + $this->enableShiftingPosition(); } else { $this->setPosition($this->getEndPosition()); } @@ -221,14 +233,6 @@ protected function getMaxPosition(): ?int return $this->newPositionQuery()->max($this->getPositionColumn()); } - /** - * Determine if the model is moving to the new position. - */ - public function isMoving(): bool - { - return $this->isDirty($this->getPositionColumn()); - } - /** * Shift models in a sequence before the move to a new position. * From 89fc2c83445e99bebd520fd881bfeec92b061603 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 14:51:07 +0200 Subject: [PATCH 10/23] Refactor --- src/HasPosition.php | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index a102560..f7c6f13 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -37,17 +37,22 @@ public static function bootHasPosition(): void }); static::updating(static function (self $model) { - if ($model->isDirty($model->getPositionColumn())()) { - $model->shiftBeforeMove($model->getPosition(), $model->getOriginal($model->getPositionColumn())); + if ($model->isDirty($model->getPositionColumn())) { + $model->enableShiftingPosition(); } }); -// static::updated(static function (self $model) { -// if ($model->shouldShiftPosition()) { -// $model->shiftBeforeMove($model->getPosition(), $model->getOriginal($model->getPositionColumn())); -// $model->disableShiftingPosition(); -// } -// }); + static::updated(static function (self $model) { + if ($model->shouldShiftPosition()) { + if ($model->getPosition() < $model->getOriginal($model->getPositionColumn())) { + $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition(), $model->getOriginal($model->getPositionColumn())); + } elseif ($model->getPosition() > $model->getOriginal($model->getPositionColumn())) { + $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToStart($model->getOriginal($model->getPositionColumn()), $model->getPosition()); + } + + $model->disableShiftingPosition(); + } + }); static::deleted(static function (self $model) { $model->newPositionQuery()->shiftToStart($model->getPosition()); @@ -232,18 +237,4 @@ protected function getMaxPosition(): ?int { return $this->newPositionQuery()->max($this->getPositionColumn()); } - - /** - * Shift models in a sequence before the move to a new position. - * - * @todo rework to shift after move. - */ - protected function shiftBeforeMove(int $newPosition, int $oldPosition): void - { - if ($newPosition < $oldPosition) { - $this->newPositionQuery()->shiftToEnd($newPosition, $oldPosition); - } elseif ($newPosition > $oldPosition) { - $this->newPositionQuery()->shiftToStart($oldPosition, $newPosition); - } - } } From c42219069c29b4cc9d17bcd1bf0ff4d095fc839c Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 14:51:57 +0200 Subject: [PATCH 11/23] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74744e2..ef11b6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Rename method `getInitPosition` to `startPosition` +- Models now are shifted after the model update ## [0.4.1] - 2023-02-08 From b6a5ca5e313381e9c1d0f57e122272643e3e82a9 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 15:22:49 +0200 Subject: [PATCH 12/23] Refactor --- src/Scopes/PositioningScope.php | 61 +++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/src/Scopes/PositioningScope.php b/src/Scopes/PositioningScope.php index 92fd5ef..407b56e 100644 --- a/src/Scopes/PositioningScope.php +++ b/src/Scopes/PositioningScope.php @@ -12,11 +12,12 @@ class PositioningScope implements Scope /** * Extend the query builder with the needed functions. */ - public function extend(Builder $builder): void + public function extend(Builder $query): void { - $builder->macro('arrangeByKeys', [$this, 'arrangeByKeys']); - $builder->macro('shiftToStart', [$this, 'shiftToStart']); - $builder->macro('shiftToEnd', [$this, 'shiftToEnd']); + $query->macro('wherePositionBetween', [$this, 'wherePositionBetween']); + $query->macro('shiftToStart', [$this, 'shiftToStart']); + $query->macro('shiftToEnd', [$this, 'shiftToEnd']); + $query->macro('arrangeByKeys', [$this, 'arrangeByKeys']); } /** @@ -32,41 +33,51 @@ public function apply(Builder $query, Model $model): void } /** - * Arrange the models according to the given ordered keys. + * Select all models that are between the given positions. */ - public function arrangeByKeys(Builder $builder, array $keys, int $startPosition = null): void + public function wherePositionBetween(Builder $query, int $startPosition = null, int $endPosition = null): Builder { - $startPosition = $startPosition ?: $builder->getModel()->startPosition(); + $query->when($startPosition !== null, static function (Builder $query) use ($startPosition) { + $query->where($query->getModel()->getPositionColumn(), '>=', $startPosition); + }); - foreach ($keys as $position => $key) { - (clone $builder)->whereKey($key) - ->update([ - $builder->getModel()->getPositionColumn() => $startPosition + $position, - ]); - } + $query->when($endPosition !== null, static function (Builder $query) use ($endPosition) { + $query->where($query->getModel()->getPositionColumn(), '<=', $endPosition); + }); + + return $query; } /** * Shift all models that are between the given positions to the beginning of the sequence. */ - public function shiftToStart(Builder $builder, int $startPosition, int $stopPosition = null): int + public function shiftToStart(Builder $query, int $fromPosition = null, int $toPosition = null, int $amount = 1): int { - return $builder->where($builder->getModel()->getPositionColumn(), '>=', $startPosition) - ->when($stopPosition, static function (Builder $builder) use ($stopPosition) { - $builder->where($builder->getModel()->getPositionColumn(), '<=', $stopPosition); - }) - ->decrement($builder->getModel()->getPositionColumn()); + return $query->wherePositionBetween($fromPosition, $toPosition) + ->decrement($query->getModel()->getPositionColumn(), $amount); } /** * Shift all models that are between the given positions to the end of the sequence. */ - public function shiftToEnd(Builder $builder, int $startPosition, int $stopPosition = null): int + public function shiftToEnd(Builder $query, int $fromPosition, int $toPosition = null, int $amount = 1): int { - return $builder->where($builder->getModel()->getPositionColumn(), '>=', $startPosition) - ->when($stopPosition, static function (Builder $builder) use ($stopPosition) { - $builder->where($builder->getModel()->getPositionColumn(), '<=', $stopPosition); - }) - ->increment($builder->getModel()->getPositionColumn()); + return $query->wherePositionBetween($fromPosition, $toPosition) + ->increment($query->getModel()->getPositionColumn(), $amount); + } + + /** + * Arrange the models according to the given ordered keys. + */ + public function arrangeByKeys(Builder $query, array $keys, int $startPosition = null): void + { + $startPosition = $startPosition ?: $query->getModel()->startPosition(); + + foreach ($keys as $position => $key) { + (clone $query)->whereKey($key) + ->update([ + $query->getModel()->getPositionColumn() => $startPosition + $position, + ]); + } } } From f0099be328fff97494ad15fcd50c0f919b10fdd7 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 15:22:56 +0200 Subject: [PATCH 13/23] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef11b6e..4ae6de1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Possibility to create model in the middle of the sequence - Possibility to create model in the beginning of the sequence +- Extra argument for shift amount in `shiftToStart` and `shiftToEnd` methods ### Changed From 6180421490adb4ecd0ec532bc20f66429a411f65 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 15:26:46 +0200 Subject: [PATCH 14/23] Refactor --- src/HasPosition.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index f7c6f13..6a056d0 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -44,10 +44,12 @@ public static function bootHasPosition(): void static::updated(static function (self $model) { if ($model->shouldShiftPosition()) { - if ($model->getPosition() < $model->getOriginal($model->getPositionColumn())) { - $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition(), $model->getOriginal($model->getPositionColumn())); - } elseif ($model->getPosition() > $model->getOriginal($model->getPositionColumn())) { - $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToStart($model->getOriginal($model->getPositionColumn()), $model->getPosition()); + [$currentPosition, $previousPosition] = [$model->getPosition(), $model->getOriginal($model->getPositionColumn())]; + + if ($currentPosition < $previousPosition) { + $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($currentPosition, $previousPosition); + } elseif ($currentPosition > $previousPosition) { + $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToStart($previousPosition, $currentPosition); } $model->disableShiftingPosition(); @@ -172,6 +174,7 @@ public function move(int $newPosition): bool */ public function swap(self $that): void { + // @todo refactor with disabled shifting... static::withoutEvents(function () use ($that) { $thisPosition = $this->getPosition(); $thatPosition = $that->getPosition(); From b8418bf6072acdfb631ce2bef80299bfdd8a1577 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 15:44:14 +0200 Subject: [PATCH 15/23] Rename method `move` to `shift` --- CHANGELOG.md | 1 + README.md | 6 ++-- src/HasPosition.php | 26 +++++++++++------ tests/MoveTest.php | 56 ------------------------------------- tests/ShiftTest.php | 68 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 89 insertions(+), 68 deletions(-) delete mode 100644 tests/MoveTest.php create mode 100644 tests/ShiftTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ae6de1..dac7a65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Rename method `getInitPosition` to `startPosition` +- Rename method `move` to `shift` - Models now are shifted after the model update ## [0.4.1] - 2023-02-08 diff --git a/README.md b/README.md index 7e88b10..b767c46 100644 --- a/README.md +++ b/README.md @@ -119,12 +119,12 @@ $category->update([ The positions of other models will be automatically recalculated as well. -#### Move +#### Shift / Move -You can also use the `move` method that sets a new position value and updates the model immediately: +You can also use the `shift method that sets a new position value and updates the model immediately: ```php -$category->move(3); +$category->shift(3); ``` #### Swap diff --git a/src/HasPosition.php b/src/HasPosition.php index 6a056d0..6493def 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -32,18 +32,13 @@ public static function bootHasPosition(): void static::created(static function (self $model) { if ($model->shouldShiftPosition()) { $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); - $model->disableShiftingPosition(); - } - }); - static::updating(static function (self $model) { - if ($model->isDirty($model->getPositionColumn())) { - $model->enableShiftingPosition(); + $model->disableShiftingPosition(); } }); static::updated(static function (self $model) { - if ($model->shouldShiftPosition()) { + if ($model->isDirty($model->getPositionColumn())) { [$currentPosition, $previousPosition] = [$model->getPosition(), $model->getOriginal($model->getPositionColumn())]; if ($currentPosition < $previousPosition) { @@ -71,6 +66,19 @@ public function initializeHasPosition(): void ]); } + public static function withoutShiftingPosition(callable $callback) + { + $shifting = static::$shiftPosition; + + static::$shiftPosition = false; + + $result = $callback(); + + static::$shiftPosition = $shifting; + + return $result; + } + /** * Get the name of the "position" column. */ @@ -156,9 +164,9 @@ public function scopeOrderByInversePosition(Builder $query): Builder } /** - * Move the model to the new position. + * Shift the model to the new position. */ - public function move(int $newPosition): bool + public function shift(int $newPosition): bool { $oldPosition = $this->getPosition(); diff --git a/tests/MoveTest.php b/tests/MoveTest.php deleted file mode 100644 index 33d28ce..0000000 --- a/tests/MoveTest.php +++ /dev/null @@ -1,56 +0,0 @@ -position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); - $category2 = CategoryFactory::new()->position(2)->create(); - - $category2->move(0); - - static::assertSame(0, $category2->fresh()->getPosition()); - static::assertSame(1, $category0->fresh()->getPosition()); - static::assertSame(2, $category1->fresh()->getPosition()); - } - - /** - * @test - */ - public function it_can_move_model_to_increase_position(): void - { - $category0 = CategoryFactory::new()->position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); - $category2 = CategoryFactory::new()->position(2)->create(); - - $category0->move(2); - - static::assertSame(0, $category1->fresh()->getPosition()); - static::assertSame(1, $category2->fresh()->getPosition()); - static::assertSame(2, $category0->fresh()->getPosition()); - } - - /** - * @test - */ - public function it_does_not_move_model_to_the_same_position(): void - { - $category = CategoryFactory::new()->position(3)->create(); - - DB::connection()->enableQueryLog(); - - $result = $category->move(3); - - static::assertEmpty(DB::connection()->getQueryLog()); - static::assertFalse($result); - } -} diff --git a/tests/ShiftTest.php b/tests/ShiftTest.php new file mode 100644 index 0000000..0fefd61 --- /dev/null +++ b/tests/ShiftTest.php @@ -0,0 +1,68 @@ +createMany(3); + + $categories[2]->shift(0); + + static::assertSame(0, $categories[2]->fresh()->getPosition()); + static::assertSame(1, $categories[0]->fresh()->getPosition()); + static::assertSame(2, $categories[1]->fresh()->getPosition()); + } + + /** + * @test + */ + public function it_can_shift_model_to_increase_position(): void + { + $categories = CategoryFactory::new()->createMany(3); + + $categories[0]->shift(2); + + static::assertSame(0, $categories[1]->fresh()->getPosition()); + static::assertSame(1, $categories[2]->fresh()->getPosition()); + static::assertSame(2, $categories[0]->fresh()->getPosition()); + } + + /** + * @test + */ + public function it_does_not_shift_model_to_the_same_position(): void + { + $category = CategoryFactory::new()->position(3)->create(); + + DB::connection()->enableQueryLog(); + + $result = $category->shift(3); + + static::assertEmpty(DB::connection()->getQueryLog()); + static::assertFalse($result); + } + +// /** +// * @test +// */ +// public function it_can_update_position_without_shifting_others(): void +// { +// $categories = CategoryFactory::new()->createMany(3); +// +// Category::withoutShiftingPosition(function () use ($categories) { +// $categories[0]->shift(2); +// }); +// +// static::assertSame(2, $categories[0]->fresh()->getPosition()); +// static::assertSame(1, $categories[1]->fresh()->getPosition()); +// static::assertSame(2, $categories[2]->fresh()->getPosition()); +// } +} From 912a87a1d129dd53be9ce626640748eb569c3d7d Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 15:47:43 +0200 Subject: [PATCH 16/23] Rollback --- CHANGELOG.md | 1 - src/HasPosition.php | 4 ++-- tests/{ShiftTest.php => MoveTest.php} | 18 +++++++++--------- 3 files changed, 11 insertions(+), 12 deletions(-) rename tests/{ShiftTest.php => MoveTest.php} (75%) diff --git a/CHANGELOG.md b/CHANGELOG.md index dac7a65..4ae6de1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Rename method `getInitPosition` to `startPosition` -- Rename method `move` to `shift` - Models now are shifted after the model update ## [0.4.1] - 2023-02-08 diff --git a/src/HasPosition.php b/src/HasPosition.php index 6493def..ad9876b 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -164,9 +164,9 @@ public function scopeOrderByInversePosition(Builder $query): Builder } /** - * Shift the model to the new position. + * Move the model to the new position. */ - public function shift(int $newPosition): bool + public function move(int $newPosition): bool { $oldPosition = $this->getPosition(); diff --git a/tests/ShiftTest.php b/tests/MoveTest.php similarity index 75% rename from tests/ShiftTest.php rename to tests/MoveTest.php index 0fefd61..cde5a3a 100644 --- a/tests/ShiftTest.php +++ b/tests/MoveTest.php @@ -5,16 +5,16 @@ use Illuminate\Support\Facades\DB; use Nevadskiy\Position\Tests\Support\Factories\CategoryFactory; -class ShiftTest extends TestCase +class MoveTest extends TestCase { /** * @test */ - public function it_can_shift_model_to_decrease_position(): void + public function it_can_move_model_to_decrease_position(): void { $categories = CategoryFactory::new()->createMany(3); - $categories[2]->shift(0); + $categories[2]->move(0); static::assertSame(0, $categories[2]->fresh()->getPosition()); static::assertSame(1, $categories[0]->fresh()->getPosition()); @@ -24,11 +24,11 @@ public function it_can_shift_model_to_decrease_position(): void /** * @test */ - public function it_can_shift_model_to_increase_position(): void + public function it_can_move_model_to_increase_position(): void { $categories = CategoryFactory::new()->createMany(3); - $categories[0]->shift(2); + $categories[0]->move(2); static::assertSame(0, $categories[1]->fresh()->getPosition()); static::assertSame(1, $categories[2]->fresh()->getPosition()); @@ -38,13 +38,13 @@ public function it_can_shift_model_to_increase_position(): void /** * @test */ - public function it_does_not_shift_model_to_the_same_position(): void + public function it_does_not_move_model_to_the_same_position(): void { $category = CategoryFactory::new()->position(3)->create(); DB::connection()->enableQueryLog(); - $result = $category->shift(3); + $result = $category->move(3); static::assertEmpty(DB::connection()->getQueryLog()); static::assertFalse($result); @@ -53,12 +53,12 @@ public function it_does_not_shift_model_to_the_same_position(): void // /** // * @test // */ -// public function it_can_update_position_without_shifting_others(): void +// public function it_can_update_position_without_moveing_others(): void // { // $categories = CategoryFactory::new()->createMany(3); // // Category::withoutShiftingPosition(function () use ($categories) { -// $categories[0]->shift(2); +// $categories[0]->move(2); // }); // // static::assertSame(2, $categories[0]->fresh()->getPosition()); From 8d33fdde8ac6299c43fa11f02b2d7ea9ec0c8e34 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 15:49:40 +0200 Subject: [PATCH 17/23] Fix --- src/HasPosition.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index ad9876b..8cddc2b 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -46,8 +46,6 @@ public static function bootHasPosition(): void } elseif ($currentPosition > $previousPosition) { $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToStart($previousPosition, $currentPosition); } - - $model->disableShiftingPosition(); } }); From ef26d6fc84633eefea72e6e1e001ffee47ebecf1 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 16:01:05 +0200 Subject: [PATCH 18/23] Refactor --- src/HasPosition.php | 56 ++++++++++++++++---------------------------- tests/CreateTest.php | 26 ++++++++++---------- tests/MoveTest.php | 31 ++++++++++++------------ 3 files changed, 49 insertions(+), 64 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index 8cddc2b..3adedf0 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -16,7 +16,7 @@ trait HasPosition * * @var bool */ - protected $shiftPosition = false; + protected static $shiftPosition = true; /** * Boot the trait. @@ -30,15 +30,14 @@ public static function bootHasPosition(): void }); static::created(static function (self $model) { - if ($model->shouldShiftPosition()) { + // @todo consider marking as dirty as well... + if (static::shouldShiftPosition()) { $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); - - $model->disableShiftingPosition(); } }); static::updated(static function (self $model) { - if ($model->isDirty($model->getPositionColumn())) { + if (static::shouldShiftPosition() && $model->isDirty($model->getPositionColumn())) { [$currentPosition, $previousPosition] = [$model->getPosition(), $model->getOriginal($model->getPositionColumn())]; if ($currentPosition < $previousPosition) { @@ -50,7 +49,9 @@ public static function bootHasPosition(): void }); static::deleted(static function (self $model) { - $model->newPositionQuery()->shiftToStart($model->getPosition()); + if (static::shouldShiftPosition()) { + $model->newPositionQuery()->shiftToStart($model->getPosition()); + } }); } @@ -64,19 +65,6 @@ public function initializeHasPosition(): void ]); } - public static function withoutShiftingPosition(callable $callback) - { - $shifting = static::$shiftPosition; - - static::$shiftPosition = false; - - $result = $callback(); - - static::$shiftPosition = $shifting; - - return $result; - } - /** * Get the name of the "position" column. */ @@ -102,31 +90,27 @@ public function alwaysOrderByPosition(): bool } /** - * Determine if the model should shift position of other models in the sequence. + * Execute a callback without shifting position of models. */ - public function shouldShiftPosition(): bool + public static function withoutShiftingPosition(callable $callback) { - return $this->shiftPosition; - } + $shifting = static::$shiftPosition; - /** - * Specify that the model should shift position of other models in the sequence. - */ - public function enableShiftingPosition(): self - { - $this->shiftPosition = true; + static::$shiftPosition = false; - return $this; + $result = $callback(); + + static::$shiftPosition = $shifting; + + return $result; } /** - * Specify that the model should not shift position of other models in the sequence. + * Determine if the model should shift position of other models in the sequence. */ - public function disableShiftingPosition(): self + public static function shouldShiftPosition(): bool { - $this->shiftPosition = true; - - return $this; + return static::$shiftPosition; } /** @@ -211,7 +195,7 @@ protected function assignPosition(): void } if ($this->getPosition() !== null) { - $this->enableShiftingPosition(); + // @todo mark as dirty... } else { $this->setPosition($this->getEndPosition()); } diff --git a/tests/CreateTest.php b/tests/CreateTest.php index 67c2cc5..d8ab1d9 100644 --- a/tests/CreateTest.php +++ b/tests/CreateTest.php @@ -32,19 +32,19 @@ public function it_creates_model_at_end_of_sequence(): void // @todo what if we start to update position after that? previous "shift" value is still working. } - /** - * @test - */ - public function it_executes_2_queries_to_create_model_at_end_of_sequence(): void - { - CategoryFactory::new()->createMany(2); - - Category::query()->getConnection()->enableQueryLog(); - - CategoryFactory::new()->create(); - - self::assertCount(2, Category::query()->getConnection()->getQueryLog()); - } +// /** +// * @test +// */ +// public function it_executes_2_queries_to_create_model_at_end_of_sequence(): void +// { +// CategoryFactory::new()->createMany(2); +// +// Category::query()->getConnection()->enableQueryLog(); +// +// CategoryFactory::new()->create(); +// +// self::assertCount(2, Category::query()->getConnection()->getQueryLog()); +// } /** * @test diff --git a/tests/MoveTest.php b/tests/MoveTest.php index cde5a3a..78553b3 100644 --- a/tests/MoveTest.php +++ b/tests/MoveTest.php @@ -4,6 +4,7 @@ use Illuminate\Support\Facades\DB; use Nevadskiy\Position\Tests\Support\Factories\CategoryFactory; +use Nevadskiy\Position\Tests\Support\Models\Category; class MoveTest extends TestCase { @@ -50,19 +51,19 @@ public function it_does_not_move_model_to_the_same_position(): void static::assertFalse($result); } -// /** -// * @test -// */ -// public function it_can_update_position_without_moveing_others(): void -// { -// $categories = CategoryFactory::new()->createMany(3); -// -// Category::withoutShiftingPosition(function () use ($categories) { -// $categories[0]->move(2); -// }); -// -// static::assertSame(2, $categories[0]->fresh()->getPosition()); -// static::assertSame(1, $categories[1]->fresh()->getPosition()); -// static::assertSame(2, $categories[2]->fresh()->getPosition()); -// } + /** + * @test + */ + public function it_can_update_position_without_moving_others(): void + { + $categories = CategoryFactory::new()->createMany(3); + + Category::withoutShiftingPosition(function () use ($categories) { + $categories[0]->move(2); + }); + + static::assertSame(2, $categories[0]->fresh()->getPosition()); + static::assertSame(1, $categories[1]->fresh()->getPosition()); + static::assertSame(2, $categories[2]->fresh()->getPosition()); + } } From 2f342e54c53e7b005fbcb1c12c97086158b6bcc4 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 16:14:58 +0200 Subject: [PATCH 19/23] Add possibility to update positions without shifting other models --- CHANGELOG.md | 1 + src/HasPosition.php | 24 ++++++++++++++++-------- tests/CreateTest.php | 28 +++++++++++++--------------- tests/MoveTest.php | 4 ++-- tests/SwapTest.php | 33 ++++++++++++++++++++++----------- 5 files changed, 54 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ae6de1..dbebe14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Possibility to create model in the middle of the sequence - Possibility to create model in the beginning of the sequence - Extra argument for shift amount in `shiftToStart` and `shiftToEnd` methods +- Possibility to update positions without shifting other models ### Changed diff --git a/src/HasPosition.php b/src/HasPosition.php index 3adedf0..ad188a2 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -30,14 +30,13 @@ public static function bootHasPosition(): void }); static::created(static function (self $model) { - // @todo consider marking as dirty as well... - if (static::shouldShiftPosition()) { + if (static::shouldShiftPosition() && $model->isMoving()) { $model->newPositionQuery()->whereKeyNot($model->getKey())->shiftToEnd($model->getPosition()); } }); static::updated(static function (self $model) { - if (static::shouldShiftPosition() && $model->isDirty($model->getPositionColumn())) { + if (static::shouldShiftPosition() && $model->isMoving()) { [$currentPosition, $previousPosition] = [$model->getPosition(), $model->getOriginal($model->getPositionColumn())]; if ($currentPosition < $previousPosition) { @@ -159,13 +158,20 @@ public function move(int $newPosition): bool return $this->setPosition($newPosition)->save(); } + /** + * Determine if the model is currently moving to a new position. + */ + public function isMoving(): bool + { + return $this->isDirty($this->getPositionColumn()); + } + /** * Swap the model position with another model. */ public function swap(self $that): void { - // @todo refactor with disabled shifting... - static::withoutEvents(function () use ($that) { + static::withoutShiftingPosition(function () use ($that) { $thisPosition = $this->getPosition(); $thatPosition = $that->getPosition(); @@ -194,11 +200,13 @@ protected function assignPosition(): void $this->setPosition($this->nextPosition()); } - if ($this->getPosition() !== null) { - // @todo mark as dirty... - } else { + if ($this->getPosition() === null) { $this->setPosition($this->getEndPosition()); + + // Sync original attribute to not shift other models when the model will be created + $this->syncOriginalAttribute($this->getPositionColumn()); } + } /** diff --git a/tests/CreateTest.php b/tests/CreateTest.php index d8ab1d9..f423d3d 100644 --- a/tests/CreateTest.php +++ b/tests/CreateTest.php @@ -28,23 +28,21 @@ public function it_creates_model_at_end_of_sequence(): void $category = CategoryFactory::new()->create(); static::assertSame(2, $category->position); - - // @todo what if we start to update position after that? previous "shift" value is still working. } -// /** -// * @test -// */ -// public function it_executes_2_queries_to_create_model_at_end_of_sequence(): void -// { -// CategoryFactory::new()->createMany(2); -// -// Category::query()->getConnection()->enableQueryLog(); -// -// CategoryFactory::new()->create(); -// -// self::assertCount(2, Category::query()->getConnection()->getQueryLog()); -// } + /** + * @test + */ + public function it_executes_2_queries_to_create_model_at_end_of_sequence(): void + { + CategoryFactory::new()->createMany(2); + + Category::query()->getConnection()->enableQueryLog(); + + CategoryFactory::new()->create(); + + self::assertCount(2, Category::query()->getConnection()->getQueryLog()); + } /** * @test diff --git a/tests/MoveTest.php b/tests/MoveTest.php index 78553b3..f21fa92 100644 --- a/tests/MoveTest.php +++ b/tests/MoveTest.php @@ -43,11 +43,11 @@ public function it_does_not_move_model_to_the_same_position(): void { $category = CategoryFactory::new()->position(3)->create(); - DB::connection()->enableQueryLog(); + Category::query()->getConnection()->enableQueryLog(); $result = $category->move(3); - static::assertEmpty(DB::connection()->getQueryLog()); + static::assertEmpty(Category::query()->getConnection()->getQueryLog()); static::assertFalse($result); } diff --git a/tests/SwapTest.php b/tests/SwapTest.php index a8b8400..6dcedbc 100644 --- a/tests/SwapTest.php +++ b/tests/SwapTest.php @@ -3,6 +3,7 @@ namespace Nevadskiy\Position\Tests; use Nevadskiy\Position\Tests\Support\Factories\CategoryFactory; +use Nevadskiy\Position\Tests\Support\Models\Category; class SwapTest extends TestCase { @@ -11,14 +12,26 @@ class SwapTest extends TestCase */ public function it_can_swap_models(): void { - $category0 = CategoryFactory::new()->create(); - $category1 = CategoryFactory::new()->create(); - $category2 = CategoryFactory::new()->create(); + $categories = CategoryFactory::new()->createMany(3); - $category0->swap($category2); + $categories[0]->swap($categories[2]); - static::assertSame($category0->fresh()->getPosition(), 2); - static::assertSame($category2->fresh()->getPosition(), 0); + static::assertSame($categories[0]->fresh()->getPosition(), 2); + static::assertSame($categories[2]->fresh()->getPosition(), 0); + } + + /** + * @test + */ + public function it_executes_only_2_queries_to_swap_models(): void + { + $categories = CategoryFactory::new()->createMany(3); + + Category::query()->getConnection()->enableQueryLog(); + + $categories[0]->swap($categories[2]); + + self::assertCount(2, Category::query()->getConnection()->getQueryLog()); } /** @@ -26,12 +39,10 @@ public function it_can_swap_models(): void */ public function it_does_not_break_another_positions(): void { - $category0 = CategoryFactory::new()->create(); - $category1 = CategoryFactory::new()->create(); - $category2 = CategoryFactory::new()->create(); + $categories = CategoryFactory::new()->createMany(3); - $category2->swap($category0); + $categories[2]->swap($categories[0]); - static::assertSame($category1->fresh()->getPosition(), 1); + static::assertSame($categories[1]->fresh()->getPosition(), 1); } } From 023cac01e61506467aa8a9fc3d8824618363afe1 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 16:17:15 +0200 Subject: [PATCH 20/23] WIP --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index b767c46..93853af 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,18 @@ The `swap` method swaps the position of two models. $category->swap($anotherCategory); ``` +#### Without shifting + +By default, the package automatically updates position of other models when the model position is updated. + +[//]: # (TODO:) + +```php +Category::withoutShifting(function () { + // @todo +}) +``` + #### Arrange It is also possible to arrange models by their IDs. From bd35e6d603de902825bbad7a74c9e10490e6c5c1 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 19:17:14 +0200 Subject: [PATCH 21/23] Update README.md --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 93853af..b28a70b 100644 --- a/README.md +++ b/README.md @@ -137,13 +137,13 @@ $category->swap($anotherCategory); #### Without shifting -By default, the package automatically updates position of other models when the model position is updated. +By default, the package automatically updates the position of other models when the model position is updated. -[//]: # (TODO:) +If you want to update the model position without shifting the positions of other models, you can use the `withoutShifting` method: ```php Category::withoutShifting(function () { - // @todo + $category->move(5); }) ``` From 215d8656ed8c7151bf0aca8d9f86c0f5e6d4e29f Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 19:24:25 +0200 Subject: [PATCH 22/23] Refactor --- tests/ArrangeTest.php | 52 ++++++++++++++------------------- tests/DeleteTest.php | 22 ++++++-------- tests/OrderTest.php | 68 +++++++++++++++++++++++++++++++++---------- tests/UpdateTest.php | 24 +++++++-------- 4 files changed, 94 insertions(+), 72 deletions(-) diff --git a/tests/ArrangeTest.php b/tests/ArrangeTest.php index 226f2db..d26d0c5 100644 --- a/tests/ArrangeTest.php +++ b/tests/ArrangeTest.php @@ -12,25 +12,21 @@ class ArrangeTest extends TestCase */ public function it_can_arrange_models_by_keys(): void { - $category0 = CategoryFactory::new()->create(); - $category1 = CategoryFactory::new()->create(); - $category2 = CategoryFactory::new()->create(); - $category3 = CategoryFactory::new()->create(); - $category4 = CategoryFactory::new()->create(); + $categories = CategoryFactory::new()->createMany(5); Category::arrangeByKeys([ - $category2->getKey(), - $category3->getKey(), - $category0->getKey(), - $category4->getKey(), - $category1->getKey(), + $categories[2]->getKey(), + $categories[3]->getKey(), + $categories[0]->getKey(), + $categories[4]->getKey(), + $categories[1]->getKey(), ]); - static::assertSame(0, $category2->fresh()->getPosition()); - static::assertSame(1, $category3->fresh()->getPosition()); - static::assertSame(2, $category0->fresh()->getPosition()); - static::assertSame(3, $category4->fresh()->getPosition()); - static::assertSame(4, $category1->fresh()->getPosition()); + static::assertSame(0, $categories[2]->fresh()->getPosition()); + static::assertSame(1, $categories[3]->fresh()->getPosition()); + static::assertSame(2, $categories[0]->fresh()->getPosition()); + static::assertSame(3, $categories[4]->fresh()->getPosition()); + static::assertSame(4, $categories[1]->fresh()->getPosition()); } /** @@ -40,24 +36,20 @@ public function it_can_arrange_models_with_start_positions_by_keys(): void { $startPosition = 5; - $category0 = CategoryFactory::new()->create(); - $category1 = CategoryFactory::new()->create(); - $category2 = CategoryFactory::new()->create(); - $category3 = CategoryFactory::new()->create(); - $category4 = CategoryFactory::new()->create(); + $categories = CategoryFactory::new()->createMany(5); Category::arrangeByKeys([ - $category2->getKey(), - $category3->getKey(), - $category0->getKey(), - $category4->getKey(), - $category1->getKey(), + $categories[2]->getKey(), + $categories[3]->getKey(), + $categories[0]->getKey(), + $categories[4]->getKey(), + $categories[1]->getKey(), ], $startPosition); - static::assertSame(0 + $startPosition, $category2->fresh()->getPosition()); - static::assertSame(1 + $startPosition, $category3->fresh()->getPosition()); - static::assertSame(2 + $startPosition, $category0->fresh()->getPosition()); - static::assertSame(3 + $startPosition, $category4->fresh()->getPosition()); - static::assertSame(4 + $startPosition, $category1->fresh()->getPosition()); + static::assertSame(0 + $startPosition, $categories[2]->fresh()->getPosition()); + static::assertSame(1 + $startPosition, $categories[3]->fresh()->getPosition()); + static::assertSame(2 + $startPosition, $categories[0]->fresh()->getPosition()); + static::assertSame(3 + $startPosition, $categories[4]->fresh()->getPosition()); + static::assertSame(4 + $startPosition, $categories[1]->fresh()->getPosition()); } } diff --git a/tests/DeleteTest.php b/tests/DeleteTest.php index 861fe60..01106a2 100644 --- a/tests/DeleteTest.php +++ b/tests/DeleteTest.php @@ -11,16 +11,14 @@ class DeleteTest extends TestCase */ public function it_updates_position_values_on_another_model_delete(): void { - $category0 = CategoryFactory::new()->create(); - $category1 = CategoryFactory::new()->create(); - $category2 = CategoryFactory::new()->create(); + $categories = CategoryFactory::new()->createMany(3); - static::assertSame(2, $category2->getPosition()); + static::assertSame(2, $categories[2]->getPosition()); - $category1->delete(); + $categories[1]->delete(); - static::assertSame(1, $category2->fresh()->getPosition()); - static::assertSame(0, $category0->fresh()->getPosition()); + static::assertSame(1, $categories[2]->fresh()->getPosition()); + static::assertSame(0, $categories[0]->fresh()->getPosition()); } /** @@ -28,13 +26,11 @@ public function it_updates_position_values_on_another_model_delete(): void */ public function it_does_not_update_positions_when_last_record_is_deleted(): void { - $category0 = CategoryFactory::new()->create(); - $category1 = CategoryFactory::new()->create(); - $category2 = CategoryFactory::new()->create(); + $categories = CategoryFactory::new()->createMany(3); - $category2->delete(); + $categories[2]->delete(); - static::assertSame(0, $category0->fresh()->getPosition()); - static::assertSame(1, $category1->fresh()->getPosition()); + static::assertSame(0, $categories[0]->fresh()->getPosition()); + static::assertSame(1, $categories[1]->fresh()->getPosition()); } } diff --git a/tests/OrderTest.php b/tests/OrderTest.php index 19a95b9..bc12627 100644 --- a/tests/OrderTest.php +++ b/tests/OrderTest.php @@ -13,11 +13,21 @@ class OrderTest extends TestCase */ public function it_can_order_models_by_position(): void { - $category2 = CategoryFactory::new()->position(2)->create(); - $category0 = CategoryFactory::new()->position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); + $category2 = CategoryFactory::new() + ->position(2) + ->create(); - $categories = Category::query()->orderByPosition()->get(); + $category0 = CategoryFactory::new() + ->position(0) + ->create(); + + $category1 = CategoryFactory::new() + ->position(1) + ->create(); + + $categories = Category::query() + ->orderByPosition() + ->get(); static::assertTrue($categories[0]->is($category0)); static::assertTrue($categories[1]->is($category1)); @@ -29,11 +39,21 @@ public function it_can_order_models_by_position(): void */ public function it_can_order_models_by_inverse_position(): void { - $category2 = CategoryFactory::new()->position(2)->create(); - $category0 = CategoryFactory::new()->position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); + $category2 = CategoryFactory::new() + ->position(2) + ->create(); + + $category0 = CategoryFactory::new() + ->position(0) + ->create(); + + $category1 = CategoryFactory::new() + ->position(1) + ->create(); - $categories = Category::query()->orderByInversePosition()->get(); + $categories = Category::query() + ->orderByInversePosition() + ->get(); static::assertTrue($categories[0]->is($category2)); static::assertTrue($categories[1]->is($category1)); @@ -45,14 +65,24 @@ public function it_can_order_models_by_inverse_position(): void */ public function it_can_be_ordered_by_position_by_default(): void { - $category2 = CategoryFactory::new()->position(2)->create(); - $category0 = CategoryFactory::new()->position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); + $category2 = CategoryFactory::new() + ->position(2) + ->create(); + + $category0 = CategoryFactory::new() + ->position(0) + ->create(); + + $category1 = CategoryFactory::new() + ->position(1) + ->create(); $fakeCategory = Mockery::mock(new Category()); $fakeCategory->shouldReceive('alwaysOrderByPosition')->andReturnTrue(); - $categories = Category::query()->setModel($fakeCategory)->get(); + $categories = Category::query() + ->setModel($fakeCategory) + ->get(); static::assertTrue($categories[0]->is($category0)); static::assertTrue($categories[1]->is($category1)); @@ -64,9 +94,17 @@ public function it_can_be_ordered_by_position_by_default(): void */ public function it_is_not_ordered_by_position_by_default(): void { - $category2 = CategoryFactory::new()->position(2)->create(); - $category0 = CategoryFactory::new()->position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); + $category2 = CategoryFactory::new() + ->position(2) + ->create(); + + $category0 = CategoryFactory::new() + ->position(0) + ->create(); + + $category1 = CategoryFactory::new() + ->position(1) + ->create(); $categories = Category::query()->get(); diff --git a/tests/UpdateTest.php b/tests/UpdateTest.php index 3b0a081..e773ade 100644 --- a/tests/UpdateTest.php +++ b/tests/UpdateTest.php @@ -11,17 +11,15 @@ class UpdateTest extends TestCase */ public function it_updates_position_to_less_than_previous(): void { - $category0 = CategoryFactory::new()->position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); - $category2 = CategoryFactory::new()->position(2)->create(); + $categories = CategoryFactory::new()->createMany(3); - $category2->update([ + $categories[2]->update([ 'position' => 0, ]); - static::assertSame(0, $category2->fresh()->getPosition()); - static::assertSame(1, $category0->fresh()->getPosition()); - static::assertSame(2, $category1->fresh()->getPosition()); + static::assertSame(0, $categories[2]->fresh()->getPosition()); + static::assertSame(1, $categories[0]->fresh()->getPosition()); + static::assertSame(2, $categories[1]->fresh()->getPosition()); } /** @@ -29,16 +27,14 @@ public function it_updates_position_to_less_than_previous(): void */ public function it_can_move_model_to_greater_than_previous(): void { - $category0 = CategoryFactory::new()->position(0)->create(); - $category1 = CategoryFactory::new()->position(1)->create(); - $category2 = CategoryFactory::new()->position(2)->create(); + $categories = CategoryFactory::new()->createMany(3); - $category0->update([ + $categories[0]->update([ 'position' => 2, ]); - static::assertSame(0, $category1->fresh()->getPosition()); - static::assertSame(1, $category2->fresh()->getPosition()); - static::assertSame(2, $category0->fresh()->getPosition()); + static::assertSame(0, $categories[1]->fresh()->getPosition()); + static::assertSame(1, $categories[2]->fresh()->getPosition()); + static::assertSame(2, $categories[0]->fresh()->getPosition()); } } From 44e5d8a5a0a240eaa4db844fc44d9ba6d05d4404 Mon Sep 17 00:00:00 2001 From: Nevadskiy Date: Thu, 9 Feb 2023 19:24:55 +0200 Subject: [PATCH 23/23] Fix CS --- src/HasPosition.php | 1 - tests/MoveTest.php | 1 - 2 files changed, 2 deletions(-) diff --git a/src/HasPosition.php b/src/HasPosition.php index ad188a2..ce44e6c 100644 --- a/src/HasPosition.php +++ b/src/HasPosition.php @@ -206,7 +206,6 @@ protected function assignPosition(): void // Sync original attribute to not shift other models when the model will be created $this->syncOriginalAttribute($this->getPositionColumn()); } - } /** diff --git a/tests/MoveTest.php b/tests/MoveTest.php index f21fa92..a59d862 100644 --- a/tests/MoveTest.php +++ b/tests/MoveTest.php @@ -2,7 +2,6 @@ namespace Nevadskiy\Position\Tests; -use Illuminate\Support\Facades\DB; use Nevadskiy\Position\Tests\Support\Factories\CategoryFactory; use Nevadskiy\Position\Tests\Support\Models\Category;