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" } } ]