Move code from plugins install endpoint to new Plugins Library#8464
Move code from plugins install endpoint to new Plugins Library#8464
Conversation
16bbfdc to
d367156
Compare
d367156 to
4df2bb8
Compare
_inc/lib/plugins.php
Outdated
| */ | ||
| public static function install_plugin( $slug ) { | ||
| if ( is_multisite() && ! current_user_can( 'manage_network' ) ) { | ||
| return new WP_Error( 'not_allowed', 'You are not allowed to install plugins on this site.' ); |
There was a problem hiding this comment.
Is this error message something the user would see? If so, then it should be translatable.
_inc/lib/plugins.php
Outdated
| } | ||
|
|
||
| protected static function generate_wordpress_org_plugin_download_link( $plugin_slug ) { | ||
| return "https://downloads.wordpress.org/plugin/{$plugin_slug}.latest-stable.zip"; |
There was a problem hiding this comment.
The curly braces are not needed here since the expression is simply a string.
_inc/lib/plugins.php
Outdated
| } | ||
|
|
||
| protected static function get_slug_from_file_path( $plugin_file ) { | ||
| // Simular to get_plugin_slug() method. |
There was a problem hiding this comment.
You folks both probably meant "Similar" 😉
|
Ready for review! |
There was a problem hiding this comment.
Wonderful work 👍 ❤️
- Plugin installation (from WP.org): ✅
- Theme installation (from WP.org): ✅ (tested with
twentyseventeen-wpcom) - Plugin installation (via upload): ✅
- Theme installation (via upload): ✅
Verified no other endpoints use the install skin.
Other than an indentation issue, this LGTM 👍
_inc/lib/plugins.php
Outdated
|
|
||
| $result = $upgrader->install( $zip_url ); | ||
|
|
||
| if ( is_wp_error( $result ) ) { |
There was a problem hiding this comment.
Something seems to have happened with the indentation here and on the following lines.
|
I can haz review pls @oskosk and/or @eliorivero? 😁 |
Tested it and it does work. It's the endpoint used by Calypso. When testing on the dev console, just don't scope the request to
|
oskosk
left a comment
There was a problem hiding this comment.
Tests great! 🎉 LGTM
There's a little tiny codestyle issue involving a lack of spaces around an array index which I'm not commenting about in the code because I really don't care.
🚢
|
That's a bit too verbose, but don't worry about it! The label is mostly there for us, to help us build the changelog; we will come up with something as we go through all the PRs with that label at once. Thanks a lot for giving us a headstart though! |
* Changelog 5.8: create base for changelog. * Update 5.8 release post link * fix 5.8 release date * Updates to plugin description * Changelog: add #8499 * Changelog: add #8506 * Changelog: add #8509 * Changelog: add #8516 * Changelog: add #8517 * Changelog: add #8523 * Changelog: add #8547 * Changelog: add #8496 * Changelog: add #8584 * Changelog: add #8595 * Changelog: add #8445 * Changelog: add #8431 * Changelog: add #8284 * Changelog: add #8270 * Changelog: add #8124 * Changelog: add #8581 * Changelog: add #8463 * Changelog: add #8568 (#8646) * Updates to testing list and changelog * Changelog: add #8443 * Changelog: add #8459 * Changelog: add #8469 * Changelog: add #8464 * Changelog: add #8478 and #8479 * Changelog: add #8483 * Changelog: add #8488 * Changelog: add #8513 * Changelog: add #8555 * Changelog: add #8565 * Changelog: add #8601 * Changelog: add #8612 * Changelog: add first pass at Search items. * Changelog: add more info to help test Search. * Changelog: add #8144 * Changelog: add #8313 * Changelog: add #8419 * Changelog: add #8465 * Changelog: add #8515 * Changelog: add #8587 * Changelog: add #8591 * Changelog: add #8659 * Changelog: add #8661 * Changelog: add #8671 * Changelog: add 5.7.1 to archived changelog too. * Reverted changes to readme, removed entry about backups.
Some prep work for
#8450#8482. De-duplicate all the things!Move:
Jetpack_Automatic_Install_Skinhelper class to_inc/lib/class.jetpack-automatic-install-skin.phpJetpack_JSON_API_Plugins_Install_EndpointtoJetpack_Pluginsin_inc/lib/plugins.php.To test: Make sure all affected endpoints still work. Note that
Jetpack_Automatic_Install_Skinis used by a few other endpoints (grep to verify!):POSTto/sites/%s/plugins/%s/install(trywoocommerce)POSTto/sites/%s/themes/%s/install(trytwentyseventeen) -- see note at bottomPOSTto/sites/%s/plugins/%s/new-- use Calypso to test (wordpress.com/plugins/upload/:site)POSTto/sites/%s/themes/%s/new-- use Calypso to test (wordpress.com/themes/upload/:site)Note:
/sites/%s/themes/%s/installdoesn't work at all, returns a 404. However, that behavior seems to be preexisting!