-
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
feat(router): add proxy-confirm-intent api for payments in v2 flow #7215
base: main
Are you sure you want to change the base?
Changes from all commits
c088d28
9ddd2d7
76ccac4
f3338d8
c846fab
a0759eb
db0ebaf
d34f32f
c9ff63e
684b5e7
f3e12bc
42d67e2
e0eedec
1ae403f
81c3cb5
5aae46b
1345b0b
fe61e7d
40b7b36
db079f2
f53ecd5
0bf7d6b
155fc9c
1d20405
0eb0053
c3bba5f
f671019
dfbe3df
90f5cda
ff0e36e
bcb3aa0
34e675f
093d2af
fd2958a
5ca3d40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -225,11 +225,14 @@ impl TaxDetails { | |
/// Get the tax amount | ||
/// If default tax is present, return the default tax amount | ||
/// If default tax is not present, return the tax amount based on the payment method if it matches the provided payment method type | ||
pub fn get_tax_amount(&self, payment_method: PaymentMethodType) -> Option<MinorUnit> { | ||
pub fn get_tax_amount(&self, payment_method: Option<PaymentMethodType>) -> Option<MinorUnit> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change the field name to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is a subfield in |
||
self.payment_method_type | ||
.as_ref() | ||
.filter(|payment_method_type_tax| payment_method_type_tax.pmt == payment_method) | ||
.map(|payment_method_type_tax| payment_method_type_tax.order_tax_amount) | ||
.zip(payment_method) | ||
.filter(|(payment_method_type_tax, payment_method)| { | ||
payment_method_type_tax.pmt == *payment_method | ||
}) | ||
.map(|(payment_method_type_tax, _)| payment_method_type_tax.order_tax_amount) | ||
.or_else(|| self.get_default_tax_amount()) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
#[cfg(feature = "v2")] | ||
use common_enums::{enums::PaymentConnectorTransmission, PaymentMethod, PaymentMethodType}; | ||
use common_enums::enums::PaymentConnectorTransmission; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do not import these types, qualify them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is of someone else's PR, I have quantify them in my pr as suggested by you |
||
use common_utils::{hashing::HashedString, pii, types::MinorUnit}; | ||
use diesel::{ | ||
sql_types::{Json, Jsonb}, | ||
|
@@ -137,9 +137,9 @@ pub struct PaymentRevenueRecoveryMetadata { | |
/// Billing Connector Payment Details | ||
pub billing_connector_payment_details: BillingConnectorPaymentDetails, | ||
///Payment Method Type | ||
pub payment_method_type: PaymentMethod, | ||
pub payment_method_type: common_enums::enums::PaymentMethod, | ||
/// PaymentMethod Subtype | ||
pub payment_method_subtype: PaymentMethodType, | ||
pub payment_method_subtype: common_enums::enums::PaymentMethodType, | ||
} | ||
|
||
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -225,7 +225,7 @@ impl AmountDetails { | |
let order_tax_amount = match self.skip_external_tax_calculation { | ||
common_enums::TaxCalculationOverride::Skip => { | ||
self.tax_details.as_ref().and_then(|tax_details| { | ||
tax_details.get_tax_amount(confirm_intent_request.payment_method_subtype) | ||
tax_details.get_tax_amount(Some(confirm_intent_request.payment_method_subtype)) | ||
}) | ||
} | ||
common_enums::TaxCalculationOverride::Calculate => None, | ||
|
@@ -243,6 +243,42 @@ impl AmountDetails { | |
}) | ||
} | ||
|
||
pub fn proxy_create_attempt_amount_details( | ||
&self, | ||
_confirm_intent_request: &api_models::payments::ProxyPaymentsRequest, | ||
) -> payment_attempt::AttemptAmountDetails { | ||
let net_amount = self.calculate_net_amount(); | ||
|
||
let surcharge_amount = match self.skip_surcharge_calculation { | ||
common_enums::SurchargeCalculationOverride::Skip => self.surcharge_amount, | ||
common_enums::SurchargeCalculationOverride::Calculate => None, | ||
}; | ||
|
||
let tax_on_surcharge = match self.skip_surcharge_calculation { | ||
common_enums::SurchargeCalculationOverride::Skip => self.tax_on_surcharge, | ||
common_enums::SurchargeCalculationOverride::Calculate => None, | ||
}; | ||
|
||
let order_tax_amount = match self.skip_external_tax_calculation { | ||
common_enums::TaxCalculationOverride::Skip => self | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we will always get the default tax amount here, and not payment method type tax There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bro since im passing payment method type from |
||
.tax_details | ||
.as_ref() | ||
.and_then(|tax_details| tax_details.get_tax_amount(None)), | ||
common_enums::TaxCalculationOverride::Calculate => None, | ||
}; | ||
|
||
payment_attempt::AttemptAmountDetails::from(payment_attempt::AttemptAmountDetailsSetter { | ||
net_amount, | ||
amount_to_capture: None, | ||
surcharge_amount, | ||
tax_on_surcharge, | ||
// This will be updated when we receive response from the connector | ||
amount_capturable: MinorUnit::zero(), | ||
shipping_cost: self.shipping_cost, | ||
order_tax_amount, | ||
}) | ||
} | ||
|
||
pub fn update_from_request(self, req: &api_models::payments::AmountDetailsUpdate) -> Self { | ||
Self { | ||
order_amount: req | ||
|
@@ -585,7 +621,7 @@ where | |
|
||
// TODO: Check if this can be merged with existing payment data | ||
#[cfg(feature = "v2")] | ||
#[derive(Clone)] | ||
#[derive(Clone, Debug)] | ||
pub struct PaymentConfirmData<F> | ||
where | ||
F: Clone, | ||
|
@@ -595,6 +631,7 @@ where | |
pub payment_attempt: PaymentAttempt, | ||
pub payment_method_data: Option<payment_method_data::PaymentMethodData>, | ||
pub payment_address: payment_address::PaymentAddress, | ||
pub mandate_data: Option<api_models::payments::MandateIds>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should not be using types from the api models here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should i create this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes please, create a struct in domain models for this. |
||
} | ||
|
||
#[cfg(feature = "v2")] | ||
|
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 use payments response here instead of creating a new response?
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.
applied_authentication_type
this is a required field in confirmIntentResponse and is not relevant to proxyResponseThere 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 still, let's use the same response, if there are any changes make those fields optional