Add ECE support for multiple product types #8987
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: +652 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
asumaran
force-pushed
the
as-8871-ece-on-supported-product-types
branch
from
d889127 to
246aebe
Compare
asumaran
force-pushed
the
as-8871-ece-on-supported-product-types
branch
from
246aebe to
6f7c254
Compare
Tested multiples scenarios and was never necessary
Fixes an issue where ECE fails if the cookies are removed before refresh.
Fixes an issue where the UI is stuck if the request to create the order returns an invalid response.
The bundle plugin has multiple .qty inputs, and the one we use to determine the quantity is the last input, which is of the number type.
asumaran
requested review from
a team and
cesarcosta99
and removed request for
a team
# Conflicts: # client/express-checkout/index.js # client/express-checkout/utils/index.ts
There was a problem hiding this comment.
@asumaran, I’m still working through the many test cases, but one issue stands out: we’re rendering ECE payment methods in scenarios where they’re known to fail or charge incorrectly, especially concerning taxes.
We should verify the cart and store settings to enhance the shopper experience, ensuring we only display payment methods when we’re confident the payment can be processed successfully.
The other feedback that I can give based on what I've already tested:
Bundle - Virtual Products
- Product page
- Worked for taxes calculated based on billing address but failed when based on shipping address. When based on shipping address, it adds taxes based in the billing address from GPay.
- Also, when calculated based on billing, it shows the incorrect amount in the GPay prompt but charges correctly. For instance, it shows the value without taxes but charges with taxes, which was the correct amount.
Bundle - Mixed Products
- Product page: the buttons are rendering in the page (if they don't work we shouldn't render them), but it's not possible to finish the payment:
The following problems were found: The payment request button is not supported in United States (US) because some required fields couldn't be verified. Please proceed to the checkout page and try again. Shipping First name is a required field. Shipping Last name is a required field. Shipping Country / Region is a required field. Shipping Street address is a required field. Shipping Town / City is a required field. Shipping State is a required field. Shipping ZIP Code is a required field. Please enter an address to continue.
@rafaelzaleski That's correct. I believe hiding ECE for those cases deserves its own issue in case we don't find a solution. I didn't include it in this PR because the testing could have been nearly endless 😀 (or maybe not, but I thought on handling it separately to simplify the testing process).
You sure? You might get charged the correct amount but what was shown to the user was incorrect. 
Assuming: Check this example using a Bundle of virtual products. It shows However the user is charged Although the end result is correct, to me that's incorrect and I marked them as failed. We can't show the user an incorrect amount then charge another.
I agree. As mentioned above I think we should handle it in a separate issue. |
|
The only inconsistency I found when testing Bundle - virtual products
I'll test Bookings on Monday. |
There was a problem hiding this comment.
Overall, the code looks good.
I noticed extensive use of jQuery for simple selectors, likely because the code was copied from PRBs. I won’t extend this review further on that point, as we have a dedicated issue to reduce jQuery usage in the ECE implementation.
I also noticed code related to WooCommerce Deposits, even though these aren’t listed in WC_Payments_Express_Checkout_Button_Helper->supported_product_types. While this is in the scope of issue #8871, I’d rather not complicate this PR further. Could you handle this in a separate PR? Please either remove the code related to deposits if we aren’t supporting them, or ensure all necessary support is included before closing issue #8871.
Please focus on my comments as I believe they are blockers.
Thanks for all your hard work! 🙏
includes/express-checkout/class-wc-payments-express-checkout-ajax-handler.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I appreciate the updates you did, great work! Just one last thing that came through in the latest changes.
All tests that I run passed 🎉
- Virtual Product ✅
- Physical Product ✅
- Without Tax
- With Tax (Customer Shipping Address)
- Ensure the ECE button renders with a variable product ✅
- Product Type:
variation✅- Virtual Products
- Physical Products
- Without Tax
- With Tax (Customer Shipping Address)
- Mixed Products (Virtual & Physical)
- Virtual Variation
- Physical Variation
- Without Tax
- With Tax (Customer Shipping Address)
- Product Type:
subscription✅- Virtual
- Physical
- Without Tax
- With Tax (Customer Shipping Address)
- Ensure the ECE button renders with a variable subscription ✅
- Product Type:
subscription_variation✅- Virtual
- Physical
- Without Tax
- With Tax (Customer Shipping Address)
- Mixed (Virtual & Physical)
- Virtual Variation
- Physical Variation
- Without Tax
- With Tax (Customer Shipping Address)
Given that the tests that didn't pass won't work on PRBs as well we should work on disabling ECE for those particular cases or figuring out a solution in a separate issue.
– #8987 (comment)
We should verify the cart and store settings to enhance the shopper experience, ensuring we only display payment methods when we’re confident the payment can be processed successfully.
– #8987 (review)
To touch on the above, it looks like this tax thing is already an issue present in PRBs (#8373). Since this is not a regression, IMO, we should use that spike to address this. The testing in this PR is extremely extensive already, and during that spike we can take advantage of the test instructions here to ensure everything is working as expected.
Another aspect we should consider when revisiting this is to confirm if it really makes sense to tax virtual products. Maybe we should consult the team working on the tax feature and/or tax experts to be sure we're applying taxes correctly. Based on whether tax or not virtual products, we can decide to either hide the ECE button, or fix the application of the tax for the (virtual) products where it's buggy. cc @bborman22
|
I’ve finished testing WC Bookings, and it performs as described in the testing instructions. |
@rafaelzaleski Deposits wasn't part of the list on the issue and I missed it completely. I'll create an issue to make sure we are supporting Deposits as well. -- Issue created #9067 |
This is to make it possible it can be used from class-wc-payments-express-checkout-ajax-handler.php
@rafaelzaleski You are correct. I must have copied it incorrectly from my list, which correctly shows that bundles with virtual products fail with taxes. I've updated the PR description to reflect the correct outcome. |
|
Thanks for your feedback, @cesarcosta99 @rafaelzaleski. I think I addressed all your comments and code suggestions, also created issues where needed. If the PR looks good to you, would you mind approving it? |
Co-authored-by: César Costa <10233985+cesarcosta99@users.noreply.github.com>
Co-authored-by: César Costa <10233985+cesarcosta99@users.noreply.github.com>
Changes proposed in this Pull Request
Closes #8871
Fixes #8981
Add support for the same product types that we currently support with PRBs.
Testing Instructions
Important
I recommend skipping the failed test cases and not wasting time reviewing them, as they fail due to limitations in ECE and PRBs. For more information, refer to issues #9020 and #8373.
Important
Due to the amount of testing cases I only tested it on Chrome with Google Pay using my own credit cards. Ensure your site is in test mode.
Important
I'd recommend clearing cookies before running each test case to catch any possible errors if some data is missing.
Setup
as-8871-ece-on-supported-product-types.wp option update _wcpay_feature_stripe_ece 1in your merchant's root directory.To alternate how the tax is calculated:
Product Type:
simpleTesting steps
In order to test this product type you'll need to create:
Product Page
Also test in the Cart Shortcode Page, Checkout shortcode Page, Cart Block and Checkout Block.
Product Type:
variableThis product is the parent of variations. It cannot be purchased; only its variations can be bought. Ensuring the ECE button renders properly on the product page is sufficient to verify that ECE is compatible with this product type.
Product Type:
variationTest cases
In order to test this product type you'll need to create the following products:
A variable product with virtual variations
color,red|green. Check the "Used for variations" options.A variable product with physical (simple products) variations.
color,red|green.A variable product with mixed products (simple and virtual) variations.
color,red|green.Product Page
(Shipping Address)
Product Type:
subscriptionTest cases
In order to test this product type you'll need to download and install the "WooCommerce Subscriptions" extension from https://woocommerce.com/my-account/downloads/.
Create two simple subscriptions:
Product Page
Product type:
variable-subscriptionThis product is the parent of subscription variations. Ensuring the ECE button renders properly on the product page of a variable subscription is sufficient to verify that ECE is compatible with this product type.
Product type:
subscription_variationTest cases
In order to test this product type you'll need to install the WooCommerce Subscriptions plugin. You can download it from here.
Create A Variable subscription with virtual variations
size,small|big. Check the "Used for variations" options.A Variable subscription with simple variations.
size,small|big.A variable product with mixed products (simple and virtual) variations.
size,small|big.Product Page
(Shipping Address)**
Product type:
bookingTesting steps
In order to test this product type you'll need to install the WooCommerce Bookings plugin. You can download it from here.
Create a simple booking
Create a virtual booking
Note: No need to test the ECE on the product page for this product type as it's currently not supported in that page.
Test cases
Product type:
bundlesTesting steps
In order to test this product type you'll need to install the Product Bundles plugin. You can download it from here.
Create a bundle with simple products
Create a bundle with virtual products
Create a bundle with mixed products (simple and virtual)
Test cases
Product type:
compositeTesting steps
In order to test this product type you'll need to install the Composite Products plugin. You can download it from here.
Create a composite product with simple products
Note: No need to test the ECE on the product page for this product type as it's currently not supported in that page.
Test cases
Product type:
mix-and-matchTesting steps
In order to test this product type you'll need to install the Mix and Match Products plugin. You can download it from here.
Create a "Mix and Match" product with simple products
Note: No need to test the ECE on the product page for this product type as it's currently not supported in that page.
Test cases
Error types
Default customer location" option is set to.