New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(payments): add webhooks for mangopay #1323
Conversation
3f8b77d
to
26f5f7f
Compare
26f5f7f
to
304fafa
Compare
WalkthroughThe recent updates to the payment system focus on integrating and enhancing functionalities with the MangoPay API. New structures and methods for managing disputes, pay-ins, payouts, refunds, and wallets have been introduced. Additionally, significant improvements include the setup and management of webhooks for real-time event handling, along with task scheduling enhancements for fetching bank accounts, transactions, users, and wallets. These changes aim to streamline payment operations and ensure more efficient and descriptive error handling. Changes
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (15)
- components/payments/cmd/connectors/internal/connectors/configtemplate/types.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/dispute.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/payin.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/payout.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/refund.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/wallets.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/connector.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_bank_accounts.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_transactions.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_users.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_wallets.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_resolve.go (3 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go (1 hunks)
Files skipped from review due to trivial changes (1)
- components/payments/cmd/connectors/internal/connectors/configtemplate/types.go
Additional comments: 25
components/payments/cmd/connectors/internal/connectors/mangopay/loader.go (2)
- 4-4: The import of
net/http
is appropriate for handling HTTP requests and responses in the new webhook management functionality.- 41-45: The modifications to the
Router
method correctly set up a newmux.Router
and handle POST requests on the root path for webhook management. It would be beneficial to add a comment explaining the specific purpose of the root path handler, such as which webhooks it's intended to manage.components/payments/cmd/connectors/internal/connectors/mangopay/client/payin.go (2)
- 13-30: The
PayinResponse
struct is appropriately defined with relevant fields and JSON tags, aligning with the expected response format from MangoPay.- 32-65: The implementation of the
GetPayin
method is correct, efficiently fetching payin details from MangoPay and handling errors appropriately. It would be beneficial to add error logging for HTTP status codes other thanhttp.StatusOK
to aid in debugging and monitoring.components/payments/cmd/connectors/internal/connectors/mangopay/client/refund.go (2)
- 13-31: The
Refund
struct is correctly defined with relevant fields and JSON tags, accurately representing the expected data structure for a refund response from MangoPay.- 33-66: The implementation of the
GetRefund
method is correct, effectively fetching refund details from MangoPay. Consider adding detailed logging for different error scenarios to improve monitoring and troubleshooting capabilities.components/payments/cmd/connectors/internal/connectors/mangopay/client/dispute.go (2)
- 13-30: The
Dispute
struct is appropriately defined with relevant fields and JSON tags, accurately representing the expected data structure for a dispute response from MangoPay.- 32-65: The implementation of the
GetDispute
method is correct, efficiently fetching dispute details from MangoPay. Consider adding more detailed error handling for HTTP status codes other thanhttp.StatusOK
to enhance debugging and monitoring.components/payments/cmd/connectors/internal/connectors/mangopay/client/wallets.go (2)
- 15-16: The addition of the
Owners
field to theWallet
struct is a valuable enhancement, allowing the representation of multiple owners for a single wallet.- 67-99: The implementation of the
GetWallet
method is correct, effectively retrieving wallet details by ID. Consider adding logging for HTTP status codes other thanhttp.StatusOK
to improve troubleshooting and monitoring.components/payments/cmd/connectors/internal/connectors/mangopay/task_resolve.go (3)
- 22-23: The introduction of
taskNameCreateWebhook
andtaskNameHandleWebhook
constants is appropriate for identifying tasks related to webhook management.- 36-36: The addition of the
WebhookID
field to theTaskDescriptor
struct is a valuable enhancement, enabling the association of specific webhooks with tasks, particularly for handling webhook events.- 70-73: The modifications to the
resolveTasks
function to handletaskNameCreateWebhook
andtaskNameHandleWebhook
tasks are correctly implemented. Adding comments explaining the purpose and expected behavior of these new tasks would enhance code readability and maintainability.components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_users.go (2)
- 21-21: The addition of the
FetchCount
field to thefetchUsersState
struct, marked asjson:"-"
, is approved as it likely serves an internal tracking purpose. Consider adding a comment to clarify its intended use.- 109-110: The added comment explaining that bank accounts are never fetched using webhooks, hence the need for polling, provides valuable context and rationale for this approach.
components/payments/cmd/connectors/internal/connectors/mangopay/client/payout.go (1)
- 98-98: The updates to error messages in the
GetPayout
function for request creation and response handling errors enhance clarity and descriptiveness. Ensure consistency in error handling and messaging across all client methods for a uniform error reporting experience.Also applies to: 103-103, 119-119
components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_bank_accounts.go (1)
- 23-23: Adding the
config *Config
parameter totaskFetchBankAccounts
function signature is a significant change. Ensure that all calls to this function throughout the codebase have been updated to include the newconfig
argument. This change enhances the function's configurability, allowing it to access configuration settings, which could be crucial for its operation.components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_wallets.go (1)
- 171-171: The modification to schedule transaction tasks immediately (
OPTIONS_RUN_NOW
) instead of periodically is a significant change. This adjustment implies a shift towards more immediate processing of transaction tasks, which could enhance responsiveness but might also increase the load on the system. Ensure that this change aligns with the intended behavior and performance expectations.components/payments/cmd/connectors/internal/connectors/mangopay/connector.go (1)
- 67-82: Introducing a new task for webhook setup (
taskNameCreateWebhook
) to run synchronously before the main task scheduling (OPTIONS_RUN_NOW_SYNC
) is a strategic enhancement. This ensures that webhooks are set up before any other tasks are scheduled, potentially improving the reliability of event handling. It's crucial to verify that this synchronous setup does not introduce any unintended delays or bottlenecks in the system's startup sequence.components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go (1)
- 210-210: Similarly, in the
UpdateHook
method, consider verifying the appropriate HTTP status code for successful updates. Whilehttp.StatusOK
might be correct depending on the API's design, some APIs usehttp.StatusNoContent
(204) to indicate that an update was successful but no content is returned. Confirm the expected status code with the API documentation.components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go (5)
- 59-130: The
taskCreateWebhooks
function is well-structured and handles the creation and updating of webhooks efficiently. However, there are a couple of areas that could be improved for better error handling and maintainability:
- The environment variable
STACK_PUBLIC_URL
is fetched directly within the function. Consider injecting this as a parameter or configuration to make the function more testable and decouple it from the environment.- The loop for checking and updating or creating webhooks could benefit from more detailed logging, especially in error scenarios, to aid in debugging.
Consider refactoring to inject
STACK_PUBLIC_URL
through configuration and enhance error logging within the webhook management loop.
- 232-305: The
handleTransfer
function is responsible for handling transfer webhooks. It's well-implemented with clear logic for fetching wallet information and creating payment records. However, there are a few areas for potential improvement:
- Error messages could be more descriptive, especially when parsing amounts fails. Including the context, such as the function name or operation being performed, can aid in debugging.
- The use of
big.Int
for amounts is appropriate, but consider validating the currency and amount for additional robustness.Enhance error messages with more context and consider adding validation for currency and amount values.
- 307-371: Similar to
handleTransfer
, thehandlePayIn
function processes payin webhooks effectively. The suggestions forhandleTransfer
regarding error messages and validation apply here as well.Apply the same refinements suggested for
handleTransfer
to improve error messaging and validation.
- 373-437: The
handlePayout
function follows the same pattern ashandleTransfer
andhandlePayIn
. The previous suggestions for improving error messages and adding validation are applicable here too.Enhance error messages and consider adding validation for currency and amount values, similar to the suggestions for
handleTransfer
.
- 510-583: The
fetchWallet
function is crucial for fetching wallet information and ingesting it into the system. Observations and suggestions include:
- The function is well-implemented, with clear logic for fetching and processing wallet information.
- Consider adding more detailed logging, especially in error scenarios, to aid in debugging and monitoring.
Enhance logging within the
fetchWallet
function to provide more context in error scenarios.
components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go
Outdated
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go
Outdated
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_transactions.go
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go
Outdated
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/connector.go (3 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_wallets.go (10 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_resolve.go (4 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go
- components/payments/cmd/connectors/internal/connectors/mangopay/connector.go
- components/payments/cmd/connectors/internal/connectors/mangopay/task_fetch_wallets.go
- components/payments/cmd/connectors/internal/connectors/mangopay/task_resolve.go
- components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files selected for processing (15)
- components/payments/cmd/connectors/internal/api/connectormodule.go (1 hunks)
- components/payments/cmd/connectors/internal/api/connectors_manager/loader.go (3 hunks)
- components/payments/cmd/connectors/internal/connectors/adyen/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/bankingcircle/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/currencycloud/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/dummypay/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/modulr/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/moneycorp/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/stripe/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/wise/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/storage/webhooks.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go
- components/payments/cmd/connectors/internal/connectors/mangopay/loader.go
- components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go
Additional comments: 13
components/payments/cmd/connectors/internal/connectors/wise/loader.go (1)
- 5-5: The import of
storage
is correctly added to enable theRouter
method to accept a*storage.Storage
parameter. This change aligns with the PR's objective of enhancing webhook management.components/payments/cmd/connectors/internal/connectors/moneycorp/loader.go (1)
- 5-5: The import of
storage
is correctly added to enable theRouter
method to accept a*storage.Storage
parameter. This change aligns with the PR's objective of enhancing webhook management.components/payments/cmd/connectors/internal/connectors/bankingcircle/loader.go (1)
- 5-5: The import of
storage
is correctly added to enable theRouter
method to accept a*storage.Storage
parameter. This change aligns with the PR's objective of enhancing webhook management.components/payments/cmd/connectors/internal/connectors/currencycloud/loader.go (1)
- 7-7: The import of
storage
is correctly added to enable theRouter
method to accept a*storage.Storage
parameter. This change aligns with the PR's objective of enhancing webhook management.components/payments/cmd/connectors/internal/connectors/stripe/loader.go (1)
- 8-8: The import of
storage
is correctly added to enable theRouter
method to accept a*storage.Storage
parameter. This change aligns with the PR's objective of enhancing webhook management.components/payments/cmd/connectors/internal/connectors/modulr/loader.go (1)
- 5-5: The import of
storage
is correctly added to enable theRouter
method to accept a*storage.Storage
parameter. This change aligns with the PR's objective of enhancing webhook management.components/payments/cmd/connectors/internal/connectors/adyen/loader.go (2)
- 7-7: The import of
storage
is correctly added to enable theRouter
method to accept a*storage.Storage
parameter. This change aligns with the PR's objective of enhancing webhook management.- 41-41: The
Router
method in theadyen
package'sloader.go
file is the only one among the reviewed files that implements logic within the method. This is a positive step towards utilizing the newly accepted*storage.Storage
parameter. Ensure that thehandleStandardWebhooks()
function is correctly implemented and tested.components/payments/cmd/connectors/internal/connectors/atlar/loader.go (1)
- 56-56: The
Router
method now accepts a*storage.Storage
parameter but does not utilize it, returningnil
. If this is a placeholder for future implementation, consider documenting the intended usage or future plans. Otherwise, if the storage is meant to be used, the implementation is missing.components/payments/cmd/connectors/internal/connectors/dummypay/loader.go (1)
- 49-49: The
Router
method now accepts a*storage.Storage
parameter but does not utilize it, returningnil
. If this is a placeholder for future implementation, consider documenting the intended usage or future plans. Otherwise, if the storage is meant to be used, the implementation is missing.components/payments/cmd/connectors/internal/api/connectors_manager/loader.go (2)
- 19-19: The
Router
method signature in theLoader
interface now includes a*storage.Storage
parameter, indicating an intention to use storage in router setups. Ensure that all implementations of this interface are updated accordingly and that the intended use of storage is documented or implemented.- 103-103: The
Router
method in theBuiltLoader
struct now accepts a*storage.Storage
parameter but does not utilize it, returningnil
. If this is a placeholder for future implementation, consider documenting the intended usage or future plans. Otherwise, if the storage is meant to be used, the implementation is missing.components/payments/cmd/connectors/internal/api/connectormodule.go (1)
- 81-81: The modification to include
store *storage.Storage
as an additional argument in thewebhookConnectorRouter
function call is consistent with changes across other components. Ensure that the integration and usage of storage within thewebhookConnectorRouter
function are fully implemented and tested.
components/payments/cmd/connectors/internal/storage/webhooks.go
Outdated
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/moneycorp/loader.go
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/bankingcircle/loader.go
Show resolved
Hide resolved
components/payments/cmd/connectors/internal/connectors/currencycloud/loader.go
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (12)
- components/payments/cmd/connectors/internal/connectors/adyen/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/atlar/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/bankingcircle/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/currencycloud/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/dummypay/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go (1 hunks)
- components/payments/cmd/connectors/internal/connectors/modulr/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/moneycorp/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/stripe/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/connectors/wise/loader.go (2 hunks)
- components/payments/cmd/connectors/internal/storage/webhooks.go (2 hunks)
Files skipped from review as they are similar to previous changes (12)
- components/payments/cmd/connectors/internal/connectors/adyen/loader.go
- components/payments/cmd/connectors/internal/connectors/atlar/loader.go
- components/payments/cmd/connectors/internal/connectors/bankingcircle/loader.go
- components/payments/cmd/connectors/internal/connectors/currencycloud/loader.go
- components/payments/cmd/connectors/internal/connectors/dummypay/loader.go
- components/payments/cmd/connectors/internal/connectors/mangopay/client/webhooks.go
- components/payments/cmd/connectors/internal/connectors/mangopay/task_webhooks.go
- components/payments/cmd/connectors/internal/connectors/modulr/loader.go
- components/payments/cmd/connectors/internal/connectors/moneycorp/loader.go
- components/payments/cmd/connectors/internal/connectors/stripe/loader.go
- components/payments/cmd/connectors/internal/connectors/wise/loader.go
- components/payments/cmd/connectors/internal/storage/webhooks.go
Summary by CodeRabbit