strawberry-graphql / strawberry-django

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

`annotate` and other query optimizations fail in mutations #635

Open erwinfeser opened 1 month ago

erwinfeser commented 1 month ago

Describe the Bug

Let's imagine we have two Django models: Cage and Animal where the second one has a FK to the first one. Then we have the following strawberry definition:

@strawberry_django.type(models.Cage)
class Cage:
    id: int

    @strawberry_django.field(annotate={"total_animals": Count("animals")})
    def total_animals(
        self,
        root: models.Cage,
    ) -> int:
        return root.total_animals

It works great when I ask for totalAnimals in a query but it fails if I ask for the same field in a mutation. Examples:

It works fine

query MyQuery {
  cages {
    id
    totalAnimals
  }
}

It fails with the error Error: "\'Cage\' object has no attribute \'total_animals\'"

mutation MyMutation {
  updateCage(data: {id: 10}) {
    id
    totalAnimals
  }
}

System Information

Upvote & Fund

Fund with Polar

erwinfeser commented 1 month ago

@bellini666 a similar error raises if we use prefetch_related optimization with to_attr in the Prefetch definition, given its result is a list instead of a queryset.

bellini666 commented 1 month ago

I'm checking here, this is happening because we disable optimizations while doing CUD mutations to avoid unexpected issues inside it, but then we don't run the optimization again when returning the model.

The most straightforward fix would be to run the query to fetch the field again, which might lead to an extra query, but I'm not sure if we can do much better...

What do you think?

philipstarkey commented 4 weeks ago

Hi @bellini666, I've also run into this recently as well in strawberry-django and I agree that running a query again seems like the best (and possibly only?) way to fix this completely.

I've had a similar issue with Django REST Framework 'mutations' in the past, and I solved that similarly by refetching the Django model instance using the QuerySet that would normally be used when responding to a query. It's the only way I can see to ensure that all required annotations/prefetches are done, taking into account any changes made as part of the mutation. Annotations computed in a query prior to making the mutation leads to out-of-date values for annotations being returned in the response.

I believe this will also apply to create mutations as well as update mutations, as newly created instances won't have any annotated field on them unless they are refetched after creation.

So my vote would be to query again after performing the mutation, using the query optimiser (if enabled).