From 73033e13aedb6ed7b05dc0bad8ec4abdd2f40b6a Mon Sep 17 00:00:00 2001 From: William Allen Date: Thu, 28 May 2026 09:24:18 -0400 Subject: [PATCH] Allow clients to specify GraphQL relationship order All relationships currently have a hardcoded ordering by ID, with the direction chosen somewhat arbitrarily based on the expected page render order. This commit switches the default order for all filterable relationships to ID, ascending. This commit also adds the ability for clients to specify an `orderBy` input parameter which accepts a list of columns and their respective orders. For example: `orderBy: [{column: NAME, order: ASC}]`. This capability allows us to integrate client-side table sorting with the GraphQL queries used to fill those tables. Additionally, this feature will allow us to use GraphQL to query data for plots throughout the site. --- app/GraphQL/Directives/FilterDirective.php | 128 +++++++++ graphql/schema.graphql | 50 ++-- tests/Feature/GraphQL/FilterTest.php | 259 ++++++++++++++++++ tests/Feature/GraphQL/TestImageTypeTest.php | 12 +- .../GraphQL/TestMeasurementTypeTest.php | 10 +- .../Build/data/instrumentation-result.json | 16 +- 6 files changed, 431 insertions(+), 44 deletions(-) diff --git a/app/GraphQL/Directives/FilterDirective.php b/app/GraphQL/Directives/FilterDirective.php index 53ef56f4d6..5174426f50 100644 --- a/app/GraphQL/Directives/FilterDirective.php +++ b/app/GraphQL/Directives/FilterDirective.php @@ -4,6 +4,7 @@ namespace App\GraphQL\Directives; +use GraphQL\Error\InvariantViolation; use GraphQL\Error\SyntaxError; use GraphQL\Language\AST\FieldDefinitionNode; use GraphQL\Language\AST\InputObjectTypeDefinitionNode; @@ -20,11 +21,13 @@ use Illuminate\Support\Str; use InvalidArgumentException; use JsonException; +use Nuwave\Lighthouse\OrderBy\OrderByDirective; use Nuwave\Lighthouse\Schema\AST\ASTHelper; use Nuwave\Lighthouse\Schema\AST\DocumentAST; use Nuwave\Lighthouse\Schema\Directives\BaseDirective; use Nuwave\Lighthouse\Support\Contracts\ArgBuilderDirective; use Nuwave\Lighthouse\Support\Contracts\ArgManipulator; +use Nuwave\Lighthouse\Support\Utils; final class FilterDirective extends BaseDirective implements ArgBuilderDirective, ArgManipulator { @@ -41,6 +44,7 @@ public static function definition(): string /** * @throws SyntaxError * @throws JsonException + * @throws InvariantViolation */ public function manipulateArgDefinition( DocumentAST &$documentAST, @@ -92,6 +96,130 @@ public function manipulateArgDefinition( ) ); } + + // For list/Connection fields, inject an orderBy argument covering all @filterable fields. + $isListField = $parentField->type instanceof ListTypeNode + || Str::endsWith(ASTHelper::getUnderlyingTypeName($parentField), 'Connection'); + + if ($isListField) { + $this->injectOrderByArgument($documentAST, $parentField, $parentType); + } + } + + /** + * Inject an orderBy argument into $parentField for all @filterable fields on the return type, + * then invoke OrderByDirective's manipulateArgDefinition so the enum types are generated. + * + * @throws InvariantViolation + * @throws SyntaxError + * @throws JsonException + */ + private function injectOrderByArgument( + DocumentAST &$documentAST, + FieldDefinitionNode &$parentField, + ObjectTypeDefinitionNode|InterfaceTypeDefinitionNode &$parentType, + ): void { + // Avoid injecting twice if @filter is somehow applied more than once on this field + foreach ($parentField->arguments as $existingArg) { + if ($existingArg->name->value === 'orderBy') { + return; + } + } + + $returnTypeName = Str::replaceEnd('Connection', '', ASTHelper::getUnderlyingTypeName($parentField)); + $returnType = $documentAST->types[$returnTypeName] ?? null; + + if (!$returnType instanceof ObjectTypeDefinitionNode && !$returnType instanceof InterfaceTypeDefinitionNode) { + return; + } + + // Collect filterable fields: GraphQL field name -> DB column name + /** @var array $filterableFields fieldName => dbColumn */ + $filterableFields = []; + foreach ($returnType->fields as $field) { + $isFilterable = false; + foreach ($field->directives as $directive) { + if ($directive->name->value === 'filterable') { + $isFilterable = true; + break; + } + } + + if (!$isFilterable) { + continue; + } + + $graphqlFieldName = $field->name->value; + + // Use @rename attribute value as the DB column name, otherwise use the field name + $dbColumn = $graphqlFieldName; + foreach ($field->directives as $directive) { + if ($directive->name->value === 'rename') { + $renameValue = $directive->arguments[0]->value; + if (property_exists($renameValue, 'value')) { + $dbColumn = (string) $renameValue->value; + } + break; + } + } + + $filterableFields[$graphqlFieldName] = $dbColumn; + } + + if ($filterableFields === []) { + return; + } + + // Build a custom columns enum using GraphQL field names as user-visible enum values, + // with @enum(value: "dbColumn") to map each to the actual database column. + $placeholderArg = Parser::inputValueDefinition('orderBy: _'); + $enumName = ASTHelper::qualifiedArgType( + $placeholderArg, + $parentField, + $parentType, + ) . 'OrderByColumn'; + + $enumValues = []; + foreach ($filterableFields as $fieldName => $dbColumn) { + $enumValueName = Utils::toEnumValueName($fieldName); + $enumValues[] = "{$enumValueName} @enum(value: \"{$dbColumn}\")"; + } + $enumValuesString = implode("\n ", $enumValues); + + $documentAST->setTypeDefinition(Parser::enumTypeDefinition(/* @lang GraphQL */ <<arguments[] = $orderByArg; + + // Manually invoke OrderByDirective's manipulateArgDefinition so it generates the enum types. + // hydrate() expects the DirectiveNode itself (the @orderBy node on the arg) plus the node it is attached to. + $orderByDirectiveNode = null; + foreach ($orderByArg->directives as $directive) { + if ($directive->name->value === 'orderBy') { + $orderByDirectiveNode = $directive; + break; + } + } + + if ($orderByDirectiveNode !== null) { + /** @var OrderByDirective $orderByDirective */ + $orderByDirective = app(OrderByDirective::class); + $orderByDirective->hydrate($orderByDirectiveNode, $orderByArg); + $orderByDirective->manipulateArgDefinition($documentAST, $orderByArg, $parentField, $parentType); + } } /** diff --git a/graphql/schema.graphql b/graphql/schema.graphql index f227340eb4..9ca9da2427 100644 --- a/graphql/schema.graphql +++ b/graphql/schema.graphql @@ -29,7 +29,7 @@ type Query { "Get multiple users." users( filters: _ @filter - ): [User!]! @paginate(type: CONNECTION) @orderBy(column: "id") + ): [User!]! @paginate(type: CONNECTION) "Find a single project." project( @@ -43,7 +43,7 @@ type Query { "List the projects available to the current user." projects( filters: _ @filter - ): [Project!]! @paginate(scopes: ["forUser"], type: CONNECTION) @orderBy(column: "id") + ): [Project!]! @paginate(scopes: ["forUser"], type: CONNECTION) "Find a single build by ID." build( @@ -214,7 +214,7 @@ type User { projects( filters: _ @filter - ): [Project!]! @hasMany(type: CONNECTION, scopes: ["forUser"]) @orderBy(column: "id") + ): [Project!]! @hasMany(type: CONNECTION, scopes: ["forUser"]) authenticationTokens: [AuthToken!]! @canRoot(ability: "viewAuthenticationTokens") @hasMany(type: CONNECTION) @orderBy(column: "id") } @@ -345,7 +345,7 @@ type Project { builds( filters: _ @filter - ): [Build!]! @hasMany(type: CONNECTION) @orderBy(column: "id") + ): [Build!]! @hasMany(type: CONNECTION) """ An efficient way to get the number of builds matching a given filter. @@ -363,7 +363,7 @@ type Project { "The sites which have submitted a build to this project." sites( filters: _ @filter - ): [Site!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id") + ): [Site!]! @belongsToMany(type: CONNECTION) "Users with the lowest user role for this project." basicUsers: [User!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id") @@ -842,14 +842,14 @@ type Build { "Test results associated with this build." tests( filters: _ @filter - ): [Test!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [Test!]! @hasMany(type: CONNECTION) """ A list of warnings and errors submitted for this build. """ buildErrors( filters: _ @filter - ): [BuildError!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [BuildError!]! @hasMany(type: CONNECTION) project: Project! @belongsTo @@ -873,11 +873,11 @@ type Build { """ children( filters: _ @filter - ): [Build!]! @hasMany(type: CONNECTION) @orderBy(column: "id") + ): [Build!]! @hasMany(type: CONNECTION) coverage( filters: _ @filter - ): [Coverage!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [Coverage!]! @belongsToMany(type: CONNECTION) """ Get the percentage of lines covered in all files starting with the provided path. @@ -890,15 +890,15 @@ type Build { labels( filters: _ @filter - ): [Label!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [Label!]! @belongsToMany(type: CONNECTION) targets( filters: _ @filter - ): [Target!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: ASC) + ): [Target!]! @hasMany(type: CONNECTION) commands( filters: _ @filter - ): [BuildCommand!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: ASC) + ): [BuildCommand!]! @hasMany(type: CONNECTION) urls: [UploadedUrl!]! @hasMany(relation: "uploadedFiles", type: CONNECTION, scopes: ["urls"]) @orderBy(column: "id", direction: ASC) @@ -910,7 +910,7 @@ type Build { dynamicAnalyses( filters: _ @filter - ): [DynamicAnalysis!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: ASC) + ): [DynamicAnalysis!]! @hasMany(type: CONNECTION) updateStep: Update @belongsTo @@ -949,15 +949,15 @@ type Test { testMeasurements( filters: _ @filter - ): [TestMeasurement!]! @hasMany @orderBy(column: "id", direction: DESC) + ): [TestMeasurement!]! @hasMany testImages( filters: _ @filter - ): [TestImage!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [TestImage!]! @hasMany(type: CONNECTION) labels( filters: _ @filter - ): [Label!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [Label!]! @belongsToMany(type: CONNECTION) } enum TestStatus { @@ -1036,7 +1036,7 @@ type BuildCommand @model(class: "App\\Models\\BuildCommand") { measurements( filters: _ @filter - ): [BuildMeasurement!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: ASC) + ): [BuildMeasurement!]! @hasMany(type: CONNECTION) """ Output filenames and corresponding sizes associated with this command. Most command types only @@ -1044,7 +1044,7 @@ type BuildCommand @model(class: "App\\Models\\BuildCommand") { """ outputs( filters: _ @filter - ): [BuildCommandOutput!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: ASC) + ): [BuildCommandOutput!]! @hasMany(type: CONNECTION) } @@ -1135,7 +1135,7 @@ type BuildError { labels( filters: _ @filter - ): [Label!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id") + ): [Label!]! @belongsToMany(type: CONNECTION) } @@ -1169,7 +1169,7 @@ type Site { "The users who have signed up to maintain this site." maintainers( filters: _ @filter - ): [User!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id") + ): [User!]! @belongsToMany(type: CONNECTION) } @@ -1313,7 +1313,7 @@ type Coverage @model(class: "App\\Models\\CoverageView") { labels( filters: _ @filter - ): [Label!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [Label!]! @belongsToMany(type: CONNECTION) } @@ -1360,11 +1360,11 @@ type Target { commands( filters: _ @filter - ): [BuildCommand!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: ASC) + ): [BuildCommand!]! @hasMany(type: CONNECTION) labels( filters: _ @filter - ): [Label!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [Label!]! @belongsToMany(type: CONNECTION) } @@ -1419,7 +1419,7 @@ type DynamicAnalysis { labels( filters: _ @filter - ): [Label!]! @belongsToMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [Label!]! @belongsToMany(type: CONNECTION) } @@ -1451,7 +1451,7 @@ type Update @model(class: "App\\Models\\BuildUpdate") { updateFiles( filters: _ @filter - ): [UpdateFile!]! @hasMany(type: CONNECTION) @orderBy(column: "id", direction: DESC) + ): [UpdateFile!]! @hasMany(type: CONNECTION) } diff --git a/tests/Feature/GraphQL/FilterTest.php b/tests/Feature/GraphQL/FilterTest.php index d0a66bb06e..fed97c58dc 100644 --- a/tests/Feature/GraphQL/FilterTest.php +++ b/tests/Feature/GraphQL/FilterTest.php @@ -1128,6 +1128,265 @@ public function testFilterNonPaginatedList(): void ]); } + public function testOrderProjectsAscending(): void + { + $this->actingAs($this->users['admin'])->graphQL(' + query { + projects(orderBy: [{column: NAME, order: ASC}]) { + edges { + node { + name + } + } + } + } + ')->assertExactJson([ + 'data' => [ + 'projects' => [ + 'edges' => [ + [ + 'node' => [ + 'name' => 'private1', + ], + ], + [ + 'node' => [ + 'name' => 'private2', + ], + ], + [ + 'node' => [ + 'name' => 'public1', + ], + ], + [ + 'node' => [ + 'name' => 'public2', + ], + ], + [ + 'node' => [ + 'name' => 'public3', + ], + ], + [ + 'node' => [ + 'name' => 'public4', + ], + ], + [ + 'node' => [ + 'name' => 'public5', + ], + ], + ], + ], + ], + ]); + } + + public function testOrderProjectsDescending(): void + { + $this->actingAs($this->users['admin'])->graphQL(' + query { + projects(orderBy: [{column: NAME, order: DESC}]) { + edges { + node { + name + } + } + } + } + ')->assertExactJson([ + 'data' => [ + 'projects' => [ + 'edges' => [ + [ + 'node' => [ + 'name' => 'public5', + ], + ], + [ + 'node' => [ + 'name' => 'public4', + ], + ], + [ + 'node' => [ + 'name' => 'public3', + ], + ], + [ + 'node' => [ + 'name' => 'public2', + ], + ], + [ + 'node' => [ + 'name' => 'public1', + ], + ], + [ + 'node' => [ + 'name' => 'private2', + ], + ], + [ + 'node' => [ + 'name' => 'private1', + ], + ], + ], + ], + ], + ]); + } + + public function testOrderBuildsWithinProject(): void + { + // Generate 3 random UUIDs and sort them so we know the expected ASC order + $names = [ + Str::uuid()->toString(), + Str::uuid()->toString(), + Str::uuid()->toString(), + ]; + sort($names); + [$nameFirst, $nameSecond, $nameThird] = $names; + + // Insert in non-sorted order to verify ordering is applied by the query + $this->projects['public1']->builds()->create([ + 'name' => $nameSecond, + 'uuid' => Str::uuid()->toString(), + ]); + $this->projects['public1']->builds()->create([ + 'name' => $nameThird, + 'uuid' => Str::uuid()->toString(), + ]); + $this->projects['public1']->builds()->create([ + 'name' => $nameFirst, + 'uuid' => Str::uuid()->toString(), + ]); + + $this->actingAs($this->users['admin'])->graphQL(' + query($projectId: ID!) { + project(id: $projectId) { + builds(orderBy: [{column: NAME, order: ASC}]) { + edges { + node { + name + } + } + } + } + } + ', [ + 'projectId' => $this->projects['public1']->id, + ])->assertExactJson([ + 'data' => [ + 'project' => [ + 'builds' => [ + 'edges' => [ + [ + 'node' => [ + 'name' => $nameFirst, + ], + ], + [ + 'node' => [ + 'name' => $nameSecond, + ], + ], + [ + 'node' => [ + 'name' => $nameThird, + ], + ], + ], + ], + ], + ], + ]); + } + + public function testOrderByAndFilterCanBeUsedTogether(): void + { + // Use a shared prefix so the filter can match all three builds + $prefix = 'build-'; + + // Generate 4 random suffixes and sort so we know the expected ASC order + $suffixes = [ + Str::uuid()->toString(), + Str::uuid()->toString(), + Str::uuid()->toString(), + Str::uuid()->toString(), + ]; + sort($suffixes); + [$suffixFirst, $suffixSecond, $suffixThird, $suffixFourth] = $suffixes; + + // Insert in non-sorted order to verify ordering is applied by the query + $this->projects['public1']->builds()->create([ + 'name' => $prefix . $suffixSecond, + 'uuid' => Str::uuid()->toString(), + ]); + $this->projects['public1']->builds()->create([ + 'name' => $prefix . $suffixFirst, + 'uuid' => Str::uuid()->toString(), + ]); + $this->projects['public1']->builds()->create([ + 'name' => $suffixThird, + 'uuid' => Str::uuid()->toString(), + ]); + $this->projects['public1']->builds()->create([ + 'name' => $prefix . $suffixFourth, + 'uuid' => Str::uuid()->toString(), + ]); + + // Filter to only include builds with the shared prefix, ordered DESC + $this->actingAs($this->users['admin'])->graphQL(' + query($projectId: ID!, $prefix: String!) { + project(id: $projectId) { + builds( + filters: {contains: {name: $prefix}} + orderBy: [{column: NAME, order: DESC}] + ) { + edges { + node { + name + } + } + } + } + } + ', [ + 'projectId' => $this->projects['public1']->id, + 'prefix' => $prefix, + ])->assertExactJson([ + 'data' => [ + 'project' => [ + 'builds' => [ + 'edges' => [ + [ + 'node' => [ + 'name' => $prefix . $suffixFourth, + ], + ], + [ + 'node' => [ + 'name' => $prefix . $suffixSecond, + ], + ], + [ + 'node' => [ + 'name' => $prefix . $suffixFirst, + ], + ], + ], + ], + ], + ], + ]); + } + public function testFilterByBelongsToRelationship(): void { $site1 = $this->makeSite(['name' => 'site1']); diff --git a/tests/Feature/GraphQL/TestImageTypeTest.php b/tests/Feature/GraphQL/TestImageTypeTest.php index 743aa1ca75..24ac4bcdb7 100644 --- a/tests/Feature/GraphQL/TestImageTypeTest.php +++ b/tests/Feature/GraphQL/TestImageTypeTest.php @@ -111,16 +111,16 @@ public function testBasicFieldAccess(): void 'edges' => [ [ 'node' => [ - 'id' => (string) $testImageWithImage->id, - 'role' => $testImageWithImage->role, - 'url' => url('/image/' . $this->image->id), + 'id' => (string) $testImageNoImage->id, + 'role' => $testImageNoImage->role, + 'url' => null, ], ], [ 'node' => [ - 'id' => (string) $testImageNoImage->id, - 'role' => $testImageNoImage->role, - 'url' => null, + 'id' => (string) $testImageWithImage->id, + 'role' => $testImageWithImage->role, + 'url' => url('/image/' . $this->image->id), ], ], ], diff --git a/tests/Feature/GraphQL/TestMeasurementTypeTest.php b/tests/Feature/GraphQL/TestMeasurementTypeTest.php index 9055bc84ed..93bce5c34b 100644 --- a/tests/Feature/GraphQL/TestMeasurementTypeTest.php +++ b/tests/Feature/GraphQL/TestMeasurementTypeTest.php @@ -112,16 +112,16 @@ public function testBasicFieldAccess(): void 'node' => [ 'name' => 'test1', 'testMeasurements' => [ - [ - 'name' => 'measurement 2', - 'type' => 'numeric/double', - 'value' => '6', - ], [ 'name' => 'measurement 1', 'type' => 'text/string', 'value' => 'test', ], + [ + 'name' => 'measurement 2', + 'type' => 'numeric/double', + 'value' => '6', + ], ], ], ], diff --git a/tests/Feature/Submission/Build/data/instrumentation-result.json b/tests/Feature/Submission/Build/data/instrumentation-result.json index ee7e01730e..0e0549c288 100644 --- a/tests/Feature/Submission/Build/data/instrumentation-result.json +++ b/tests/Feature/Submission/Build/data/instrumentation-result.json @@ -405,12 +405,12 @@ "edges": [ { "node": { - "text": "Label4" + "text": "MakeTable" } }, { "node": { - "text": "MakeTable" + "text": "Label4" } } ] @@ -679,12 +679,12 @@ "edges": [ { "node": { - "text": "Label3" + "text": "MathFunctions" } }, { "node": { - "text": "MathFunctions" + "text": "Label3" } } ] @@ -999,12 +999,12 @@ "edges": [ { "node": { - "text": "Label3" + "text": "MathFunctions" } }, { "node": { - "text": "MathFunctions" + "text": "Label3" } } ] @@ -1568,7 +1568,7 @@ "edges": [ { "node": { - "text": "Label2" + "text": "Tutorial" } }, { @@ -1578,7 +1578,7 @@ }, { "node": { - "text": "Tutorial" + "text": "Label2" } } ]