-
Notifications
You must be signed in to change notification settings - Fork 221
Add schema name as Executor argument & fix default schema #713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Vincz
commented
Jul 12, 2020
- Add schemaName in the ExecutorArgumentsEvent
- Make the GraphQL collector retrieve schema from ExecutorArgument instead of routing
- Consider default schema as the one with name 'default' or the first one defined
- Apply same logic to Annotation Parser
- Update doc
Q | A |
---|---|
Bug fix? | yes |
New feature? | no |
BC breaks? | yes |
Deprecations? | no |
Tests pass? | yes |
Documented? | yes |
Fixed tickets | #608, #712 |
License | MIT |
- Add schemaName in the ExecutorArgumentsEvent - Make the GraphQL collector retrieve schema from ExecutorArgument instead of routing - Consider default schema as the one with name 'default' or the first one defined - Apply same logic to Annotation Parser - Update doc
Not sure why /** @var ExtensibleSchema $schema */ ExecutorArgumentsEvent::create($schemaName, $schema, $requestString, $contextValue, $rootValue, $variableValue, $operationName), was not accepted anymore to type hint $schema
foreach ($configs['definitions']['schema'] as $schemaName => $schema) { | ||
$schemaQuery = $schema['query'] ?? null; | ||
$schemaMutation = $schema['mutation'] ?? null; | ||
|
||
if ($schemaQuery && $gqlName === $schemaQuery) { | ||
$isRoot = true; | ||
if ('default' == $schemaName) { | ||
if ($defaultSchemaName == $schemaName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use strict check here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And a think the "if" conditions could also be simplified with $schemaQuery === $gqlName
and $schemaMutation === $gqlName
$isDefault = true; | ||
} | ||
} elseif ($schemaMutation && $gqlName === $schemaMutation) { | ||
$isMutation = true; | ||
$isRoot = true; | ||
if ('default' == $schemaName) { | ||
if ($defaultSchemaName == $schemaName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use strict check here?
@@ -28,15 +28,19 @@ public function collect(Request $request, Response $response, Throwable $excepti | |||
{ | |||
$error = false; | |||
$count = 0; | |||
$schema = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use null intead
*/ | ||
public function getSchema(): string | ||
public function getSchema() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
: string