From 469cca88215a921f3d38a91d841f2221094bab91 Mon Sep 17 00:00:00 2001 From: Oskars Germovs Date: Tue, 19 Nov 2024 23:15:42 +0200 Subject: [PATCH 1/5] 1.) added: refactored configuration 2.) added: refactored views Signed-off-by: Oskars Germovs --- README.md | 109 ++++++++++++---- config/eloquent-sortable.php | 19 ++- src/EloquentModelSortedEvent.php | 2 + src/EloquentSortableServiceProvider.php | 2 + src/Sortable.php | 31 ++++- src/SortableTrait.php | 96 +++++++++++++- tests/Dummy.php | 2 + tests/DummyWithGlobalScope.php | 2 + tests/DummyWithSoftDeletes.php | 2 + tests/DummyWithTimestamps.php | 2 + tests/SortableTest.php | 164 ++++++++++++++++++++++-- tests/TestCase.php | 2 + 12 files changed, 388 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index 3e5078c..3ab1131 100644 --- a/README.md +++ b/README.md @@ -48,32 +48,46 @@ Optionally you can publish the config file with: php artisan vendor:publish --tag=eloquent-sortable-config ``` -This is the content of the file that will be published in `config/eloquent-sortable.php` +This is the content of the file that will be published in `config/eloquent-sortable.php`: ```php return [ - /* - * The name of the column that will be used to sort models. - */ - 'order_column_name' => 'order_column', - - /* - * Define if the models should sort when creating. When true, the package - * will automatically assign the highest order number to a new model - */ - 'sort_when_creating' => true, - - /* - * Define if the timestamps should be ignored when sorting. - * When true, updated_at will not be updated when using setNewOrder - */ - 'ignore_timestamps' => false, + /* + * The name of the column that will be used to sort models. + */ + 'order_column_name' => 'order_column', + + /* + * Define if the models should sort when creating. + * When true, the package will automatically assign the highest order number to a new model. + */ + 'sort_when_creating' => true, + + /* + * Define if the models should sort when updating. + * When true, the package will automatically update the order of models when one is updated. + */ + 'sort_when_updating' => true, + + /* + * Define if the models should sort when deleting. + * When true, the package will automatically update the order of models when one is deleted. + */ + 'sort_when_deleting' => true, + + /* + * Define if the timestamps should be ignored when sorting. + * When true, `updated_at` will not be updated when using `setNewOrder`, `setMassNewOrder`, + * or when models are reordered automatically during creation, updating, or deleting. + */ + 'ignore_timestamps' => false, ]; ``` ## Usage To add sortable behaviour to your model you must: + 1. Implement the `Spatie\EloquentSortable\Sortable` interface. 2. Use the trait `Spatie\EloquentSortable\SortableTrait`. 3. Optionally specify which column will be used as the order column. The default is `order_column`. @@ -91,6 +105,8 @@ class MyModel extends Model implements Sortable public $sortable = [ 'order_column_name' => 'order_column', 'sort_when_creating' => true, + 'sort_when_updating' => true, + 'sort_when_deleting' => true, ]; // ... @@ -113,12 +129,24 @@ $myModel->save(); // order_column for this record will be set to 2 $myModel = new MyModel(); $myModel->save(); // order_column for this record will be set to 3 - -//the trait also provides the ordered query scope +// The trait also provides the ordered query scope $orderedRecords = MyModel::ordered()->get(); ``` -You can set a new order for all the records using the `setNewOrder`-method +### New Sorting Methods + +#### Mass Update Ordering + +You can set a new order for all the records using the `setMassNewOrder`-method: + +```php +$newOrder = [5, 3, 1, 4, 2]; +MyModel::setMassNewOrder($newOrder); +``` + +This will reorder the records in the order specified by `$newOrder`. + +#### Setting a New Order for All Records ```php /** @@ -126,10 +154,10 @@ You can set a new order for all the records using the `setNewOrder`-method * the record for model id 1 will have order_column value 2 * the record for model id 2 will have order_column value 3 */ -MyModel::setNewOrder([3,1,2]); +MyModel::setNewOrder([3, 1, 2]); ``` -Optionally you can pass the starting order number as the second argument. +Optionally, you can pass the starting order number as the second argument. ```php /** @@ -137,7 +165,7 @@ Optionally you can pass the starting order number as the second argument. * the record for model id 1 will have order_column value 12 * the record for model id 2 will have order_column value 13 */ -MyModel::setNewOrder([3,1,2], 10); +MyModel::setNewOrder([3, 1, 2], 10); ``` You can modify the query that will be executed by passing a closure as the fourth argument. @@ -148,11 +176,12 @@ You can modify the query that will be executed by passing a closure as the fourt * the record for model id 1 will have order_column value 12 * the record for model id 2 will have order_column value 13 */ -MyModel::setNewOrder([3,1,2], 10, null, function($query) { +MyModel::setNewOrder([3, 1, 2], 10, null, function ($query) { $query->withoutGlobalScope(new ActiveScope); }); ``` +#### Setting New Order by Custom Column To sort using a column other than the primary key, use the `setNewOrderByCustomColumn`-method. @@ -184,6 +213,8 @@ MyModel::setNewOrderByCustomColumn('uuid', [ ], 10); ``` +### Additional Methods for Sorting + You can also move a model up or down with these methods: ```php @@ -211,6 +242,36 @@ You can swap the order of two models: MyModel::swapOrder($myModel, $anotherModel); ``` +### Handling Model Updates and Deletions + +If you want your model to automatically reorder upon updating or deleting a record, ensure the relevant configuration values (`sort_when_updating`, `sort_when_deleting`) are set to `true` in the configuration file. This will allow your models to maintain the correct order without needing to manually update the ordering each time a change is made. + +For example, if `sort_when_updating` is set to `true`, any changes to a model's attributes will automatically adjust the order, ensuring consistency. + +In addition to automatic reordering, you can also manually trigger sorting for specific scenarios. Here is an example of how you can manually trigger sorting: + +```php +$model = $this->model::query()->find(1); +$model->forceFill(['updated_at' => now()]); +$model->sortables = [1, 2, 3]; // The `sortables` array contains the IDs of the records that need to be reordered. +$model->save(); +``` + +In this scenario, the model is being updated with a new order, and the `sortables` property is set before saving. This ensures the correct order is applied manually when necessary. + +#### Sorting When Deleting + +If `sort_when_deleting` is enabled, the order of the remaining models will be automatically adjusted when a model is deleted. For example: + +```php +$model = MyModel::find(1); +$model->delete(); // The remaining records will be reordered automatically. +``` + +This helps maintain the correct sequence without any manual intervention. + +In this scenario, the model is being updated with a new order, and the `sortables` property is set before saving. This ensures the correct order is applied manually when necessary. + ### Grouping If your model/table has a grouping field (usually a foreign key): `id, `**`user_id`**`, title, order_column` diff --git a/config/eloquent-sortable.php b/config/eloquent-sortable.php index f05614e..5f1191b 100644 --- a/config/eloquent-sortable.php +++ b/config/eloquent-sortable.php @@ -1,5 +1,7 @@ true, + /* + * Define if the models should sort when updating. + * When true, the package will automatically update the order of models when one is updated. + */ + 'sort_when_updating' => true, + + /* + * Define if the models should sort when deleting. + * When true, the package will automatically update the order of models when one is deleted. + */ + 'sort_when_deleting' => true, + /* * Define if the timestamps should be ignored when sorting. - * When true, updated_at will not be updated when using setNewOrder + * When true, `updated_at` will not be updated when using `setNewOrder`, `setMassNewOrder`, + * or when models are reordered automatically during creation, updating, or deleting. */ 'ignore_timestamps' => false, ]; diff --git a/src/EloquentModelSortedEvent.php b/src/EloquentModelSortedEvent.php index 3c78bec..d86e5a2 100644 --- a/src/EloquentModelSortedEvent.php +++ b/src/EloquentModelSortedEvent.php @@ -1,5 +1,7 @@ shouldSortWhenCreating()) { $model->setHighestOrderNumber(); } }); + + static::updating(function (Model $model): void { + if ($model instanceof Sortable && $model->shouldSortWhenUpdating() && !empty($model->sortables)) { + self::setMassNewOrder($model->sortables); + } + }); + + static::deleting(function (Model $model): void { + if ($model instanceof Sortable && $model->shouldSortWhenDeleting() && !empty($model->sortables)) { + self::setMassNewOrder($model->sortables); + } + }); } public function setHighestOrderNumber(): void @@ -36,7 +54,7 @@ public function getLowestOrderNumber(): int return (int)$this->buildSortQuery()->min($this->determineOrderColumnName()); } - public function scopeOrdered(Builder $query, string $direction = 'asc') + public function scopeOrdered(Builder $query, string $direction = 'asc'): Builder { return $query->orderBy($this->determineOrderColumnName(), $direction); } @@ -47,7 +65,7 @@ public static function setNewOrder( string $primaryKeyColumn = null, callable $modifyQuery = null ): void { - if (! is_array($ids) && ! $ids instanceof ArrayAccess) { + if (!is_array($ids) && !$ids instanceof ArrayAccess) { throw new InvalidArgumentException('You must pass an array or ArrayAccess object to setNewOrder'); } @@ -79,6 +97,62 @@ public static function setNewOrder( } } + public static function setMassNewOrder( + array $getSortables, + int $incrementOrder = 1, + ?string $primaryKeyColumn = null + ): void { + $model = new static(); + $orderColumnName = $model->determineOrderColumnName(); + $ignoreTimestamps = config('eloquent-sortable.ignore_timestamps', false); + + if (is_null($primaryKeyColumn)) { + $primaryKeyColumn = $model->getQualifiedKeyName(); + } + + if ($ignoreTimestamps) { + static::$ignoreTimestampsOn = array_values(array_merge(static::$ignoreTimestampsOn, [static::class])); + } + + $caseStatement = collect($getSortables)->reduce(function (string $carry, int $id) use (&$incrementOrder) { + $incrementOrder++; + $carry .= "WHEN {$id} THEN {$incrementOrder} "; + return $carry; + }, ''); + + $getSortablesId = implode(', ', $getSortables); + + DB::transaction( + function () use ( + $model, + $primaryKeyColumn, + $orderColumnName, + $caseStatement, + $getSortablesId, + $ignoreTimestamps + ) { + $timestampUpdate = $ignoreTimestamps ? '' : ", `updated_at` = NOW()"; + + DB::update( + " + UPDATE {$model->getTable()} + SET `{$orderColumnName}` = CASE {$primaryKeyColumn} + {$caseStatement} + END + {$timestampUpdate} + WHERE {$primaryKeyColumn} IN ({$getSortablesId}) + " + ); + } + ); + + Event::dispatch(new EloquentModelSortedEvent(static::class)); + + if ($ignoreTimestamps) { + static::$ignoreTimestampsOn = array_values(array_diff(static::$ignoreTimestampsOn, [static::class])); + } + } + public static function setNewOrderByCustomColumn(string $primaryKeyColumn, $ids, int $startOrder = 1) { self::setNewOrder($ids, $startOrder, $primaryKeyColumn); @@ -97,6 +171,16 @@ public function shouldSortWhenCreating(): bool return $this->sortable['sort_when_creating'] ?? config('eloquent-sortable.sort_when_creating', true); } + public function shouldSortWhenUpdating(): bool + { + return $this->sortable['sort_when_updating'] ?? config('eloquent-sortable.sort_when_updating', true); + } + + public function shouldSortWhenDeleting(): bool + { + return $this->sortable['sort_when_deleting'] ?? config('eloquent-sortable.sort_when_deleting', true); + } + public function moveOrderDown(): static { $orderColumnName = $this->determineOrderColumnName(); @@ -106,7 +190,7 @@ public function moveOrderDown(): static ->where($orderColumnName, '>', $this->$orderColumnName) ->first(); - if (! $swapWithModel) { + if (!$swapWithModel) { return $this; } @@ -122,7 +206,7 @@ public function moveOrderUp(): static ->where($orderColumnName, '<', $this->$orderColumnName) ->first(); - if (! $swapWithModel) { + if (!$swapWithModel) { return $this; } diff --git a/tests/Dummy.php b/tests/Dummy.php index 19e9da0..e56cd7e 100644 --- a/tests/Dummy.php +++ b/tests/Dummy.php @@ -1,5 +1,7 @@ delete(); - $this->assertEquals(DummyWithSoftDeletes::withTrashed()->count(), (new DummyWithSoftDeletes())->getHighestOrderNumber()); + $this->assertEquals( + DummyWithSoftDeletes::withTrashed()->count(), + (new DummyWithSoftDeletes())->getHighestOrderNumber() + ); } /** @test */ public function it_can_set_a_new_order() { - Event::fake(EloquentModelSortedEvent::class); $newOrder = Collection::make(Dummy::all()->pluck('id'))->shuffle()->toArray(); @@ -391,9 +393,9 @@ public function it_can_move_a_model_to_the_last_place() public function it_can_use_config_properties() { config([ - 'eloquent-sortable.order_column_name' => 'order_column', - 'eloquent-sortable.sort_when_creating' => true, - ]); + 'eloquent-sortable.order_column_name' => 'order_column', + 'eloquent-sortable.sort_when_creating' => true, + ]); $model = new class () extends Dummy { public $sortable = []; @@ -408,9 +410,9 @@ public function it_can_override_config_properties() { $model = new class () extends Dummy { public $sortable = [ - 'order_column_name' => 'my_custom_order_column', - 'sort_when_creating' => false, - ]; + 'order_column_name' => 'my_custom_order_column', + 'sort_when_creating' => false, + ]; }; $this->assertEquals($model->determineOrderColumnName(), 'my_custom_order_column'); @@ -432,4 +434,150 @@ public function it_can_tell_if_element_is_last_in_order() $this->assertTrue($model[$model->count() - 1]->isLastInOrder()); $this->assertFalse($model[$model->count() - 2]->isLastInOrder()); } + + /** @test */ + public function it_sets_mass_new_order_correctly() + { + $newOrder = Collection::make(Dummy::all()->pluck('id'))->shuffle()->toArray(); + + Dummy::setMassNewOrder($newOrder); + + foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { + $this->assertEquals($newOrder[$i], $dummy->id); + } + } + + /** @test */ + public function it_updates_order_when_sortables_property_is_set() + { + $model = Dummy::first(); + $originalOrder = Dummy::pluck('order_column', 'id'); + + // Shuffle order and set it on the model as sortables + $newOrder = $originalOrder->keys()->shuffle()->toArray(); + $model->sortables = $newOrder; + + $model->save(); + + foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { + $this->assertEquals($newOrder[$i], $dummy->id); + } + } + + /** @test */ + public function it_does_not_update_order_when_sortables_is_not_set_on_update() + { + $model = Dummy::first(); + $originalOrder = Dummy::pluck('order_column', 'id'); + + // Do not provide sortables to the model + $model->name = 'Updated Name'; + $model->save(); + + foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { + $this->assertEquals($originalOrder[$i], $dummy->id); + } + } + + /** @test */ + public function it_updates_order_when_sortables_property_is_set_on_delete() + { + $modelToDelete = Dummy::first(); + $remainingModels = Dummy::where('id', '!=', $modelToDelete->id)->pluck('id'); + + $newOrder = $remainingModels->shuffle()->toArray(); + $modelToDelete->sortables = $newOrder; + + $modelToDelete->delete(); + + foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { + $this->assertEquals($newOrder[$i], $dummy->id); + } + } + + /** @test */ + public function it_does_not_update_order_when_sortables_is_not_set_on_delete() + { + $modelToDelete = Dummy::first(); + $remainingModels = Dummy::where('id', '!=', $modelToDelete->id)->pluck('id'); + + $originalOrder = $remainingModels->values()->toArray(); + + // Do not provide sortables to the model before deleting + $modelToDelete->delete(); + + foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { + $this->assertEquals($originalOrder[$i], $dummy->id); + } + } + + /** @test */ + public function it_dispatches_sorted_event_on_mass_update_for_sortables() + { + Event::fake(EloquentModelSortedEvent::class); + + $newOrder = Collection::make(Dummy::all()->pluck('id'))->shuffle()->toArray(); + Dummy::setMassNewOrder($newOrder); + + Event::assertDispatched(EloquentModelSortedEvent::class, function (EloquentModelSortedEvent $event) { + return $event->isFor(Dummy::class); + }); + } + + /** @test */ + public function it_respects_ignore_timestamps_on_mass_update_for_sortables() + { + $this->setUpTimestamps(); + DummyWithTimestamps::query()->update(['updated_at' => now()]); + $originalTimestamps = DummyWithTimestamps::all()->pluck('updated_at'); + + // Update with timestamps enabled + config()->set('eloquent-sortable.ignore_timestamps', false); + $newOrder = Collection::make(DummyWithTimestamps::all()->pluck('id'))->shuffle()->toArray(); + DummyWithTimestamps::setMassNewOrder($newOrder); + + foreach (DummyWithTimestamps::orderBy('order_column')->get() as $i => $dummy) { + $this->assertNotEquals($originalTimestamps[$i], $dummy->updated_at); + } + + // Update with timestamps disabled + config()->set('eloquent-sortable.ignore_timestamps', true); + DummyWithTimestamps::setMassNewOrder($newOrder); + + foreach (DummyWithTimestamps::orderBy('order_column')->get() as $i => $dummy) { + $this->assertEquals($originalTimestamps[$i], $dummy->updated_at); + } + } + + /** @test */ + public function it_respects_sort_when_updating_setting() + { + $model = new class () extends Dummy { + public $sortable = ['sort_when_updating' => true]; + }; + + $this->assertTrue($model->shouldSortWhenUpdating()); + + $model = new class () extends Dummy { + public $sortable = ['sort_when_updating' => false]; + }; + + $this->assertFalse($model->shouldSortWhenUpdating()); + } + + /** @test */ + public function it_respects_sort_when_deleting_setting() + { + $model = new class () extends Dummy { + public $sortable = ['sort_when_deleting' => true]; + }; + + $this->assertTrue($model->shouldSortWhenDeleting()); + + $model = new class () extends Dummy { + public $sortable = ['sort_when_deleting' => false]; + }; + + $this->assertFalse($model->shouldSortWhenDeleting()); + } } diff --git a/tests/TestCase.php b/tests/TestCase.php index a606fd0..8991da7 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -1,5 +1,7 @@ Date: Fri, 22 Nov 2024 19:17:05 +0200 Subject: [PATCH 2/5] 1.) added: amend to sortables to array to avoid crashing 2.) added: bugfix to bootSortableTrait is not set if not used in model as fallback Signed-off-by: Oskars Germovs --- src/SortableTrait.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SortableTrait.php b/src/SortableTrait.php index b2d58ff..02542dc 100644 --- a/src/SortableTrait.php +++ b/src/SortableTrait.php @@ -14,24 +14,24 @@ trait SortableTrait { - public array|null $sortables; + public array $sortables = []; public static function bootSortableTrait(): void { static::creating(function (Model $model) { - if ($model instanceof Sortable && $model->shouldSortWhenCreating()) { + if (($model instanceof Sortable || $model instanceof Model) && $model->shouldSortWhenCreating()) { $model->setHighestOrderNumber(); } }); static::updating(function (Model $model): void { - if ($model instanceof Sortable && $model->shouldSortWhenUpdating() && !empty($model->sortables)) { + if (($model instanceof Sortable || $model instanceof Model) && $model->shouldSortWhenUpdating()) { self::setMassNewOrder($model->sortables); } }); static::deleting(function (Model $model): void { - if ($model instanceof Sortable && $model->shouldSortWhenDeleting() && !empty($model->sortables)) { + if (($model instanceof Sortable || $model instanceof Model) && $model->shouldSortWhenDeleting()) { self::setMassNewOrder($model->sortables); } }); From 8a3bd63698f0ac77667aa51cd51116437264c6a8 Mon Sep 17 00:00:00 2001 From: Oskars Germovs Date: Fri, 22 Nov 2024 23:14:07 +0200 Subject: [PATCH 3/5] 1.) added: amends to logic to avoid crashing tests Signed-off-by: Oskars Germovs --- src/SortableTrait.php | 50 +++++++++++++---------- tests/SortableTest.php | 91 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 103 insertions(+), 38 deletions(-) diff --git a/src/SortableTrait.php b/src/SortableTrait.php index 02542dc..68124d7 100644 --- a/src/SortableTrait.php +++ b/src/SortableTrait.php @@ -10,6 +10,7 @@ use Illuminate\Database\Eloquent\SoftDeletingScope; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Schema; use InvalidArgumentException; trait SortableTrait @@ -102,6 +103,10 @@ public static function setMassNewOrder( int $incrementOrder = 1, ?string $primaryKeyColumn = null ): void { + if (count($getSortables) === 0) { + return; + } + $model = new static(); $orderColumnName = $model->determineOrderColumnName(); $ignoreTimestamps = config('eloquent-sortable.ignore_timestamps', false); @@ -110,10 +115,6 @@ public static function setMassNewOrder( $primaryKeyColumn = $model->getQualifiedKeyName(); } - if ($ignoreTimestamps) { - static::$ignoreTimestampsOn = array_values(array_merge(static::$ignoreTimestampsOn, [static::class])); - } - $caseStatement = collect($getSortables)->reduce(function (string $carry, int $id) use (&$incrementOrder) { $incrementOrder++; $carry .= "WHEN {$id} THEN {$incrementOrder} "; @@ -122,6 +123,10 @@ public static function setMassNewOrder( $getSortablesId = implode(', ', $getSortables); + if ($ignoreTimestamps) { + $model->timestamps = false; + } + DB::transaction( function () use ( $model, @@ -131,29 +136,32 @@ function () use ( $getSortablesId, $ignoreTimestamps ) { - $timestampUpdate = $ignoreTimestamps ? '' : ", `updated_at` = NOW()"; - - DB::update( - " - UPDATE {$model->getTable()} - SET `{$orderColumnName}` = CASE {$primaryKeyColumn} - {$caseStatement} - END - {$timestampUpdate} - WHERE {$primaryKeyColumn} IN ({$getSortablesId}) - " - ); + $updateQuery = " + UPDATE {$model->getTable()} + SET `{$orderColumnName}` = CASE {$primaryKeyColumn} + {$caseStatement} + END"; + + if ($model->timestamps && Schema::hasColumn($model->getTable(), 'updated_at')) { + $consistentTimestamp = now(); + $connection = DB::connection()->getDriverName(); + $timestampUpdate = $connection === 'sqlite' + ? ", updated_at = '{$consistentTimestamp->format('Y-m-d H:i:s')}'" + : ", updated_at = '{$consistentTimestamp->toDateTimeString()}'"; + + $updateQuery .= $timestampUpdate; + } + + $updateQuery .= " WHERE {$primaryKeyColumn} IN ({$getSortablesId})"; + + DB::update($updateQuery); } ); Event::dispatch(new EloquentModelSortedEvent(static::class)); - - if ($ignoreTimestamps) { - static::$ignoreTimestampsOn = array_values(array_diff(static::$ignoreTimestampsOn, [static::class])); - } } - public static function setNewOrderByCustomColumn(string $primaryKeyColumn, $ids, int $startOrder = 1) + public static function setNewOrderByCustomColumn(string $primaryKeyColumn, $ids, int $startOrder = 1): void { self::setNewOrder($ids, $startOrder, $primaryKeyColumn); } diff --git a/tests/SortableTest.php b/tests/SortableTest.php index 951263f..efbf1b5 100644 --- a/tests/SortableTest.php +++ b/tests/SortableTest.php @@ -1,5 +1,7 @@ keys()->shuffle()->toArray(); - $model->sortables = $newOrder; + $newOrder = Dummy::pluck('id')->shuffle()->toArray(); // Get IDs and shuffle them + $model = Dummy::first(); + $model->sortables = $newOrder; // Assuming this property is used for ordering $model->save(); - foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { - $this->assertEquals($newOrder[$i], $dummy->id); + // Create CASE statement to order by the shuffled IDs + $orderByClause = "CASE id "; + foreach ($newOrder as $index => $id) { + $orderByClause .= "WHEN {$id} THEN {$index} "; + } + $orderByClause .= "END"; + + // Retrieve the dummies in the shuffled order using CASE statement + $dummies = Dummy::whereIn('id', $newOrder) + ->orderByRaw($orderByClause) + ->get(); + + // Verify that the new order matches the expected order + foreach ($dummies as $index => $dummy) { + $this->assertEquals($newOrder[$index], $dummy->id); } } /** @test */ public function it_does_not_update_order_when_sortables_is_not_set_on_update() { + // Get the first model $model = Dummy::first(); - $originalOrder = Dummy::pluck('order_column', 'id'); - // Do not provide sortables to the model + // Get the original order + $originalOrder = Dummy::orderBy('order_column')->pluck('id')->toArray(); // Ensure order is consistent + + // Update the model without changing the sortables $model->name = 'Updated Name'; $model->save(); - foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { - $this->assertEquals($originalOrder[$i], $dummy->id); + // Retrieve models in the current order and compare with the original + $currentOrder = Dummy::orderBy('order_column')->pluck('id')->toArray(); + + // Verify that the order has not changed + foreach ($originalOrder as $i => $id) { + $this->assertEquals( + $id, + $currentOrder[$i], + "Order mismatch at index {$i}. Expected {$id}, got {$currentOrder[$i]}" + ); } } @@ -527,25 +551,58 @@ public function it_dispatches_sorted_event_on_mass_update_for_sortables() /** @test */ public function it_respects_ignore_timestamps_on_mass_update_for_sortables() { + // Set up a consistent timestamp + $consistentTimestamp = now(); + + // Set up timestamps on the models using the consistent timestamp $this->setUpTimestamps(); - DummyWithTimestamps::query()->update(['updated_at' => now()]); + DummyWithTimestamps::query()->update(['updated_at' => $consistentTimestamp]); + + // Pluck the original timestamps to use for comparison $originalTimestamps = DummyWithTimestamps::all()->pluck('updated_at'); + // Move forward in time by one minute for the next round of updates + $this->travelTo($consistentTimestamp->copy()->addMinute()); + // Update with timestamps enabled config()->set('eloquent-sortable.ignore_timestamps', false); + $this->assertFalse(config('eloquent-sortable.ignore_timestamps'), 'ignore_timestamps should be false'); + $newOrder = Collection::make(DummyWithTimestamps::all()->pluck('id'))->shuffle()->toArray(); DummyWithTimestamps::setMassNewOrder($newOrder); - foreach (DummyWithTimestamps::orderBy('order_column')->get() as $i => $dummy) { - $this->assertNotEquals($originalTimestamps[$i], $dummy->updated_at); + // Verify that the timestamps have been updated + $dummies = DummyWithTimestamps::orderBy('order_column')->get(); + + foreach ($dummies as $i => $dummy) { + $this->assertNotEquals( + $originalTimestamps[$i], + $dummy->updated_at, + "Timestamps should have been updated, but they were not. Index: {$i}" + ); } + $dummyWithTimestamps = new DummyWithTimestamps(); + $dummyWithTimestamps->timestamps = false; + $dummyWithTimestamps::setMassNewOrder($newOrder); + $dummyWithTimestamps->refresh(); + + // Move forward in time by another minute for the next round of updates + $this->travelTo($consistentTimestamp->copy()->addMinutes()); + // Update with timestamps disabled config()->set('eloquent-sortable.ignore_timestamps', true); - DummyWithTimestamps::setMassNewOrder($newOrder); + $this->assertTrue(config('eloquent-sortable.ignore_timestamps'), 'ignore_timestamps should be true'); - foreach (DummyWithTimestamps::orderBy('order_column')->get() as $i => $dummy) { - $this->assertEquals($originalTimestamps[$i], $dummy->updated_at); + // Verify that the timestamps have not changed + $currentTimestamps = $dummyWithTimestamps::orderBy('order_column')->pluck('updated_at')->toArray(); + + foreach ($dummyWithTimestamps->all()->pluck('updated_at') as $i => $timestamp) { + $this->assertEquals( + $timestamp, + $currentTimestamps[$i], + "Timestamps should not have been updated, but they were. Index: {$i}" + ); } } From f53e5a635af3fe9220a922b9fbc6863bead6b97c Mon Sep 17 00:00:00 2001 From: Oskars Germovs Date: Sun, 24 Nov 2024 11:53:47 +0200 Subject: [PATCH 4/5] 1.) added: amend to properly re-order when deleting Signed-off-by: Oskars Germovs --- src/SortableTrait.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SortableTrait.php b/src/SortableTrait.php index 68124d7..5f98a75 100644 --- a/src/SortableTrait.php +++ b/src/SortableTrait.php @@ -33,7 +33,9 @@ public static function bootSortableTrait(): void static::deleting(function (Model $model): void { if (($model instanceof Sortable || $model instanceof Model) && $model->shouldSortWhenDeleting()) { - self::setMassNewOrder($model->sortables); + self::setMassNewOrder( + $model::query()->ordered()->where('id', '!=', $model->id)->pluck('id')->toArray() + ); } }); } From f88f73d9f0f01e5ce4f15745744cf41697f91752 Mon Sep 17 00:00:00 2001 From: Oskars Germovs Date: Sun, 24 Nov 2024 12:53:02 +0200 Subject: [PATCH 5/5] 1.) added: bugfix to SortableTrait for deleting 2.) added: bugfix and proper test coverage for deleting 3.) added: changes to config to avoid issues with old version package users Signed-off-by: Oskars Germovs --- config/eloquent-sortable.php | 4 ++-- src/SortableTrait.php | 2 +- tests/SortableTest.php | 32 +++++++++++++++++++++++++------- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/config/eloquent-sortable.php b/config/eloquent-sortable.php index 5f1191b..980a102 100644 --- a/config/eloquent-sortable.php +++ b/config/eloquent-sortable.php @@ -18,13 +18,13 @@ * Define if the models should sort when updating. * When true, the package will automatically update the order of models when one is updated. */ - 'sort_when_updating' => true, + 'sort_when_updating' => false, /* * Define if the models should sort when deleting. * When true, the package will automatically update the order of models when one is deleted. */ - 'sort_when_deleting' => true, + 'sort_when_deleting' => false, /* * Define if the timestamps should be ignored when sorting. diff --git a/src/SortableTrait.php b/src/SortableTrait.php index 5f98a75..c475ad3 100644 --- a/src/SortableTrait.php +++ b/src/SortableTrait.php @@ -118,8 +118,8 @@ public static function setMassNewOrder( } $caseStatement = collect($getSortables)->reduce(function (string $carry, int $id) use (&$incrementOrder) { - $incrementOrder++; $carry .= "WHEN {$id} THEN {$incrementOrder} "; + $incrementOrder++; return $carry; }, ''); diff --git a/tests/SortableTest.php b/tests/SortableTest.php index efbf1b5..22c24b3 100644 --- a/tests/SortableTest.php +++ b/tests/SortableTest.php @@ -28,6 +28,7 @@ public function it_can_get_the_highest_order_number() public function it_can_get_the_highest_order_number_with_trashed_models() { $this->setUpSoftDeletes(); + config()->set('eloquent-sortable.sort_when_deleting', false); DummyWithSoftDeletes::first()->delete(); @@ -504,18 +505,35 @@ public function it_does_not_update_order_when_sortables_is_not_set_on_update() } /** @test */ - public function it_updates_order_when_sortables_property_is_set_on_delete() + public function it_reorders_models_when_a_model_is_deleted() { - $modelToDelete = Dummy::first(); - $remainingModels = Dummy::where('id', '!=', $modelToDelete->id)->pluck('id'); + // Use initial data from SQLite - $newOrder = $remainingModels->shuffle()->toArray(); - $modelToDelete->sortables = $newOrder; + // Store the original order before deletion + $originalOrder = Dummy::orderBy('order_column')->pluck('order_column')->toArray(); + // Ensure the config is set to reorder on delete + config()->set('eloquent-sortable.sort_when_deleting', true); + $this->assertTrue(config('eloquent-sortable.sort_when_deleting'), 'sort_when_deleting should be true'); + + // Act: Delete one model (e.g., the one with order_column = 1) + $modelToDelete = Dummy::query()->find(1); $modelToDelete->delete(); - foreach (Dummy::orderBy('order_column')->get() as $i => $dummy) { - $this->assertEquals($newOrder[$i], $dummy->id); + // Fetch the new order after deletion + $newOrder = Dummy::orderBy('order_column')->pluck('order_column')->toArray(); + + // Assert: The new order should not match the original order + $this->assertNotEquals($newOrder, $originalOrder, 'The models should have been reordered after deletion.'); + + // Additional Assert: Ensure `order_column` values are gapless and sequential + $remainingModels = Dummy::orderBy('order_column')->get(); + foreach ($remainingModels as $index => $model) { + $this->assertSame( + $index + 1, + (int)$model->order_column, + "The order_column value should be sequential and gapless after deletion." + ); } }