-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
fix(core): Validate values which are intentionally 0 #12382
base: master
Are you sure you want to change the base?
Conversation
This was especially problematic in the Postgres Insert Node when a user wants to insert `0` as a value in a table in Fixed mode. In `validateValueAgainstSchema`, when a resolvedValue is `0`, the value is considered invalid since in Javascript, `0` is considered a falsey value which makes `!0` is equal to `true`.
3fb5268
to
ad473cc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
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.
besides a minor comment, LGTM
@@ -44,7 +44,7 @@ const validateResourceMapperValue = ( | |||
!skipRequiredCheck && | |||
schemaEntry?.required === true && | |||
schemaEntry.type !== 'boolean' && | |||
!resolvedValue | |||
resolvedValue === undefined |
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.
I don't actually know what the correct behavior here should be, but changing this means that now we will treat nulls and empty string also differently than before. Perhaps that is okay, perhaps it's not. Maybe we need to expand the check here to block both undefined
and null
.
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.
I'll also add a check for null
. In the worst case, if this change does break things, we can always revert 🫡
Well, I'm not 100% what happened, but this now works for Expressions :) So I can say that this resolves #11939 |
Summary
Allow for resource mapper nodes to handle intentional
0
values.This was especially problematic in the Postgres Insert Node when a user
wants to insert
0
as a value in a table in both Fixed and Expressionmode.
In
validateValueAgainstSchema
, when a resolvedValue is0
, the valueis considered invalid since in Javascript,
0
is considered a falseyvalue which makes
!0
is equal totrue
.Workflow for testing:
Related Linear tickets, Github issues, and Community forum posts
Resolves https://linear.app/n8n/issue/NODE-2085/community-issue-postgres-node-validation-fails-with-0
Resolves #11939
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)