strawberry-graphql / strawberry-django

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

After update to version 0.8 the GraphQL type of field with method resolver is generated as 'DjangoModelType' #209

Closed Zlira closed 2 years ago

Zlira commented 2 years ago

Describe the Bug

Given the models and other definitions from the example app, if I declare the Fruit type as following:

from strawberry import auto, field

@strawberry_django.type(models.Fruit)
class Fruit:
    id: auto
    name: auto

    @field
    def color(self, info) -> "Color":
        # some additional logic here
        return self.color

The generated GraphQL type of the color field is DjangoModelType instead of Color:

type Fruit {
  id: ID!
  name: String!
  color: DjangoModelType
}

I think, the work-around would be to declare get_color as a standalone function and then do this:

class Fruit:
    ...
    color: "Color" = field(resolver=get_color)

But the default behaviour seems unexpected to me.

System Information

Upvote & Fund

Fund with Polar

bellini666 commented 2 years ago

@patrick91 maybe something related to the issue you fixed last week? I'll try to take a look at this tomorrow

patrick91 commented 2 years ago

I think so! That's annoying, also strange that we didn't find it in the tests 🤔

patrick91 commented 2 years ago

@Zlira can you show us where Color is imported from?

Zlira commented 2 years ago

@patrick91 It wasn't imported, I just edited the definition of the Fruit type in the example app. The Color is a few lines below that.

Anyway, here's a smaller schema that reproduces the same behaviour (the models are from the same app):

import strawberry
import strawberry_django
from strawberry import auto, field

from . import models

@strawberry_django.type(models.Color)
class Color:
    id: auto
    name: auto

@strawberry_django.type(models.Fruit)
class Fruit:
    id: auto
    name: auto

    @field
    def color(self, info) -> "Color":
        # some additional logic here
        return self.color

@strawberry.type
class Query:
    fruit: Fruit = strawberry_django.field()

schema = strawberry.Schema(query=Query)

And the SDL for this schema is:

type Query {
  fruit(pk: ID): Fruit!
}

type Fruit {
  id: ID!
  name: String!
  color: DjangoModelType
}

type DjangoModelType {
  pk: ID!
}
bellini666 commented 2 years ago

Apparently there are two issues here:

1) Because the type annotation gets resolved before the resolver itself, we can't just force auto on unannotated fields (which is the case of fields without resolvers), otherwise it will be used and resolved to a generic DjangoModelType

2) When creating the type, the code that parses the fields could set their annotation as UNRESOLVED when using forward refs.

This PR solves both issues. Can you take a look @patrick91 ?