Skip to content

Commit

Permalink
FIX: Don't attempt to correct sort when passed as an argument (closes #…
Browse files Browse the repository at this point in the history 
…571)
  • Loading branch information
@kinglozzer
kinglozzer committed Feb 28, 2024
1 parent 2aabfb0 commit e8b485a
Show file tree
Hide file tree
Showing 4 changed files with 249 additions and 115 deletions.
7 changes: 7 additions & 0 deletions src/Schema/Traits/SortTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace SilverStripe\GraphQL\Schema\Traits;

use GraphQL\Language\AST\ObjectValueNode;
use GraphQL\Type\Definition\ResolveInfo;

trait SortTrait
Expand Down Expand Up @@ -44,6 +45,12 @@ private static function getSortOrder(ResolveInfo $info, string $fieldName)
continue;
}

// If the sort has been passed as a variable, we can't attempt to fix it
// See https://github.com/silverstripe/silverstripe-graphql/issues/573
if (!$arg->value instanceof ObjectValueNode) {
continue;
}

// Get the sort order from the query
$sortOrder = [];
foreach ($arg->value->fields as $field) {
Expand Down
323 changes: 208 additions & 115 deletions tests/Schema/IntegrationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,126 +674,106 @@ public function testFieldAliases()
public function provideFilterAndSortOnlyRead(): array
{
return [
'read with sort' => [
'fixture' => '_QuerySort',
'query' => <<<GRAPHQL
query {
readDataObjectFakes(sort: { author: { id: DESC }, myField: ASC }) {
nodes {
myField
author {
firstName
}
}
}
}
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 sort' => [
'fixture' => '_QuerySort',
'query' => <<<GRAPHQL
query {
readDataObjectFakes(sort: { author: { id: DESC }, myField: ASC }) {
nodes {
myField
author {
firstName
}
}
}
}
GRAPHQL,
'expected' => [
["myField" => "test2", "author" => ["firstName" => "tester2"]],
["myField" => "test3", "author" => ["firstName" => "tester2"]],
["myField" => "test1", "author" => ["firstName" => "tester1"]],
],
],
],
'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 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 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 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 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 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 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" => []],
'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" => []],
],
],
],
];
}

Expand Down Expand Up @@ -847,6 +827,119 @@ public function testFilterAndSortOnlyRead(string $fixture, string $query, array
$this->assertResults($expected, $records);
}

public function provideFilterAndSortWithArgsOnlyRead(): array
{
return [
'read with sort - with sort arg' => [
'fixture' => '_QuerySort',
'query' => <<<GRAPHQL
query (\$sort: DataObjectFakeSortFields) {
readDataObjectFakes(sort: \$sort) {
nodes {
myField
author {
firstName
}
}
}
}
GRAPHQL,
'args' => [
'sort' => ['author' => ['id' => 'DESC'], 'myField' => 'ASC']
],
'expected' => [
["myField" => "test2", "author" => ["firstName" => "tester2"]],
["myField" => "test3", "author" => ["firstName" => "tester2"]],
["myField" => "test1", "author" => ["firstName" => "tester1"]],
],
],
'read with sorter files ParentID DESC, name ASC - with sort args' => [
'fixture' => '_SortPlugin',
'query' => <<<GRAPHQL
query (\$sort: DataObjectFakeSortFields, \$fileSort: FilesSimpleSortFields) {
readDataObjectFakes(sort: \$sort) {
nodes {
myField
files(sort: \$fileSort) {
title
}
}
}
}
GRAPHQL,
'args' => [
'sort' => ['myField' => 'ASC'],
'fileSort' => ['ParentID' => 'DESC', 'name' => 'ASC'],
],
'expected' => [
["myField" => "test1", "files" => [["title" => "file2"], ["title" => "file4"], ["title" => "file1"], ["title" => "file3"]]],
["myField" => "test2", "files" => []],
["myField" => "test3", "files" => []],
],
],
];
}

/**
* @dataProvider provideFilterAndSortWithArgsOnlyRead
*/
public function testFilterAndSortWithArgsOnlyRead(string $fixture, string $query, array $args, array $expected)
{
$author = Member::create(['FirstName' => 'tester1']);
$author->write();

$author2 = Member::create(['FirstName' => 'tester2']);
$author2->write();

$dataObject1 = DataObjectFake::create(['MyField' => 'test1', 'AuthorID' => $author->ID]);
$dataObject1->write();

$dataObject2 = DataObjectFake::create(['MyField' => 'test2', 'AuthorID' => $author2->ID]);
$dataObject2->write();

$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, $args);
$this->assertSuccess($result);
$records = $result['data']['readDataObjectFakes']['nodes'] ?? [];

// We intentionally don't check the sort order, as it's expected it may not match:
// See https://github.com/silverstripe/silverstripe-graphql/issues/573
usort($expected, function($a, $b) {
return strcmp($a['myField'], $b['myField']);
});
usort($records, function($a, $b) {
return strcmp($a['myField'], $b['myField']);
});

$this->assertResults($expected, $records);
}

public function testAggregateProperties()
{
$file1 = File::create(['Title' => '1']);
Expand Down
12 changes: 12 additions & 0 deletions tests/Schema/_testFilterAndSortWithArgsOnlyRead_QuerySort/models.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
SilverStripe\GraphQL\Tests\Fake\DataObjectFake:
operations:
read:
plugins:
sort:
before: paginateList
fields:
myField: true
author:
fields:
id: true
firstName: true
22 changes: 22 additions & 0 deletions tests/Schema/_testFilterAndSortWithArgsOnlyRead_SortPlugin/models.yml
View file
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit e8b485a

Please sign in to comment.