Update salesforce to use `connection` client #1063

mtuchi · 2025-03-18T17:55:38Z

Summary

Replace state.connection with a global connection client and update unit tests

Fixes

Details

Update configuration-schema to default loginUrl to https://login.salesforce.com and apiVersion to 50.0. Because of defaultConnectionConfig in V3. See V3 connection.ts:L109, V3 connection.ts:L111
Removed util.createBasicConnection().
Removed util.createTokenConnection
Add connect function for connecting to salesforce using basic or access_token
Add setMockConnection() function, which make it easy to setup fake connection
Update unit test to use setMockConnection() for setting up fake connection
Update execute() function to createConnection and removed removeConnection function
Update all Adaptor function to use the global connection instead of state.connection
Remove lodash and the use of flatten in execute function

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?

remove removeConnection function

moved createConnection to Adaptor.js

mtuchi · 2025-03-19T17:14:45Z

Hiya @josephjclark , this one is ready for review. I can't express enough how grateful i am with the unit tests. They helped me a lot on getting this done 🙌🏽

josephjclark · 2025-03-19T18:14:33Z

packages/salesforce/src/Adaptor.js

    return commonExecute(
      util.loadAnyAscii,
-      ...operations
+      createConnection,
+      ...flatten(operations)


Why did we introduce flatten here? it wasn't here before?

@josephjclark, flatten was there before. I just moved createConnection from util.js to Adaptor.js and removed removeConnection function

Ah then I'm getting confused.

Ok, I've looked over this code. You can remove flatten completely and close #1067. Let's just do this here, it's a simple change and it's getting merged into an epic anyway

@josephjclark i have removed the use of flatten and deleted lodash package

Great, thanks!

packages/salesforce/src/util.js

packages/salesforce/test/integration.js

josephjclark · 2025-03-21T09:50:51Z

packages/salesforce/src/Adaptor.js

+  }
+  const { configuration } = state;
+  configuration.access_token
+    ? await tokenAuth(configuration)


hmm, sorry, i was just about to merge but this ternary just caught my eye and it's needling me. I think if I let this go I'll regret it forever.

IMO this is a "naughty ternary" because it doesn't return anything. This should really be an if statement.

And something about the structure and naming of this is bothering me 🤔

I would either:

Move all the code inside this connect function, so that you have a big connect function. I don't really understand why basicAuth and tokenAuth need to be in their own function. We only call them from one place and presumably we're not unit testing them. If you move them inside if/else branches in connect, you can also move checkConnection inside (which is what I was getting at before). And I think that simplifies all the code.

Rename basicAuth to connectWithBasicAuth (because this function does something, it really needs a verb in the name) and tokenAuth to connectWithTokenAuth, then have those functions accept state and return state. Now connect can end with return config.access_token? connectWithBasicAuth(state) : connectWithTokenAuth(state), which at least feels like a cleaner ternary and a nicer way to end the function.

I haven't seen both approaches written down but my instinct says option 1.

What do you think?

Options 1 definitely, it makes the whole implementation of connect clean, let me work on that

Hiya @josephjclark I have cleanup the connect function and remove the helper function

@josephjclark just a heads up, #1070 is rebased against 687-use-client branch. So that PR should be merged before this one to avoid conflict resolution

josephjclark

Thank you @mtuchi , I think this is much cleaner 🙌

* 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

* update jsforce to v3 (#1060) * update jsforce to v3 * add changeset * Update salesforce to use `connection` client (#1063) * 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 * Add `http` function in salesforce (#1070) * 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]> * bump version and update changelog * update changelog --------- Co-authored-by: Emmanuel Evance <[email protected]>

mtuchi added 4 commits March 18, 2025 20:58

wip refactor create connection

2ba6aef

fix unit test

9c6fb6f

wip fix intergration test

f30d74a

remove log

560f82c

mtuchi force-pushed the 687-use-client branch from 129af96 to 560f82c Compare March 18, 2025 18:27

mtuchi changed the title ~~687 use client~~ Update salesforce to use connection client Mar 18, 2025

mtuchi added 8 commits March 19, 2025 16:34

revert use of flatten function

a6227e6

remove removeConnection function

improve basicAuth and tokenAuth

3db9c0e

moved createConnection to Adaptor.js

setup mock connection

55df631

remove .skip and .only

e923f00

cleanup

be16616

remove logLevel

a1958ed

remove default configuration

d6a7ef5

add changeset

2bbd4d9

mtuchi requested a review from josephjclark March 19, 2025 17:12

mtuchi marked this pull request as ready for review March 19, 2025 17:12

josephjclark reviewed Mar 19, 2025

View reviewed changes

mtuchi added 2 commits March 20, 2025 11:30

use checkConnection in basicAuth and tokenAuth only

d7197b8

move basicAuth, tokenAuth and checkConnection to Adaptor.js

1555898

mtuchi requested a review from josephjclark March 20, 2025 09:01

josephjclark mentioned this pull request Mar 20, 2025

Add http function in salesforce #1070

Merged

12 tasks

mtuchi added 3 commits March 21, 2025 11:36

remove array in execute() function

c051a2f

remove flatten from lodash

5505682

remove lodash package

7d65261

josephjclark reviewed Mar 21, 2025

View reviewed changes

update connect function

2cef9b8

mtuchi requested a review from josephjclark March 21, 2025 11:08

josephjclark approved these changes Mar 21, 2025

View reviewed changes

josephjclark merged commit 1ecbaed into epic/salesforce Mar 21, 2025
2 checks passed

josephjclark deleted the 687-use-client branch March 21, 2025 14:56

mtuchi self-assigned this Apr 2, 2025

mtuchi mentioned this pull request Apr 4, 2025

saleforce remove flatten from lodash #1067

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update salesforce to use `connection` client #1063

Update salesforce to use `connection` client #1063

mtuchi commented Mar 18, 2025 •

edited

Loading

mtuchi commented Mar 19, 2025

josephjclark Mar 19, 2025

mtuchi Mar 20, 2025

josephjclark Mar 20, 2025

mtuchi Mar 21, 2025

josephjclark Mar 21, 2025

josephjclark Mar 21, 2025

mtuchi Mar 21, 2025

mtuchi Mar 21, 2025

mtuchi Mar 21, 2025

josephjclark left a comment

Update salesforce to use connection client #1063

Update salesforce to use connection client #1063

Conversation

mtuchi commented Mar 18, 2025 • edited Loading

Summary

Details

AI Usage

Review Checklist

mtuchi commented Mar 19, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephjclark left a comment

Choose a reason for hiding this comment

Update salesforce to use `connection` client #1063

Update salesforce to use `connection` client #1063

mtuchi commented Mar 18, 2025 •

edited

Loading