feat: Moov connector for Payments #425
Conversation
|
This PR is estimated to cost 10 credits. |
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
We have prepared a new estimate of 170 credits for this pull request. |
5b8b49d to
3061d55
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #425 +/- ##
==========================================
- Coverage 69.65% 63.27% -6.39%
==========================================
Files 627 645 +18
Lines 32208 33385 +1177
==========================================
- Hits 22436 21123 -1313
- Misses 8558 11133 +2575
+ Partials 1214 1129 -85 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of Implementation Choices
{
"message": "activity error",
"source": "GoSDK",
"cause": {
"message": "not found",
"source": "GoSDK",
"cause": {
"message": "not found",
"source": "GoSDK",
"applicationFailureInfo": {
"type": "fundamental"
}
},
"applicationFailureInfo": {
"type": "STORAGE",
"nonRetryable": true
}
},
"activityFailureInfo": {
"scheduledEventId": "23",
"startedEventId": "24",
"identity": "267@dc56a64c1844@",
"activityType": {
"name": "StorageBankAccountsGet"
},
"activityId": "23",
"retryState": "RETRY_STATE_NON_RETRYABLE_FAILURE"
}
}After some investigation, we found that the error is being handled in This change correctly handles the error as intended. payments/internal/storage/error.go Lines 72 to 96 in 86499e0 payments/internal/connectors/engine/workflow/utils.go Lines 181 to 186 in 86499e0
|
| type Config struct { | ||
| PublicKey string `json:"publicKey" validate:"required"` | ||
| PrivateKey string `json:"privateKey" validate:"required"` | ||
| Endpoint string `json:"endpoint" validate:"required"` |
There was a problem hiding this comment.
Might be better to validate that this is a real url. If it is parsed as a URL you can also more easily trim the url scheme below.
| endpoint: endpoint, | ||
| publicKey: publicKey, | ||
| secretKey: secretKey, |
There was a problem hiding this comment.
Why persist these values in memory if we've already passed the credentials to the moov client?
| endpoint: endpoint, | ||
| publicKey: publicKey, | ||
| secretKey: secretKey, | ||
| accountID: accountID, |
There was a problem hiding this comment.
where is accountID actually used?
There was a problem hiding this comment.
When using the SDK, you are required to pass the ID of the Moov merchant account as accountID in order to fetch transfer options and create a transfer.
go.mod
Outdated
| @@ -31,6 +32,7 @@ require ( | |||
| github.com/jackc/pgx/v5 v5.7.5 | |||
| github.com/jackc/pgxlisten v0.0.0-20241005155529-9d952acd6a6c | |||
| github.com/lib/pq v1.10.9 | |||
| github.com/moovfinancial/moov-go v0.12.0 | |||
There was a problem hiding this comment.
There's already a v0.19.0 available. Let's upgrade if possible?
| package client | ||
|
|
||
| const ( | ||
| moovMetadataSpecNamespace = "com.moov.spec/" |
There was a problem hiding this comment.
| moovMetadataSpecNamespace = "com.moov.spec/" | |
| moovMetadataSpecNamespace = "io.moov.spec/" |
| } | ||
|
|
||
| // PaymentMethodMap maps payment method types to their details | ||
| var PaymentMethodMap = map[PaymentMethodType]PaymentMethodInfo{ |
There was a problem hiding this comment.
I don't think there is any reason to store these locally. If they add payment limits or change them it becomes unmaintainable. I think it's better to allow users to set arbitrarily high amounts and simply handle the errors that come back from the API if possible.
|
|
||
| // Sales tax metadata keys | ||
| MoovSalesTaxAmountCurrencyMetadataKey = moovMetadataSpecNamespace + "salesTaxAmountCurrency" | ||
| MoovSalesTaxAmountValueMetadataKey = moovMetadataSpecNamespace + "salesTaxAmountvalue" |
There was a problem hiding this comment.
| MoovSalesTaxAmountValueMetadataKey = moovMetadataSpecNamespace + "salesTaxAmountvalue" | |
| MoovSalesTaxAmountValueMetadataKey = moovMetadataSpecNamespace + "salesTaxAmountValue" |
| if transfer.FacilitatorFee != nil { | ||
| metadata[client.MoovFacilitatorFeeTotalMetadataKey] = fmt.Sprintf("%d", transfer.FacilitatorFee.Total) | ||
| metadata[client.MoovFacilitatorFeeTotalDecimalMetadataKey] = transfer.FacilitatorFee.TotalDecimal | ||
| } |
There was a problem hiding this comment.
Seems like these do not get set correctly in polled payments even when facilitator fees are present. See for example payment with reference 1241b8f1-f357-4794-babd-9afc3068a378 which has a fee of -$0.50
There was a problem hiding this comment.
For this payment, the FacilitatorFee is indeed 0 USD, as confirmed from the raw data retrieved from the API. However, since other keys capture Moov-related fees, we’ll add metadata fields, moovFee and moovFeeDecimal to properly reflect these values.
There was a problem hiding this comment.
Oh that's interesting... because I created the payment via payments service by setting the Facilitator fee in metadata (eg. "com.moov.spec/facilitatorFeeTotal":"50"). I would assume that metadata tag sets the Facillitator fee and not some other arbitrary fee type?
Can you check what is happening there?
There was a problem hiding this comment.
Sure we can take a look, perhaps you can also share a sample of the request body you used in creating this transaction?
There was a problem hiding this comment.
It was something like this:
{
"reference":"transfer_250620_1823",
"connectorID":"eyJQcm92aWRlciI6Im1vb3YiLCJSZWZlcmVuY2UiOiIxYjM2MTU3Yi1iODEzLTQ4OWYtYjQxYS1mMjA4NjliMzUwMGIifQ",
"type":"PAYOUT",
"description":"mooov payout with sales tax",
"asset":"USD/2",
"amount":411,
"destinationAccountID":"eyJDb25uZWN0b3JJRCI6eyJQcm92aWRlciI6Im1vb3YiLCJSZWZlcmVuY2UiOiIxYjM2MTU3Yi1iODEzLTQ4OWYtYjQxYS1mMjA4NjliMzUwMGIifSwiUmVmZXJlbmNlIjoiZmE1M2Y5YTMtYjEyYS00NTU5LTg0MGItNTkwYjVjZjhlNWVlIn0",
"sourceAccountID":"eyJDb25uZWN0b3JJRCI6eyJQcm92aWRlciI6Im1vb3YiLCJSZWZlcmVuY2UiOiIxYjM2MTU3Yi1iODEzLTQ4OWYtYjQxYS1mMjA4NjliMzUwMGIifSwiUmVmZXJlbmNlIjoiOTY4NjA5YjgtMjg4MC00MDI3LWI4ZmYtOTgzYjQ3MWVkNTk2In0",
"metadata":{
"com.moov.spec/sourcePaymentMethodId":"93a34443-4998-471b-8e63-2a979c75957c",
"com.moov.spec/sourcePaymentMethodType":"moov-wallet",
"com.moov.spec/sourceWalletId":"968609b8-2880-4027-b8ff-983b471ed596",
"com.moov.spec/destinationPaymentMethodId":"e92254fe-d4df-447f-a741-8658fa8b7d17",
"com.moov.spec/destinationPaymentMethodType":"rtp-credit",
"com.moov.spec/salesTaxAmountvalue":"51",
"com.moov.spec/salesTaxAmountCurrency":"USD/2",
"com.moov.spec/facilitatorFeeTotal":"50"
}
}
This PR was created by GitStart to address the requirements from this ticket: LNK-51.
This PR implements a new Moov connector for the Payments service, enabling integration with the Moov payment platform.
The connector supports:
The implementation uses the Moov Go SDK and follows the established patterns in the Formance codebase, particularly the Stripe connector.
Key files: