-
Notifications
You must be signed in to change notification settings - Fork 815
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
WooCommerce Services JITM #6183
Changes from 17 commits
3e7ed4a
54f0f33
2be7eea
014ba86
e318a8d
d863d11
752dfdc
f7c1aa2
9da1e76
f5328c4
e581bfd
c383dac
fdf129d
55dc750
caff508
68e5e75
8c09a93
8594906
6a84c14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
<?php | ||
|
||
if ( ! defined( 'ABSPATH' ) ) { | ||
exit; | ||
} | ||
|
||
class WC_Services_Installer { | ||
|
||
/** | ||
* @var WC_Services_Installer | ||
**/ | ||
private static $instance = null; | ||
|
||
static function init() { | ||
if ( is_null( self::$instance ) ) { | ||
self::$instance = new WC_Services_Installer(); | ||
} | ||
return self::$instance; | ||
} | ||
|
||
public function __construct() { | ||
add_action( 'admin_init', array( $this, 'add_error_notice' ) ); | ||
add_action( 'admin_init', array( $this, 'try_install' ) ); | ||
} | ||
|
||
public function try_install() { | ||
if ( isset( $_GET['wc-services-action'] ) && ( 'install' === $_GET['wc-services-action'] ) ) { | ||
check_admin_referer( 'wc-services-install' ); | ||
|
||
if ( ! current_user_can( 'install_plugins' ) ) { | ||
return; | ||
} | ||
|
||
$result = $this->install(); | ||
$redirect = wp_get_referer(); | ||
|
||
if ( false === $result ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May be easier to just do |
||
$redirect = add_query_arg( 'wc-services-install-error', true, $redirect ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep - the value end up going through |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to add an else clause so we could append a get argument confirming the installation happened? Or do we not care about showing a confirmation message on the page we're directing them to (which is fine)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not concerned initially since the first activation of the plugin will cause a welcome/TOS message. |
||
|
||
wp_safe_redirect( $redirect ); | ||
|
||
exit; | ||
} | ||
} | ||
|
||
public function add_error_notice() { | ||
if ( ! empty( $_GET['wc-services-install-error'] ) ) { | ||
add_action( 'admin_notices', array( $this, 'error_notice' ) ); | ||
} | ||
} | ||
|
||
public function error_notice() { | ||
?> | ||
<div class="notice notice-error is-dismissible"> | ||
<p><?php _e( 'There was an error installing WooCommerce Services.', 'jetpack' ); ?></p> | ||
</div> | ||
<?php | ||
} | ||
|
||
private function install() { | ||
include_once( ABSPATH . '/wp-admin/includes/admin.php' ); | ||
include_once( ABSPATH . '/wp-admin/includes/plugin-install.php' ); | ||
include_once( ABSPATH . '/wp-admin/includes/plugin.php' ); | ||
include_once( ABSPATH . '/wp-admin/includes/class-wp-upgrader.php' ); | ||
include_once( ABSPATH . '/wp-admin/includes/class-plugin-upgrader.php' ); | ||
|
||
$api = plugins_api( 'plugin_information', array( 'slug' => 'connect-for-woocommerce' ) ); | ||
|
||
if ( is_wp_error( $api ) ) { | ||
return false; | ||
} | ||
|
||
$upgrader = new Plugin_Upgrader( new Automatic_Upgrader_Skin() ); | ||
$result = $upgrader->install( $api->download_link ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this work on environments where WordPress doesn't have write access to the filesystem? Is it just the catchall error in I know we have a lot of caveats in our plugin installation rest api endpoint -- https://github.com/Automattic/jetpack/blob/master/json-endpoints/jetpack/class.jetpack-json-api-plugins-install-endpoint.php#L12-L54 // cc: @roccotripaldi There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just tested this, and it is caught by our generic error handler. |
||
|
||
if ( true !== $result ) { | ||
return false; | ||
} | ||
|
||
$result = activate_plugin( 'connect-for-woocommerce/woocommerce-connect-client.php' ); | ||
|
||
// activate_plugin() returns null on success | ||
return is_null( $result ); | ||
} | ||
} | ||
|
||
WC_Services_Installer::init(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,18 @@ function prepare_jitms( $screen ) { | |
add_action( 'admin_enqueue_scripts', array( $this, 'jitm_enqueue_files' ) ); | ||
add_action( 'admin_notices', array( $this, 'backups_updates_msg' ) ); | ||
} | ||
elseif ( ! Jetpack::is_plugin_active( 'woocommerce-services/woocommerce-services.php' ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be on the previous line, as per WP Coding Standards -- https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#brace-style (really, so should all the prior conditionals above -- but as you're just matching style, this is less critical to fix) |
||
$pages_to_display = array( | ||
'woocommerce_page_wc-settings', // WooCommerce > Settings | ||
'edit-shop_order', // WooCommerce > Orders | ||
'shop_order', // WooCommerce > Edit Order | ||
); | ||
|
||
if ( in_array( $screen->id, $pages_to_display ) ) { | ||
add_action( 'admin_enqueue_scripts', array( $this, 'jitm_enqueue_files' ) ); | ||
add_action( 'admin_notices', array( $this, 'woocommerce_services_msg' ) ); | ||
} | ||
} | ||
} | ||
|
||
/* | ||
|
@@ -414,6 +426,60 @@ function videopress_media_upload_warning_msg() { | |
<?php | ||
} | ||
|
||
/** | ||
* Display message prompting user to install WooCommerce Services. | ||
* | ||
* @since 4.6 | ||
*/ | ||
function woocommerce_services_msg() { | ||
if ( ! current_user_can( 'manage_woocommerce' ) || ! current_user_can( 'install_plugins' ) ) { | ||
return; | ||
} | ||
|
||
if ( isset( self::$jetpack_hide_jitm['woocommerce_services'] ) ) { | ||
return; | ||
} | ||
|
||
if ( ! function_exists( 'wc_get_base_location' ) ) { | ||
return; | ||
} | ||
|
||
$base_location = wc_get_base_location(); | ||
|
||
switch ( $base_location['country'] ) { | ||
case 'US': | ||
$message = __( 'Try our new service for USPS shipping & label-printing.', 'jetpack' ); | ||
break; | ||
case 'CA': | ||
$message = __( 'Try our new Canada Post shipping service.', 'jetpack' ); | ||
break; | ||
default: | ||
return; | ||
} | ||
|
||
$install_url = wp_nonce_url( add_query_arg( array( 'wc-services-action' => 'install' ) ), 'wc-services-install' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably should provide a base page to the link here, instead of just coasting off current. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm.. okay - after a quick discussion, I think we'll take the user to the plugins page upon installation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could also do |
||
|
||
?> | ||
<div class="jp-jitm woo-jitm"> | ||
<a href="#" data-module="woocommerce_services" class="dismiss"><span class="genericon genericon-close"></span></a> | ||
<div class="jp-emblem"> | ||
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" id="Layer_1" x="0" y="0" viewBox="0 0 24 24" enable-background="new 0 0 24 24" xml:space="preserve"> | ||
<path d="M18,8h-2V7c0-1.105-0.895-2-2-2H4C2.895,5,2,5.895,2,7v10h2c0,1.657,1.343,3,3,3s3-1.343,3-3h4c0,1.657,1.343,3,3,3s3-1.343,3-3h2v-5L18,8z M7,18.5c-0.828,0-1.5-0.672-1.5-1.5s0.672-1.5,1.5-1.5s1.5,0.672,1.5,1.5S7.828,18.5,7,18.5z M4,14V7h10v7H4z M17,18.5c-0.828,0-1.5-0.672-1.5-1.5s0.672-1.5,1.5-1.5s1.5,0.672,1.5,1.5S17.828,18.5,17,18.5z" /> | ||
</svg> | ||
</div> | ||
<p class="msg"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be wrapped in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not the most straightforward logic perhaps, but if you don't fall into either the |
||
<?php echo esc_html( $message ); ?> | ||
</p> | ||
<p> | ||
<a href="<?php echo esc_url( $install_url ); ?>" title="<?php esc_attr_e( 'Install WooCommerce Services', 'jetpack' ); ?>" data-module="woocommerce_services" class="button button-jetpack launch show-after-enable"><?php esc_html_e( 'Install WooCommerce Services', 'jetpack' ); ?></a> | ||
</p> | ||
</div> | ||
<?php | ||
//jitm is being viewed, track it | ||
$jetpack = Jetpack::init(); | ||
$jetpack->stat( 'jitm', 'woocommerce_services-viewed-' . JETPACK__VERSION ); | ||
} | ||
|
||
/* | ||
* Function to enqueue jitm css and js | ||
*/ | ||
|
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.
Security concern: Nonces are for verifying intent, not capabilities. This doesn't seem to in any way test for the
install_plugins
capability. :\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.
Good catch - we'll add the check for the
install_plugins
capability as well.