-
Notifications
You must be signed in to change notification settings - Fork 51.7k
fix(Phantombuster Node): Don't send empty objects in request #22303
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
base: master
Are you sure you want to change the base?
fix(Phantombuster Node): Don't send empty objects in request #22303
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
E2E Tests: n8n tests passed after 9m 21.5s Run Details
Groups
This message was posted automatically by
currents.dev | Integration Settings
|
RomanDavydchuk
left a comment
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.
The fix works as expected and code looks good 🙌
Also good job on the tests, just left one small nitpick (not required to address it, only if you want to)
But I noticed that your commits are unverified, i.e. without signature, I think it's required for us to have the commit signature set up, so I think you'd need to configure it and repush your changes 😓 But once that is done, we can merge it
This comment has been minimized.
This comment has been minimized.
…e-not-filled-in-optional
RomanDavydchuk
left a comment
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.
Nice, great that you fixed the signature as well. One last comment and we can merge it 🙌
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 think this workflow file can be removed since it's no longer used in tests
Summary
Phantombuster's API throws an error if we send argument or bonusArgument as empty objects, which happened if the user didn't provide any values for those. This fix adds validation to ensure we omit the objects entirely if they are empty.
Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/NODE-4002/community-issue-phantombuster-node-not-filled-in-optional-fields-are
fixes #18087
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)