-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
refactor(graph): refactor the Knowledge Graph to include configs check, while eligibility analysis #4687
Conversation
…ac-eligibity-analysis
…hyperswitch into refac-eligibity-analysis
crates/kgraph_utils/src/mca.rs
Outdated
.filter(|p| enabled_pmt.clone().into_iter().any(|d| &d != p)) | ||
.collect::<Vec<_>>(); | ||
global_vector | ||
.clone() |
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.
redundant .clone()
cgraph::Relation::Positive, | ||
cgraph::Strength::Normal, | ||
)); | ||
if let Some(country) = config.country.clone() { |
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.
I feel like the .clone()
can be avoided.
)) | ||
} | ||
|
||
if let Some(currency) = config.currency.clone() { |
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.
The .clone()
here probably can be avoided too.
crates/kgraph_utils/src/mca.rs
Outdated
}) | ||
.transpose()?; | ||
|
||
if let Some(_country_currency) = curr { |
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.
I'm kinda surprised clippy didn't suggest this.
if let Some(_country_currency) = curr { | |
if curr.is_some() { |
crates/kgraph_utils/src/mca.rs
Outdated
// PaymentMethodsEnabled { | ||
// payment_method: api_enums::PaymentMethod::Wallet, | ||
// payment_method_types: Some(vec![RequestPaymentMethodTypes { | ||
// payment_method_type: api_enums::PaymentMethodType::GooglePay, | ||
// payment_experience: None, | ||
// card_networks: None, | ||
// accepted_currencies: Some(AcceptedCurrencies::EnableOnly(vec![ | ||
// api_enums::Currency::USD, | ||
// api_enums::Currency::INR, | ||
// ])), | ||
// accepted_countries: None, | ||
// minimum_amount: Some(10), |
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.
remove commented code
impl IntoDirValue for api_enums::PaymentMethodType { | ||
fn into_dir_value(self) -> Result<dir::DirValue, KgraphError> { | ||
match self { | ||
Self::Credit => Ok(dirval!(CardType = Credit)), | ||
Self::Debit => Ok(dirval!(CardType = Debit)), | ||
Self::Giropay => Ok(dirval!(BankRedirectType = Giropay)), | ||
Self::Ideal => Ok(dirval!(BankRedirectType = Ideal)), | ||
Self::Sofort => Ok(dirval!(BankRedirectType = Sofort)), | ||
Self::Eps => Ok(dirval!(BankRedirectType = Eps)), | ||
Self::Klarna => Ok(dirval!(PayLaterType = Klarna)), | ||
Self::Affirm => Ok(dirval!(PayLaterType = Affirm)), | ||
Self::AfterpayClearpay => Ok(dirval!(PayLaterType = AfterpayClearpay)), | ||
Self::GooglePay => Ok(dirval!(WalletType = GooglePay)), | ||
Self::ApplePay => Ok(dirval!(WalletType = ApplePay)), | ||
Self::Paypal => Ok(dirval!(WalletType = Paypal)), | ||
Self::CryptoCurrency => Ok(dirval!(CryptoType = CryptoCurrency)), |
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.
Don't have this as an IntoDirValue
impl. Instead have it as a private function in kgraph_utils::mca
, since we don't want to encourage its usage.
crates/kgraph_utils/src/utils.rs
Outdated
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.
I would argue naming the file types.rs
is more appropriate. But utils.rs
works too.
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.
have empty lines between function definitions
@@ -28,7 +28,7 @@ impl From<DomainId> for DomainIdOrIdentifier<'_> { | |||
Self::DomainId(value) | |||
} | |||
} | |||
|
|||
#[derive(Debug)] |
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 possible we can revert the added Derive implemetations.
Type of Change
Description
Refactor the Knowledge Graph to include configs(pm_filters) check, while eligibility analysis
Additional Changes
Motivation and Context
How did you test it?
Wrote unit test
To test it,
You can create MCA for 2 connectors
Do a payment using BOA and with some currency that it doesn't support , for eg:INR
Checklist
cargo +nightly fmt --all
cargo clippy