This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Refactor Payment Methods Integration API to fire onPaymentProcessing event with saved tokens.
#3982
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
|
Size Change: -9 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
nerrad
force-pushed
the
add/expose-onPaymentProcessing-event-for-saved-payment-methods
branch
from
083d883 to
9c8875a
Compare
Also for TS typing I changed `paymentMethodData` to be optional for both the `success` and `started` action creators. This is because the behaviour allows for paymentMethodData to be retained in the state if it is not explicitly provided on dispatch.
…cher. The implementation now allows for receiving payment method data when the `start` status is dispatched.
It is intended that if paymentMethodData is undefined, that is simply passed through to the dispatched action. This signals the reducer to retain the existing paymentMethodData in state (when undefined). The correct way to clear the paymentMethodData is to either explictly provide an empty object, or set the status to pristine.
This changeset also configures the reducer to retain the existing paymentMethodData in state (and related correlated information0 when the provided paymentMethodData property is undefined. The only time paymentMethodData should be reset in state is when it is explicitly provided or the status is set to PRISTINE.
Also restores previous paymentMethodData when express payment cancelled.
…atched action instead of success. This change ensures that savedToken handlers registered by payment methods have access to the `onPaymentProcessing` checkout event.
Really just need to ensure types are used anywhere, this is a temporary change due to the time sensitive needs for this PR.
nerrad
force-pushed
the
add/expose-onPaymentProcessing-event-for-saved-payment-methods
branch
from
9c8875a to
c3d5163
Compare
nerrad
added
category: extensibility
nerrad
commented
Mar 20, 2021
Comment on lines
-117
to
+119
| if ( token === '0' ) { | ||
| setPaymentStatus().started(); | ||
| } | ||
| setActiveSavedToken( token ); | ||
| }, | ||
| [ setActiveSavedToken, setPaymentStatus ] | ||
| [ setActiveSavedToken ] |
There was a problem hiding this comment.
This logic hasn't been needed ever since we switched away from tabs and radios to only radios for payment method selection. There is no longer a possibility for a token with a value of '0' in the checkout flow.
Aljullu
approved these changes
Mar 21, 2021
assets/js/base/context/cart-checkout/payment-methods/reducer.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Albert Juhé Lluveras <[email protected]>
nerrad
deleted the
add/expose-onPaymentProcessing-event-for-saved-payment-methods
branch
nerrad
added
the
needs: dev note
ralucaStan
added
the
type: enhancement
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes: #3980
Currently there exists what is arguably a flaw in the checkout event system in that
onPaymentProcessingevent can be observed by express payment methods and regular payment methods but not by saved payment methods (implementing thesavedTokenComponentconfiguration property). This inconsistency is a flaw in that some payment methods implementing this component will have no way to reliably update thepaymentMethodDatain the checkout state before it is passed along on the checkout endpoint request for server side processing. You can read more about where this can manifest in #3980.A significant reason for why this inconsistency existed is because when the system was initially designed, an assumption was made that saved tokens would always be processed server side and payment method logic would never need to interact on the client. This was a faulty assumption which I started rectifying in #3961 and now finished with this PR here.
As a result, this PR implements the following changes to ensure the
onPaymentProcessingevent is accessible to thesavedTokenComponent:startedaction creator that returns an action object that when dispatched will allow for updating the payment method data in the checkout state.startedstatus for the payment method and store data about the saved token in the checkout state. This replaces the existing logic that was setting the payment method status for saved tokens to "success".startedandsuccesspayment method status changes allow for retaining existing payment method data in the state if it's not provided by the action. This improves performance because there are many places where just the payment status needs updated by checkout without changing anything else in the state.successtostartedin order to reflect the changes made for saved token handling.Outside of the necessary TypeScript type changes for existing typescript files, I didn't implement typescript anywhere else mostly because there's some time sensitivity around the need for this API change.
Regarding documentation, I did do a pass through the existing documentation and I don't see anywhere that needs updated. Indeed, the existing documentation already pointed to the
onPaymentProcessingevent being available forsavedTokenComponentso the changes here are actually more in line with what the documentation already indicates!To Test
The changes here implement any payment method type processing, so it's important to verify there is no impact to existing payment method handling by these changes. This means setting up an environment where you have the core payment methods and Stripe (along with Stripe express payments, either Chrome Pay or Apple Pay) available for testing.
Besides the above smoke tests, try intentionally confusing or breaking the system. Some example scenarios:
4000002760003184), then the 3DS validation process still works when attempting to make a payment using the token.The above all validates that the changes in this PR don't break existing behaviour. There's no easy way to test (currently) the actual implementation exposing
onPaymentProcessingevent for savedTokenComponents without manually implementing a dummy test. I did this to verify it works as expected. However, this PR will also be tested as a part of the work being done for the Square extension.Changelog