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

Make Decimal type return None if parsing an empty string #1360

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

asnoeyink
Copy link

Would fix #927.

When using forms in the frontend, it's often useful for the "null" value of the form field to be an empty string, as HTML Input elements don't take null for a value. When converting that form value into a Graphene Decimal type, an empty string will currently cause a ConversionError. This PR makes empty strings a valid "null" value for Decimal.

@rzane
Copy link

rzane commented Mar 1, 2022

Hey @abs25, I'm not a contributor, but I also encountered this issue. I think the issue here is that the original author of Graphene's Decimal type assumed that a ValueError would be raised in the case of a value that couldn't be parsed. However, that's not the case.

Rather than checking for a blank string, I a better solution would be to change except ValueError to except decimal.InvalidOperation. This will handle scenarios where someone sends "abc" to the server as well.

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.

Empty value for Decimal scalar
2 participants