silverstripe · GuySartorelli · Nov 27, 2023 · Nov 26, 2023
diff --git a/src/Schema/DataObject/Plugin/QuerySort.php b/src/Schema/DataObject/Plugin/QuerySort.php
@@ -19,12 +19,15 @@
 use SilverStripe\ORM\Sortable;
 use Exception;
 use GraphQL\Type\Definition\ResolveInfo;
+use SilverStripe\GraphQL\Schema\Traits\SortTrait;
 
 /**
  * Adds a sort parameter to a DataObject query
  */
 class QuerySort extends AbstractQuerySortPlugin
 {
+    use SortTrait;
+
     const IDENTIFIER = 'sort';
 
     public function getIdentifier(): string
@@ -105,7 +108,7 @@ public static function sort(array $context): closure
                 return $list;
             }
 
-            $sortArgs = static::getSortArgs($info, $args, $rootType, $fieldName);
+            $sortArgs = self::getSortArgs($info, $args, $fieldName);
             $paths = NestedInputBuilder::buildPathsFromArgs($sortArgs);
             if (empty($paths)) {
                 return $list;
@@ -138,56 +141,6 @@ public static function sort(array $context): closure
         };
     }
 
-    private static function getSortArgs(ResolveInfo $info, array $args, string $rootType, string $fieldName): array
-    {
-        $sortArgs = [];
-        $sortOrder = self::getSortOrder($info, $fieldName);
-
-        foreach ($sortOrder as $orderName) {
-            if (!isset($args[$fieldName][$orderName])) {
-                continue;
-            }
-            $sortArgs[$orderName] = $args[$fieldName][$orderName];
-            unset($args[$fieldName][$orderName]);
-        }
-
-        return array_merge($sortArgs, $args[$fieldName]);
-    }
-
-    /**
-     * Gets the original order of fields to be sorted based on the query args order.
-     *
-     * This is necessary because the underlying GraphQL implementation we're using ignores the
-     * order of query args, and uses the order that fields are defined in the schema instead.
-     */
-    private static function getSortOrder(ResolveInfo $info, string $fieldName)
-    {
-        $relevantNode = $info->fieldDefinition->getName();
-
-        // Find the query field node that matches the schema
-        foreach ($info->fieldNodes as $node) {
-            if ($node->name->value !== $relevantNode) {
-                continue;
-            }
-
-            // Find the sort arg
-            foreach ($node->arguments as $arg) {
-                if ($arg->name->value !== $fieldName) {
-                    continue;
-                }
-
-                // Get the sort order from the query
-                $sortOrder = [];
-                foreach ($arg->value->fields as $field) {
-                    $sortOrder[] = $field->name->value;
-                }
-                return $sortOrder;
-            }
-        }
-
-        return [];
-    }
-
     /**
      * @param NestedInputBuilder $builder
      */

diff --git a/src/Schema/Plugin/SortPlugin.php b/src/Schema/Plugin/SortPlugin.php
@@ -14,14 +14,17 @@
 use SilverStripe\GraphQL\Schema\Resolver\ResolverReference;
 use SilverStripe\GraphQL\Schema\Schema;
 use SilverStripe\GraphQL\Schema\Services\NestedInputBuilder;
+use SilverStripe\GraphQL\Schema\Traits\SortTrait;
 use SilverStripe\GraphQL\Schema\Type\InputType;
 use SilverStripe\ORM\Sortable;
 use Closure;
+use GraphQL\Type\Definition\ResolveInfo;
 
 class SortPlugin implements FieldPlugin, SchemaUpdater
 {
     use Configurable;
     use Injectable;
+    use SortTrait;
 
     const IDENTIFIER = 'sorter';
 
@@ -86,11 +89,16 @@ public function apply(Field $field, Schema $schema, array $config = []): void
     public static function sort(array $context): Closure
     {
         $fieldName = $context['fieldName'];
-        return function (?Sortable $list, array $args) use ($fieldName) {
+        return function (?Sortable $list, array $args, array $context, ResolveInfo $info) use ($fieldName) {
             if ($list === null) {
                 return null;
             }
-            $sortArgs = $args[$fieldName] ?? [];
+
+            if (!isset($args[$fieldName])) {
+                return $list;
+            }
+
+            $sortArgs = self::getSortArgs($info, $args, $fieldName);
             $list = $list->sort($sortArgs);
 
             return $list;

diff --git a/src/Schema/Traits/SortTrait.php b/src/Schema/Traits/SortTrait.php
@@ -0,0 +1,58 @@
+<?php
+
+namespace SilverStripe\GraphQL\Schema\Traits;
+
+use GraphQL\Type\Definition\ResolveInfo;
+
+trait SortTrait
+{
+    private static function getSortArgs(ResolveInfo $info, array $args, string $fieldName): array
+    {
+        $sortArgs = [];
+        $sortOrder = self::getSortOrder($info, $fieldName);
+
+        foreach ($sortOrder as $orderName) {
+            if (!isset($args[$fieldName][$orderName])) {
+                continue;
+            }
+            $sortArgs[$orderName] = $args[$fieldName][$orderName];
+            unset($args[$fieldName][$orderName]);
+        }
+
+        return array_merge($sortArgs, $args[$fieldName]);
+    }
+
+    /**
+     * Gets the original order of fields to be sorted based on the query args order.
+     *
+     * This is necessary because the underlying GraphQL implementation we're using ignores the
+     * order of query args, and uses the order that fields are defined in the schema instead.
+     */
+    private static function getSortOrder(ResolveInfo $info, string $fieldName)
+    {
+        $relevantNode = $info->fieldDefinition->getName();
+
+        // Find the query field node that matches the schema
+        foreach ($info->fieldNodes as $node) {
+            if ($node->name->value !== $relevantNode) {
+                continue;
+            }
+
+            // Find the sort arg
+            foreach ($node->arguments as $arg) {
+                if ($arg->name->value !== $fieldName) {
+                    continue;
+                }
+
+                // Get the sort order from the query
+                $sortOrder = [];
+                foreach ($arg->value->fields as $field) {
+                    $sortOrder[] = $field->name->value;
+                }
+                return $sortOrder;
+            }
+        }
+
+        return [];
+    }
+}
diff --git a/tests/Schema/IntegrationTest.php b/tests/Schema/IntegrationTest.php
@@ -685,14 +685,119 @@ public function provideFilterAndSortOnlyRead(): array
               }
             }
             GRAPHQL,
+            'expected' => [
+              ["myField" => "test2", "author" => ["firstName" => "tester2"]],
+              ["myField" => "test3", "author" => ["firstName" => "tester2"]],
+              ["myField" => "test1", "author" => ["firstName" => "tester1"]],
+            ],
+          ],
+          'read with sorter files title DESC' => [
+            'fixture' => '_SortPlugin',
+            'query' => <<<GRAPHQL
+            query {
+              readDataObjectFakes(sort: { myField: ASC }) {
+                nodes {
+                  myField
+                  files(sort: { title: DESC }) {
+                    title
+                  }
+                }
+              }
+            }
+            GRAPHQL,
+            'expected' => [
+              ["myField" => "test1", "files" => [["title" => "file4"], ["title" => "file3"], ["title" => "file2"], ["title" => "file1"]]],
+              ["myField" => "test2", "files" => []],
+              ["myField" => "test3", "files" => []],
+            ],
+          ],
+          'read with sorter files ParentID ACS, name DESC' => [
+            'fixture' => '_SortPlugin',
+            'query' => <<<GRAPHQL
+            query {
+              readDataObjectFakes(sort: { myField: ASC }) {
+                nodes {
+                  myField
+                  files(sort: { ParentID: ASC, name: DESC }) {
+                    title
+                  }
+                }
+              }
+            }
+            GRAPHQL,
+            'expected' => [
+              ["myField" => "test1", "files" => [["title" => "file2"],["title" => "file1"], ["title" => "file4"],["title" => "file3"]]],
+              ["myField" => "test2", "files" => []],
+              ["myField" => "test3", "files" => []],
+            ],
+          ],
+          'read with sorter files ParentID DESC, name ASC' => [
+            'fixture' => '_SortPlugin',
+            'query' => <<<GRAPHQL
+            query {
+              readDataObjectFakes(sort: { myField: ASC }) {
+                nodes {
+                  myField
+                  files(sort: { ParentID: DESC, name: ASC }) {
+                    title
+                  }
+                }
+              }
+            }
+            GRAPHQL,
+            'expected' => [
+              ["myField" => "test1", "files" => [["title" => "file3"],["title" => "file4"], ["title" => "file1"],["title" => "file2"]]],
+              ["myField" => "test2", "files" => []],
+              ["myField" => "test3", "files" => []],
+            ],
+          ],
+          'read with sorter files name ASC, ParentID DESC' => [
+            'fixture' => '_SortPlugin',
+            'query' => <<<GRAPHQL
+            query {
+              readDataObjectFakes(sort: { myField: ASC }) {
+                nodes {
+                  myField
+                  files(sort: { name: ASC, ParentID: DESC }) {
+                    title
+                  }
+                }
+              }
+            }
+            GRAPHQL,
+            'expected' => [
+              ["myField" => "test1", "files" => [["title" => "file3"],["title" => "file1"], ["title" => "file4"],["title" => "file2"]]],
+              ["myField" => "test2", "files" => []],
+              ["myField" => "test3", "files" => []],
+            ],
+          ],
+          'read with sorter files name DESC, ParentID ASC' => [
+            'fixture' => '_SortPlugin',
+            'query' => <<<GRAPHQL
+            query {
+              readDataObjectFakes(sort: { myField: ASC }) {
+                nodes {
+                  myField
+                  files(sort: { name: DESC, ParentID: ASC }) {
+                    title
+                  }
+                }
+              }
+            }
+            GRAPHQL,
+            'expected' => [
+              ["myField" => "test1", "files" => [["title" => "file2"],[ "title" => "file4"],["title" => "file1"],["title" => "file3"]]],
+              ["myField" => "test2", "files" => []],
+              ["myField" => "test3", "files" => []],
+            ],
           ],
         ];
     }
 
     /**
      * @dataProvider provideFilterAndSortOnlyRead
      */
-    public function testFilterAndSortOnlyRead($fixture, $query)
+    public function testFilterAndSortOnlyRead(string $fixture, string $query, array $expected)
     {
         $author = Member::create(['FirstName' => 'tester1']);
         $author->write();
@@ -709,18 +814,34 @@ public function testFilterAndSortOnlyRead($fixture, $query)
         $dataObject3 = DataObjectFake::create(['MyField' => 'test3', 'AuthorID' => $author2->ID]);
         $dataObject3->write();
 
+        $file1 = File::create(['Title' => 'file1', 'Name' => 'asc_name']);
+        $file1->ParentID = 1;
+        $file1->write();
+
+        $file2 = File::create(['Title' => 'file2', 'Name' => 'desc_name']);
+        $file2->ParentID = 1;
+        $file2->write();
+
+        $file3 = File::create(['Title' => 'file3', 'Name' => 'asc_name']);
+        $file3->ParentID = 2;
+        $file3->write();
+
+        $file4 = File::create(['Title' => 'file4', 'Name' => 'desc_name']);
+        $file4->ParentID = 2;
+        $file4->write();
+
+        $dataObject1->Files()->add($file1);
+        $dataObject1->Files()->add($file2);
+        $dataObject1->Files()->add($file3);
+        $dataObject1->Files()->add($file4);
 
         $factory = new TestSchemaBuilder(['_' . __FUNCTION__ . $fixture]);
         $schema = $this->createSchema($factory);
 
         $result = $this->querySchema($schema, $query);
         $this->assertSuccess($result);
         $records = $result['data']['readDataObjectFakes']['nodes'] ?? [];
-        $this->assertResults([
-              ["myField" => "test2", "author" => ["firstName" => "tester2"]],
-              ["myField" => "test3", "author" => ["firstName" => "tester2"]],
-              ["myField" => "test1", "author" => ["firstName" => "tester1"]],
-        ], $records);
+        $this->assertResults($expected, $records);
     }
 
     public function testAggregateProperties()

diff --git a/tests/Schema/_testFilterAndSortOnlyRead_SortPlugin/models.yml b/tests/Schema/_testFilterAndSortOnlyRead_SortPlugin/models.yml
@@ -0,0 +1,22 @@
+SilverStripe\GraphQL\Tests\Fake\DataObjectFake:
+  operations:
+    read:
+      plugins:
+        sort:
+          before: paginateList
+  fields:
+    myField: true
+    AuthorID: true
+    author:
+      fields:
+        firstName: true
+    files:
+      fields:
+        title: true
+      plugins:
+        sorter:
+          fields:
+            id: true
+            title: true
+            name: true
+            ParentID: true