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
Changes from 23 commits
e960e0c
785ee7f
1cb716c
e790e6e
f99e030
164fbb8
069baca
b8c5821
c975c55
08da3ea
3fa28af
3190345
1ce254c
5405235
5f9633c
296cee1
f12d080
e72b38b
c6f46ce
3203ffc
e9fd863
cd68352
4b290c6
ff304d6
5605ff3
8aebbbd
91320db
8cc5265
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
use common_enums as storage_enums; | ||
use diesel::{AsChangeset, Identifiable, Insertable, Queryable}; | ||
use masking::{Deserialize, Serialize}; | ||
use time::PrimitiveDateTime; | ||
|
@@ -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 commentThe 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 commentThe 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 |
||
} | ||
|
||
#[derive(router_derive::Setter, Clone, Debug, Insertable, router_derive::DebugAsDisplay)] | ||
|
@@ -46,6 +48,7 @@ pub struct FraudCheckNew { | |
pub metadata: Option<serde_json::Value>, | ||
pub modified_at: PrimitiveDateTime, | ||
pub last_step: FraudCheckLastStep, | ||
pub frm_capture_method: Option<storage_enums::CaptureMethod>, | ||
} | ||
|
||
#[derive(Debug, Clone, Serialize, Deserialize)] | ||
|
@@ -59,6 +62,7 @@ pub enum FraudCheckUpdate { | |
metadata: Option<serde_json::Value>, | ||
modified_at: PrimitiveDateTime, | ||
last_step: FraudCheckLastStep, | ||
frm_capture_method: Option<storage_enums::CaptureMethod>, | ||
}, | ||
ErrorUpdate { | ||
status: FraudCheckStatus, | ||
|
@@ -76,6 +80,7 @@ pub struct FraudCheckUpdateInternal { | |
frm_error: Option<Option<String>>, | ||
metadata: Option<serde_json::Value>, | ||
last_step: FraudCheckLastStep, | ||
frm_capture_method: Option<storage_enums::CaptureMethod>, | ||
} | ||
|
||
impl From<FraudCheckUpdate> for FraudCheckUpdateInternal { | ||
|
@@ -89,13 +94,15 @@ impl From<FraudCheckUpdate> for FraudCheckUpdateInternal { | |
metadata, | ||
modified_at: _, | ||
last_step, | ||
frm_capture_method, | ||
} => Self { | ||
frm_status: Some(frm_status), | ||
frm_transaction_id, | ||
frm_reason, | ||
frm_score, | ||
metadata, | ||
last_step, | ||
frm_capture_method, | ||
..Default::default() | ||
}, | ||
FraudCheckUpdate::ErrorUpdate { | ||
|
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 fieldsThere 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: @AnandKGanesh @bernard-eugine
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.cc: @kashif-m @AnandKGanesh
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: @srujanchikkeHaving 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. IfRequiresMerchantAction
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