REST API: JPO: Enable WooCommerce installation#8482
Conversation
bfb7994 to
4939b5d
Compare
_inc/lib/plugins.php
Outdated
There was a problem hiding this comment.
Note that activate_plugin returns null on successful activation, so you'll want to update the logic here to return true on success (probably by checking if it's NOT a WP_Error). You might be able to benefit from the other review comments from this review (I mentioned that there): #8450 (review).
There was a problem hiding this comment.
Oops, my bad. Oh those dreaded return values.
Move: * the `Jetpack_Automatic_Install_Skin` helper class to `_inc/lib/class.jetpack-automatic-install-skin.php` * some static helper methods from `Jetpack_JSON_API_Plugins_Install_Endpoint` to `Jetpack_Plugins` in `_inc/lib/plugins.php`. (Preparatory work for #8482. De-duplicate all the things!)
4939b5d to
cc93681
Compare
|
Ready for review! |
tyxla
left a comment
There was a problem hiding this comment.
Brilliant work ❤️ 😍
Code looks great and tests as expected. LGTM 👍
Left a couple of comments only, nothing blocking.
_inc/lib/plugins.php
Outdated
| // Check if get_plugins() function exists. This is required on the front end of the | ||
| // site, since it is in a file that is normally only loaded in the admin. | ||
| if ( ! function_exists( 'get_plugins' ) ) { | ||
| require_once ABSPATH . 'wp-admin/includes/plugin.php'; |
There was a problem hiding this comment.
Indentation appears to be use a space instead of a tab here.
| } | ||
|
|
||
| public static function get_plugin_id_by_slug( $slug ) { | ||
| // Check if get_plugins() function exists. This is required on the front end of the |
There was a problem hiding this comment.
We're also not in the front end of the site, are we?
There was a problem hiding this comment.
Indeed. I was copy-pasting this from elsewhere. Since #8464 worked without it, I guess I can just drop it?
There was a problem hiding this comment.
I'd prefer keeping it just in case, because those admin files are often not specifically loaded. We can consider removing it only if we're sure that in all cases this method is called, the file is specifically required.
There was a problem hiding this comment.
Nope, has to remain for our use case:
[08-Jan-2018 15:54:56 UTC] PHP Fatal error: Uncaught Error: Call to undefined function get_plugins() in ~/public_html/wp-content/plugins/jetpack/_inc/lib/plugins.php:104
|
Soliciting review from JP Crew members @oskosk and @eliorivero 🙏 |
Add
install_and_activate_plugins()to the Plugins Helper class, and use that to install WooCommerce from the JP Onboarding endpoint.Counterpart to Automattic/wp-calypso#21237 -- find testing instructions there.