[ECE] Add ECE support for WooCommerce Deposits. #9081
Merged
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: +80 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
asumaran
changed the title
asumaran
requested review from
a team and
reykjalin
and removed request for
a team
asumaran
commented
Jul 12, 2024
includes/express-checkout/class-wc-payments-express-checkout-button-helper.php
Outdated
Show resolved
Hide resolved
reykjalin
requested changes
Jul 12, 2024
There was a problem hiding this comment.
The payments failed for me in several places which I think we need to address:
Virtual Simple (50% deposit)
- ⛔️ Fails when using GPay from the product page.
- ✅ Success when using GPay from the Cart Block.
- ⛔️ Fails when using GPay from the Checkout Block.
- ✅ Success when using GPay from the Shortcode Checkout.
- ✅ Success when using GPay from the Shortcode Cart
Physical product (30% then 70%)
- ⛔️ Payment sheet doesn't show up from the product page (default choice is Pay full, change to Deposit and click button).
- ✅ Success when using GPay from the Cart Block.
- ⛔️ Fails when using GPay from the Checkout Block.
- ✅ Success when using GPay from the Shortcode Checkout.
- ✅ Success when using GPay from the Shortcode Cart
Payment Plan (physical)
- ✅ Success when using GPay from the product page.
- I have no idea why this one succeeded but the other ones failed.
- ✅ Success when using GPay from the Cart Block.
- ⛔️ Fails when using GPay from the Checkout Block.
- ✅ Success when using GPay from the Shortcode Checkout.
- ✅ Success when using GPay from the Shortcode Cart
client/express-checkout/index.js
Outdated
| $( | ||
| 'input[name=wc_deposit_option],input[name=wc_deposit_payment_plan]' | ||
| ) | ||
| .off( 'change' ) |
There was a problem hiding this comment.
What's the purpose of this off? Do we need to remove all other event handlers from the change event? 🤔
There was a problem hiding this comment.
I was being extra cautious because the ECE element can be reinitialized, event handlers are attached each time it happens. To avoid attaching multiple handlers to the same selector, I remove any existing event handlers first. Although the elements are not being removed and added again, it seems unnecessary, but I prefer to keep it this way for safety. Does that sound good to you?
There was a problem hiding this comment.
I appreciate the caution, but if we're going to do that I think we should pass the specific handler we're removing to the function so we're not just removing every single event handler that may have been attached.
See the off() docs for details.
includes/express-checkout/class-wc-payments-express-checkout-button-helper.php
Outdated
Show resolved
Hide resolved
We need to do this because the accumulated amount in the item list is higher than the total passed.
Nice catch. Fixed via 2203c28
Works for me now.
I can reproduce this. I've fixed it via 263027c. Race condition issue.
Works for me now.
Works for me now. |
reykjalin
requested changes
Jul 14, 2024
There was a problem hiding this comment.
I'm still seeing some failures in the Checkout block:
Virtual Simple (50% deposit)
- ✅ Success when using GPay from the product page.
- ✅ Success when using GPay from the Cart Block.
- ⛔️ Fails when using GPay from the Checkout Block.
- ✅ Success when using GPay from the Shortcode Checkout.
- ✅ Success when using GPay from the Shortcode Cart
Physical product (30% then 70%)
- ✅ Success sheet doesn't show up from the product page (default choice is Pay full, change to Deposit and click button).
- ✅ Success when using GPay from the Cart Block.
- ⛔️ Fails when using GPay from the Checkout Block.
- ✅ Success when using GPay from the Shortcode Checkout.
- ✅ Success when using GPay from the Shortcode Cart
Payment Plan (physical)
- ✅ Success when using GPay from the product page.
- I have no idea why this one succeeded but the other ones failed.
- ✅ Success when using GPay from the Cart Block.
- ⛔️ Fails when using GPay from the Checkout Block.
- ✅ Success when using GPay from the Shortcode Checkout.
- ✅ Success when using GPay from the Shortcode Cart
All I can tell from the network console is that it's failing in the create_order part of the flow. Maybe because I have multi-currency enabled? 🤔 Although I doubt that's the problem since this works via the same APIs from other pages. I suspect there's something that needs to be changed in the blocks integration?
|
@reykjalin does it fail for you with PRBs too? |
|
I couldn’t reproduce the failed test cases reported by @reykjalin in Safari. Could @reykjalin check if the issue still persists in the Pressable environment? This could help us understand why it’s failing for Kris and determine whether to address it here or in a new issue, especially if it’s specific to certain setups. |
|
I reran some tests with the failed cases, and it's working fine for me on Chrome. I will need more details to reproduce it, or maybe it's related to your setup, @reykjalin? |
They're not present there so I'll do some more testing in my local tomorrow to try to track this down. |
reykjalin
approved these changes
Jul 17, 2024
lovo-h
pushed a commit
that referenced
this pull request
Aug 1, 2024
Co-authored-by: Rafael Zaleski <[email protected]>
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.
Closes #9067
Changes proposed in this Pull Request
Testing instructions
Based on testing instructions of #7910 which added support for WooCommerce Deposits with PRBs. The functionality should be identical with ECE.
Virtual Product
Physical Product
(product price + shipping) * 0.3Virtual product with payment plan
Note
If you want to test to pay the remaining balance you have to go to the order page in the admin area then click "Invoice Remaining Balance"
It will create a new order, and you will be able to pay the remaining using the "Customer payment page" link. Note that the ECE button didn't work on the Pay for Order page, so code has been added to disable it on that page in this PR.
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