From eacca89b503f7b4f7abd03a20c16234a320e023d Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Thu, 23 Nov 2023 09:52:47 +1300 Subject: [PATCH] FIX QuerySort::sort method should support sorting for multi arguments --- src/Schema/DataObject/Plugin/QuerySort.php | 78 ++++++- tests/Schema/IntegrationTest.php | 225 ++++++++++++--------- 2 files changed, 204 insertions(+), 99 deletions(-) diff --git a/src/Schema/DataObject/Plugin/QuerySort.php b/src/Schema/DataObject/Plugin/QuerySort.php index 1647bc34..611fbee2 100644 --- a/src/Schema/DataObject/Plugin/QuerySort.php +++ b/src/Schema/DataObject/Plugin/QuerySort.php @@ -18,6 +18,7 @@ use Closure; use SilverStripe\ORM\Sortable; use Exception; +use GraphQL\Type\Definition\ResolveInfo; /** * Adds a sort parameter to a DataObject query @@ -88,6 +89,10 @@ protected function buildAllFieldsConfig(ModelType $modelType, Schema $schema, in } + /** + * @param array $context + * @return Closure + */ /** * @param array $context * @return Closure @@ -96,12 +101,21 @@ public static function sort(array $context): closure { $fieldName = $context['fieldName']; $rootType = $context['rootType']; - return function (?Sortable $list, array $args, array $context) use ($fieldName, $rootType) { + return function (?Sortable $list, array $args, array $context, ResolveInfo $info) use ($fieldName, $rootType) { if ($list === null) { return null; } - $filterArgs = $args[$fieldName] ?? []; - $paths = NestedInputBuilder::buildPathsFromArgs($filterArgs); + + if (!isset($args[$fieldName])) { + return $list; + } + + $sortArgs = static::getSortArgs($info, $args, $rootType, $fieldName); + $paths = NestedInputBuilder::buildPathsFromArgs($sortArgs); + if (empty($paths)) { + return $list; + } + $schemaContext = SchemaConfigProvider::get($context); if (!$schemaContext) { throw new Exception(sprintf( @@ -111,6 +125,7 @@ public static function sort(array $context): closure )); } + $normalisedPaths = []; foreach ($paths as $path => $value) { $normalised = $schemaContext->mapPath($rootType, $path); Schema::invariant( @@ -120,13 +135,66 @@ public static function sort(array $context): closure $path, $rootType ); - $list = $list->sort($normalised, $value); + + $normalisedPaths[$normalised] = $value; } - return $list; + return $list->sort($normalisedPaths); }; } + private static function getSortArgs(ResolveInfo $info, array $args, string $rootType, string $fieldName): array + { + $sortArgs = []; + $sortOrder = self::getSortOrder($info, $rootType, $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 $rootType, string $fieldName) + { + // If we don't have the right field definition, just use the existing order + if ($info->fieldDefinition->getType()->name ?? '' === $rootType) { + $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/tests/Schema/IntegrationTest.php b/tests/Schema/IntegrationTest.php index 372835cf..20b37fd2 100644 --- a/tests/Schema/IntegrationTest.php +++ b/tests/Schema/IntegrationTest.php @@ -415,7 +415,100 @@ public function testNestedFieldDefinitions() $this->assertMissingField($result, 'title'); } - public function testFilterAndSort() + public function provideFilterAndSort(): array + { + return [ + [ + 'query' => << 'id', + 'placeholderRecord' => 'fake1', + 'expected' => 'fake1', + ], + [ + 'query' => << 'id', + 'placeholderRecord' => 'fake1', + 'expected' => 'fake2', + ], + [ + 'query' => << 'myField', + 'placeholderRecord' => '', + 'expected' => 'test1', + ], + [ + 'query' => << 'myField', + 'placeholderRecord' => '', + 'expected' => 'test2', + ], + [ + 'query' => << 'myField', + 'placeholderRecord' => '', + 'expected' => 'test1', + ], + [ + 'query' => << 'myField', + 'placeholderRecord' => '', + 'expected' => 'test4', + ], + [ + 'query' => << 'myField', + 'placeholderRecord' => 'fake3', + 'expected' => 'test4', + ], + ]; + } + + /** + * @dataProvider provideFilterAndSort + */ + public function testFilterAndSort(string $query, string $testAgainst, string $placeholderRecord, string $expected): void { $dir = '_' . __FUNCTION__; @@ -434,6 +527,9 @@ public function testFilterAndSort() $dataObject3 = DataObjectFake::create(['MyField' => 'test3', 'AuthorID' => $author2->ID]); $dataObject3->write(); + $dataObject4 = DataObjectFake::create(['MyField' => 'test4', 'AuthorID' => $author1->ID]); + $dataObject4->write(); + $file1 = File::create(['Title' => 'file1']); $file1->write(); @@ -446,104 +542,45 @@ public function testFilterAndSort() $id1 = $dataObject1->ID; $id2 = $dataObject2->ID; $id3 = $dataObject3->ID; + $id4 = $dataObject4->ID; + + if ($testAgainst === 'id') { + switch ($expected) { + case 'fake1': + $expected = $id1; + break; + case 'fake2': + $expected = $id2; + break; + case 'fake3': + $expected = $id3; + break; + default: + throw new LogicException("No ID known for '$expected'"); + } + } - $schema = $this->createSchema(new TestSchemaBuilder([$dir])); - - $query = <<querySchema($schema, $query); - $this->assertSuccess($result); - $this->assertResult('readOneDataObjectFake.id', $id1, $result); - - $query = <<querySchema($schema, $query); - $this->assertSuccess($result); - $this->assertResult('readOneDataObjectFake.id', $id2, $result); - - $query = <<querySchema($schema, $query); - $this->assertSuccess($result); - $this->assertResult('readOneDataObjectFake.myField', 'test1', $result); - - $query = <<querySchema($schema, $query); - $this->assertSuccess($result); - $this->assertResult('readOneDataObjectFake.myField', 'test2', $result); + $placeholderID = null; + switch ($placeholderRecord) { + case 'fake1': + $placeholderID = $id1; + break; + case 'fake2': + $placeholderID = $id2; + break; + case 'fake3': + $placeholderID = $id3; + break; + } + if ($placeholderID) { + $query = str_replace('_ID_PLACEHOLDER_', (string) $placeholderID, $query); + } - $query = <<querySchema($schema, $query); - $this->assertSuccess($result); - $this->assertResult('readOneDataObjectFake.myField', 'test3', $result); + $schema = $this->createSchema(new TestSchemaBuilder([$dir])); - $query = <<querySchema($schema, $query); $this->assertSuccess($result); - $this->assertResult('readOneDataObjectFake.myField', 'test2', $result); - - $query = <<querySchema($schema, $query); - // Nested fields aren't working. Needs refactoring. -// $this->assertSuccess($result); -// $this->assertResult('readOneDataObjectFake.author.firstName', 'tester1', $result); - - $query = <<querySchema($schema, $query); - -// $this->assertSuccess($result); -// $this->assertNull($result['data']['readOneDataObjectFake']); + $this->assertResult("readOneDataObjectFake.{$testAgainst}", $expected, $result); }