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
Fix syntax for advanced filters in WC 7.8 and over #6617
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: +710 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
@@ -125,12 +126,22 @@ export const getFilters = ( | |||
export const getAdvancedFilters = ( | |||
customerCurrencyOptions?: TransactionsFilterEntryType[] | |||
): any => { | |||
// TODO: Remove this and all the checks once we drop support of WooCommerce 7.7 and below. | |||
const wooCommerceVersionString = getSetting( 'wcVersion' ); |
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.
Will this, in any case, will return null
or undefined
?
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.
I don't think WCPay can run without WC installed, so I don't think so :) Still, this is a temporary solution that we'll be able to drop once we drop support for 7.7
…om/Automattic/woocommerce-payments into hotfix/6533-advanced-filters-error
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.
LGTM, works as expected on both WC7.8 and older versions.
We should make a follow up issue to look into handling this in a more generic way in future and to update the tests (because I used a slightly hacky fix to get them working).
The only other thing is the changelog should be updated before merging.
Thanks for working on this!
Fixes #6533
Changes proposed in this Pull Request
This change allows Advanced Filters to be compatible with the newly introduced syntax in WC 7.8 woocommerce/woocommerce#37967
It introduces a check for the WC version and picks the right format depending if WC is less than 7.8 or not, in order to make it backward compatible.
Because the string is part of the translation function, it needs to be literal, so more elegant approaches with string concatenation were not available for this.
### Testing instructions
Install WooCommerce 7.8 or bigger
Go to Transactions
All the advanced filters should be functional and displayed without any error
Go to Deposits
All the advanced filters should be functional and displayed without any error
Go to Disputes
All the advanced filters should be functional and displayed without any error
Go to Documents (enable it in Dev tools if not appearing)
All the advanced filters should be functional and displayed without any error
Install WP Rollback
Rollback WooCommerce to any version under 7.8 (7.7, for example)
Go to Transactions
All the advanced filters should be functional and displayed without any error
Go to Deposits
All the advanced filters should be functional and displayed without any error
Go to Disputes
All the advanced filters should be functional and displayed without any error
Go to Documents (enable it in Dev tools if not appearing)
All the advanced filters should be functional and displayed without any error
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