Skip to content

Add targetType property to Mutation #639

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add targetType property to Mutation #639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/annotations/annotations-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@ This annotation applies on methods for classes tagged with the `@Provider` annot
The resulting field is added to the root Mutation type (defined in configuration at key `overblog_graphql.definitions.schema.mutation`).
The class exposing the mutation(s) must be declared as a [service](https://symfony.com/doc/current/service_container.html).

Optional attributes:

- **targetType** : The GraphQL type to attach the field to (by default, it'll be the root Mutation type).

Example:

This will add an `updateUserEmail` mutation, with as resolver `@=service('App\Graphql\MutationProvider').updateUserEmail(...)`.
Expand Down
6 changes: 6 additions & 0 deletions src/Annotation/Mutation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,10 @@
*/
final class Mutation extends Field
{
/**
* The target type to attach this mutation to.
*
* @var string
*/
public $targetType;
}
18 changes: 16 additions & 2 deletions src/Config/Parser/AnnotationParser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,22 @@ private static function typeAnnotationToGQLConfiguration(
$isRootMutation = ($rootMutationType && $gqlName === $rootMutationType);
$currentValue = ($isRootQuery || $isRootMutation) ? \sprintf("service('%s')", self::formatNamespaceForExpression($reflectionEntity->getName())) : 'value';

$queryType = $isRootMutation ? 'Mutation' : 'Query';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part is incomplete and confusing. The Query and Mutation types are considered "root" regardless of the schema, so they should behave the same way (the $currentValue for example).
Also, if we can target whatever type with the @query, the @mutation should only target "root" Mutations.
I think we should check this and raise an exception if something other than a Mutation is targeted by a @Mutation.
Then we should change the isRootQuery & isRootMutation to check in all schema.
And the last part are the default Query / Mutation.
Now, we must decide what we want to target if there is no targetType provided. We used to target the root query and mutation of the default schema. Now, we can have multiple schemas and we are not sure that we have one with the name "default". So, my suggestion would be to target the one with name "default" if it exists, and if not the first one of the list of schemas.
And the last parameter of getGraphQLFieldsFromProviders should be rename $isDefault as it is used to know if the current type is the default one (the one targeted when no targetType is defined).
The tests should also be updated accordingly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your feedback! I will try to implement the changes you mentioned tomorrow. I am new to using this bundle and I have some troubles understanding what do you mean by: the @mutation should only target "root" Mutations. Do you mean that there might be targetType set to something other than mutation type? And how can a @query target whatever type we choose? This confuses me a little and I guess everything will clear in my head once I understand this concept. Can you guide me to a piece of documentation that explains this and I might have missed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of the @Provider is to be able to add query or mutation from various services.
In the case of Query, we can target the "root" Query (ie. The Query define at the key "query" in the schema configuration)...or...any other type (for example, I could target the type "Droid" like in the test fixtures) and the effect will be that I'll have a new queryable field on type Droid.
Let's say I want to provide a "getVictims" query on the Droid type, it would be queried like that for example:

query {
    getDroid(id: 'HKB-3') { // Return a Droid
        getVictims {
            name
        }
    }
}

In the case of Mutation, the GraphQL specs says that mutations must be defined on the Mutation object (ie. The "root" mutation define at the key "mutation" in the schema config). So I couldn't target the Droid with my mutation. So the targetType should always be a root Mutation as Mutation are always "root".

Hope it'll help :)


if (isset($configs['definitions']['schema'])) {
$mutationTypes = [];

foreach ($configs['definitions']['schema'] as $key => $value) {
if (isset($configs['definitions']['schema'][$key]['mutation'])) {
$mutationTypes[] = $configs['definitions']['schema'][$key]['mutation'];
}
}

$queryType = \in_array($gqlName, $mutationTypes) ? 'Mutation' : 'Query';
}

$gqlConfiguration = self::graphQLTypeConfigFromAnnotation($classAnnotation, $classAnnotations, $properties, $methods, $reflectionEntity->getNamespaceName(), $currentValue);
$providerFields = self::getGraphQLFieldsFromProviders($reflectionEntity->getNamespaceName(), $isRootMutation ? 'Mutation' : 'Query', $gqlName, ($isRootQuery || $isRootMutation));
$providerFields = self::getGraphQLFieldsFromProviders($reflectionEntity->getNamespaceName(), $queryType, $gqlName, ($isRootQuery || $isRootMutation));
$gqlConfiguration['config']['fields'] = $providerFields + $gqlConfiguration['config']['fields'];

if ($classAnnotation instanceof GQL\Relay\Edge) {
Expand Down Expand Up @@ -673,7 +687,7 @@ private static function getGraphQLFieldsFromProviders(string $namespace, string
continue;
}

$annotationTarget = 'Query' === $annotationName ? $annotation->targetType : null;
$annotationTarget = \in_array($annotationName, ['Query', 'Mutation']) ? $annotation->targetType : null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With providers, $annotationName will always be either Query or Mutation, so this line is useless.

if (!$annotationTarget && $isRoot) {
$annotationTarget = $targetType;
}
Expand Down
25 changes: 25 additions & 0 deletions tests/Config/Parser/AnnotationParserTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class AnnotationParserTest extends TestCase
'definitions' => [
'schema' => [
'default' => ['query' => 'RootQuery', 'mutation' => 'RootMutation'],
'alternative' => ['query' => 'AlternativeQuery', 'mutation' => 'AlternativeMutation'],
],
],
'doctrine' => [
Expand Down Expand Up @@ -229,6 +230,30 @@ public function testProviders(): void
],
],
]);

$this->expect('AlternativeQuery', 'object', [
'fields' => [
'planet_sortPlanets' => [
'type' => '[Planet]',
'args' => ['direction' => ['type' => 'String!']],
'resolve' => "@=call(service('Overblog\\\\GraphQLBundle\\\\Tests\\\\Config\\\\Parser\\\\fixtures\\\\annotations\\\\Repository\\\\PlanetRepository').sortPlanets, arguments({direction: \"String!\"}, args))",
'access' => '@=default_access',
'public' => '@=default_public',
],
],
]);

$this->expect('AlternativeMutation', 'object', [
'fields' => [
'planet_destroyPlanet' => [
'type' => 'Planet',
'args' => ['planetInput' => ['type' => 'PlanetInput!']],
'resolve' => "@=call(service('Overblog\\\\GraphQLBundle\\\\Tests\\\\Config\\\\Parser\\\\fixtures\\\\annotations\\\\Repository\\\\PlanetRepository').destroyPlanet, arguments({planetInput: \"PlanetInput!\"}, args))",
'access' => '@=default_access',
'public' => '@=override_public',
],
],
]);
}

public function testFullqualifiedName(): void
Expand Down
21 changes: 21 additions & 0 deletions tests/Config/Parser/fixtures/annotations/Repository/PlanetRepository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ public function searchPlanet(string $keyword)
return [];
}

/**
* @GQL\Query(type="[Planet]", targetType="AlternativeQuery", args={
* @GQL\Arg(type="String!", name="direction")
* })
*/
public function sortPlanets(string $direction)
{
return [];
}

/**
* @GQL\Mutation(type="Planet", args={
* @GQL\Arg(type="PlanetInput!", name="planetInput")
Expand All @@ -34,6 +44,17 @@ public function createPlanet(array $planetInput)
return [];
}

/**
* @GQL\Mutation(type="Planet", targetType="AlternativeMutation", args={
* @GQL\Arg(type="PlanetInput!", name="planetInput")
* })
* @GQL\IsPublic("override_public")
*/
public function destroyPlanet(array $planetInput)
{
return [];
}

/**
* @GQL\Query(type="[Planet]", targetType="Droid", name="allowedPlanets")
* @GQL\Access("override_access")
Expand Down
14 changes: 14 additions & 0 deletions tests/Config/Parser/fixtures/annotations/Type/AlternativeMutation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Overblog\GraphQLBundle\Tests\Config\Parser\fixtures\annotations\Type;

use Overblog\GraphQLBundle\Annotation as GQL;

/**
* @GQL\Type
*/
class AlternativeMutation
{
}
14 changes: 14 additions & 0 deletions tests/Config/Parser/fixtures/annotations/Type/AlternativeQuery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Overblog\GraphQLBundle\Tests\Config\Parser\fixtures\annotations\Type;

use Overblog\GraphQLBundle\Annotation as GQL;

/**
* @GQL\Type
*/
class AlternativeQuery
{
}