feat: Type-safe "optional-nullable" fields #3779 #3791
Conversation
Reviewer's Guide by SourceryThis pull request introduces Updated class diagram for optional-nullable fieldsclassDiagram
class Maybe~T~
class UnsetType
class Union~T, UnsetType, None~
class TypeGuard~Union[T, None]~
UnsetType <|-- Maybe~T~
Union~T, UnsetType, None~ ..> Maybe~T~ : includes
TypeGuard~Union[T, None]~ ..> Union~T, UnsetType, None~ : checks type of
note for Maybe~T~ "Represents a value that can be either of type T, Unset, or None"
note for UnsetType "Represents an unset value"
note for Union~T, UnsetType, None~ "Type alias for T | UnsetType | None"
note for TypeGuard~Union[T, None]~ "Type guard to check if a value is not Unset"
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
for more information, see https://pre-commit.ci
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nrbnlulu - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a changelog entry to describe the new
strawberry.Maybeandstrawberry.isnt_unsetfeatures.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @strawberry.input | ||
| class UpdateUserInput: |
There was a problem hiding this comment.
suggestion (testing): Suggest adding a test case for default values with Maybe.
It would be beneficial to add a test case that specifically checks the behavior of strawberry.Maybe when a default value is provided in the input type. This will ensure that the default value is correctly handled when the field is not provided in the mutation input.
| phone: strawberry.Maybe[str] = ( | ||
| strawberry.UNSET |
There was a problem hiding this comment.
suggestion (testing): Test case for explicitly providing UNSET.
While the current tests cover setting the value to None and omitting the field, it would be helpful to have a test case where strawberry.UNSET is explicitly passed as the input value. This would ensure that the intended behavior of UNSET is maintained and differentiated from None.
Suggested implementation:
import pytest
@pytest.mark.asyncio
async def test_explicit_unset():
input_data = UpdateUserInput(phone=UNSET)
# Simulate update logic or simply check that phone remains UNSET
# In a full mutation, UNSET should signal that the field is not updated.
assert input_data.phone is UNSETIf your update mutation uses UpdateUserInput to modify a user's data, please integrate this test with the mutation execution and assert that the user's phone remains unchanged when phone is UNSET.
RELEASE.md
Outdated
|
|
||
| Now you can use `strawberry.Maybe` and `strawberry.isnt_unset` to identify if a value was provided or not. | ||
|
|
||
| i.e |
There was a problem hiding this comment.
suggestion (typo): Use "e.g." instead of "i.e."
"i.e." means "that is", while "e.g." means "for example". Since you're providing an example, "e.g." is more appropriate.
| i.e | |
| e.g. |
RELEASE.md
Outdated
| phone = ( | ||
| input.phone | ||
| ) # could be `str | None` in case we want to nullify the phone |
There was a problem hiding this comment.
issue (bug_risk): Handle the case where input.phone is unset
The example code doesn't define phone outside the if block. It should initialize phone with the existing value or None before the if block.
|
Thanks for adding the Here's a preview of the changelog: This release adds a new (preferable) way to handle optional updates. Now you can use i.e import strawberry
@strawberry.type
class User:
name: str
phone: str | None
@strawberry.input
class UpdateUserInput:
name: str
phone: strawberry.Maybe[str]
@strawberry.type
class Mutation:
def update_user(self, info, input: UpdateUserInput) -> User:
if strawberry.isnt_unset(input.phone):
phone = (
input.phone
) # could be `str | None` in case we want to nullify the phone
return User(name=input.name, phone=phone)Here's the tweet text: |
|
Thanks for adding the Here's a preview of the changelog: This release adds a new (preferable) way to handle optional updates. Now you can use i.e import strawberry
@strawberry.type
class User:
name: str
phone: str | None
@strawberry.input
class UpdateUserInput:
name: str
phone: strawberry.Maybe[str]
@strawberry.type
class Mutation:
def update_user(self, info, input: UpdateUserInput) -> User:
if strawberry.isnt_unset(input.phone):
phone = (
input.phone
) # could be `str | None` in case we want to nullify the phone
return User(name=input.name, phone=phone)Here's the tweet text: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3791 +/- ##
=======================================
Coverage 97.23% 97.24%
=======================================
Files 503 503
Lines 33529 33593 +64
Branches 1716 1717 +1
=======================================
+ Hits 32603 32667 +64
Misses 707 707
Partials 219 219 |
CodSpeed Performance ReportMerging #3791 will not alter performanceComparing Summary
|
Description
Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Adds
strawberry.Maybeandstrawberry.isnt_unsetto handle optional and nullable input fields in mutations, providing a more type-safe and intuitive way to determine if a value is present or absent. It also includes tests and documentation for the new feature.New Features:
strawberry.Maybeandstrawberry.isnt_unsetto handle optional and nullable input fields in mutations, providing a more type-safe and intuitive way to determine if a value is present or absent.Enhancements:
strawberry.UNSETwithstrawberry.Maybeandstrawberry.isnt_unsetfor optional input fields, improving code readability and reducing potential errors.Documentation:
strawberry.Maybefeature and how to use it.Tests:
strawberry.Maybewith different scenarios, including setting values from None to Some, Some to None, and handling absent values.