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

Allow the user to change InputObjectType's default value on non-specified inputs to a sentinel value #1506

Merged

Conversation

flipbit03
Copy link
Contributor

@flipbit03 flipbit03 commented May 29, 2023

This PR aims to remove the ambiguity that happens with InputObjectTypes when accessing optional fields using the input.<attribute> "dot access" syntax.

Current behavior:

If an optional input field is defined but has not been specified in an incoming InputObjectType, it comes back as a None

This is ambiguous, since the user could also have set that specific field to None, meaning we wouldn't be able to tell between an explicit setting to None versus the field not being set at all.

Proposed behavior in this PR:

Introduce a non-breaking, opt-in method to allow the users to change this default value getter behavior in InputObjectTypes to any value that they want to. Graphene already provides such concept in the form of the graphl.Undefined value, which is a good example of a value that the user could use to differentiate between an optional input being explicitly set to None versus not being set at all.

The proposed API is a function called set_input_object_type_default_value() that allows the user to change the sentinel value that will be returned in optional inputs (it should be called early in the code, before any InputObjectTypes are defined). The default value remains None, aligning with the current behavior, so as not to break existing code that relies on the current assumption.

(Next steps and parting thoughts)

Moving forward, I believe it'd be best to discuss around introducing a well documented breaking change that fixes this behavior altogether, eliminating such possibility. But the current PR should allow us to keep making progress while bigger fish are being fried 🐟 🧑‍🍳

Thanks!

…ndefined` instead of `None`, removing ambiguity of undefined optional inputs using dot notation access syntax.
@codecov
Copy link

codecov bot commented May 29, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (8ede21e) 96.00% compared to head (a355083) 96.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1506   +/-   ##
=======================================
  Coverage   96.00%   96.00%           
=======================================
  Files          51       51           
  Lines        1750     1753    +3     
=======================================
+ Hits         1680     1683    +3     
  Misses         70       70           
Impacted Files Coverage Δ
graphene/types/inputobjecttype.py 97.50% <100.00%> (+0.20%) ⬆️
graphene/validation/depth_limit.py 96.96% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

…test.py for sharing it between multiple test files.
@erikwrede erikwrede merged commit 2da8e9d into graphql-python:master Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants