-
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
refactor(router): store network_transaction_id
for off_session
payments irrespective of the is_connector_agnostic_mit_enabled
config
#7083
Open
ShankarSinghC
wants to merge
10
commits into
main
Choose a base branch
from
ntid/store-id-everytime
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ShankarSinghC
added
A-core
Area: Core flows
A-payment-methods
Area: Payment Methods
C-refactor
Category: Refactor
A-payments
Area: payments
labels
Jan 22, 2025
Changed Files
|
…d/store-id-everytime
let network_transaction_id = | ||
let network_transaction_id = if payment_data.payment_intent.setup_future_usage | ||
== Some(diesel_models::enums::FutureUsage::OffSession) | ||
{ | ||
if let Some(network_transaction_id) = pm_resp_network_transaction_id { |
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.
what is the purpose of this if let? Just for logger this can be achieved via an if statement as well
Narayanbhat166
approved these changes
Jan 22, 2025
Aprabhat19
approved these changes
Jan 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-core
Area: Core flows
A-payment-methods
Area: Payment Methods
A-payments
Area: payments
C-refactor
Category: Refactor
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of Change
Description
Currently, we store the
network_transaction_id
for the setup_mandate/off_session payments only if theis_connector_agnostic_mit_enabled
config is enabled. During the MITs, we refer to this flag to decide whether to use theconnector_mandate_id
or thenetwork_transaction_id
for the MIT.Instead of using the flag for multiple purposes, it should be used solely to determine whether to use the
connector_mandate_id
or thenetwork_transaction_id
for the MIT. Therefore, this change will ensure that thenetwork_transaction_id
is always stored for off-session payments if it is present in the connector response.Additional Changes
Motivation and Context
Instead of using the
is_connector_agnostic_mit_enabled
flag for multiple purposes, it should be used solely to determine whether to use theconnector_mandate_id
or thenetwork_transaction_id
for the MIT. And is the connector is returning thenetwork_transaction_id
for theoff_session
payment it should always be stored.How did you test it?
-> Create a business profile and
is_connector_agnostic_mit_enabled
disabled.Initial CIT
-> Make an off_session payment. Even though the
is_connector_agnostic_mit_enabled
is falsenetwork_transaction_id
should be stored in the payment methods table.-> Db screenshot shown
network_transaction_id
being stored in the payment methods table.Recurring MIT
-> Create a payment for the same customer with confirm false
-> List payment methods using the above client_secret
-> Confirm the payment with the above listed token. Even though
network_transaction_id
is present, as theis_connector_agnostic_mit_enabled
is false the MIT should be processed by usingconnector_mandate_id
.-> Logs showing
connector_mandate_id
is used for the MITRecurring MIT by passing different connector in the routing.
-> Create a
network_transaction_id
support connector (stripe, adyen or cybersource)-> Create a payment for the same customer
-> List payment methods using the above client_secret
-> Confirm the payment by pass the newly configured connector (the connector that was not used in the initially CIT). Even though cybersource was sent in the routing input, the payment was processed with stripe as the
is_connector_agnostic_mit_enabled
is false andconnector_mandate_id
that is present is of stripe.-> Log showing routing order is
[Cybersource, Stripe]
andconnector_mandate_id
was used to process the MITChecklist
cargo +nightly fmt --all
cargo clippy