-
Notifications
You must be signed in to change notification settings - Fork 798
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: Unnecessary query calls without WooCommerce - PR ready for review #10664
Fix: Unnecessary query calls without WooCommerce - PR ready for review #10664
Conversation
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: November 26, 2018. Generated by 🚫 dangerJS |
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 would revert to using the Jetpack function in this case.
* | ||
* This action is documented in https://docs.woocommerce.com/document/create-a-plugin | ||
*/ | ||
if ( !is_plugin_active( 'woocommerce/woocommerce.php' ) ) { |
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 would recommend that you keep using Jetpack::get_active_plugins()
here; it accounts for more use-cases than is_plugin_active
(e.g. multisite).
You can check the get_active_plugins
function in class.jetpack.php
to find out more.
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.
Thank you @jeherve I have checked it out and the change is reverted to Jetpack's get_active_plugins() function.
This reverts commit bbfac8b.
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 looks good to me. 👍
@greenafrican Would you mind taking a look as well, and merge if this looks good to you?
Thank you!
* Add first version of the Changelog and testing list for 6.9 * Changelog: add #10710 * changelog: add #10538 * changelog: add #10741 * changelog: add #10749 * changelog: add #10664 * changelog: add #10224 * changelog: add #10788 * Changelog: add #10560 * Chanegelog: add #10812 * changelog: add #10556 * Changelog: add #10668 * Changelog: add #10846 * Changelog: add #10947 * Changelog: add #10962 * Changelog: add #10956 * Changelog: add #10940 * Changelog: add #10934 * Changelog: add #10912 * changelog: add #10866 * changelog: add #10924 * Changelog: add #10936 * Changelog: add #10833 * changelog: add #10867 * Changelog: add #10960 * Changelog: add #10888 * changelog: add #10840 * changelog: add #10972 * Changelog: add #10979 * changelog: add #10909 * Changelog: add #10958 * Changelog: add #10981 * Changelog: add #10564 * Changelog: add #10809 * Changelog: add #10982 * Changelog: add #10706 * Changelog: add #10978 * Changelog: add #10132 * Changelog: add #11022 * Changelog: add #11024 * Changelog: add #10875 * Changelog: add #11030 * Changelog: add #11053 * Changelog: add #10880 * Changelog: add #9359 * Changelog: add #11037 * Update block list * Changelog: add #11060 * Changelog: add #10755 * changelog: add #11000 * Changelog: add #10786 * Changelog: add #10945 * Changelog: add #10597
As the issue #9736 states, there are a number of unnecessary query calls from the wp-woocommerce-analytics.php.
Changes proposed in this Pull Request:
I have changed the order of the calling of the functions, to make the check if WooCommerce is installed/active to avoid other functions calls that result in unnecessary query calls. As it was mentioned in the issue description there might be other ways to check if the plugin is active, I have changed (and tested) the function to is_plugin_active() . Not sure if it has any effect on the performance, but it's less code and works well.
Testing instructions:
I recommend testing with the Query Monitor Plugin.
Check the Query Monitor > Queries by Component > Plugin: jetpack menu, with WooCommerce activated and deactivated. The mentioned queries should only show up when it's activated.
WooCommerce Activated:
WooCommerce Deactivated:
Proposed changelog entry for your changes:
None required.