[UPD] update OpenSPP adaptor with new functions #435
Conversation
| } | ||
| return nextState; | ||
| } catch (err) { | ||
| console.log(`✗ Error: ${err}`); |
There was a problem hiding this comment.
I think I gave you bad advice before - I'm not sure what I was thinking now!
You may not always want to return state if there's an error. It depends on the error really. Sometimes it's fine to throw the error and let the job "crash". You should only trap the error if it's non-fatal or the next operation should still run.
In this case, if there's a validation error on the data, the job will carry on running. That might not be what you want - it might be better for the job to die, and alert the team that action needs to be taken.
packages/openspp/src/Adaptor.js
Outdated
| if (res.length > 0) { | ||
| let groupMembershipIds = res.map( i => i.id ); | ||
| let now = new Date(); | ||
| let sppDateTimeNowString = |
There was a problem hiding this comment.
I am reluctant to suggest this because I think it's broken in the CLI and it's not really a pattern I'm keen to support... but the common adaptor exports a dateFns objects. dateFns has a format function which is really useful for this stuff: https://date-fns.org/v2.16.1/docs/format
If it works it'll let you do something like this:
let sppDateTimeNowString = format (new Date(), 'y-M-d H:m:s')`
| @@ -3,3 +3,14 @@ | |||
| ## 1.0.0 | |||
|
|
|||
| - Create OpenSPP adaptor | |||
|
|
|||
| ## 1.0.1 | |||
There was a problem hiding this comment.
First off can you put the most recent changes first in the log?
Secondly, for future reference, next time you want to update the version, please run pnpm changeset from the repo root. That'll ask you some questions and generate a changelog for you. There should be more details in the main readme.
|
@nhatnm0612 I think this is looking great - can I just check you're happy with the error handling for now? You can always raise a PR to change it later, of course :) If you're happy we'll take it on and try and get a release out today. |
I am please with this error handling for now, we users now are technical people anyway. I will change these error handling in the future if needed @josephjclark it is a pleasure working with you, there were lots of things that I learnt from you guys 😄 |
|
@josephjclark can I push 1 more update? I found an error with dateFns format, please wait. I never met this before coz the time I test these function is not expose that error |
Summary
Update OpenSPP adaptor with new functions
Details
Including these new functions as requested:
Issues
Requests From My Company
Review Checklist
Before merging, the reviewer should check the following items:
migration guide followed?
dev only changes don't need a changeset.