From f88f73d9f0f01e5ce4f15745744cf41697f91752 Mon Sep 17 00:00:00 2001 From: Oskars Germovs Date: Sun, 24 Nov 2024 12:53:02 +0200 Subject: [PATCH] 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." + ); } }