-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(core): Added Dual Refunds Validation - Chargeback+Refund #10533
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
base: main
Are you sure you want to change the base?
Conversation
6e6a36b to
bfbf4bf
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10533 +/- ##
=======================================
Coverage ? 6.46%
=======================================
Files ? 1251
Lines ? 311812
Branches ? 0
=======================================
Hits ? 20169
Misses ? 291643
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| pub merchant_connector_id: Option<common_utils::id_type::MerchantConnectorAccountId>, | ||
| /// Shows up the total refunded amount for a payment | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub total_refunded_amount: Option<MinorUnit>, |
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.
Why do we need to send total_refunded_amount in DisputeResponse?
| /// Decider to enable or disable the connector call for dispute retrieve request | ||
| pub force_sync: Option<bool>, | ||
| /// If enabled provides refunded_amount and disputed_amount linked to the payment intent | ||
| pub expand_all: Option<bool>, |
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.
We shouldn't be having this ideally, because we don't want to show refunded_amount in dispute api
| pub tokenization: Option<common_enums::Tokenization>, | ||
| pub partner_merchant_identifier_details: | ||
| Option<common_types::payments::PartnerMerchantIdentifierDetails>, | ||
| pub state_metadata: Option<serde_json::Value>, |
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.
Can't we use strict types here?
| #[diesel(sql_type = Json)] | ||
| pub struct PaymentIntentStateMetadata { | ||
| /// Shows up the total refunded amount for a payment | ||
| pub total_refunded_amount: Option<MinorUnit>, |
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.
Shall we add refund_data, dispute_data as an object for extending this for future usecases?
| pub tokenization: Option<common_enums::Tokenization>, | ||
| pub partner_merchant_identifier_details: | ||
| Option<common_types::payments::PartnerMerchantIdentifierDetails>, | ||
| pub state_metadata: Option<Value>, |
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.
Please use strict types
|
|
||
| // Update payment_intent for the refund's payment_id | ||
| // Calculate total_refunded_amount based on all succeeded refunds for that payment_id | ||
| let all_refunds_for_payment = db |
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.
Can we have this logic in some central place where refunds core can also utilize this?
| ) | ||
| })?; | ||
|
|
||
| let mut current_state: diesel_models::types::PaymentIntentStateMetadata = payment_intent |
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.
please try to avoid mutable objects
| platform.get_processor().get_account().storage_scheme, | ||
| ) | ||
| .await | ||
| .change_context(subscriptions::errors::ApiErrorResponse::InternalServerError)?; |
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.
Please add an attach_printable
| )?; | ||
|
|
||
| // Block refund if amount exceeds total disputed amount or total captured amount | ||
| if let Some(state_metadata_value) = &payment_intent.state_metadata { |
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.
Please move all of this validations to fns, preferably impl based
Type of Change
Description
In this PR, we have added a
state_metadatacolumn inpayment_intenttable which stores the state of thetotal_refunded_amountandtotal_disputed_amountfor that particular payment_intent. The goal is to add a validation check whenever refunds are being attempted after a chargeback has already occured.In cases where refund occurs first and then chargeback we cannot stop this event from happening but to flag these cases out in the dashboard, we have added a query param
expand_allwhich can be set to true for the Dispute Sync API. Once this is called from the FE, BE will send thetotal_refunded_amountandtotal_disputed_amountDispute Sync which will allow the dashboard to flag such cases.Additional Changes
Motivation and Context
How did you test it?
Case 1: Chargeback happens and then Refund
Send a chargeback webhook to HS Server with the help of Novalnet Webhook Simulator (you being the connector) :
Step 1: Make a SEPA Payment Create + Confirm
cURL :
Response :
Step 2: Make a POST callback to HS server. Take Connector Transaction Id from Response and paste in Novalnet's webhook Simulator. Paste payment access key which is merchant secret :
cURL :
Check Intent and Dispute Table :
Step 3: Initiate a Partial Refund.
cURL :
Response :
Step 4: Intiate another Partial Refund (total disputed amount + previous refunds + new refund amount requested > amount captured => should throw error)
cURL :
Response :
Step 5: Intiate a Refund amount which is within the limits (total disputed amount + previous refunds + new refund amount requested <= amount captured => should go through)
cURL :
Response :
Case 2 : Refund happens then Chargeback (can't stop this but can definitely flag these cases in dashboard)
Step 1 : Make a SEPA Payment
cURL :
Response:
Step 2 : Make a Refund happen
Note: Refund will not get succeeded directly in test env. Need to submit connector tx id to Novalnet support team before making a refund. They will do some adjustments in their system upon which a refund can be triggered.
cURL :
Response :
Step 3: Make a POST callback to HS server for Chargeback
cURL :
Step 4 : Make a Dispute Sync API call with
expand_allquery param set to true.cURL :
Response :
Checklist
cargo +nightly fmt --allcargo clippy