Implement the Relay specification for mutations #1597
Conversation
| if ($input) { | ||
| $shortName .= 'Input'; | ||
| } elseif (null !== $mutationName) { | ||
| $shortName .= 'Payload'; | ||
| } |
There was a problem hiding this comment.
May this be shorter using sprintf?
There was a problem hiding this comment.
Not sure it will be more clear as $shortName is built from conditions
There was a problem hiding this comment.
Indeed, sprinf doesn't help.
src/Graphql/Type/SchemaBuilder.php
Outdated
| @@ -326,9 +328,13 @@ private function getResourceObjectTypeFields(string $resource, bool $input = fal | |||
| { | |||
| $fields = []; | |||
| $idField = ['type' => GraphQLType::id()]; | |||
| $clientMutationId = GraphQLType::nonNull(GraphQLType::string()); | |||
There was a problem hiding this comment.
clientMutationId can be null: https://facebook.github.io/relay/graphql/mutations.htm#sec-Mutation-inputs
There was a problem hiding this comment.
It can also be not null:
That input object type may contain an argument named clientMutationId. If provided, that argument must be a String. That argument may be non‐null.
And it is in their official examples: https://facebook.github.io/relay/docs/en/graphql-server-specification.html#mutations
I propose to be strict by default (= to keep this as is).
features/graphql/mutation.feature
Outdated
| @@ -27,21 +27,22 @@ Feature: GraphQL mutation support | |||
| And the response should be in JSON | |||
| And the header "Content-Type" should be equal to "application/json" | |||
| And the JSON node "data.__type.fields[0].name" should contain "delete" | |||
| And the JSON node "data.__type.fields[0].description" should contain "Deletes " | |||
| And the JSON node "data.__type.fields[0].type.name" should contain "DeleteMutation" | |||
| And the JSON node "data.__type.fields[0].description" should match '/^Deletes a [a-zA-Z]+\.$/' | |||
There was a problem hiding this comment.
Regex could be optimized as [A-z]
| if (isset($this->graphqlTypes[$resourceClass][$mutationName][$input])) { | ||
| return $this->graphqlTypes[$resourceClass][$mutationName][$input]; | ||
| if ($input) { | ||
| $shortName .= 'Input'; |
There was a problem hiding this comment.
What if $mutationName is null but $input is true? You will concatenate a string on a non-existing variable. But could this situation happen?
There was a problem hiding this comment.
$shortName always exists.
Implement the Relay Input Object Mutations Specification.
When this PR will be merged, we'll respect 100% of the Relay spec!