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

refactor(bank-debit): remove billingdetails from bankdebit pmd #4371

Merged
merged 16 commits into from May 8, 2024
Merged

refactor(bank-debit): remove billingdetails from bankdebit pmd #4371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
c746961
refactor: remove billingdetails from bankdebit pmd
swangi-kumari Apr 16, 2024
3378876
docs(openapi): re-generate OpenAPI specification
hyperswitch-bot[bot] Apr 16, 2024
f0c23d0
refactor: add postman test for ach-bank-debit for stripe connector
swangi-kumari Apr 17, 2024
e43ce2a
refactor: postman test for ach in stripe
swangi-kumari Apr 17, 2024
7c1e6d6
refactor: removed unused functions
swangi-kumari Apr 17, 2024
a0cc68a
refactor: fix
swangi-kumari Apr 24, 2024
1f3aa2c
docs(openapi): re-generate OpenAPI specification
hyperswitch-bot[bot] Apr 24, 2024
f1fc6a2
Merge branch 'main' into bank-debit-billing
swangi-kumari Apr 25, 2024
a7cd85e
refactor: use utils functions
swangi-kumari Apr 25, 2024
9d4239f
refactor: address
swangi-kumari Apr 26, 2024
42adde3
refactor: first name
swangi-kumari May 6, 2024
f4d4e1f
Merge remote-tracking branch 'origin' into bank-debit-billing
swangi-kumari May 6, 2024
ac4ba73
Merge branch 'main' into bank-debit-billing
swangi-kumari May 6, 2024
8491ca1
refactor: resolve conflicts
swangi-kumari May 6, 2024
dec55ed
refactor: resolve conflicts
swangi-kumari May 6, 2024
cc45fb6
refactor: resolve comments
swangi-kumari May 7, 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
42 changes: 23 additions & 19 deletions crates/api_models/src/payments.rs
View file
Open in desktop
Expand Up @@ -1143,7 +1143,7 @@ pub enum BankDebitData {
/// Payment Method data for Ach bank debit
AchBankDebit {
/// Billing details for bank debit
billing_details: BankDebitBilling,
billing_details: Option<BankDebitBilling>,
/// Account number for ach bank debit payment
#[schema(value_type = String, example = "000123456789")]
account_number: Secret<String>,
Expand All @@ -1168,7 +1168,7 @@ pub enum BankDebitData {
},
SepaBankDebit {
/// Billing details for bank debit
billing_details: BankDebitBilling,
billing_details: Option<BankDebitBilling>,
/// International bank account number (iban) for SEPA
#[schema(value_type = String, example = "DE89370400440532013000")]
iban: Secret<String>,
Expand All @@ -1178,7 +1178,7 @@ pub enum BankDebitData {
},
BecsBankDebit {
/// Billing details for bank debit
billing_details: BankDebitBilling,
billing_details: Option<BankDebitBilling>,
/// Account number for Becs payment method
#[schema(value_type = String, example = "000123456")]
account_number: Secret<String>,
Expand All @@ -1191,7 +1191,7 @@ pub enum BankDebitData {
},
BacsBankDebit {
/// Billing details for bank debit
billing_details: BankDebitBilling,
billing_details: Option<BankDebitBilling>,
/// Account number for Bacs payment method
#[schema(value_type = String, example = "00012345")]
account_number: Secret<String>,
Expand All @@ -1207,11 +1207,12 @@ pub enum BankDebitData {
impl GetAddressFromPaymentMethodData for BankDebitData {
fn get_billing_address(&self) -> Option<Address> {
fn get_billing_address_inner(
bank_debit_billing: &BankDebitBilling,
bank_debit_billing: Option<&BankDebitBilling>,
bank_account_holder_name: Option<&Secret<String>>,
) -> Option<Address> {
// We will always have address here
let mut address = bank_debit_billing.get_billing_address()?;
let mut address = bank_debit_billing
.and_then(GetAddressFromPaymentMethodData::get_billing_address)?;

// Prefer `account_holder_name` over `name`
address.address.as_mut().map(|address| {
Expand Down Expand Up @@ -1243,7 +1244,10 @@ impl GetAddressFromPaymentMethodData for BankDebitData {
billing_details,
bank_account_holder_name,
..
} => get_billing_address_inner(billing_details, bank_account_holder_name.as_ref()),
} => get_billing_address_inner(
billing_details.as_ref(),
bank_account_holder_name.as_ref(),
),
}
}
}
Expand Down Expand Up @@ -1446,7 +1450,7 @@ impl GetAddressFromPaymentMethodData for PaymentMethodData {
Self::Wallet(wallet_data) => wallet_data.get_billing_address(),
Self::PayLater(pay_later) => pay_later.get_billing_address(),
Self::BankRedirect(_) => None,
Self::BankDebit(_) => None,
Self::BankDebit(bank_debit_data) => bank_debit_data.get_billing_address(),
Self::BankTransfer(_) => None,
Self::Voucher(voucher_data) => voucher_data.get_billing_address(),
Self::Crypto(_)
Expand Down Expand Up @@ -2240,31 +2244,31 @@ impl GetAddressFromPaymentMethodData for BankTransferData {
#[derive(serde::Deserialize, serde::Serialize, Debug, Clone, ToSchema, Eq, PartialEq)]
pub struct BankDebitBilling {
/// The billing name for bank debits
#[schema(value_type = String, example = "John Doe")]
pub name: Secret<String>,
#[schema(value_type = Option<String>, example = "John Doe")]
pub name: Option<Secret<String>>,
/// The billing email for bank debits
#[schema(value_type = String, example = "example@example.com")]
pub email: Email,
#[schema(value_type = Option<String>, example = "example@example.com")]
pub email: Option<Email>,
/// The billing address for bank debits
pub address: Option<AddressDetails>,
}

impl GetAddressFromPaymentMethodData for BankDebitBilling {
fn get_billing_address(&self) -> Option<Address> {
let address = if let Some(mut address) = self.address.clone() {
address.first_name = Some(self.name.clone());
address.first_name = self.name.clone().or(address.first_name);
Address {
address: Some(address),
email: Some(self.email.clone()),
email: self.email.clone(),
phone: None,
}
} else {
Address {
address: Some(AddressDetails {
first_name: Some(self.name.clone()),
first_name: self.name.clone(),
..AddressDetails::default()
}),
email: Some(self.email.clone()),
email: self.email.clone(),
phone: None,
}
};
Expand Down Expand Up @@ -4893,14 +4897,14 @@ mod billing_from_payment_method_data {
let test_first_name = Secret::new(String::from("Chaser"));

let bank_redirect_billing = BankDebitBilling {
name: test_first_name.clone(),
name: Some(test_first_name.clone()),
address: None,
email: test_email.clone(),
email: Some(test_email.clone()),
};

let ach_bank_debit_payment_method_data =
PaymentMethodData::BankDebit(BankDebitData::AchBankDebit {
billing_details: bank_redirect_billing,
billing_details: Some(bank_redirect_billing),
account_number: Secret::new("1234".to_string()),
routing_number: Secret::new("1235".to_string()),
card_holder_name: None,
Expand Down
42 changes: 14 additions & 28 deletions crates/router/src/connector/adyen/transformers.rs
View file
Open in desktop
Expand Up @@ -1822,56 +1822,42 @@ fn build_shopper_reference(customer_id: &Option<String>, merchant_id: String) ->
.map(|c_id| format!("{}_{}", merchant_id, c_id))
}

impl<'a> TryFrom<&domain::BankDebitData> for AdyenPaymentMethod<'a> {
impl<'a> TryFrom<(&domain::BankDebitData, &types::PaymentsAuthorizeRouterData)>
for AdyenPaymentMethod<'a>
{
type Error = Error;
fn try_from(bank_debit_data: &domain::BankDebitData) -> Result<Self, Self::Error> {
fn try_from(
(bank_debit_data, item): (&domain::BankDebitData, &types::PaymentsAuthorizeRouterData),
) -> Result<Self, Self::Error> {
match bank_debit_data {
domain::BankDebitData::AchBankDebit {
account_number,
routing_number,
card_holder_name,
..
} => Ok(AdyenPaymentMethod::AchDirectDebit(Box::new(
AchDirectDebitData {
payment_type: PaymentType::AchDirectDebit,
bank_account_number: account_number.clone(),
bank_location_id: routing_number.clone(),
owner_name: card_holder_name.clone().ok_or(
errors::ConnectorError::MissingRequiredField {
field_name: "card_holder_name",
},
)?,
owner_name: item.get_billing_full_name()?,
},
))),
domain::BankDebitData::SepaBankDebit {
iban,
bank_account_holder_name,
..
} => Ok(AdyenPaymentMethod::SepaDirectDebit(Box::new(
SepaDirectDebitData {
owner_name: bank_account_holder_name.clone().ok_or(
errors::ConnectorError::MissingRequiredField {
field_name: "bank_account_holder_name",
},
)?,
domain::BankDebitData::SepaBankDebit { iban, .. } => Ok(
AdyenPaymentMethod::SepaDirectDebit(Box::new(SepaDirectDebitData {
owner_name: item.get_billing_full_name()?,
iban_number: iban.clone(),
},
))),
})),
),
domain::BankDebitData::BacsBankDebit {
account_number,
sort_code,
bank_account_holder_name,
..
} => Ok(AdyenPaymentMethod::BacsDirectDebit(Box::new(
BacsDirectDebitData {
payment_type: PaymentType::BacsDirectDebit,
bank_account_number: account_number.clone(),
bank_location_id: sort_code.clone(),
holder_name: bank_account_holder_name.clone().ok_or(
errors::ConnectorError::MissingRequiredField {
field_name: "bank_account_holder_name",
},
)?,
holder_name: item.get_billing_full_name()?,
},
))),
domain::BankDebitData::BecsBankDebit { .. } => {
Expand Down Expand Up @@ -2715,7 +2701,7 @@ impl<'a>
let browser_info = get_browser_info(item.router_data)?;
let additional_data = get_additional_data(item.router_data);
let return_url = item.router_data.request.get_return_url()?;
let payment_method = AdyenPaymentMethod::try_from(bank_debit_data)?;
let payment_method = AdyenPaymentMethod::try_from((bank_debit_data, item.router_data))?;
let country_code = get_country_code(item.router_data.get_optional_billing());
let request = AdyenPaymentRequest {
amount,
Expand Down
103 changes: 18 additions & 85 deletions crates/router/src/connector/gocardless/transformers.rs
View file
Open in desktop
Expand Up @@ -8,9 +8,8 @@ use serde::{Deserialize, Serialize};

use crate::{
connector::utils::{
self, AddressDetailsData, BankDirectDebitBillingData, BrowserInformationData,
ConnectorCustomerData, PaymentsAuthorizeRequestData, PaymentsSetupMandateRequestData,
RouterData,
self, AddressDetailsData, BrowserInformationData, ConnectorCustomerData,
PaymentsAuthorizeRequestData, PaymentsSetupMandateRequestData, RouterData,
},
core::errors,
types::{
Expand Down Expand Up @@ -79,64 +78,19 @@ impl TryFrom<&types::ConnectorCustomerRouterData> for GocardlessCustomerRequest
type Error = error_stack::Report<errors::ConnectorError>;
fn try_from(item: &types::ConnectorCustomerRouterData) -> Result<Self, Self::Error> {
let email = item.request.get_email()?;
let billing_details = match &item.request.payment_method_data {
domain::PaymentMethodData::BankDebit(bank_debit_data) => {
match bank_debit_data.clone() {
domain::BankDebitData::AchBankDebit {
billing_details, ..
} => Ok(billing_details),
domain::BankDebitData::SepaBankDebit {
billing_details, ..
} => Ok(billing_details),
domain::BankDebitData::BecsBankDebit {
billing_details, ..
} => Ok(billing_details),
domain::BankDebitData::BacsBankDebit { .. } => {
Err(errors::ConnectorError::NotImplemented(
utils::get_unimplemented_payment_method_error_message("Gocardless"),
))
}
}
}
domain::PaymentMethodData::Card(_)
| domain::PaymentMethodData::CardRedirect(_)
| domain::PaymentMethodData::Wallet(_)
| domain::PaymentMethodData::PayLater(_)
| domain::PaymentMethodData::BankRedirect(_)
| domain::PaymentMethodData::BankTransfer(_)
| domain::PaymentMethodData::Crypto(_)
| domain::PaymentMethodData::MandatePayment
| domain::PaymentMethodData::Reward
| domain::PaymentMethodData::Upi(_)
| domain::PaymentMethodData::Voucher(_)
| domain::PaymentMethodData::GiftCard(_)
| domain::PaymentMethodData::CardToken(_) => {
Err(errors::ConnectorError::NotImplemented(
utils::get_unimplemented_payment_method_error_message("Gocardless"),
))
}
}?;

let billing_details_name = billing_details.name.expose();
let billing_details_name = item.get_billing_full_name()?.expose();
Copy link
Member

Choose a reason for hiding this comment

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

This should be a secret only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are converting it back to secret while returning.


if billing_details_name.is_empty() {
Err(errors::ConnectorError::MissingRequiredField {
field_name: "billing_details.name",
})?
}
let (given_name, family_name) = billing_details_name
.trim()
.rsplit_once(' ')
.unwrap_or((&billing_details_name, &billing_details_name));

let billing_address = billing_details
.address
.ok_or_else(utils::missing_field_err("billing_details.address"))?;
let billing_address = item.get_billing_address()?;

let metadata = CustomerMetaData {
crm_id: item.customer_id.clone().map(Secret::new),
};
let region = get_region(&billing_address)?;
let region = get_region(billing_address)?;
Ok(Self {
customers: GocardlessCustomer {
email,
Expand All @@ -153,7 +107,7 @@ impl TryFrom<&types::ConnectorCustomerRouterData> for GocardlessCustomerRequest
postal_code: billing_address.zip.to_owned(),
// Should be populated based on the billing country
swedish_identity_number: None,
city: billing_address.city.map(Secret::new),
city: billing_address.city.clone().map(Secret::new),
},
})
}
Expand Down Expand Up @@ -286,7 +240,7 @@ impl TryFrom<&types::TokenizationRouterData> for CustomerBankAccount {
fn try_from(item: &types::TokenizationRouterData) -> Result<Self, Self::Error> {
match &item.request.payment_method_data {
domain::PaymentMethodData::BankDebit(bank_debit_data) => {
Self::try_from(bank_debit_data)
Self::try_from((bank_debit_data, item))
}
domain::PaymentMethodData::Card(_)
| domain::PaymentMethodData::CardRedirect(_)
Expand All @@ -310,26 +264,21 @@ impl TryFrom<&types::TokenizationRouterData> for CustomerBankAccount {
}
}

impl TryFrom<&domain::BankDebitData> for CustomerBankAccount {
impl TryFrom<(&domain::BankDebitData, &types::TokenizationRouterData)> for CustomerBankAccount {
type Error = error_stack::Report<errors::ConnectorError>;
fn try_from(item: &domain::BankDebitData) -> Result<Self, Self::Error> {
match item {
fn try_from(
(bank_debit_data, item): (&domain::BankDebitData, &types::TokenizationRouterData),
) -> Result<Self, Self::Error> {
match bank_debit_data {
domain::BankDebitData::AchBankDebit {
billing_details,
account_number,
routing_number,
bank_type,
bank_account_holder_name,
..
} => {
let bank_type = bank_type.ok_or_else(utils::missing_field_err("bank_type"))?;
let country_code = billing_details.get_billing_country()?;
let account_holder_name =
bank_account_holder_name
.clone()
.ok_or_else(utils::missing_field_err(
"payment_method_data.bank_debit.ach_bank_debit.bank_account_holder_name",
))?;
let country_code = item.get_billing_country()?;
let account_holder_name = item.get_billing_full_name()?;
let us_bank_account = USBankAccount {
country_code,
account_number: account_number.clone(),
Expand All @@ -340,18 +289,11 @@ impl TryFrom<&domain::BankDebitData> for CustomerBankAccount {
Ok(Self::USBankAccount(us_bank_account))
}
domain::BankDebitData::BecsBankDebit {
billing_details,
account_number,
bsb_number,
bank_account_holder_name,
} => {
let country_code = billing_details.get_billing_country()?;
let account_holder_name =
bank_account_holder_name
.clone()
.ok_or_else(utils::missing_field_err(
"payment_method_data.bank_debit.becs_bank_debit.bank_account_holder_name",
))?;
let country_code = item.get_billing_country()?;
let account_holder_name = item.get_billing_full_name()?;
let au_bank_account = AUBankAccount {
country_code,
account_number: account_number.clone(),
Expand All @@ -360,17 +302,8 @@ impl TryFrom<&domain::BankDebitData> for CustomerBankAccount {
};
Ok(Self::AUBankAccount(au_bank_account))
}
domain::BankDebitData::SepaBankDebit {
iban,
bank_account_holder_name,
..
} => {
let account_holder_name =
bank_account_holder_name
.clone()
.ok_or_else(utils::missing_field_err(
"payment_method_data.bank_debit.sepa_bank_debit.bank_account_holder_name",
))?;
domain::BankDebitData::SepaBankDebit { iban, .. } => {
let account_holder_name = item.get_billing_full_name()?;
let international_bank_account = InternationalBankAccount {
iban: iban.clone(),
account_holder_name,
Expand Down