Revert "Revert "Displaying Clearpay instead of Afterpay for UK based stores"" #8046
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: +602 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
gpressutto5
requested review from
a team and
timur27
and removed request for
a team
| @@ -267,13 +267,14 @@ public function __construct( | |||
| $this->localization_service = $localization_service; | |||
| $this->fraud_service = $fraud_service; | |||
|
|
|||
| $account_country = $this->get_account_country(); | |||
There was a problem hiding this comment.
Noting that I reached out to the Architecture Team about this and they confirmed that this isn't the best way to go about this: p1705510123965239-slack-C0208C3BXHP
I think that if we need the account's country for something before the plugin is fully initialized, we should likely save the account's country in an option somewhere that would allow us to pull it with WP core functions that do not try to use services in the client plugin. The account country should be static, as it is not possible to change the country on an account, so if the value was saved on a previous load it would remain.
The woocommerce_payments_account_refreshed hook could be used to call a method to save the country in its own option in the database.
We could also see if it's possible to set the country in the account object that's saved in the woocommerce_woocommerce_payments_settings, and then that option could be pulled via a private method in this class to get the country.
timur27
approved these changes
Jan 22, 2024
There was a problem hiding this comment.
The changes look good, are identical to the changes in #7995. I went through the history of these changes and I understand that we are merging this back after improving logger class & dev tools in separate PRs.
I've also tested it, Clearpay/Afterpay show up correctly depending on the country. I also performed a successful purchase by using Clearpay.
Approving this PR with the assumption that the merge conflicts will be solved as well as #8046 (comment) should be still resolved, especially given there are some options available in that thread. Thanks @gpressutto5!
6 tasks
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.
Reverts #8034
Fixes #6425
Changes proposed in this Pull Request
Based on the account country set in Stripe, we should display Clearpay instead of Afterpay everywhere.
Testing instructions
(clear)from the latest devtools works as expectedUsing this branch and a Stripe account with a UK address access all pages that show payment methods and ensure Clearpay name and logo is used instead of Afterpay. Some pages you should check:
Then update to a Stripe account from the US and ensure we show Afterpay instead of Clearpay.
I recommend using the Jurassic Ninja link bellow twice and creating different stripe test accounts.
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