Skip to content
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

Nested Mutations using Type setters #182

Closed
oojacoboo opened this issue Nov 22, 2019 · 2 comments
Closed

Nested Mutations using Type setters #182

oojacoboo opened this issue Nov 22, 2019 · 2 comments

Comments

@oojacoboo
Copy link
Collaborator

oojacoboo commented Nov 22, 2019

Overall, nested mutations has mixed opinions. I'm still fairly new to GraphQL and don't have a solid opinion on whether or not they have value. I think, like with most things, there are pros/cons.

Take this nice list of "rules" discussing namespaces, as an example.
https://graphql-rules.com/rules/mutation-namespaces

I'm not really sure if all, or even most, mutations should be nested as such, but I can certainly see the benefit of having nested mutations available.

Has there been any consideration for this? Could you do the following?

{
  product(id: 1) {
    disable(datetime: "2019-11-22T11:10:52+00:0")
  }
}

Assuming that'd be possible, you could leverage setters of a model/entity object without having to write custom root mutations.

There are a few downsides. Firstly, it could become abused. If someone wrote most of their GraphQL API this way, it certainly wouldn't be ideal. But, I'm not sure that's a valid reason to not support it.o

There is still a question about how persistence is handled in this case. I'm guessing you'd have to have some kind of generic controller to handle all nested mutations, at least for persistence reasons.

Another issue with nested mutations is the spec, itself. The spec states that mutations that are nested are executed in parallel. This could have adverse consequences if abused due to the order of operations. Adversely, mutations at the root are executed consecutively.

@oojacoboo
Copy link
Collaborator Author

From the same graphql-rules.com list, here is another:
https://graphql-rules.com/rules/mutation-business-operations

@moufmouf
Copy link
Member

Hey @oojacoboo ,

Interesting! I think GraphQLite can already support those nested mutations. If you have a look at the JS code examples at https://graphql-rules.com/rules/mutation-namespaces, you will realize that those nested mutations have nothing special. As far as GraphQL is concerned, each mutation is a "field". They are supposed to be in the root "Mutation" type, but nothing prevents you from putting the mutation code is a field that is a type returned by the root mutation. That might be a bit obscure, let's try with a code sample:

class ProductController {
    /**
     * @Mutation
     */
    public function product(int $id): Product {
        return $this->repository->getProductById($id);
    }
}

/**
 * @Type
 */
class Product {
    /**
     * @Autowire(for="repository")
     */
    public function disable(DateTimeImmutable $datetime, ProductRepositoryInterface $repository): Product {
        $this->setDisableDate($datetime);
        $repository->save($this);
        return $this;
    }
}

Would that answer (partly) your question?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants