feat(payments): Atlar connector: Add more bank details to Accounts (internal) and Payments #1607
Conversation
WalkthroughRecent updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant TaskFetchAccounts
participant Client
participant ThirdPartiesService
participant AccountsService
TaskFetchAccounts->>Client: ingestAccountsBatch(taskID, client)
Client->>ThirdPartiesService: GetV1BetaThirdPartiesID(ctx, id)
ThirdPartiesService-->>Client: Third Party Data
Client->>AccountsService: GetV1AccountsID(ctx, id)
AccountsService-->>Client: Account Data
Client-->>TaskFetchAccounts: Processed Data
sequenceDiagram
participant TaskFetchTransactions
participant Client
participant TransactionsService
participant AccountsService
participant ThirdPartiesService
TaskFetchTransactions->>Client: ingestPaymentsBatch(ctx, connectorID, taskID, pagedTransactions)
Client->>TransactionsService: GetV1TransactionsID(ctx, id)
TransactionsService-->>Client: Transaction Data
Client->>AccountsService: ExtractPaymentMetadata(account, bank)
AccountsService-->>Client: Metadata
Client->>ThirdPartiesService: GetV1BetaThirdPartiesID(ctx, id)
ThirdPartiesService-->>Client: Third Party Data
Client-->>TaskFetchTransactions: Processed Payments
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
lwagner-getmomo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (6)
components/payments/cmd/connectors/internal/connectors/atlar/task_fetch_accounts.go (1)
110-113: Update function documentation.The function signature has changed to include
taskIDandclientparameters. Ensure the function documentation is updated accordingly.components/payments/cmd/connectors/internal/connectors/atlar/task_payments.go (3)
Line range hint
1-235:
Update function documentation.The function signature has changed to include
taskIDandclientparameters. Ensure the function documentation is updated accordingly.
360-364: Update function documentation.The function signature has changed to include
taskIDandclientparameters. Ensure the function documentation is updated accordingly.
Line range hint
96-100:
Update function documentation.The function signature has changed to include
taskIDandclientparameters. Ensure the function documentation is updated accordingly.components/payments/cmd/connectors/internal/connectors/atlar/task_fetch_transactions.go (2)
69-71: Update function documentation.The function signature has changed to include
taskIDandclientparameters. Ensure the function documentation is updated accordingly.
96-100: Update function documentation.The function signature has changed to include
taskIDandclientparameters. Ensure the function documentation is updated accordingly.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- components/payments/cmd/connectors/internal/connectors/atlar/account_utils.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/client/accounts.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/client/third_parties.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/currencies.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/task_fetch_accounts.go (4 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/task_fetch_transactions.go (7 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/task_payments.go (2 hunks)
Additional comments not posted (19)
components/payments/cmd/connectors/internal/connectors/atlar/currencies.go (1)
8-8: Addition of DKK currency looks good.The addition of "DKK" to the
supportedCurrenciesWithDecimalmap is correct and consistent with the existing structure.components/payments/cmd/connectors/internal/connectors/atlar/client/third_parties.go (1)
11-22: New methodGetV1BetaThirdPartiesIDlooks good.The method correctly handles context, uses deferred metrics, and constructs the parameters properly before making the API call.
components/payments/cmd/connectors/internal/connectors/atlar/client/accounts.go (1)
11-22: New methodGetV1AccountsIDlooks good.The method correctly handles context, uses deferred metrics, and constructs the parameters properly before making the API call.
components/payments/cmd/connectors/internal/connectors/atlar/account_utils.go (1)
48-52: Modification ofExtractAccountMetadatalooks good.The function correctly uses the new
bankparameter to extract additional metadata. The changes are consistent with the rest of the code.components/payments/cmd/connectors/internal/connectors/atlar/task_fetch_accounts.go (4)
Line range hint
1-109:
LGTM!The function
FetchAccountsTaskhas no changes and looks good.
115-120: LGTM!The span attributes now include
taskID, which is useful for tracing.
137-144: Verify the timeout duration.The timeout duration for the context is set to 30 seconds. Ensure this duration is appropriate for the
GetV1BetaThirdPartiesIDcall.
156-157: LGTM!The
ExtractAccountMetadatafunction now includes thethirdPartyResponse.Payloadparameter, which is correctly used.components/payments/cmd/connectors/internal/connectors/atlar/task_payments.go (6)
236-237: LGTM!The
ingestAtlarTransactionfunction call now includes thetaskIDparameter, which is correctly used.
365-367: Verify the timeout duration.The timeout duration for the context is set to 30 seconds. Ensure this duration is appropriate for the
GetV1TransactionsIDcall.
372-373: LGTM!The
atlarTransactionToPaymentBatchElementfunction call now includes thetaskIDparameter, which is correctly used.
Line range hint
133-140:
Verify the timeout duration.The timeout duration for the context is set to 30 seconds. Ensure this duration is appropriate for the
GetV1AccountsIDcall.
Line range hint
141-148:
Verify the timeout duration.The timeout duration for the context is set to 30 seconds. Ensure this duration is appropriate for the
GetV1BetaThirdPartiesIDcall.
Line range hint
169-170:
LGTM!The
ExtractPaymentMetadatafunction now includes theaccountResponse.PayloadandthirdPartyResponse.Payloadparameters, which are correctly used.components/payments/cmd/connectors/internal/connectors/atlar/task_fetch_transactions.go (5)
Line range hint
1-68:
LGTM!The function
FetchTransactionsTaskhas no changes and looks good.
77-78: LGTM!The
atlarTransactionToPaymentBatchElementfunction call now includes thetaskIDparameter, which is correctly used.
133-140: Verify the timeout duration.The timeout duration for the context is set to 30 seconds. Ensure this duration is appropriate for the
GetV1AccountsIDcall.
141-148: Verify the timeout duration.The timeout duration for the context is set to 30 seconds. Ensure this duration is appropriate for the
GetV1BetaThirdPartiesIDcall.
169-170: LGTM!The
ExtractPaymentMetadatafunction now includes theaccountResponse.PayloadandthirdPartyResponse.Payloadparameters, which are correctly used.
| @@ -5,5 +5,6 @@ import "github.com/formancehq/payments/cmd/connectors/internal/connectors/curren | |||
| var ( | |||
| supportedCurrenciesWithDecimal = map[string]int{ | |||
| "EUR": currency.ISO4217Currencies["EUR"], // Euro | |||
| "DKK": currency.ISO4217Currencies["DKK"], | |||
There was a problem hiding this comment.
We need the DKK currency, because there is only one bank for the EUR currency in the Atlar default sandbox. After asking them for a second EUR bank, they asked us if we could use a DKK accounts for testing multi banking.
| ctx, span := connectors.StartSpan( | ||
| ctx, | ||
| "atlar.atlarTransactionToPaymentBatchElement", | ||
| attribute.String("connectorID", connectorID.String()), | ||
| attribute.String("taskID", taskID.String()), | ||
| ) | ||
| defer span.End() |
There was a problem hiding this comment.
I hope creating instead of passing a span is the correct thing to do here.
There was a problem hiding this comment.
Yep it's good, it's gonna be linked to the parent span thanks to the StartSpan function, so LGTM
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- components/payments/cmd/connectors/internal/connectors/atlar/task_payments.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- components/payments/cmd/connectors/internal/connectors/atlar/task_payments.go
We're currently preparing for adding a second bank as partner to work with.
Going forward, we will need to differentiate payments and internal accounts by bank. This PR introduces the metadata necessary to do so.