-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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(router): add support for co-badged cards #5801
base: main
Are you sure you want to change the base?
Conversation
Review changes with SemanticDiff. Analyzed 9 of 9 files. Overall, the semantic diff is 9% smaller than the GitHub diff.
|
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 you please add detailed description of the approach in PR description?
@@ -1280,6 +1280,16 @@ impl GetAddressFromPaymentMethodData for Card { | |||
|
|||
impl Card { | |||
fn apply_additional_card_info(&self, additional_card_info: AdditionalCardInfo) -> Self { | |||
let req_card_brand = self |
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 you please use the same naming convention as card_network here?
.clone() | ||
.or(additional_card_info.card_network); | ||
|
||
let card_network = match req_card_brand.clone().map(|_| self.is_cobadged_card()) { |
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 is map required here?
@@ -1303,6 +1310,28 @@ impl Card { | |||
nick_name: self.nick_name.clone(), | |||
} | |||
} | |||
fn is_cobadged_card(&self) -> bool { | |||
let c_card_number_value = self.card_number.get_card_no(); | |||
let card_number_length = i32::try_from(c_card_number_value.len()).ok(); |
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 avoid these conversions right?
Lazy::new(|| { | ||
let mut map = HashMap::new(); | ||
map.insert(common_enums::CardNetwork::Maestro, CardNetworkPattern { | ||
regex: Regex::new(r"^(5018|5081|5044|504681|504993|5020|502260|5038|603845|603123|6304|6759|676[1-3]|6220|504834|504817|504645|504775|600206|627741)[0-9]{0,15}$").ok(), |
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.
isn't it unsafe to ignore these errors?
card_network_data.allowed_cvc_length.contains(&len) | ||
}) | ||
{ | ||
matching_networks += 1; |
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 break the loop once matching_networks > 1 and return true
@@ -136,3 +140,69 @@ pub const MAX_ALLOWED_AMOUNT: i64 = 999999999; | |||
//payment attempt default unified error code and unified error message | |||
pub const DEFAULT_UNIFIED_ERROR_CODE: &str = "UE_000"; | |||
pub const DEFAULT_UNIFIED_ERROR_MESSAGE: &str = "Something went wrong"; | |||
|
|||
/// Regex for Identifying Card Network | |||
pub const CARD_NETWORK_DATA: Lazy<HashMap<common_enums::CardNetwork, CardNetworkPattern>> = |
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 reuse the CARD_NETWORK_DATA from common_utils consts
@@ -5265,3 +5274,26 @@ pub async fn validate_merchant_connector_ids_in_connector_mandate_details( | |||
} | |||
Ok(()) | |||
} | |||
|
|||
pub fn is_cobadged_card(card_number: CardNumber, card_cvc: masking::Secret<String>) -> 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.
can we please move this to impl of Card domain model
Type of Change
Description
Pass the card network parameter only when it is a cobadged cards.
Extend support for Stripe, Cybersource, Bank of America and Adyen
How did you test it?
Response
List customers
Response
Checklist
cargo +nightly fmt --all
cargo clippy