-
-
Notifications
You must be signed in to change notification settings - Fork 555
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
feat: Type-safe "optional-nullable" fields #3779 #3791
base: main
Are you sure you want to change the base?
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nrbnlulu - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a changelog entry to describe the new
strawberry.Maybe
andstrawberry.isnt_unset
features.
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.
|
||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
phone = ( | ||
input.phone | ||
) # could be `str | None` in case we want to nullify the phone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. Up until 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.not_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 33644 +115
Branches 1716 1722 +6
========================================
+ Hits 32603 32718 +115
Misses 707 707
Partials 219 219 |
CodSpeed Performance ReportMerging #3791 will not alter performanceComparing Summary
|
…SET automatically.
Description
Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Adds
strawberry.Maybe
andstrawberry.isnt_unset
to 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.Maybe
andstrawberry.isnt_unset
to 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.UNSET
withstrawberry.Maybe
andstrawberry.isnt_unset
for optional input fields, improving code readability and reducing potential errors.Documentation:
strawberry.Maybe
feature and how to use it.Tests:
strawberry.Maybe
with different scenarios, including setting values from None to Some, Some to None, and handling absent values.