diff --git a/README.md b/README.md index b4dba6f..1362b40 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ and updates the other entries based on the models position value. ## Installation -> Tested in Laravel 5.1 - 5.7, should work in all 5.*/6.* releases +> Tested in Laravel 5.4 - 8. **Install via composer** diff --git a/composer.json b/composer.json index 60fdfb7..df10fe7 100644 --- a/composer.json +++ b/composer.json @@ -2,10 +2,11 @@ "name": "pion/laravel-eloquent-position", "description": "Position logic for Eloquent models with minimum setup", "require": { - "laravel/framework": ">=5.1" + "laravel/framework": ">=5.5" }, "require-dev": { - "squizlabs/php_codesniffer": "2.*" + "squizlabs/php_codesniffer": "2.*", + "orchestra/testbench": "~3.6.7 || ~3.7.8 || ~3.8.6 || ^4.8 || ^5.2 || ^6.0" }, "scripts": { "lint": "./vendor/bin/phpcs --standard=PSR2 src/", @@ -16,6 +17,11 @@ "Pion\\Support\\Eloquent\\Position\\": "src/" } }, + "autoload-dev": { + "psr-4": { + "Tests\\": "tests/" + } + }, "license": "MIT", "authors": [ { diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..9c3fe97 --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,29 @@ + + + + + src/ + + + + + ./tests/ + + + + + + + + diff --git a/src/Commands/RecalculatePositionCommand.php b/src/Commands/RecalculatePositionCommand.php index 28542ff..c9bb14e 100644 --- a/src/Commands/RecalculatePositionCommand.php +++ b/src/Commands/RecalculatePositionCommand.php @@ -62,24 +62,25 @@ public function handle() $groups = $model->getPositionGroup(); // Run sorted query for every entry - $modelClass::sorted()->chunk(200, function (Collection $collection) use ($groups, &$positionsByGroup) { - /** @var PositionTrait|Model $model */ - foreach ($collection as $model) { - // Builds the group key to get position - $groupKey = $this->buildGroupKeyForPosition($model, $groups); - - // Set the new position - $model->setPosition($this->getPositionForGroup($groupKey, $positionsByGroup)) - ->save(); - } - }); + $modelClass::sorted() + ->chunk(200, function (Collection $collection) use ($groups, &$positionsByGroup) { + /** @var PositionTrait|Model $model */ + foreach ($collection as $model) { + // Builds the group key to get position + $groupKey = $this->buildGroupKeyForPosition($model, $groups); + + // Set the new position + $model->setPosition($this->getPositionForGroup($groupKey, $positionsByGroup)) + ->save(); + } + }); // Render the table layout about the positions $this->table([ 'Group', 'Last position' - ], collect($positionsByGroup)->map(function ($value, $key) { + ], array_map(function ($value, $key) { return [$key, $value]; - })); + }, $positionsByGroup)); return true; } @@ -107,7 +108,7 @@ protected function getPositionForGroup($groupKey, &$positionsByGroup) /** * Builds the group key from the group columns and the values form the model * - * @param Model|PositionTrait $model The eloquent model + * @param Model|PositionTrait $model The eloquent model * @param array $groups * * @return string diff --git a/src/PositionObserver.php b/src/PositionObserver.php index 7f1136a..375d2a8 100644 --- a/src/PositionObserver.php +++ b/src/PositionObserver.php @@ -42,7 +42,7 @@ public function __construct(Dispatcher $events) */ public function saving($model) { - if (!$model->isPositionUpdateDisabled()) { + if ($model->isPositionUpdateDisabled() === false) { // Get the position for current and old value $position = $model->getPosition(); @@ -61,7 +61,23 @@ public function saving($model) } /** - * Forces the new position, will be overriden if it's out of maximum bounds. + * Updates the position before saving + * + * @param Model|PositionTrait $model + */ + public function deleted($model) + { + if ($model->isPositionUpdateDisabled() === false) { + // Get the old position + $oldPosition = $model->getOriginal($model->getPositionColumn()); + + // Append deleted model as last to re-index other models + $this->appendLast($model, $oldPosition); + } + } + + /** + * Forces the new position, will be overridden if it's out of maximum bounds. * * @param Model|PositionTrait $model * @param int $position diff --git a/src/Query/LastPositionQuery.php b/src/Query/LastPositionQuery.php index 65d506f..cb005f5 100644 --- a/src/Query/LastPositionQuery.php +++ b/src/Query/LastPositionQuery.php @@ -1,4 +1,5 @@ max($this->model()->getPositionColumn()) ?: 0; - // Check if the last position is not same as original position - the same object - if (empty($this->oldPosition) || $lastPosition != $this->oldPosition) { + if (empty($this->oldPosition) === false) { + (new MoveQuery($this->model, $lastPosition, $this->oldPosition))->run(); + } else if ($this->oldPosition === null || $lastPosition != $this->oldPosition) { + // Check if the last position is not same as original position - the same object $this->model()->setPosition($lastPosition + 1); } diff --git a/src/Query/MoveQuery.php b/src/Query/MoveQuery.php index 8bb0cb8..5c7bb50 100644 --- a/src/Query/MoveQuery.php +++ b/src/Query/MoveQuery.php @@ -1,4 +1,5 @@ '; + protected $oldComparisonCondition = '>'; /** - * Comparision condition for new position value. + * Comparison condition for new position value. * In default is for decrement. * @var string */ - protected $newComparisionCondition = '<='; + protected $newComparisonCondition = '<='; /** - * PositionQuery constructor. - * * @param Model|PositionTrait $model * @param int $position * @param int|null $oldPosition @@ -56,7 +55,7 @@ public function __construct($model, $position, $oldPosition) $this->positionColumn = $model->getPositionColumn(); // Build the comparision condition - $this->buildComparisionCondition(); + $this->buildComparisonCondition(); // Prepare the query $this->query = $this->buildQuery(); @@ -76,6 +75,15 @@ public function runQuery($query) if ($this->increment) { return $query->increment($this->positionColumn); } else { + // Get the last position and move the position + $lastPosition = $query->max($this->positionColumn) ?: 0; + + // If the set position is out of the bounds of current items, force new position + if ($this->position > $lastPosition) { + $this->position = $lastPosition; + $this->model()->setPosition($this->position); + } + return $query->decrement($this->positionColumn); } } @@ -90,18 +98,18 @@ protected function buildQuery() $query = parent::buildQuery(); // Build the where condition for the position. This will ensure to update only required rows - return $query->where($this->positionColumn, $this->oldComparisionCondition, $this->oldPosition) - ->where($this->positionColumn, $this->newComparisionCondition, $this->position); + return $query->where($this->positionColumn, $this->oldComparisonCondition, $this->oldPosition) + ->where($this->positionColumn, $this->newComparisonCondition, $this->position); } /** - * Builds the correct comparision condition + * Builds the correct comparison condition */ - protected function buildComparisionCondition() + protected function buildComparisonCondition() { if ($this->increment) { - $this->oldComparisionCondition = '<'; - $this->newComparisionCondition = '>='; + $this->oldComparisonCondition = '<'; + $this->newComparisonCondition = '>='; } } //endregion diff --git a/src/Query/PositionQuery.php b/src/Query/PositionQuery.php index 1424a1d..2e86bb6 100644 --- a/src/Query/PositionQuery.php +++ b/src/Query/PositionQuery.php @@ -52,11 +52,11 @@ protected function buildQuery() $query = parent::buildQuery(); // Get the last position and move the position - $lastPosition = $query->max($this->positionColumn); + $lastPosition = $query->max($this->positionColumn) ?: 0; // If the new position is last position, just update the position or if // new position is out of bounds - if ($this->position >= $lastPosition) { + if ($this->position > $lastPosition) { $this->model()->setPosition($lastPosition + 1); } else { $this->shouldRunQuery = true; diff --git a/src/Traits/PositionTrait.php b/src/Traits/PositionTrait.php index 1b6fcc0..bad74e3 100644 --- a/src/Traits/PositionTrait.php +++ b/src/Traits/PositionTrait.php @@ -70,7 +70,7 @@ public function getPositionGroup() */ public function isPositionUpdateDisabled() { - return $this->positionOption('disablePositionUpdate'); + return $this->positionOption('disablePositionUpdate', false); } //endregion diff --git a/tests/GroupItemPositionTest.php b/tests/GroupItemPositionTest.php new file mode 100644 index 0000000..6b4bfb0 --- /dev/null +++ b/tests/GroupItemPositionTest.php @@ -0,0 +1,36 @@ +createItem('DifferentGroup', null, 2); + $this->createItem('DifferentGroup', null, 2); + $this->createItem('DifferentGroup', null, 2); + $this->createItem('DifferentGroup', null, 2); + } + + + protected function query(): Builder + { + return GroupItem::query()->where('group', 1); + } + + protected function createItem(string $name, string $position = null, int $group = 1): AbstractPositionModel + { + return GroupItem::create([ + 'name' => $name, + 'position' => $position, + 'group' => $group, + ]); + } +} diff --git a/tests/Models/AbstractPositionModel.php b/tests/Models/AbstractPositionModel.php new file mode 100644 index 0000000..220fdf9 --- /dev/null +++ b/tests/Models/AbstractPositionModel.php @@ -0,0 +1,23 @@ +loadMigrationsFrom(__DIR__ . '/database/migrations'); + } + + protected function createItem(string $name, string $position = null): AbstractPositionModel + { + return SingleItem::create([ + 'name' => $name, + 'position' => $position, + ]); + } + + public function testCreateItemsWithCorrectOrderWithoutSettingPosition() + { + for ($i = 1; $i <= 10; $i++) { + $item = $this->createItem('Test ' . $i); + $this->assertEquals($item->position, $i); + } + } + + public function testCreateItemAtLastPositionWithEmptyStringAndUsesAsLastPosition() + { + $this->assertEquals(1, $this->createItem('Test', '')->position); + $this->assertEquals(2, $this->createItem('Test', 0)->position); + } + + public function testCreateItemWithFixedPositionButEmptyListEnsuresCorrectNumber() + { + $this->assertEquals(1, $this->createItem('Test 1', 1)->position); + $this->assertEquals(2, $this->createItem('Test 2', 2)->position); + $this->assertEquals(2, $this->createItem('Test 3', 2)->position); + + $this->assertListOrder([ + 'Test 1 - 1', + 'Test 3 - 2', + 'Test 2 - 3', + ]); + + $this->assertEquals( 1, $this->createItem('Test 4', 1)->position); + + $this->assertListOrder([ + 'Test 4 - 1', + 'Test 1 - 2', + 'Test 3 - 3', + 'Test 2 - 4', + ]); + } + + public function testUpdatePosition() + { + $item = $this->createItem('Test 1'); + $this->assertEquals(1, $item->position); + $item2 = $this->createItem('Test 2'); + $this->assertEquals(2, $item2->position); + $item3 = $this->createItem('Test 3'); + $this->assertEquals(3, $item3->position); + + $this->assertListOrder([ + 'Test 1 - 1', + 'Test 2 - 2', + 'Test 3 - 3', + ]); + + $item->update(['position' => 2]); + + $this->assertListOrder([ + 'Test 2 - 1', + 'Test 1 - 2', + 'Test 3 - 3', + ]); + + $item->update(['position' => 1]); + + $this->assertListOrder([ + 'Test 1 - 1', + 'Test 2 - 2', + 'Test 3 - 3', + ]); + + $item->update(['position' => 4]); + + $this->assertListOrder([ + 'Test 2 - 1', + 'Test 3 - 2', + 'Test 1 - 3', + ]); + + } + + public function testCreateItemWithForcedPositionAndDelete() + { + for ($i = 1; $i <= 8; $i++) { + $item = $this->createItem('Test ' . $i); + $this->assertEquals($i, $item->position); + } + + $item = $this->createItem('Test middle', 5); + $this->assertEquals(5, $item->position); + + $this->assertListOrder([ + 'Test 1 - 1', + 'Test 2 - 2', + 'Test 3 - 3', + 'Test 4 - 4', + 'Test middle - 5', + 'Test 5 - 6', + 'Test 6 - 7', + 'Test 7 - 8', + 'Test 8 - 9', + ]); + $item->delete(); + + $this->assertListOrder([ + 'Test 1 - 1', + 'Test 2 - 2', + 'Test 3 - 3', + 'Test 4 - 4', + 'Test 5 - 5', + 'Test 6 - 6', + 'Test 7 - 7', + 'Test 8 - 8', + ]); + } + + protected function query(): Builder + { + return SingleItem::query(); + } + + protected function assertListOrder(array $expectedItems): void + { + $items = $this->query() + ->sorted() + ->get() + ->map(fn(AbstractPositionModel $item) => $item->name . ' - ' . $item->position) + ->all(); + + $this->assertEquals($expectedItems, $items); + } +} diff --git a/tests/database/migrations/2020_11_29_144300_create_single_item_table.php b/tests/database/migrations/2020_11_29_144300_create_single_item_table.php new file mode 100644 index 0000000..f118e6f --- /dev/null +++ b/tests/database/migrations/2020_11_29_144300_create_single_item_table.php @@ -0,0 +1,41 @@ +tableName, function (Blueprint $table) { + $table->id(); + $table->unsignedInteger('position'); + $table->string('name'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists($this->tableName); + } +} diff --git a/tests/database/migrations/2020_11_29_144310_create_group_item_table.php b/tests/database/migrations/2020_11_29_144310_create_group_item_table.php new file mode 100644 index 0000000..af28269 --- /dev/null +++ b/tests/database/migrations/2020_11_29_144310_create_group_item_table.php @@ -0,0 +1,42 @@ +tableName, function (Blueprint $table) { + $table->id(); + $table->unsignedInteger('position'); + $table->string('name'); + $table->unsignedInteger('group'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists($this->tableName); + } +}