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

Issues with strawberry.UNSET #2144

Open
ThirVondukr opened this issue Aug 31, 2022 · 7 comments
Open

Issues with strawberry.UNSET #2144

ThirVondukr opened this issue Aug 31, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@ThirVondukr
Copy link
Contributor

ThirVondukr commented Aug 31, 2022

strawberry.UNSET doesn't make parameters optional

It's not possible to use strawberry.UNSET for optional parameters:

@strawberry.input
class StrawberryUnset:
    a: int = strawberry.UNSET
    b: int = strawberry.UNSET


@strawberry.type
class Query:
    @strawberry.field
    async def test(self, input: StrawberryUnset) -> None:
        return None
query GetTest {
  test(input: {}) # fails: a and b weren't provided
}

Would fail because a and b are required.

Query to _service.sdl fails custom sentinel object is used in place of strawberry.UNSET:

_UNSET = object()

@strawberry.federation.input
class StrawberryUnset:
    a: int = _UNSET
    b: int = _UNSET

System Information

  • Strawberry version (if applicable): 0.127.3

Additional Context

Code Reference: https://gitlab.com/ThirVondukr/strawberry-info-warning/-/blob/unset/tests/test_error.py

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@ThirVondukr ThirVondukr added the bug Something isn't working label Aug 31, 2022
@patrick91
Copy link
Member

@ThirVondukr I think you need to mark the fields as optional as well:

@strawberry.input
class StrawberryUnset:
    a: Optional[int] = strawberry.UNSET
    b: Optional[int] = strawberry.UNSET

otherwise the GraphQL validation will complain 😊

@ThirVondukr
Copy link
Contributor Author

@patrick91 I think there's a use case for a non-null optional field, but I don't know if graphql spec allows that 🤔
Could be something like this:

@strawberry.input
class BookUpdate:
    title: str = strawberry.UNSET
    published_at: Optional[datetime] = strawberry.UNSET

This has benefit of checking all of the values for strawberry.UNSET and not checking some fields for None and some for UNSET in code.

@patrick91
Copy link
Member

I wonder if that's something we can provide, or at least allow, the GraphQL spec doesn't allow it, so it would be something we do in our codebase, but I'm unsure how to do that :)

@ThirVondukr
Copy link
Contributor Author

Main issue (at least for me) is that implementing partial updates is kind of awkward

@ThirVondukr
Copy link
Contributor Author

@strawberry.input
class BookUpdate:
    title: str = None
    published_at: datetime | None = strawberry.UNSET


@strawberry.type
class Mutation:
    async def book_update(
        self,
        id: strawberry.ID,
        input: BookUpdate,
    ) -> BookType:
        book = await fetch_book_somehow(id)

        if input.title is None:
            book.title = input.title

        if input.published_at is strawberry.UNSET:
            book.published_at = input.published_at

        ...

@jkimbo
Copy link
Member

jkimbo commented Aug 31, 2022

FYI I think this proposal should still be implemented: #872

@ThirVondukr
Copy link
Contributor Author

@jkimbo It seems that was the case ~10 months ago, it worked in my old project which used strawberry 0.93.4
But all non-nullable values are indeed required to be present inside of an input by graphql spec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants