Fix WooPay pre-checking place order bug when buying a subscription #9450
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
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: +28 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
|
@alefesouza I've tested you branch with the tool above to generate a JN instance using your branch and seems to be working fine. However, when I tested it locally by pulling your branch, it didn’t behave as expected. In the attached video, you can see that the first time I click the button, the form doesn’t submit. There’s no validation error or anything else that could prevent submission. The form only submits on the second click. Are you experiencing similar behavior, or could this be something specific to my setup? As I mentioned, it works fine on JN, but the issue occurs when testing locally. Screen.Recording.2024-09-17.at.1.58.33.PM.mov |
|
@alefesouza If ‘Pre-check’ is enabled and I fill in the input with my phone, it seems to work as expected. |
|
Disabling WooPay also fixes "the issue". It works as expected after it. |
|
@asumaran I could not reproduce this issue locally using the following steps:
Are the steps correct? Also I don't see the |
|
@alefesouza I've figured out why it fails for me. Normally, when I run my tests, fill in all the information and at the end, I uncheck the "Save my info" option to avoid creating an unnecessary WooPay account. But immediately after, I click "Sign up now" without waiting for the last batch request to finish. That’s why it works the second time, once the last request that updates the cart has completed. I've tested with other fields and the behavior is the same. For example, when I modify the address and quickly click the ‘Sign up now’ button, the form doesn’t submit, but it works fine after the request has finished. So, I think this is the expected behavior and I believe the "Save my info" section isn't introducing anything different." I'll do a final round of tests and will finish my review. |
|
Maybe the question is. Should the "Save my info" trigger a batch request when unchecking its checkbox? It doesn't seem to be saving anything. |
I see the request is sent here, it looks like we need it to save the phone information in session, this way we don't lose it when reloading or moving to another page, by sending an empty request when unchecking we are removing this information from session. |
|
@alefesouza I see, that makes sense! I didn't test filling in the phone number. |
asumaran
approved these changes
Sep 19, 2024
There was a problem hiding this comment.
I've tested with the following combinations:
- Checkout Block.
- Checkout Shortcode.
- WooPay enabled.
- WooPay disabled.
- Precheck disabled.
- Regular product
- Express Checkout Disabled.
- Checkout page
- With regular theme.
- With block theme.
- Check "Save my info"
- Creates woopay account.
- With PayPal enabled.
- Doesn’t support subscriptions.
- WooPay auto redirect works normally.
and didn't found an issue.
Nice fix @alefesouza!
alefesouza
deleted the
fix/woopay-pre-checking-save-my-info-place-order
branch
frosso
pushed a commit
that referenced
this pull request
Sep 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #9449
Changes proposed in this Pull Request
Save my infogetting hidden.When buying a subscription via blocks checkout while WooPay pre-checking is enabled.
This bug was introduced on #9378 due to the late loading of the payment methods section, this way this line would initially return
undefined, undefinedas it checks the existence of the section radio buttons before it's loaded.It looks like WC Blocks wait the
registerExpressPaymentMethodhere get fully loaded (since the mentioned PR it uses a Promise) to the payment methods section on the checkout page.To fix it, I simply used the hooks used by WC Blocks itself to detect the current payment method, checking if the current one is
woocommerce_paymentsand the load theSave my infosection, for the classic checkout, it works the same way it worked before.The
Place orderbutton bug was caused by the WooPay component (Save my infosection) phone number validation, it was loaded even though it was hidden.Testing instructions
Save my infoshould show up.Place orderbutton should work as expected.Save my infoshould show up.Place orderbutton should work as expected.Save my infosection should be shown only when the WCPay payment method is selected.Save my infosection should be shown only when the WCPay payment method is selected.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