-
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?
Conversation
…in payment intent for v2 flow
@@ -5272,6 +5299,73 @@ pub struct PaymentsConfirmIntentResponse { | |||
pub error: Option<ErrorDetails>, | |||
} | |||
|
|||
#[cfg(feature = "v2")] | |||
#[derive(Debug, serde::Serialize, ToSchema)] | |||
pub struct ProxyPaymentsIntentResponse { |
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 proxyResponse
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.
Yes still, let's use the same response, if there are any changes make those fields optional
@@ -591,6 +627,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 comment
The reason will be displayed to describe this comment to others. Learn more.
we should not be using types from the api models here
cc: @Aprabhat19
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.
should i create this MandateIDs
in hyperswitch_domain_model because it is not present there
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.
Yes please, create a struct in domain models for this.
@@ -215,11 +215,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 comment
The reason will be displayed to describe this comment to others. Learn more.
change the field name to payment_method_type
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.
there is a subfield in PaymentMethodType
with same name of payment_method_type
and this subfield is also being used in this function
customer_acceptance: None, | ||
profile_id: payment_intent.profile_id.clone(), | ||
organization_id: payment_intent.organization_id.clone(), | ||
payment_method_type: storage_enums::PaymentMethod::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.
we cannot set this value as card, set this to Mandate
cc: @Aprabhat19
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.
bro for our use case we are not dealing anything other than cards for so, i will create another pr for generic use case because it requires a lot of changes in the codebase
payment_method_type: storage_enums::PaymentMethod::Card, | ||
payment_method_id: None, | ||
connector_payment_id: None, | ||
payment_method_subtype: storage_enums::PaymentMethodType::Credit, |
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.
same here, use mandate / connector_token variant
cc: @Aprabhat19
Type of Change
Description
Developed the proxy-confirm-intent API, specifically designed for Merchant-Initiated Transactions (MIT) payments. This is a generic and reusable API that facilitates MIT payment confirmations.
Its working is similar to confirm-intent api but in proxy flow we will be not having customer's details, rather we need mca_id and psp_token in order to confirm intent.
This API will help in PCR as it totally depends on recurring payments(MIT transaction) so, it needs proxy-confirm-intent.
Additional Changes
Motivation and Context
How did you test it?
cURL:-
edited create_intent cURL(added feature_metadata field):-
Response:-
Checklist
cargo +nightly fmt --all
cargo clippy