-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't load Apple/Google Pay for synced subscriptions that don't have payment due today and also require shipping #7582
base: develop
Are you sure you want to change the base?
Don't load Apple/Google Pay for synced subscriptions that don't have payment due today and also require shipping #7582
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: +38 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mattallan. The fix is working as described 🎉
@mattallan I have reviewed the changes and approved the PR as things are fixed. But you may need to consider the scenario @james-allan raised about variations. |
…f ALL variations pass
…h express payment methods
Thanks @Mayisha and @james-allan. I've refactored the This means if you have a subscription that has one variation that is valid for express payment methods and another that is not, the button will still appear on the product page for it and only show for variations that are supported, here's a quick screencast showcasing this: |
65c5d02
to
4bbfc67
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mattallan.
Changes look good and works. I left a comment suggesting that we can remove the is_today()
check. I believe that isn't necessary given the is_payment_upfront()
function includes an equivalent check. :)
Co-authored-by: James Allan <james.allan@automattic.com>
Thanks for the suggestion @james-allan. I had to fix the psalm issues and unit tests but this is good for a final review Edit: I've ran through the standard tests on the latest changes |
…show PRB on the page load after
While copying these changes over to our Stripe gateway, I found a few more issues over there that are also issues in WooPayments. Here are the individual fixes I've pushed up to this PR:
@james-allan did you mind taking this PR for another spin? 🙏 Thanks! |
Fixes #3742
Changes proposed in this Pull Request
When customers attempt to use Apple Pay or Google Pay to purchase a synchronized subscription product that has no upfront costs (i.e. trial period or delayed initial payment due to product being synced) and also requires shipping, they're met with an error on the page:
Inside our
WC_Payments_Payment_Request_Button_Handler
class, we have code that prevents non-virtual (physical) products with free trials from being purchased with Apple/Google Pay, however there isn't any code to handle products that imitate a free trial period like what we do to handle synchronised subscription purchased outside of the synced date with no initial payment.In this PR, I've introduced a new function to check if the product is a subscription product and has a free trial period (either trial set on the product or is a synced product), and requires shipping, and if so, prevent loading the Apple Pay/Google Pay buttons on the page.
Testing instructions
develop
branch, view the product page and note the Payment Request buttons (PRB) are loaded. If you attempt to purchase this product with PRB, you will see the above error.Other test cases to run:
npm run changelog
to add a changelog file, choosepatch
to 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