strawberry-graphql / strawberry-django

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

Automatic resolver for single types leaves `pk` argument optional #211

Closed fireteam99 closed 1 year ago

fireteam99 commented 1 year ago

Describe the Bug

Given the following code:

@strawberry.type
class Query:
  foo: Foo = strawberry.django.field()

Will generate the following schema:

foo(pk: ID): Foo!

When it should probably generate:

foo(pk: ID!): Foo!

If you accidentally forget to pass the pk argument you will get a slightly more misleading get() returned more than one Foo -- it returned xyz!

System Information

Additional Context

Upvote & Fund

Fund with Polar

stygmate commented 1 year ago

+1 (I agree with bug label)

stygmate commented 1 year ago

here is the patch to correct this:

Index: strawberry_django/filters.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/strawberry_django/filters.py b/strawberry_django/filters.py
--- a/strawberry_django/filters.py  (revision b9c2127a179f26df05ccfe1830f35c7f42908d09)
+++ b/strawberry_django/filters.py  (revision c6bd0a1b81c4cd370106c3d0059360296c730215)
@@ -182,7 +182,7 @@
             filters = self.get_filters()
             if self.django_model and not self.is_list:
                 if self.is_relation is False:
-                    arguments.append(argument("pk", strawberry.ID))
+                    arguments.append(argument("pk", strawberry.ID, is_optional=False))
             elif filters and filters is not UNSET:
                 arguments.append(argument("filters", filters))
         return super().arguments + arguments
stygmate commented 1 year ago

here is the pull request (test passes) https://github.com/strawberry-graphql/strawberry-graphql-django/pull/214

stygmate commented 1 year ago

@fireteam99 the pull request i made to correct this is merged. You can test it. If OK you can close this issue.

bellini666 commented 1 year ago

I'll close the issue, considering that the PR fixed it. If that's not the case we can reopen it :)

fireteam99 commented 1 year ago

@stygmate thanks for fixing!