-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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
Add filter to convert WooCommerce slug for plugin dependencies #46707
Conversation
Hi @jorgeatorres, @adrianduffell, @rjchow, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Hi @adrianduffell, @rjchow, @woocommerce/ghidorah Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
1bef21f
to
dc2de46
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.
LGTM and tested well, thanks @chihsuan !
@jorgeatorres Would Proton like to review this solution too? |
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.
This works as advertised. Thanks @chihsua!
I left an (optional) suggestion as part of the feedback but I'm preemptively approving.
* @param string $slug The plugin slug to convert. | ||
* | ||
* @return string | ||
*/ |
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.
Can we add a @since
tag to this block?
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.
Yep, added in 6dbe049. Thanks!
dc2de46
to
6dbe049
Compare
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #46169.
When WC is installed in a directory other than
woocommerce
, the dependent plugins such asWooPayments
andWooCommerce Tax
can't be activated during the Core Profiler with WP 6.5 due to the slug mismatch.This PR adds the
wp_plugin_dependencies_slug
filter to convert the WooCommerce slug to the correct one to fix the issue.During the testing using live branch, I found that we can't install WooPayments from
/wp-admin/plugin-install.php
page. The install button is disabled. It seems like the button state is determined using different logic without applying the filter. There is no filter to change theget_plugins()
result. We'll need to investigate further.How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Australia
as the store location in Core ProfilerChangelog entry
Significance
Type
Message
Comment