`salesforce` update query function #1092

mtuchi · 2025-03-21T16:43:51Z

Summary

Refactor query function in salesforce adaptor to use the latest options in connection.query()

Fixes #1030

Details

Update query function to use thew new jsforce v3 connection. The new connection.query() support query options for fetching more data we reached query limit hence removing the current implementation of autoFetch
Update query options documentation link. Newly supported options are [maxFetch, headers, responseTarget, scanAll]

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to
know!):

You can read more details in our
Responsible AI Policy

Review Checklist

Before merging, the reviewer should check the following items:

Does the PR do what it claims to do?
NA: If this is a new adaptor, added the adaptor on marketing website ?
NA: If this PR includes breaking changes, do we need to update any jobs in
production? Is it safe to release?
Are there any unit tests?
Is there a changeset associated with this PR? Should there be? Note that
dev only changes don't need a changeset.
Have you ticked a box under AI Usage?

mtuchi · 2025-03-21T16:45:03Z

I will wait for #1070 to be merged so that in can fix unit test for query function

mtuchi · 2025-03-25T14:45:56Z

This is almost done, i just need to re-think the query options documentation. Because by default autoFetch: false and maxFetch: 10000 and jsdoc doesn't have good documentation. They have note section that says When maxFetch option is not set, the default value (10,000) is applied. See docs here.

I would like to point users to this query opts docs to see supported options, but it doesn't say the default values for each options

mtuchi · 2025-03-25T14:55:40Z

Hiya @hunterachieng please have a look at changes when you got time

packages/salesforce/test/Adaptor.test.js

.changeset/sweet-ties-juggle.md

* update jsforce to v3 * add changeset

* wip refactor create connection * fix unit test * wip fix intergration test * remove log * revert use of flatten function remove removeConnection function * improve basicAuth and tokenAuth moved createConnection to Adaptor.js * setup mock connection * remove .skip and .only * cleanup * remove logLevel * remove default configuration * add changeset * use checkConnection in basicAuth and tokenAuth only * move basicAuth, tokenAuth and checkConnection to Adaptor.js * remove array in execute() function * remove flatten from lodash * remove lodash package * update connect function

* wip refactor create connection * fix unit test * wip fix intergration test * remove log * revert use of flatten function remove removeConnection function * improve basicAuth and tokenAuth moved createConnection to Adaptor.js * setup mock connection * remove .skip and .only * cleanup * remove logLevel * remove default configuration * add changeset * use checkConnection in basicAuth and tokenAuth only * move basicAuth, tokenAuth and checkConnection to Adaptor.js * remove array in execute() function * remove flatten from lodash * remove lodash package * move http function to http.js * build docs * update unit test * use request helper methods * add unit test for http * add integration test for http * improve logs * add test for query params * add unit test for http.post * add salesforceRequest function use this function in http.js * update test * add changeset * remove json option in FullRequestOption * remove sinon * remove sinon package * move request into http --------- Co-authored-by: Joe Clark <[email protected]>

josephjclark · 2025-03-31T14:16:36Z

packages/salesforce/src/Adaptor.js

    console.log(`Executing query: ${resolvedQuery}`);
-    const autoFetch = resolvedOptions.autoFetch || resolvedOptions.autofetch;
+    const { autoFetch, maxFetch } = resolvedOptions;


Is autoFetch actually doing anything now?

Presumably if you pass maxFetch: 10m, it'll just download 10m records regardless of whether autofetch is enabled? This, by the way, is the correct pagination behaviour. Ask for a million records and you should be given a million records.

Okay that make sense, but i wonder if we need a better way of querying for more results. Because making the query by default autoPage make sense but you still have limitation of 10000 max records by default, if you need more you just need to adjust maxFetch which still is not a guarantee if it will return all results

I think autoFetch is still useful for when user wants to get the default maximum number of results and get the nextRecordsUrl incase they need to use it in their job code

But autofetch isn't doing anything right now. So if you explicitly want all records, what do you do?

Also we haven't documented the 10k default? It's probably worth us duplicating documentation on options we think are going to be super common. Like maxFetch right now (which in other adaptors we call limit, so maybe we should think about that) is basically essential to large queries

autoFetch allow you to fetch max of 10k records by default now. If it's not enabled it will return 2k max of results.
Regarding documentation i am happy to create our own typedef for query options but i don't think we need to remap these options to match other adaptor. I think they just need a clear description of what they do

packages/salesforce/src/Adaptor.js

josephjclark · 2025-03-31T14:21:37Z

packages/salesforce/src/Adaptor.js

+      console.log('total in database : ' + totalSize);
+      console.log('total fetched : ' + totalFetched);
+    }
+    const allResults = { done, records, totalSize, totalFetched };


Is this still the correct data structure to return?

done doesn't seem very useful. Especially now that we're using jsforce for pagination

totalFetched === records.length so isn't adding a lot

I am not sure I see the value of totalSize. Is it actually needed by anyone for anything?

I'm not necessarily asking for a change here, just sounding out the question

If autoFetch: false and we have 8000 records in the database

done: false, totalFetched: 2000, records: [{}] // 2000 objects totalSize: 8000, nextRecordsUrl: 'url-for-next-batch'

If autoFetch: true

done: true, totalFetched: 8000, records: [] // 8000 records totalSize: 8000,

I can remove totalFetched and leave the log only that says total fetched : records.length. I thought it's useful because query has a limitation of 2000 records if autFetch: false and 10000 max records if autoFetch: true and maxFetch value is not specified

Ok. Let me go back and remind myself about this data structure - I seem to remember talking about it before.

My instinct is that we should just have an API that says "get me all records" or "get me this specific set of records", and nothing else. And in that API you'd just have a flat state.data results array.

I don't know if we've talked about this, but if we were to expose the jsforce object directly, we'd have a lot of license to have more opinionated APIs for 90% of use-case, and point users to the client for the edge-cases that aren't covered by by this.

I suppose that depends on how common it is to use nextRecordsUrl

josephjclark · 2025-04-01T15:25:44Z

@mtuchi how about this:

We make the signature query(q, { limit }). We always autoFetch, and we explicitly default the limit to 10k. Pass limit: false or limit:Infinity to download everything.

No other jsforce options are supported.

We return a result array to state.data.

We return the { done, totalSize, records, nextRecordsUrl } object returned by jsforce to state.response

Users wanting to implement their own pagination need to do http.get($.response.nextRecordsUrl). I don't really want them to do this (because it's too hard in openfn). They should be using a cursor to query for the correct range of data, and processing that entire range in the step.

This is not a jsforce query wrapper. This is a salesforce query operation.

This would be a breaking change and should be released in the next major (not 6), along with the bulk API changes.

mtuchi · 2025-04-03T06:56:32Z

Okay got you @josephjclark, I will work on your suggestion and fix the conflicts in the next sprint

taylordowns2000 added this to v2 Mar 21, 2025

github-project-automation bot moved this to New Issues in v2 Mar 21, 2025

mtuchi force-pushed the 1030-sf-query branch from b2e8e36 to 52c935d Compare March 25, 2025 14:32

mtuchi marked this pull request as ready for review March 25, 2025 14:53

mtuchi requested a review from hunterachieng March 25, 2025 14:54

hunterachieng reviewed Mar 26, 2025

View reviewed changes

packages/salesforce/test/Adaptor.test.js Outdated Show resolved Hide resolved

.changeset/sweet-ties-juggle.md Outdated Show resolved Hide resolved

mtuchi and others added 3 commits March 31, 2025 14:14

update jsforce to v3 (#1060)

38de07e

* update jsforce to v3 * add changeset

mtuchi force-pushed the epic/salesforce branch from b6a132e to 8ce9783 Compare March 31, 2025 11:37

mtuchi force-pushed the 1030-sf-query branch 2 times, most recently from c7e3fc4 to 7b4c24c Compare March 31, 2025 11:55

mtuchi requested a review from josephjclark March 31, 2025 11:58

bump version and update changelog

ab4cbfb

josephjclark reviewed Mar 31, 2025

View reviewed changes

packages/salesforce/src/Adaptor.js Outdated Show resolved Hide resolved

josephjclark reviewed Mar 31, 2025

View reviewed changes

packages/salesforce/src/Adaptor.js Outdated Show resolved Hide resolved

josephjclark reviewed Mar 31, 2025

View reviewed changes

mtuchi added 9 commits March 31, 2025 17:59

update changelog

c8c5a47

update query function

ccc4b9d

fix query test

430a1ce

update query function

6f96588

update default value for autoFetch to false

d84dcd4

add changeset

867fa5b

update changeset to minor

9bb1e45

remove comment

9b1fb8b

update test

8ae306f

improve logs

406acba

mtuchi force-pushed the 1030-sf-query branch from ab47395 to 406acba Compare March 31, 2025 15:50

mtuchi mentioned this pull request Apr 2, 2025

Salesforce v6 tidy up #1089

Closed

Base automatically changed from epic/salesforce to main April 2, 2025 08:18

mtuchi added 4 commits April 3, 2025 09:53

update query option and docs

ef35a6f

docs build

4beec6a

update tests

2c4f26a

update changeset

4141c23

mtuchi self-assigned this Apr 3, 2025

mtuchi linked an issue Apr 3, 2025 that may be closed by this pull request

salesforce refactor query function #1030

Open

mtuchi mentioned this pull request Apr 3, 2025

salesforce query improvements #1111

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`salesforce` update query function #1092

`salesforce` update query function #1092

mtuchi commented Mar 21, 2025 •

edited

Loading

mtuchi commented Mar 21, 2025

mtuchi commented Mar 25, 2025

mtuchi commented Mar 25, 2025

josephjclark Mar 31, 2025

mtuchi Mar 31, 2025

mtuchi Mar 31, 2025

josephjclark Mar 31, 2025

mtuchi Mar 31, 2025

josephjclark Mar 31, 2025

josephjclark Mar 31, 2025 •

edited

Loading

mtuchi Mar 31, 2025

josephjclark Mar 31, 2025

josephjclark commented Apr 1, 2025

mtuchi commented Apr 3, 2025 •

edited

Loading

salesforce update query function #1092

Are you sure you want to change the base?

salesforce update query function #1092

Conversation

mtuchi commented Mar 21, 2025 • edited Loading

Summary

Details

AI Usage

Review Checklist

mtuchi commented Mar 21, 2025

mtuchi commented Mar 25, 2025

mtuchi commented Mar 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephjclark Mar 31, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephjclark commented Apr 1, 2025

mtuchi commented Apr 3, 2025 • edited Loading

`salesforce` update query function #1092

`salesforce` update query function #1092

mtuchi commented Mar 21, 2025 •

edited

Loading

josephjclark Mar 31, 2025 •

edited

Loading

mtuchi commented Apr 3, 2025 •

edited

Loading