feat: (payments) simplify loop logic in moneycorp plugin and add unit tests #115
Conversation
| @@ -7,6 +7,8 @@ import ( | |||
| ) | |||
|
|
|||
| var ( | |||
| ErrMissingCurrencies = errors.New("missing currencies") | |||
There was a problem hiding this comment.
Should we add this error for the non-retryable errors in temporal ?
There was a problem hiding this comment.
I'm guessing no for two reasons:
-
missing currency will happen if there is an inconsistency between our implementation and the API response... this is more a configuration error on our side (eg. the platform supports currencies that we didn't add to the whitelist). The smoothest way to deal with this problem would be for an updated version of payments to be deployed and the temporal job to be picked up automatically, allowing us to import the data
-
the resolution of the error is granular (related to 1 transaction) rather than a batch error (related to all transactions in the batch). If our users misconfigure credentials, we mark as a non-retryable error because no matter how many times we retry with that configuration we will never be able to import transactions. In this case (if the page size were 10) we might have a batch of 9 importable transactions and 1 transaction with an unsupported currency. If we want to ignore that one record with an unsupported currency we should program in a "skip" behaviour so that we can still import the rest of the transactions with valid currencies.
fixes ENG-1429
fixes ENG-1435