-
Notifications
You must be signed in to change notification settings - Fork 10
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
salesforce
Fix autoFetch
in query
#699
Conversation
@mtuchi this is targeted at main, not the salesforce epic. Is that right? |
Ok, I think I get it. In production, salesforce 4.8.1 will push an array of pages of results into
The issue in #644 ins't necessarily a bug, just an unexpected design choice. What this PR does is flattens those pages into a single item
This still isn't the design I would choose, but we're fixing that in #691. So this PR will fix the 4.x adaptor so that the old references design works a bit better, which is good for our legacy users. But we'll need to port this fix over to the new 5.x branch. |
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.
Protip: when writing tests, make a little hack to make them fail, then discard the hack.
}); | ||
}) | ||
.then(done) | ||
.catch(done); |
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.
Oh don't call done()
if there's an exception. If there's an exception we need this test to fail!
The same goes for the other tests in this describe block
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.
Removed the .catch(done)
for function in describe('query')
Hey, passing Can we maybe: |
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.
Getting closer, but still a few more changes. Thanks!
I have made the suggested improvements, please have a look |
.changeset/ten-crews-complain.md
Outdated
@@ -0,0 +1,8 @@ | |||
--- | |||
'@openfn/language-salesforce': major |
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'm gonna call this a patch release.
it's technically a breaking fix.
But we have a major version coming soon which I expect jobs to migrate to. This patch just ensures that the 4.x branch is stable and behaves as originally intended.
Summary
This PR contain a fix
autoFetch
and code improvements for salesforcequery()
function. All result records are now merged intorecords
arrayRef #644
Review Checklist
Before merging, the reviewer should check the following items:
dev only changes don't need a changeset.