Harden up the sort modifier. Closes #2450

The issue with 2450 was because when it was being sorted, the indexes were jumbled, and Antlers needs a zero indexed array to loop over.

This also removes using a key argument named "value" triggering the primitive sort. You might have a multidimensional array with keys named value that you want to sort by. You can still just sort primitive arrays by doing {{ var | sort }} or {{ var sort="true" }}

Author: jasonvarga

src/Modifiers/CoreModifiers.php

@@ -1757,23 +1757,22 @@ public function slugify($value)
      */
     public function sort($value, $params)
     {
-        $key = Arr::get($params, 0);
-        $is_descending = strtolower(Arr::get($params, 1)) == 'desc';
+        $key = Arr::get($params, 0, 'true');
+        $desc = strtolower(Arr::get($params, 1)) == 'desc';
 
-        // Enforce collection
-        $value = ($value instanceof Collection) ? $value : collect($value);
+        $value = $value instanceof Collection ? $value : collect($value);
 
-        // Random sort
         if ($key === 'random') {
             return $value->shuffle();
         }
 
-        // Primitive array sort
-        if ($key === 'true' || $key == 'value') {
-            return $is_descending ? $value->sort()->reverse() : $value->sort();
+        if ($key === 'true') {
+            $value = $desc ? $value->sort()->reverse() : $value->sort();
+        } else {
+            $value = $desc ? $value->sortByDesc($key) : $value->sortBy($key);
         }
 
-        return $is_descending ? $value->sortByDesc($key) : $value->sortBy($key);
+        return $value->values();
     }
 
     /**

tests/Modifiers/SortTest.php

@@ -0,0 +1,114 @@
+<?php
+
+namespace Tests\Modifiers;
+
+use Statamic\Modifiers\Modify;
+use Tests\TestCase;
+
+class SortTest extends TestCase
+{
+    /** @test */
+    public function it_sorts_primitive_arrays()
+    {
+        $arr = ['beta', 'zeta', 'alpha'];
+        $expected = ['alpha', 'beta', 'zeta'];
+
+        $this->assertEquals($expected, $this->modify($arr));
+        $this->assertEquals($expected, $this->modify($arr, 'true'));
+        $this->assertEquals($expected, $this->modify($arr, 'true'));
+        $this->assertEquals($expected, $this->modify($arr, ['true', 'asc']));
+
+        $this->assertEquals($expected, $this->modify(collect($arr)));
+        $this->assertEquals($expected, $this->modify(collect($arr), 'true'));
+        $this->assertEquals($expected, $this->modify(collect($arr), ['true', 'asc']));
+    }
+
+    /** @test */
+    public function it_sorts_primitive_arrays_descending()
+    {
+        $arr = ['beta', 'zeta', 'alpha'];
+        $expected = ['zeta', 'beta', 'alpha'];
+
+        $this->assertEquals($expected, $this->modify($arr, ['true', 'desc']));
+        $this->assertEquals($expected, $this->modify(collect($arr), ['true', 'desc']));
+    }
+
+    /** @test */
+    public function it_sorts_multidimensional_arrays_by_a_key()
+    {
+        $arr = [
+            ['value' => 'beta'],
+            ['value' => 'zeta'],
+            ['value' => 'alpha'],
+        ];
+
+        $expected = [
+            ['value' => 'alpha'],
+            ['value' => 'beta'],
+            ['value' => 'zeta'],
+        ];
+
+        $this->assertEquals($expected, $this->modify($arr, 'value'));
+        $this->assertEquals($expected, $this->modify(collect($arr), 'value'));
+    }
+
+    /** @test */
+    public function it_sorts_multidimensional_arrays_by_a_key_descending()
+    {
+        $arr = [
+            ['value' => 'beta'],
+            ['value' => 'zeta'],
+            ['value' => 'alpha'],
+        ];
+
+        $expected = [
+            ['value' => 'zeta'],
+            ['value' => 'beta'],
+            ['value' => 'alpha'],
+        ];
+
+        $this->assertEquals($expected, $this->modify($arr, ['value', 'desc']));
+        $this->assertEquals($expected, $this->modify(collect($arr), ['value', 'desc']));
+    }
+
+    /** @test */
+    public function it_sorts_primitive_arrays_randomly()
+    {
+        $this->assertSortsRandomly(['alpha', 'beta', 'zeta']);
+    }
+
+    /** @test */
+    public function it_sorts_multidimensional_arrays_randomly()
+    {
+        $this->assertSortsRandomly([
+            ['word' => 'alpha'],
+            ['word' => 'beta'],
+            ['word' => 'zeta'],
+        ]);
+    }
+
+    private function assertSortsRandomly($arr)
+    {
+        $combinations = [];
+
+        foreach (collect()->times(100) as $time) {
+            $modified = $this->modify($arr, 'random');
+
+            if (collect($modified)->keys()->first() != 0) {
+                $this->fail('Collection is not zero indexed');
+            }
+
+            $combinations[json_encode($modified)] = true;
+        }
+
+        $this->assertNotCount(1, $combinations);
+    }
+
+    public function modify($arr, $args = [])
+    {
+        $modified = Modify::value($arr)->sort($args)->fetch();
+
+        // It gets modified to a Collection, but it's easier to write tests as arrays.
+        return $modified->all();
+    }
+}