Rework move methods to remove reliance on delete listener

Author: oddvalue

README.md

@@ -11,12 +11,12 @@
 This package is an extension of Spatie's excellent [Eloquent Sortable](https://github.com/spatie/eloquent-sortable). It adds the following additional features:
 
 * New methods
-    - `$model->moveBefore(Sortable $target): void`
-    - `$model->moveAfter(Sortable $target): void`
-    - `$model->moveBetween(Sortable $before = null, Sortable $after = null): void` 
+    - `$model->moveBefore(int|Sortable $target): void`
+    - `$model->moveAfter(int|Sortable $target): void`
+    - `$model->moveBetween(int|Sortable $before = null, Sortable $after = null): void` 
     - `$model->moveTo(int|Sortable $newPosition): void`
-* New features
-    - Automatic re-sorting after deleting a model
+
+__Note:__ The move methods rebuild the entire sequence so this package is not recommended for very large datasets.
 
 ## Installation
 
@@ -42,19 +42,27 @@ $model = MyModel::create(['title' => 'foo']);
 
 # Move the model before the 5th item in the list
 $model->moveBefore(MyModel::where('order_column', 5)->first());
+// OR
+$model->moveBefore(5);
 
 # Move the model after the 5th item in the list
 $model->moveAfter(MyModel::where('order_column', 5)->first());
+// OR
+$model->moveAfter(5);
 
 # Move the model between the 5th and 6th item in the list
 $model->moveBetween(
     MyModel::where('order_column', 5)->first(), 
     MyModel::where('order_column', 6)->first()
 );
+// OR
+$model->moveBetween(5, 6);
 # This is useful when using a javacript library that provides the node before
 # and after the location an item is dropped
 
 # Move the model to the 5th item in the list
+$model->moveTo(MyModel::where('order_column', 5)->first());
+// OR
 $model->moveTo(5);
 ```
 

composer.json

@@ -58,16 +58,6 @@
             "phpstan/extension-installer": true
         }
     },
-    "extra": {
-        "laravel": {
-            "providers": [
-                "Oddvalue\\EloquentSortable\\EloquentSortableServiceProvider"
-            ],
-            "aliases": {
-                "EloquentSortable": "Oddvalue\\EloquentSortable\\Facades\\EloquentSortable"
-            }
-        }
-    },
     "minimum-stability": "dev",
     "prefer-stable": true
 }

src/EloquentSortableServiceProvider.php

@@ -4,8 +4,4 @@
 
 interface Sortable extends \Spatie\EloquentSortable\Sortable
 {
-    /**
-     * Determine if the siblings should resorted when deleting.
-     */
-    public function shouldSortWhenDeleting(): bool;
 }

src/Sortable.php

@@ -2,63 +2,40 @@
 
 namespace Oddvalue\EloquentSortable;
 
-use Illuminate\Database\Eloquent\Builder;
-use Spatie\EloquentSortable\Sortable;
-
 trait SortableTrait
 {
     use \Spatie\EloquentSortable\SortableTrait;
 
-    public static function bootSortableTrait(): void
-    {
-        static::deleted([static::class, 'resortRowsDeleted']);
-    }
-
     protected function getSortingValue(): int
     {
         return $this->{$this->determineOrderColumnName()};
     }
 
-    protected function setSortingValue(int $value): void
+    public function moveBetween(Sortable|int $before = null, Sortable|int $after = null): void
     {
-        $this->{$this->determineOrderColumnName()} = $value;
-    }
+        $from = $this->getSortingValue();
+        $to = null;
 
-    public static function resortRowsDeleted(Sortable $instance): void
-    {
-        if (! $instance->shouldSortWhenDeleting()) {
-            return;
+        if ($before instanceof Sortable) {
+            $before = $before->getSortingValue();
         }
-        $instance->newQuery()
-                 ->modifySortingQuery($instance)
-                 ->where($instance->determineOrderColumnName(), '>', $instance->getSortingValue())
-                 ->decrementSort();
-    }
-
-    public function shouldSortWhenDeleting(): bool
-    {
-        return $this->sortable['sort_when_deleting'] ?? config('eloquent-sortable.sort_when_deleting', true);
-    }
 
-    public function moveBetween(Sortable $before = null, Sortable $after = null): void
-    {
-        $from = $this->getSortingValue();
-        $to = null;
+        if ($after instanceof Sortable) {
+            $after = $after->getSortingValue();
+        }
 
         if ($before) {
             // if the node has a sibling before it, insert after it
-            $beforePosition = $before->getSortingValue();
-            if ($beforePosition === $from) {
+            if ($before === $from) {
                 return;
             }
-            $to = $beforePosition + ($from >= $beforePosition ? 1 : 0);
+            $to = $before + ($from >= $before ? 1 : 0);
         } elseif ($after) {
             // if the node has a sibling after it, insert before it
-            $afterPosition = $after->getSortingValue();
-            if ($afterPosition === $from) {
+            if ($after === $from) {
                 return;
             }
-            $to = $afterPosition - ($from <= $afterPosition ? 1 : 0);
+            $to = $after - ($from <= $after ? 1 : 0);
         }
 
         if ($to === null) {
@@ -68,12 +45,12 @@ public function moveBetween(Sortable $before = null, Sortable $after = null): vo
         $this->moveTo($to);
     }
 
-    public function moveBefore(Sortable $target): void
+    public function moveBefore(Sortable|int $target): void
     {
         $this->moveBetween(after: $target);
     }
 
-    public function moveAfter(Sortable $target): void
+    public function moveAfter(Sortable|int $target): void
     {
         $this->moveBetween(before: $target);
     }
@@ -84,38 +61,30 @@ public function moveTo(int|Sortable $newPosition): void
             $newPosition = $newPosition->getSortingValue();
         }
 
-        $from = $this->getSortingValue();
-
-        $difference = $from - $newPosition;
+        $difference = $this->getSortingValue() - $newPosition;
 
         if ($difference === 0) {
             return;
         }
-        $query = $this->buildSortQuery()->whereBetween(
-            $this->determineOrderColumnName(),
-            [min($from, $newPosition), max($from, $newPosition)]
-        );
-
-        if ($difference > 0) {
-            $query->incrementSort();
-        } else {
-            $query->decrementSort();
-        }
-        $this->setSortingValue($newPosition);
-        $this->save();
-    }
 
-    public function scopeIncrementSort(Builder $query): void
-    {
-        // Increment is called on the raw query builder to avoid touching the
-        // updated timestamp
-        $query->getQuery()->increment($this->determineOrderColumnName());
-    }
+        $sequence = $this->buildSortQuery()
+            ->ordered()
+            ->where($this->getKeyName(), '!=', $this->getKey())
+            ->getQuery()
+            ->get([$this->determineOrderColumnName(), $this->getKeyName()]);
+
+        [$before, $after] = $sequence->partition(function ($item) use ($newPosition, $difference) {
+            if ($difference > 0) {
+                return $item->{$this->determineOrderColumnName()} < $newPosition;
+            } else {
+                return $item->{$this->determineOrderColumnName()} <= $newPosition;
+            }
+        });
 
-    public function scopeDecrementSort(Builder $query): void
-    {
-        // Decrement is called on the raw query builder to avoid touching the
-        // updated timestamp
-        $query->getQuery()->decrement($this->determineOrderColumnName());
+        static::setNewOrder([
+            ...$before->pluck($this->getKeyName()),
+            $this->getKey(),
+            ...$after->pluck($this->getKeyName()),
+        ]);
     }
 }

src/SortableTrait.php

@@ -11,4 +11,14 @@ class Dummy extends \Illuminate\Database\Eloquent\Model implements \Oddvalue\Elo
     protected $guarded = [];
 
     public $timestamps = false;
+
+    public array $sortable = [
+        'order_column' => 'order_column',
+        'sort_when_deleting' => true,
+    ];
+
+    protected $casts = [
+        'order_column' => 'integer',
+        'original_position' => 'integer',
+    ];
 }

tests/Dummy.php

@@ -3,88 +3,86 @@
 use Oddvalue\EloquentSortable\Tests\Dummy;
 
 it('can_be_move_records', function (int $from, int $to, array $expected) {
-    $featuredProducts = Dummy::all();
+    $this->assertEquals(range(1, 10), Dummy::pluck('order_column')->all());
 
-    $this->assertEquals(range(1, 10), $featuredProducts->pluck('order_column')->all());
+    Dummy::where('order_column', $from)->first()->moveTo(Dummy::find($to));
 
-    $featuredProducts->firstWhere('order_column', $from)->moveTo($to);
-
-    $this->assertEquals($expected, $featuredProducts->fresh()->pluck('order_column')->all());
+    $this->assertEquals($expected, Dummy::ordered()->pluck('original_position')->all());
 })->with([
     [1, 2,  [2, 1, 3, 4, 5, 6, 7, 8, 9, 10]],
-    [2, 6,  [1, 6, 2, 3, 4, 5, 7, 8, 9, 10]],
-    [3, 10, [1, 2, 10, 3, 4, 5, 6, 7, 8, 9]],
-    [4, 1,  [2, 3, 4, 1, 5, 6, 7, 8, 9, 10]],
+    [2, 6,  [1, 3, 4, 5, 6, 2, 7, 8, 9, 10]],
+    [3, 10, [1, 2, 4, 5, 6, 7, 8, 9, 10, 3]],
+    [4, 1,  [4, 1, 2, 3, 5, 6, 7, 8, 9, 10]],
     [5, 5,  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]],
-    [6, 10, [1, 2, 3, 4, 5, 10, 6, 7, 8, 9]],
-    [7, 1,  [2, 3, 4, 5, 6, 7, 1, 8, 9, 10]],
-    [8, 5,  [1, 2, 3, 4, 6, 7, 8, 5, 9, 10]],
-    [9, 3,  [1, 2, 4, 5, 6, 7, 8, 9, 3, 10]],
-    [10, 1, [2, 3, 4, 5, 6, 7, 8, 9, 10, 1]],
+    [6, 10, [1, 2, 3, 4, 5, 7, 8, 9, 10, 6]],
+    [7, 1,  [7, 1, 2, 3, 4, 5, 6, 8, 9, 10]],
+    [8, 5,  [1, 2, 3, 4, 8, 5, 6, 7, 9, 10]],
+    [9, 3,  [1, 2, 9, 3, 4, 5, 6, 7, 8, 10]],
+    [10, 1, [10, 1, 2, 3, 4, 5, 6, 7, 8, 9]],
 ]);
 
 it('can_move_items_between', function (int $from, array $to, array $expected) {
-    $featuredProducts = Dummy::all();
-
     [$before, $after] = $to;
 
-    $featuredProducts->firstWhere('order_column', $from)
+    Dummy::where('order_column', $from)->first()
         ->moveBetween(
-            $featuredProducts->firstWhere('order_column', $before),
-            $featuredProducts->firstWhere('order_column', $after)
+            Dummy::where('order_column', $before)->first(),
+            Dummy::where('order_column', $after)->first()
         );
 
-    $this->assertEquals($expected, $featuredProducts->fresh()->pluck('order_column')->all());
+    $this->assertEquals($expected, Dummy::ordered()->pluck('original_position')->all());
 })->with([
     [1,  [1, 2],     [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]],
-    [2,  [5, 6],     [1, 5, 2, 3, 4, 6, 7, 8, 9, 10]],
-    [3,  [10, null], [1, 2, 10, 3, 4, 5, 6, 7, 8, 9]],
-    [4,  [null, 1],  [2, 3, 4, 1, 5, 6, 7, 8, 9, 10]],
+    [2,  [5, 6],     [1, 3, 4, 5, 2, 6, 7, 8, 9, 10]],
+    [3,  [10, null], [1, 2, 4, 5, 6, 7, 8, 9, 10, 3]],
+    [4,  [null, 1],  [4, 1, 2, 3, 5, 6, 7, 8, 9, 10]],
     [5,  [4, 5],     [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]],
-    [6,  [10, null], [1, 2, 3, 4, 5, 10, 6, 7, 8, 9]],
-    [7,  [null, 1],  [2, 3, 4, 5, 6, 7, 1, 8, 9, 10]],
-    [8,  [4, 5],     [1, 2, 3, 4, 6, 7, 8, 5, 9, 10]],
+    [6,  [10, null], [1, 2, 3, 4, 5, 7, 8, 9, 10, 6]],
+    [7,  [null, 1],  [7, 1, 2, 3, 4, 5, 6, 8, 9, 10]],
+    [8,  [4, 5],     [1, 2, 3, 4, 8, 5, 6, 7, 9, 10]],
     [9,  [10, null], [1, 2, 3, 4, 5, 6, 7, 8, 10, 9]],
-    [10, [1, null],  [1, 3, 4, 5, 6, 7, 8, 9, 10, 2]],
+    [10, [3, 4],     [1, 2, 3, 10, 4, 5, 6, 7, 8, 9]],
+    [10, [100, 100], [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]],
 ]);
 
 it('can_move_items_before', function ($from, $to, $expected) {
-    $featuredProducts = Dummy::all();
-
-    $featuredProducts->firstWhere('order_column', $from)->moveBefore($featuredProducts->firstWhere('order_column', $to));
+    Dummy::where('order_column', $from)->first()->moveBefore($to);
 
-    $this->assertEquals($expected, $featuredProducts->fresh()->pluck('order_column')->all());
+    $this->assertEquals($expected, Dummy::ordered()->pluck('original_position')->all());
 })->with([
     [1, 2,  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]],
-    [2, 6,  [1, 5, 2, 3, 4, 6, 7, 8, 9, 10]],
-    [3, 10, [1, 2, 9, 3, 4, 5, 6, 7, 8, 10]],
-    [4, 1,  [2, 3, 4, 1, 5, 6, 7, 8, 9, 10]],
+    [2, 6,  [1, 3, 4, 5, 2, 6, 7, 8, 9, 10]],
+    [3, 10, [1, 2, 4, 5, 6, 7, 8, 9, 3, 10]],
+    [4, 1,  [4, 1, 2, 3, 5, 6, 7, 8, 9, 10]],
     [5, 5,  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]],
     [6, 8,  [1, 2, 3, 4, 5, 7, 6, 8, 9, 10]],
-    [7, 2,  [1, 3, 4, 5, 6, 7, 2, 8, 9, 10]],
-    [8, 5,  [1, 2, 3, 4, 6, 7, 8, 5, 9, 10]],
-    [9, 3,  [1, 2, 4, 5, 6, 7, 8, 9, 3, 10]],
-    [10, 1, [2, 3, 4, 5, 6, 7, 8, 9, 10, 1]],
+    [7, 2,  [1, 7, 2, 3, 4, 5, 6, 8, 9, 10]],
+    [8, 5,  [1, 2, 3, 4, 8, 5, 6, 7, 9, 10]],
+    [9, 3,  [1, 2, 9, 3, 4, 5, 6, 7, 8, 10]],
+    [10, 1, [10, 1, 2, 3, 4, 5, 6, 7, 8, 9]],
 ]);
 
-/**
- * @dataProvider itemsToMoveAfter
- */
 it('can_move_items_after', function ($from, $to, $expected) {
-    $featuredProducts = Dummy::all();
+    Dummy::where('order_column', $from)->first()->moveAfter($to);
 
-    $featuredProducts->firstWhere('order_column', $from)->moveAfter($featuredProducts->firstWhere('order_column', $to));
-
-    $this->assertEquals($expected, $featuredProducts->fresh()->pluck('order_column')->all());
+    $this->assertEquals($expected, Dummy::ordered()->pluck('original_position')->all());
 })->with([
     [1, 2,  [2, 1, 3, 4, 5, 6, 7, 8, 9, 10]],
-    [2, 6,  [1, 6, 2, 3, 4, 5, 7, 8, 9, 10]],
-    [3, 10, [1, 2, 10, 3, 4, 5, 6, 7, 8, 9]],
-    [4, 1,  [1, 3, 4, 2, 5, 6, 7, 8, 9, 10]],
+    [2, 6,  [1, 3, 4, 5, 6, 2, 7, 8, 9, 10]],
+    [3, 10, [1, 2, 4, 5, 6, 7, 8, 9, 10, 3]],
+    [4, 1,  [1, 4, 2, 3, 5, 6, 7, 8, 9, 10]],
     [5, 5,  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]],
-    [6, 8,  [1, 2, 3, 4, 5, 8, 6, 7, 9, 10]],
-    [7, 2,  [1, 2, 4, 5, 6, 7, 3, 8, 9, 10]],
-    [8, 5,  [1, 2, 3, 4, 5, 7, 8, 6, 9, 10]],
-    [9, 3,  [1, 2, 3, 5, 6, 7, 8, 9, 4, 10]],
-    [10, 1, [1, 3, 4, 5, 6, 7, 8, 9, 10, 2]],
+    [6, 8,  [1, 2, 3, 4, 5, 7, 8, 6, 9, 10]],
+    [7, 2,  [1, 2, 7, 3, 4, 5, 6, 8, 9, 10]],
+    [8, 5,  [1, 2, 3, 4, 5, 8, 6, 7, 9, 10]],
+    [9, 3,  [1, 2, 3, 9, 4, 5, 6, 7, 8, 10]],
+    [10, 1, [1, 10, 2, 3, 4, 5, 6, 7, 8, 9]],
 ]);
+
+it('performs_acceptably', function () {
+    Dummy::query()->delete();
+    collect(range(1, 5000))->each(fn ($i) => Dummy::create());
+    $start = microtime(true);
+    Dummy::first()->moveAfter(Dummy::inRandomOrder()->first());
+    expect(microtime(true) - $start)->toBeLessThanOrEqual(2);
+});