Fix WooPay Multiple Redirects #7215
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +314 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
bborman22
requested review from
a team and
ricardo
and removed request for
a team
|
I find we can still trigger multiple requests to @bborman22 maybe we can tackle this in a separate issue by introducing some global flag which prevents multiple modals from stacking on top of each other? multiple-requests-screen-recording.mov |
There was a problem hiding this comment.
Code LGTM and tests well. Nice fix! 💯
I'm able to replicate the behavior of multiple requests to wcpay_init_woopay in develop, and not in this branch. This behavior only happens to me when I click the WooPay button from the cart page, not the checkout page.
However, as I mentioned in my previous comment, if I click the WooPay button from the checkout page while the automatic redirect is already in progress, I still face the same issue with multiple requests and multiple draft orders.
I guess we can tackle this in a separate PR, totally up to you. 👍
|
Thanks for taking a look at this. After our discussion I did some experimenting and actually was able to reproduce the issue on product and checkout pages with just the button (ignoring the email input flow for that test). I also checked this against cart and checkout blocks as well. Also from our discussion I explored moving this check into the To truly solve the email input + button click issue we would have to resort to some sort of global flag on the page. I would personally consider the failing scenario an edge case (shopper loads checkout, email lookup triggers auto redirect, and shopper clicks on WooPay) and would be hesitant to add a global for just that case. Overall, I think this area needs a bit of a refactor to get more of the code shared and that would be the proper way to solve this edge case in my opinion. @ricardo if you wouldn't mind taking another look at this PR and let me know what you think of this |
There was a problem hiding this comment.
I would personally consider the failing scenario an edge case (shopper loads checkout, email lookup triggers auto redirect, and shopper clicks on WooPay) and would be hesitant to add a global for just that case.
I agree. It's probably better to explore improving the overall integration of express checkout button + email input than introducing a global flag right now.
I explored moving this check into the api object.
This approach looks much cleaner 👏.
The latest changes tested well for me. Code LGTM. ✅
Changes proposed in this Pull Request
This is one piece of the fix for https://github.com/Automattic/woopay/issues/2125. This fix prevents cases where WooPay sending multiple redirect messages will trigger too many WooPay session init calls and create too many draft orders causing issues with WooPay checkout.
This change was needed because there was a scenario where 3rd party cookies were not blocked and the shopper was already logged into WooPay where the
redirect_to_woopaypost message was getting posted to the merchant store multiple times. This triggered multiple session init calls which created an issue with draft orders being created on the merchant store and at times preventing WooPay checkout.This PR addresses that issue by checking if we are already in the process of redirecting the shopper and prevents any further redirects. It does this check by using a flag within the WooPay front end specific code.
Below are the network logs from dev tools after clicking the WooPay button while already logged into WooPay.
Before
After
Testing instructions
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