Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usage with default and default_factory arguments #3333

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Release type: minor

Fix using strawberry with default or default_factory (dataclasses basics)
10 changes: 9 additions & 1 deletion strawberry/field.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,15 @@
if self.base_resolver:
return self.base_resolver(*args, **kwargs)

return self.default_resolver(source, self.python_name)
try:
return self.default_resolver(source, self.python_name)
except AttributeError as exc:
# fallback if source is not available or is not an proper instance
if self.default:
return self.default
if self.default_factory:
return self.default_factory()
raise exc

Check warning on line 232 in strawberry/field.py

View check run for this annotation

Codecov / codecov/patch

strawberry/field.py#L231-L232

Added lines #L231 - L232 were not covered by tests

@property
def is_basic_field(self) -> bool:
Expand Down
40 changes: 40 additions & 0 deletions tests/schema/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,3 +580,43 @@ class FooBar2:
FooBar(foo=1)
FooBar(bar=2)
FooBar(foo=1, bar=2)


def test_use_default_factory_on_root():
@strawberry.type
class Sub:
foo: str = strawberry.field(default="bar")

@strawberry.type
class Query:
sub: Sub = strawberry.field(default_factory=Sub)

schema = strawberry.Schema(query=Query)

result = schema.execute_sync(
"""
query TestQuery{
sub {
foo
}
}
"""
)
Comment on lines +596 to +604
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this fixed by passing a root value to schema.execute? 😊

Copy link
Contributor Author

@devkral devkral Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good. There should be a default root value in execute instead of None.

But this also happens on sub types. There should be an automatic initialization of root queries and co

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get you, but I think that would make us quite opinionated, and also we can't guess how all types will be instantiated, this would only work if types don't require any argument 😊

Copy link
Contributor Author

@devkral devkral Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This task isn't easy as you have to provide a mutation or query instance depending on the query to execute.

this is a big task, instead of the small quick and dirty fix

assert result.data == {"sub": {"foo": "bar"}}


def test_use_default_on_root():
@strawberry.type
class Query:
sub: bool = strawberry.field(default=True)

schema = strawberry.Schema(query=Query)

result = schema.execute_sync(
"""
query TestQuery{
sub
}
"""
)
assert result.data == {"sub": True}