blb-ventures / strawberry-django-plus

Enhanced Strawberry GraphQL integration with Django
MIT License
178 stars 47 forks source link

FieldDoesNotExist when using field via mixin #188

Open blueyed opened 1 year ago

blueyed commented 1 year ago

Given:

# from strawberry_django import field as django_field
# from strawberry_django import type as django_type
from strawberry_django_plus.gql.django import field as django_field
from strawberry_django_plus.gql.django import type as django_type

class UrnFieldMixin:
    urn: str = django_field()

@django_type(models.Foo, filters=TrayTypeFilter, pagination=True)
class Foo(UrnFieldMixin):
    ...

I get:


../../../Vcs/django/django/db/models/options.py:669: in get_field
    return self.fields_map[field_name]
E   KeyError: 'field'

During handling of the above exception, another exception occurred:
testing/api/test_foo.py:3: in <module>
    from csd.api.schema import schema
.../schema.py:11: in <module>
    from ...api_types import Baz
.../api_types.py:11: in <module>
    from ....api_types import Bar
.../api_types.py:51: in <module>
    @django_type(models.Foo, filters=FooFilter, pagination=True)
../../../Vcs/strawberry-django-plus/strawberry_django_plus/type.py:379: in wrapper
    return _process_type(
../../../Vcs/strawberry-django-plus/strawberry_django_plus/type.py:276: in _process_type
    fields = list(_get_fields(django_type).values())
../../../Vcs/strawberry-django-plus/strawberry_django_plus/type.py:210: in _get_fields
    fields[name] = _from_django_type(
../../../Vcs/strawberry-django-plus/strawberry_django_plus/type.py:149: in _from_django_type
    model_field = get_model_field(
../../../Vcs/strawberry-graphql-django/strawberry_django/fields/types.py:252: in get_model_field
    raise e
../../../Vcs/strawberry-graphql-django/strawberry_django/fields/types.py:235: in get_model_field
    return model._meta.get_field(field_name)
../../../Vcs/django/django/db/models/options.py:671: in get_field
    raise FieldDoesNotExist(
E   django.core.exceptions.FieldDoesNotExist: Foo has no field named 'field', did you mean ...

When not using a mixin, or when not using strawberry-django-plus it works.

I am happy to debug this further, but would appreciate some pointer(s).

bellini666 commented 1 year ago

hey @blueyed ,

Hrm, I wonder if the issue has to do with the fact that the base type is not a dataclass. Maybe defining it as:

@dataclasses.dataclass
class UrnFieldMixin:
    urn: str = django_field()

But not really sure...

blueyed commented 1 year ago

Yeah, that fixes it. Thanks!

Might be worth adding to the documentation? (where?) Could it also be detected and handled with a clearer error message then?

bellini666 commented 1 year ago

Yeah, that fixes it. Thanks!

Might be worth adding to the documentation? (where?) Could it also be detected and handled with a clearer error message then?

Yeah, for sure! I think this actually applies to any dataclass implementation where you are trying to do introspection in it. It is probably because it uses dataclasses.fields to extract the fields, so if an annotation was defined in a class that is not a dataclass it won't be retrieved by it.

Maybe what might be confusing is that someone might not know that @strawberry.type is basically the same as @dataclasses.dataclass with some extra functionalities (it calls dataclasses.dataclass behind the scenes), so any rules applied to dataclasses also applies to strawberry types.

blueyed commented 1 year ago

Unfortunately it also causes only to not be applied/considered. Do you think this could that be done/handled? It's likely because it is not on the concrete class itself, but via the mixin. My idea with the mixin was to define a common field only once, but can do it explicitly then - without the mixin class.

bellini666 commented 1 year ago

@blueyed mixins work, but issue is that the mixin should also be a dataclass/strawberry type, otherwise their annotations won't be considered as fields.