strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
https://strawberry.rocks/docs/django
MIT License
421 stars 122 forks source link

Enable atomic nested mutations #628

Open keithhackbarth opened 2 months ago

keithhackbarth commented 2 months ago

Strawberry supports serialization for nested mutations but Strawberry-Django doesn't seem to be able do it in an atomic way

Feature Request Type

Description

In the below example, I expect these things to happen in an atomic way so that if a credit card charge fails we do have to go back out previous transactions.

I've seen similar offers from other libraries such as Prisma.

mutation (
  $createInvoice: CreateInvoiceInput!
  $chargeCreditCard: ChargeCreditCardInput!
  atomic: true
) {
  createInvoice: invoice {
    createInvoice(input: $createInvoice) {
      id
    }
  }

  chargeCreditCard: invoice {
    chargeCreditCard(input: $chargeCreditCard) {
      id
    }
  }
}

Upvote & Fund

Fund with Polar

bellini666 commented 2 months ago

That should be the behavior.

GraphQL states that queries should be resolved in parallel while mutations should happen sequentially, meaning that if one fails, the remaining ones will not be executed.

Having said that, a fail means a GraphQL fail, which could not be the case when using "nice" error returns, such as those: https://strawberry.rocks/docs/django/guide/mutations#django-errors-handling

Is that not what you are seeing? If so, could you provide an example? Asking because that would probably be an issue at https://github.com/graphql-python/graphql-core level.

keithhackbarth commented 2 months ago

Thanks, @bellini666. My question is whether we should have a better mechanism to roll back previous actions if one fails. Should we be treating these mutations as an atomic transaction?

Right now, during checkout, we execute four or five different mutations (invoicing, payment, shipping, etc.). If one, like shipping, fails, we end up in an incomplete state. This might be acceptable for now, but I’m curious how other GraphQL libraries handle this. The detailed implementation of atomicity is more framework-specific (e.g., Django), so I wonder if it is less something that would be in the core but more framework-specific as well? Or if everyone is creating very specific custom mutations (ie checkout)?

Happy to close as "won't do" was just more curious about what the Django approach is?

bellini666 commented 2 months ago

@keithhackbarth hrm I see what you mean...

From the top of my head, I would say that this could be achieved by using extensions, more specifically the on_execute lifecycle hook.

It can check if the operation is a mutation, and if it is, it starts a migration in it.

Not sure if we could do anything on the django integration side to facilitate that. We should try to check how other frameworks do that (if they do) to get some ideas :)