OpenFn / adaptors

The new home for OpenFn adaptors; re-usable connectors for the most common DPGs and DPI building blocks.
GNU General Public License v3.0
4 stars 8 forks source link

salesforce: update API #662

Open josephjclark opened 2 weeks ago

josephjclark commented 2 weeks ago

New API design for salesforce, including adding composeNextState and removing old code.

Delivery Plan

We should break this down into three parts:

Each part should be treated like its own story. It should have its own PR which is reviewed and merged into an epic branch called epic/salesforce.

Overview

All functions should use composeNextState and write to state.data.

references behaviour should be preserved, but state.data should be the primary usage from now on.

We should probably go on and fix #509 here because we're going to be working so close to that problem.

Only basic documentation is included in this issue. We'll do a deep dive on docs later.

We are going to be bold and REMOVE callbacks entirely from the API :scream: This may result in worse performance for this adaptor version on v1, but I think that's acceptable. And then() support, which is coming super soon, will empower this adaptor on v2.

It makes sense to use the new expandReferences API, but that's not technically part of the issue.

Note that the HTTP helpers (request, get, post) should use connection.request. Docs should make it super clear that these helpers only call salesforce APIs. Later, we will add a generic HTTP API with common (see #668)

This work should only have minimal unit tests. We'll do a deep dive on tests in #666

Errors should be thrown, not caught and logged , unless there is any specific messaging that can be added

API

bulkQuery(query, options)

bulk(operation, sObjectName, records, options)

create(sObjectName, records) // use records instead of attrs. Default to batch.

describe(sObjectName)

describeAll()

destroy(sObjectName, ids, options) // rename attrs (which is wrong) to ids

insert(sObjectName, records)

query(soql, options)

request(url, options)

get(url, options)

post(url, data, options)

retrieve(sObjectName, id)

update(sObjectName, records)

upsert(sObjectName, externalId, records)

toUTF8(as-is)

fnIf() // exported out of common

Code Removal

To be clear, the list above is the whole API. The following functions should all be removed.

josephjclark commented 2 weeks ago

This may fix #644

josephjclark commented 2 weeks ago

This is a huge issue.

We should create an epic/salesforce branch and merge steps into it.

I strongly recommend we do this over several branches. Each one should be opened as a PR, reviewed, and merged into the epic

  1. Remove old/unused functions, fix tests
  2. Use composeNextState and fix state.references, update tests
  3. Update API to new signatures, with basic docs changes
mtuchi commented 3 days ago

Just a thought exercise 🧠 , Should we keep exporting the following function from @openfn/language-common 🤔 ?

mtuchi commented 3 days ago

How important is export const relationship =(relationshipName, externalId, dataSource)=>({ [externalId]: dataSource }) ? 🤔 Could this be a fix for {externald.abc: '123'} vs {externalId: { abc: '123'}}

mtuchi commented 3 days ago

Good start today, i have done lots of cleanup but still work in progress. I will keep at it tomorrow

mtuchi commented 2 days ago

Okay today went by so quick, i haven't done much in this issue but i have manage to start clearing the callbacks in each function. I need another programming block tomorrow tidy this up

mtuchi commented 1 day ago

Hiya @aleksa-krolls , I don't think we need to create another issue as this issue is technically done, I need to create a changeset and update the PR description #682. I will need approx 1 hrs to finish the remaining tasks

Notes For Next Step:

josephjclark commented 1 day ago

@mtuchi please keep the changeset really BRIEF! I would rather ask you to add detail than to remove it

commcare 2.0 is a really good model for the right level of detail