-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(core): [Payouts] Add payout_method_details to response #5887
Conversation
crates/api_models/src/payouts.rs
Outdated
Card(Box<CardPayoutResponse>), | ||
Bank(Box<BankPayoutResponse>), | ||
Wallet(Box<WalletPayoutResponse>), |
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.
These can directly be CardAdditionalData, BankAdditionalData and WalletAdditionalData
We had added details
and we were flattening it for payments for backwards compatibility only
} | ||
|
||
#[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, ToSchema)] | ||
pub struct CardAdditionalData { |
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.
Possible to re-use this struct from payments? Or is there anything different here for payins vs payouts?
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.
struct from payment have additional payment fields which are optional but I think it would be better to have our own struct to contain any specific data for card in payouts if required
additional_payout_method_data, | ||
}; | ||
|
||
payout_attempt = 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.
Is this added for update / confirm flows?
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.
Yes, this is added for update as well as confirm call
@@ -30,6 +30,7 @@ pub struct PayoutAttempt { | |||
pub profile_id: common_utils::id_type::ProfileId, | |||
pub merchant_connector_id: Option<common_utils::id_type::MerchantConnectorAccountId>, | |||
pub routing_info: Option<serde_json::Value>, | |||
pub additional_payout_method_data: 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.
Let's make it the optional type - AdditionalPayoutMethodData
? We will not need to manually encode or decode the values in our flows that way.
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.
small improvements can be made, other than that, it looks good to me 👍
"authentication_data": null | ||
} | ||
}"#))] | ||
pub payout_method_data: Option<PayoutMethodDataResponse>, |
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 re-use AdditionalPayoutMethodData
?
match pm_data { | ||
api::PayoutMethodData::Card(card_data) => { | ||
let card_isin = Some(card_data.card_number.get_card_isin()); | ||
let enable_extended_bin =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.
Is this file not formatted? 🤔
+ &"*".repeat(user_identifier.len() - unmasked_char_count); | ||
format!("{}@{}", masked_user_identifier, domain) | ||
} else { | ||
let masked_value = apply_mask(src.as_ref(), unmasked_char_count, 8); |
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.
If we're unable to split by @
, then I feel it's better to mask everything, to be on the safer side.
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.
It only unmask the last two and first two character only if the size is greater than 12, else it would only unmask the last two character.
0481e86
Type of Change
Description
This PR helps in showing masked payout method details in payout response for each payout method.
Additional Changes
add payout_method_data object in response
Eg.
Motivation and Context
How did you test it?
Tested through postman:
Case 1
Case 2
Checklist
cargo +nightly fmt --all
cargo clippy