1.) added: amends to logic to avoid crashing tests

Signed-off-by: Oskars Germovs <[email protected]>
spatie · Nov 22, 2024 · 8a3bd63 · 8a3bd63
1 parent 59bd4a1
commit 8a3bd63
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 38 deletions.
diff --git 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
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Spatie\EloquentSortable\Test;
 
 use Illuminate\Support\Collection;
@@ -450,32 +452,54 @@ public function it_sets_mass_new_order_correctly()
     /** @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;
+        $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}"
+            );
         }
     }