Integrate split UPE payments with WooCommerce Blocks #4681
Conversation
harriswong
changed the base branch from
develop
to
poc/upe-instances-multiplied
|
Update
|
…omattic/woocommerce-payments into fix/4603-block-split-upe
|
@harriswong I noticed one more thing - currently, with blocks, we are rendering the UPE credit card (at the bottom in the first screenshot) instead of the legacy credit card that uses Stripe Elements (the top one):
Whereas during the classic checkout with split UPE turned on, we're rendering the legacy credit card. There is some background of why it looks like that on the classic checkout in the description of this PR. My first thought was that we should align them and make blocks checkout behave the same as classic checkout. Do you agree with this? If yes, I'll cover this within this issue as well. |
|
@timur27 Thanks for the note! Yes, I think making both behaviours the same would be good. This way, both will use the classic checkout for cards, and UPE elements for other payment methods. That said, the card classic checkout doesn't seem to work with Link, so whatever fix we decided upon will have to be applied to both in the future. Please check with @FangedParakeet though. |
|
@timur27, yeah, the Blocks element should use the legacy payment element for the card payment method, but the UPE element for all other payment methods. This should align with how the classic checkout functions in the split-UPE sanctum. The purpose for doing this is because the UPE is currently incompatible with WooPay, so this split-UPE approach will allow us to hurdle this obstacle by using the compatible legacy element for the WooPay card element alone, while using the UPE for all other payment methods. I am honestly not sure how Link will eventually play into this (sorry lol), but there's more discussion on this on #4720. |
timur27
force-pushed
the
fix/4603-block-split-upe
branch
from
4a4e312 to
e85f6cf
Compare
…omattic/woocommerce-payments into fix/4603-block-split-upe
|
Size Change: +571 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
timur27
force-pushed
the
fix/4603-block-split-upe
branch
from
d6af1e6 to
2b40054
Compare
timur27
force-pushed
the
fix/4603-block-split-upe
branch
from
54e5ed2 to
6a10a65
Compare
|
@FangedParakeet today, I found one inconsistency between classic UPE checkout vs. the blocks one, and with this message, I'd like to clarify the performance expectations and if there is a need for alignment between these two. How the classic split UPE checkout creates payment intentsAs we know, the classic split UPE checkout is implemented in a way that How the blocks split UPE checkout creates payment intentsWith woocommerce-blocks, things are implemented a bit differently, and I noticed a dissimilarity in the strategy of sending the requests to create a payment intent. We have the effect hook, which is responsible for creating a payment intent after rendering the component (unless At the same time, the registerPaymentMethod function from woocommerce-blocks creates a payment method config within blocks but does not render the payment method yet. The rendering occurs when users open a particular payment method. Effectively, a payment intent is created only for a specific form of payment when its block component is chosen from the list. What I'd like to clarifyWhile there should be a way for Blocks implementation to create payment intents on the page load, I wanted to confirm the expectations. Should each payment method with blocks open immediately like in the classic checkout, or the loading time due to the intent creation API call for each method is acceptable in Blocks? Alternatively, maybe at this point, the most important thing for us is to make the Blocks checkout work, and we will get back to the performance later. These are the questions I don't have an answer to yet. 😊 Thank you for clarifying all this! |
This is actually quite a dense philosophical answer and one I don't entirely have an answer for. You are correct in that the classic checkout creates payment intents for all payment method types immediately on page load. This is "seamless", such that the end user will not have to wait for a new element to load while changing payment methods at checkout; however, this isn't tremendously efficient. We spend background resources to make these requests and moreover we create lots of additional perhaps superfluous requests to Stripe. We create a new payment intent for every enabled payment method, irregardless of whether every payment method is even selected at checkout. An end user might never select a payment method and never see its payment element, but we will still trouble Stripe to create an intent and the front-end resources required to support this unseen element. These additional requests and unused payment intents might look a little suspicious from Stripe's perspective and is usually a symptom of some spammy sickness on a site's checkout. It is designed this way not by some enlightened decision making, but simply because I understandably chose the simplest path when writing the initial PoC. Personally, I think the latency to mount a new payment element (create intent, create element using intent, and mount it to DOM) is generally a relatively negligible, constant-time operation, whereas the negatives of avoiding this scenario by preloading all payment elements is rather noticeable (multiplying our requests to Stripe, multiplying unused payment intents, etc.). So I would lean towards refactoring our existing code to only prepare and mount UPE elements when they are required, as opposed to loading them all on page load.
That being said, I'd still hold this goal in higher regard, so for now I think following the shortest path is the most sensible option. It's worth remaining aware that once Stripe finally drops their new UPE implementation, this entire debate will be nullified, since we will no longer require a payment intent in order to present a UPE payment element to our customers. So I feel that perhaps expending too much effort on the most optimal solution in this case will be somewhat in vain, since this will not be a long-term concern for us. Anyways, I'd vote for now on finding the simplest solution for Blocks right now and since that also happens to involve delaying payment intent creation until the moment is unavoidable, I'm even happier with choosing this path forwards. @mdmoore, we've chatted about this a few times here and there: let me know if you have any contradictory thoughts on this subject--or even better, if you happen to agree with me! 😁 |
I agree completely. It might be worth sharing your findings in #4672 here. Hopefully we'll ultimately move to the same approach in classic checkout as well and keep the intent history tidy. If blocks checkout is giving us an assist on that, I'd say go with it! |
timur27
changed the title
…ditor with the main payment method UI
timur27
force-pushed
the
fix/4603-block-split-upe
branch
from
cdfd88a to
6304292
Compare
…omattic/woocommerce-payments into fix/4603-block-split-upe
There was a problem hiding this comment.
This issue is probably the largest one we have remaining and the one I've been keeping the most distance from, so awesome work, @timur27, for tackling it without breaking a sweat!
Generally everything here is working even better than expected! I was able to do pretty muche everything, except for completing a checkout with a saved UPE payment method. However, I think this may be related to #4530, so I'm happy to ignore this issue.
Here are a few oddities that I discovered.
Invisible notices
I'm not sure why, but every time I managed to coax an error out of the checkout, I found a blank notice at the top of the page.
Mysterious invisible error notice example
By using the Redux debug tools, I could find each missing error notice stored correctly in the state, but for some reason this is not being communicated to the front-end. Let me know if you're able to replicate this as well!
Payment gateway re-renders
I left another comment related to this issue, but essentially, whenever we select a new UPE payment gateway at checkout, the entire element re-renders and we remount a new payment element. I see that we discussed this earlier, but I didn't realise that the entire element still re-renders even if the select payment gateway has been selected before.
This is a little irritating, but not a goliath gaffe. However, I would maybe suggest that we might want to block the checkout--or at least the Place Order button--while the element is rendering, if possible. Several of the redirection UPE payment gateways do not require any additional payment information and when one of these gateways is selected, it isn't necessarily clear that anything is loading.
If you submit the form while the payment element is mounting, you are greeted with a rather strange error message. I had to sift through the Redux state to find it, due to the former issue, but I believe the error message states, "Invalid payment method. Please input a new card number" when the currently selected payment gateway is not the card gateway. I'm not entirely sure what's going on here, but if it's possible to block the "Place Order" button during this loading time, I think that would negate this unkindness.
Those are the only issues I could consistently replicate with these changes, so top work once more! Let me know your thoughts, as I wouldn't be opposed to opening new issues to address these challenges, if you'd like to move on from this current issue, as I do think you have accomplished the core heart of this task successfully.
@FangedParakeet, this is a valid observation, thank you for bringing this up! 🙌🏼 I spent some time trying to find the root cause since I could reproduce it on the non-UPE blocks checkout and on the Finally, I discovered that the After updating the plugin on my local WCPay environment, I confirmed that the issue is not appearing anymore. |
Nice one! Can confirm that upgrading the Blocks plugin has now resolved this issue for me. 💪 Ma b. |
There was a problem hiding this comment.
Thanks for responding to all of my feedback from the last time! Don't worry, I only have a little more this time. 😅
Basically, everything is working as expected! The regular Blocks checkout flow, excluding saved payment methods, is working happily. Moreover the loading time while switching between payment methods at checkout is much healthier now that we persist the payment intent for the entire duration of a checkout. Great work!
However, sadly I discovered that the regular shortcode UPE checkout flow is now unfortunately broken and returns the following error consistently.
paymentMethodsConfig[paymentMethodType] is undefined
However, I've left another comment that explains why I'm very confident this is happening (you need to fix the parameters used on this function call), which should hopefully be a very easy amend. Please ping me again once you've fixed that and I don't think I'll have any other reason not to approve this PR. Cheers!
|
@FangedParakeet today, I found one more thing to align - The reason for that was that the payment gateway is considered separate when using Blocks, and adding actions in the Somehow this worked with the classic UPE checkout, and I guess that hooks' callbacks from I now fixed this behavior to work everywhere, and the problem disappeared. I tend to think that the solution is not ideal, but it seems to be bulletproof within our area of interest. |
There was a problem hiding this comment.
Thank you so much for outlasting me and my endless feedback in this marathon PR. Code looks great and works like a dream.
I found one more thing to align - paymentIntents weren't removed from the session after an order gets processed successfully.
Fantastic find here as well! I agree that the solution looks a little ugly (sorry 😁 ), but I can't think of a better way of ensuring that these actions definitely fire when the UPE is enabled, but only fire once. In fact, in the long term, I think this is a much more robust solution that allows us to potentially disable the card payment method entirely, without affecting these hooks.
I ran through a few regular checkout flows, using both the Blocks and shortcode checkout, and everything appears to be working as expected! I hereby offer the entirety of my grattitude and blessings.
L. G. T. M. 
Fixes #4603
Changes proposed in this Pull Request
Block checkout registers UPE as a payment method (Introduced by #2429). What this PR attempts to do is to register multiple payment methods instead.
Difference in payment methods management in classic checkout vs Blocks 🧐
Classic checkout renders and manages individual payment methods via WC Core and
woocommerce_payment_gatewaysfilter.WooCommerce Blocks provide the
PaymentMethodRegistryfor the method to be registered. During the payment method registration, we rollout js scripts and styles, and this time the side responsible for the methods rendering is the front-end where blocks give us an assist in the form of registerPaymentMethod.Problem solution 👨🏽💻
Having in mind the above, I took the opportunity and used the
paymentMethodsConfigfield, which comes from the back-end and is exposed in JavaScript. This field represents all the enabled payment methods from WCPay settings. On the front end, I take all the enabled payment methods and register a payment method for each of them. For each payment method, its name is being provided to the component in order to be used later on in the effect hook to create a payment intent, right after opening the payment method on the UI.I also provide testingInstructions for each particular payment method to be displayed on the UI, if we are in the Test mode.
How the basic Card Element replaced the card Stripe payment element
The idea of replacing the card payment method with the legacy card is pretty straightforward - ignore the card payment method while attaching all the enabled UPE payment methods and attach the legacy card separately. The same idea is applied in the classic split UPE checkout. My first thought was to update the UPE implementation to somehow be able to inject a legacy card there. But then I thought - we already have the fully working separate component with the working legacy card, which is used when UPE aren't enabled.
Finally, what I did is I rolled out two components here - the legacy card and UPE payment elements without the card. This approach helped us to reduce the amount of global refactoring and allowed us to roll back the solution very quickly in case we will ever need to take a new path.
Testing instructions
wp-adminconsole and create a new page. Search for the checkout block and add it to the page.Payments->Settingsand disable some of the payment methods from the list. Then, go to the blocks checkout page and confirm, that the rendered methods list reflects only the enabled payment methods.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge