Improve MultiCurrency cache strategy and switch to using API v2 for currency rates #10800
Conversation
This file contains hidden or 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: 0 B Total Size: 1.26 MB ℹ️ View Unchanged
|
marcinbot
reviewed
May 26, 2025
| /** | ||
| * Communicates with the WooPayments v2 API. | ||
| */ | ||
| class WC_Payments_API_V2_Client extends WC_Payments_API_Client { |
There was a problem hiding this comment.
Let's avoid inheritance. I think adding something like V2_ENDPOINT_REST_BASE to the base class and using that in the method will be sufficient. It also allows the classes that call the API client not to be concerned of versions.
And we will be able to get rid of all the static/self changes, reducing the diff size.
And it will address the concern from the comment of 1:1 route match, which we will never have :)
| $ttl = DAY_IN_SECONDS; | ||
| } | ||
| break; | ||
| case self::CURRENCIES_KEY: | ||
| // Refresh the errored currencies quickly, otherwise cache for 6h. | ||
| $ttl = $cache_contents['errored'] ? 2 * MINUTE_IN_SECONDS : 6 * HOUR_IN_SECONDS; | ||
| if ( defined( 'DOING_CRON' ) || is_admin() || Utils::is_admin_api_request() ) { |
There was a problem hiding this comment.
Not sure if we need the DOING_CRON check there TBH.
We should also check for is_cart and is_checkout in addition to or instead of the admin checks. We'd want to show updated prices on those pages I imagine.
There was a problem hiding this comment.
Or feel free to ignore, perhaps this is good enough 🤔
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.
Contributes to TRAPLAT-3377
Changes proposed in this Pull Request
We change the caching strategy for MultiCurrency (MCCY) rates by:
This way, we avoid excessive API spamming from busy stores while triggering cache refreshes by less demanding contexts (WP admin).
At the same time, we switch to using the API v2 endpoint which will has a better response time due to its internal caching of currency rates.
Context: p1748001425970059-slack-C01BPL3ALGP
Testing instructions
wp-content/uploads/wc-logs/woopayments-<date>...login your local client WordPress install.DELETE FROM wp_options WHERE option_name='wcpay_multi_currency_cached_currencies'to delete thewcpay_multi_currency_cached_currenciesDB cache entry.WooCommerce > Settings > Multi-CurrencySELECT * FROM wp_options WHERE option_name='wcpay_multi_currency_cached_currencies'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