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(FRM): Revise post FRM core flows #4394
Conversation
@@ -25,6 +26,7 @@ pub struct FraudCheck { | |||
pub metadata: Option<serde_json::Value>, | |||
pub modified_at: PrimitiveDateTime, | |||
pub last_step: FraudCheckLastStep, | |||
pub frm_capture_method: Option<storage_enums::CaptureMethod>, // In postFrm, we are updating capture method from automatic to manual. To store the merchant actual capture method, we are storing the actual capture method in frm_capture_method. It will be useful while approving the FRM decision. |
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 can have a separate enum for this, just in case we might need to have different enum variants for these
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.
Since we are using same capture method type as payment capture method, I am just renaming frm_capture_method
to payment_capture_method
as we discussed.
crates/common_enums/src/enums.rs
Outdated
@@ -1176,6 +1179,7 @@ pub enum IntentStatus { | |||
Processing, | |||
RequiresCustomerAction, | |||
RequiresMerchantAction, | |||
FrmRequiresMerchantAction, |
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 can use RequiresMerchantAction
, this is meant to indicate that there is some MerhantAction that is required based on the frm message or some additional fields
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.
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.
@Narayanbhat166 , We have introduced this status to not interfere with payments which doesn't go through FRM. In future there might decisions/actions we might take based on FrmRequiredMerchantAction
status for FRM webhooks which doesn't apply for payment connectors.
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.
Right, this status is introduced for explicitly specifying the fact that merchant needs to take an FRM related action on the payment. Using existing variant RequiresMerchantAction
does not give enough context about what exactly merchant needs to do. This variant is being used in one of the flows for some crypto connector cc: @srujanchikke
Having a separate variant in this case gives us full context about the type of actions that can be taken by the merchant (payment approval / cancellation in this case)
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.
cc : @jarnura
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 should not introduce the same status RequiresMerchantAction
with some prefix again because the merchant wants to do something different by this. If RequiresMerchantAction
is the status, then always the merchant should go to some details field or reason field, which tells what is issue and what merchant needs do.
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.
Let's also add the feasibility to be able to list only the fraudulent transactions in payments list / filter APIs using the details / reason field
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.
Sure, will do that it in subsequent PR
api_models::enums::AttemptStatus::FrmUnresolved | ||
) | ||
{ | ||
payment_data.payment_intent.status = IntentStatus::RequiresCapture; // In Approve flow, payment which has payment_capture_method "manual" and attempt status as "FrmUnresolved", |
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.
Payment data update should not happen within frm flow, only pass the suggestion on what to do with the transaction let the payments_core decide whether to consider the frm_suggestion or not. Check the update_trackers of payment_confirm
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.
Thanks for the info, I have added new frmSuggestion and moved this logic to approve->update trackers
) -> RouterResult<Option<FrmData>> { | ||
if matches!(frm_data.fraud_check.frm_status, FraudCheckStatus::Fraud) | ||
&& matches!(frm_configs.frm_action, FrmAction::AutoRefund) | ||
&& matches!(frm_configs.frm_action, FrmAction::CancelTxn) |
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 had three frm configs right?
- Pre frm -> Cancel on fraud
- Pre frm -> Complete transaction and mark as fraud. merchant will trigger refund if needed
- Post frm -> Auto refund on fraud
- Post frm -> Authorize the payment and send it for manual review. Cancel or Capture payment based on merchant decision
Why are we removing the 3 option here?
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.
In Revised flow, We don't actually capture the payment rather we authorize the payments(Post Frm) then take a call whether to cancel/hold/capture . When merchant approves ,we capture the payment or else we cancel the payment.
…to-env * 'main' of github.com:juspay/hyperswitch: Refactor(core): remove payment_method_id from RouterData struct (#4485) fix(euclid_wasm): connector config wasm metadata update (#4460) chore(version): 2024.04.30.0 feat(user): add single purpose token and auth (#4470) feat: stripe connect integration for payouts (#2041) feat(router): handle authorization for frictionless flow in external 3ds flow (#4471) feat(FRM): Revise post FRM core flows (#4394) feat(router): send poll_config in next_action of confirm response for external 3ds flow (#4443) chore(version): 2024.04.29.0 feat(connector): [CRYPTOPAY] Report underpaid/overpaid amount in outgoing webhooks (#4468) feat(users): use cookie for auth (#4434) refactor(scheduler): join frequency and count in `RetryMapping` (#4313) refactor(required_fields): change required fields for billing address (#4258) refactor(access_token): use `merchant_connector_id` for storing access token (#4462) chore(version): 2024.04.26.0 chore(postman): update Postman collection files
* 'main' of github.com:juspay/hyperswitch: refactor(cypress): read creds from env instead of hardcoding the path (#4430) ci: fix paypal postman tests (#4501) Refactor(core): remove payment_method_id from RouterData struct (#4485) fix(euclid_wasm): connector config wasm metadata update (#4460) chore(version): 2024.04.30.0 feat(user): add single purpose token and auth (#4470) feat: stripe connect integration for payouts (#2041) feat(router): handle authorization for frictionless flow in external 3ds flow (#4471) feat(FRM): Revise post FRM core flows (#4394) feat(router): send poll_config in next_action of confirm response for external 3ds flow (#4443) chore(version): 2024.04.29.0 feat(connector): [CRYPTOPAY] Report underpaid/overpaid amount in outgoing webhooks (#4468) feat(users): use cookie for auth (#4434) refactor(scheduler): join frequency and count in `RetryMapping` (#4313) refactor(required_fields): change required fields for billing address (#4258) refactor(access_token): use `merchant_connector_id` for storing access token (#4462) chore(version): 2024.04.26.0 chore(postman): update Postman collection files
Type of Change
Description
PostAuth flow
proceed with the txn is based on the status / score returned.
Possible actions based on the status
- Continue with the transaction
- Mark the transaction as cancelled
- Hold the txn in manual review state. Merchants can list and review such transactions.
- If approved, payment is captured
- If declined, payment is voided
This flow works same as payments flow incase of 3ds, non 3ds, manual and auto capture for Legit payments.
Additional Changes
up.sql
down.sql
Motivation and Context
How did you test it?
Create merchant connector account with stripe and then create new merchant connector account with signifyd.
TEST CASE 1 : Create legit card payment(amount =150) for both credit and debit. This should go as normal payment flow. Interchange CaptureMethod and CardType(3ds, non 3ds).
TEST CASE 2 : Create Credit(pmt type) Payment with amount equals to 15000. This should Fail the payment .
TESTCASE 3 : Create a payment with payment method type as
debit
. set capture method toautomatic
and use same json above. Payment status should be moved toRequiresMerchantAction
.Response looks like this
/approve
and/reject
should hit after above step for the same payment which will mark payment as succeeded and failed by merchant.Request curl :
Response json :
Checklist
cargo +nightly fmt --all
cargo clippy