search

blb-ventures / strawberry-django-plus

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

strawberry_django_plus.filters.filter makes fields required by default?! #55

Open blueyed opened 2 years ago

blueyed commented 2 years ago

When using strawberry_django_plus.filters.filter instead of strawberry_django.filter, the annotated fields appear to be required, i.e. activated by default in GraphiQL, and using deprecation_reason causes the schema to validate:

❌ Required input field FooFilter.bar_status cannot be deprecated.

@filter(models.Foo)
class FooFilter:
    bar_status: str = gql.django.field(deprecation_reason="use … instead")

I've seen that | None can be used with the annotation to make it optional, but I think it should be optional by default.

code ref: https://github.com/blb-ventures/strawberry-django-plus/blob/273d906ac7191f5eb073e1bf18b96cd1268466f9/strawberry_django_plus/filters.py#L65-L92

bellini666 commented 2 years ago

Hey @blueyed , are you sure that strawberry_django forthis should work just like strawberry_django forces the type annotation to optional?

I'll take a look over the weekend to check what is strawberry_django_plus doing differently

blueyed commented 2 years ago

are you sure that strawberry_django forthis should work just like strawberry_django forces the type annotation to optional?

Can you please rephrase / fix your question? :)

I cannot really tell though: from my experience filters are optional/gql.UNSET by default, and while it appears to be a nice feature to require them, I do not think this should be the default behavior.

bellini666 commented 2 years ago

are you sure that strawberry_django forthis should work just like strawberry_django forces the type annotation to optional?

Can you please rephrase / fix your question? :)

OMG, I started typing something, than changed my mind in the middle and didn't reread what I wrote.

What I meant was to ask is: Are you sure strawberry_django works like that? Because the behaviour here should be the same AFAIK.

I cannot really tell though: from my experience filters are optional/gql.UNSET by default, and while it appears to be a nice feature to require them, I do not think this should be the default behavior.

I agree with you! It is just strange from the typing point of view. But well, if strawberry_django is already doing that (based on the question above), strawberry_django_plus shouldn't be working differently.