-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
refactor(core): Update shipping_cost and order_tax_amount to net_amount of payment_attempt #5844
Conversation
@@ -1278,6 +1278,22 @@ impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for Paymen | |||
None => (None, None, None), | |||
}; | |||
|
|||
let shipping_cost = payment_data.payment_intent.shipping_cost; | |||
|
|||
let pmt_order_tax_amount = payment_data |
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.
here you should check if the payment method type that is passed in the confirm request is the same as the one that was used when calculating the tax, only then consider this tax amount
pub mod setup_mandate_flow; | ||
pub mod tax_calculation_flow; |
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 you delete this file?
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.
this file does not exist, as I have renamed the file name from tax_calculation_flow.rs
to session_update_flow.rs
@@ -571,6 +571,8 @@ pub fn make_new_payment_attempt( | |||
customer_acceptance: Default::default(), | |||
profile_id: old_payment_attempt.profile_id, | |||
organization_id: old_payment_attempt.organization_id, | |||
shipping_cost: old_payment_attempt.shipping_cost, | |||
order_tax_amount: old_payment_attempt.order_tax_amount, |
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.
This should be set to None right? This will be dependent on the payment method that will be chosen. This would be updated later in the update trackers
Ok(services::ApplicationResponse::JsonWithHeaders(( | ||
Self { net_amount: amount }, | ||
Self { | ||
net_amount: amount, |
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.
do we need to show net amount here? @SamraatBansal @jarnura I think this will be confusing because this would reflect the net_amount
field that we already have in the payment attempt. But this is just a derived field sent in the response. Net amount can be calculated derived as well where this api is consumed. Let me know your thoughts
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 have decided to keep it as net_amount
.
cc- @Narayanbhat166 @jarnura
.payment_intent | ||
.skip_external_tax_calculation | ||
.unwrap_or(false); | ||
if is_tax_connector_enabled && !skip_external_tax_calculation { | ||
*should_continue_confirm_transaction = false; |
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.
why are we setting this to false?
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 also send order tax amount in the payments 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.
approving on behalf of connector
3c12cb4
@SamraatBansal I have a doubt, when calling tax jar in payments create, after calculating the tax amount, should we update the net amount in the response and send? |
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.
Dashboard changes are fine
* 'main' of github.com:juspay/hyperswitch: (51 commits) feat(connector): [DEUTSCHEBANK] Integrate SEPA Payments (#5826) feat(payments_v2): payment intent diesel and domain models changes v2 (#5783) feat(connector): [Fiuu] ADD Wasm Configs (#5874) chore(version): 2024.09.13.0 refactor(core): Update shipping_cost and order_tax_amount to net_amount of payment_attempt (#5844) refactor: return optional request body from build_request_v2 in ConnectorIntegrationV2 trait (#5865) feat(refunds): Refunds aggregate api (#5795) refactor: handle redirections for iframed content (#5591) refactor(payment_links): Update API contract for dynamic transaction details and upgrade UI (#5849) fix(router): add payment_method check in `get_mandate_type` (#5828) fix(connector): [ZSL] compare consr_paid_amt with the total amount for identifying partial payments (#5873) feat(connector): [Novalnet] add Payment flows for cards (#5726) chore(version): 2024.09.12.0 fix(router): return `collect_billing_details_from_wallet_connector` if `always_collect_billing_details_from_wallet_connector ` is false in merchant payment method list (#5854) feat(opensearch): add profile_id and organization_id to /search APIs (#5705) build(deps): bump `sqlx` to `0.8.2` (#5859) refactor: Remove unwanted commented lines (#5851) feat(payments): add support for profile aggregates (#5845) Feat(connector): [Fiuu] Add DuitNow/FPX PaymentMethod (#5841) chore: remove Connectors enum dependency from ConnectorIntegrationV2 trait (#5840) ...
Type of Change
Description
shipping_cost
andorder_tax_amount
tonet_amount
of payment_attempt/tax_calculation
responsetax_calculation_flow.rs
tosession_update_flow.rs
PaymentTaxCalculation
toTaxCalculation
as it is inside connector trait not in payments trait.auth_type
validation for taxJar connectorpayments_create
,payment_update
andtax_calculation
had a skip_tax_calculation as true or notorder_tax_amount
in payment response.Additional Changes
Motivation and Context
How did you test it?
Create a Payment Connector Create of Cybersource for Googlepay
4.Payment Connector Create for TaxJar
Response
confirm the payment
Response:
In response check for "
order_tax_amount
"Checklist
cargo +nightly fmt --all
cargo clippy