Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(connector): [Multisafepay] Add support for Ideal and Giropay #4398

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(connector): [Multisafepay] Add support for Ideal and Giropay #4398

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0824c5d
Add support for Ideal and Giropay bank redirect in Multisafepay conne…
TakshPanchal Apr 19, 2024
b729cdd
Updated default hands
TakshPanchal Apr 22, 2024
5cff0ad
Merge branch 'main' into ideal-geropay-with-multisafe
TakshPanchal Apr 22, 2024
c495c6a
Refactor payment method data types in Multisafepay connector
TakshPanchal Apr 22, 2024
d125f9a
Convert MultisafepayBankNames to enum
TakshPanchal Apr 26, 2024
7ad9ab8
Refactor MultisafepayBankNames enum in transformer.rs
TakshPanchal Apr 26, 2024
ee0b441
Update configs for giropay and ideal
TakshPanchal May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
82 changes: 80 additions & 2 deletions crates/router/src/connector/multisafepay/transformers.rs
View file
Open in desktop
@@ -1,3 +1,4 @@
use api_models::enums::BankNames;
use common_utils::pii::{Email, IpAddress};
use masking::ExposeInterface;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -63,6 +64,8 @@ pub enum Gateway {
Klarna,
Googlepay,
Paypal,
Ideal,
Giropay,
Comment on lines +68 to +69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add Currency and Country configs for both Ideal and Giropay in config/development.toml, config/deployments/sandbox.toml, config/deployments/integration_test.toml, config/deployments/production.toml for Multisafepay

}

#[serde_with::skip_serializing_none]
Expand Down Expand Up @@ -173,6 +176,7 @@ pub enum GatewayInfo {
Card(CardInfo),
Wallet(WalletInfo),
PayLater(PayLaterInfo),
BankRedirect(BankRedirectInfo),
}

#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
Expand All @@ -181,6 +185,46 @@ pub enum WalletInfo {
GooglePay(GpayInfo),
}

#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these all derives required? If not please remove the redundant ones from all the enums and structs. #[derive(Debug, Serialize, Clone)] should work for requests and #[derive(Debug, Deserialize, Clone)] should work for response.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For BankRedirectInfo and IdealInfo, Eq and PartialEq will be used for comparing different bank's Info that is why Eq is needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please point to the code where you are comparing different bank's info?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SamraatBansal on line no. 760.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

#[serde(untagged)]
pub enum BankRedirectInfo {
Ideal(IdealInfo),
}

#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct IdealInfo {
pub issuer_id: Option<String>,
}

pub struct MultisafepayBankNames<'a>(&'a str);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you convert this to an enum. You can use serde rename here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completed the change @srujanchikke


impl<'a> TryFrom<&BankNames> for MultisafepayBankNames<'a> {
type Error = error_stack::Report<errors::ConnectorError>;
fn try_from(bank: &BankNames) -> Result<Self, Self::Error> {
Ok(match bank {
BankNames::AbnAmro => Self("0031"),
BankNames::AsnBank => Self("0761"),
BankNames::Bunq => Self("4371"),
BankNames::Ing => Self("0721"),
BankNames::Knab => Self("0801"),
BankNames::N26 => Self("9926"),
BankNames::NationaleNederlanden => Self("9927"),
BankNames::Rabobank => Self("0021"),
BankNames::Regiobank => Self("0771"),
BankNames::Revolut => Self("1099"),
BankNames::SnsBank => Self("0751"),
BankNames::TriodosBank => Self("0511"),
BankNames::VanLanschot => Self("0161"),
BankNames::Yoursafe => Self("0806"),
BankNames::Handelsbanken => Self("1235"),
_ => Err(errors::ConnectorError::NotSupported {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use default here, rather mention all the enums. It will help us easily debug any issues when new enum is added.

message: String::from("BankRedirect"),
connector: "Multisafepay",
})?,
})
}
}

#[derive(Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
pub struct DeliveryObject {
first_name: Secret<String>,
Expand Down Expand Up @@ -311,6 +355,13 @@ impl TryFrom<&MultisafepayRouterData<&types::PaymentsAuthorizeRouterData>>
))?,
},
api::PaymentMethodData::PayLater(ref _paylater) => Type::Redirect,
api::PaymentMethodData::BankRedirect(ref bank_data) => match bank_data {
api::BankRedirectData::Giropay { .. } => Type::Redirect,
api::BankRedirectData::Ideal { .. } => Type::Direct,
_ => Err(errors::ConnectorError::NotImplemented(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, don't use defaults

utils::get_unimplemented_payment_method_error_message("multisafepay"),
))?,
},
_ => Type::Redirect,
};

Expand Down Expand Up @@ -355,9 +406,15 @@ impl TryFrom<&MultisafepayRouterData<&types::PaymentsAuthorizeRouterData>>
},
) => Some(Gateway::Klarna),
api::PaymentMethodData::MandatePayment => None,
api::PaymentMethodData::BankRedirect(ref bank_data) => Some(match bank_data {
api::BankRedirectData::Giropay { .. } => Gateway::Giropay,
api::BankRedirectData::Ideal { .. } => Gateway::Ideal,
_ => Err(errors::ConnectorError::NotImplemented(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

utils::get_unimplemented_payment_method_error_message("multisafepay"),
))?,
}),
api::PaymentMethodData::CardRedirect(_)
| api::PaymentMethodData::PayLater(_)
| api::PaymentMethodData::BankRedirect(_)
| api::PaymentMethodData::BankDebit(_)
| api::PaymentMethodData::BankTransfer(_)
| api::PaymentMethodData::Crypto(_)
Expand Down Expand Up @@ -503,8 +560,29 @@ impl TryFrom<&MultisafepayRouterData<&types::PaymentsAuthorizeRouterData>>
}))
}
api::PaymentMethodData::MandatePayment => None,
api::PaymentMethodData::BankRedirect(ref bank_redirect_data) => {
match bank_redirect_data {
api::BankRedirectData::Ideal {
billing_details: _,
bank_name,
country: _,
} => Some(GatewayInfo::BankRedirect(BankRedirectInfo::Ideal(
IdealInfo {
issuer_id: Some(
MultisafepayBankNames::try_from(&bank_name.ok_or(
errors::ConnectorError::MissingRequiredField {
field_name: "eps.bank_name",
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ideal.bank_name .

)?)?
.0
.to_string(),
),
},
))),
_ => None,
}
}
api::PaymentMethodData::CardRedirect(_)
| api::PaymentMethodData::BankRedirect(_)
| api::PaymentMethodData::BankDebit(_)
| api::PaymentMethodData::BankTransfer(_)
| api::PaymentMethodData::Crypto(_)
Expand Down
16 changes: 8 additions & 8 deletions crates/router/src/types/api/payments.rs
View file
Open in desktop
@@ -1,12 +1,12 @@
pub use api_models::payments::{
AcceptanceType, Address, AddressDetails, Amount, AuthenticationForStartResponse, Card,
CryptoData, CustomerAcceptance, HeaderPayload, MandateAmountData, MandateData,
MandateTransactionType, MandateType, MandateValidationFields, NextActionType, OnlineMandate,
PayLaterData, PaymentIdType, PaymentListConstraints, PaymentListFilterConstraints,
PaymentListFilters, PaymentListResponse, PaymentListResponseV2, PaymentMethodData,
PaymentMethodDataRequest, PaymentMethodDataResponse, PaymentOp, PaymentRetrieveBody,
PaymentRetrieveBodyWithCredentials, PaymentsApproveRequest, PaymentsCancelRequest,
PaymentsCaptureRequest, PaymentsExternalAuthenticationRequest,
AcceptanceType, Address, AddressDetails, Amount, AuthenticationForStartResponse,
BankRedirectData, Card, CryptoData, CustomerAcceptance, HeaderPayload, MandateAmountData,
MandateData, MandateTransactionType, MandateType, MandateValidationFields, NextActionType,
OnlineMandate, PayLaterData, PaymentIdType, PaymentListConstraints,
PaymentListFilterConstraints, PaymentListFilters, PaymentListResponse, PaymentListResponseV2,
PaymentMethodData, PaymentMethodDataRequest, PaymentMethodDataResponse, PaymentOp,
PaymentRetrieveBody, PaymentRetrieveBodyWithCredentials, PaymentsApproveRequest,
PaymentsCancelRequest, PaymentsCaptureRequest, PaymentsExternalAuthenticationRequest,
TakshPanchal marked this conversation as resolved.
Show resolved Hide resolved
PaymentsIncrementalAuthorizationRequest, PaymentsRedirectRequest, PaymentsRedirectionResponse,
PaymentsRejectRequest, PaymentsRequest, PaymentsResponse, PaymentsResponseForm,
PaymentsRetrieveRequest, PaymentsSessionRequest, PaymentsSessionResponse, PaymentsStartRequest,
Expand Down