Skip to content
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.

Sign up for GitHub

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

Prevent module activation when plan doesn't support them #8983

Merged
merged 8 commits into from
Mar 7, 2018
Merged

Prevent module activation when plan doesn't support them #8983

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8730bf1
Deactivate search when downgrading Jetpack plan
gravityrail Mar 5, 2018
43c6662
Centralise active plan checking and use all modules in 'supports' list
gravityrail Mar 5, 2018
7477f02
Add 'free' to plan classes, always
gravityrail Mar 5, 2018
1905189
Fix bug showing correct plan support in premium
gravityrail Mar 5, 2018
ff8fce0
Return better error message via API
gravityrail Mar 5, 2018
9b2712c
Remove rogue error_log
gravityrail Mar 5, 2018
5ed8a9f
Fix typo
gravityrail Mar 5, 2018
0850e6a
Fix spacing
gravityrail Mar 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions _inc/lib/admin-pages/class.jetpack-admin-page.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,15 @@ function check_plan_deactivate_modules( $page ) {
$active = Jetpack::get_active_modules();
switch ( $current->plan->product_slug ) {
case 'jetpack_free':
$to_deactivate = array( 'seo-tools', 'videopress', 'google-analytics', 'wordads' );
$to_deactivate = array( 'seo-tools', 'videopress', 'google-analytics', 'wordads', 'search' );
break;
case 'jetpack_personal':
case 'jetpack_personal_monthly':
$to_deactivate = array( 'seo-tools', 'videopress', 'google-analytics', 'wordads' );
$to_deactivate = array( 'seo-tools', 'videopress', 'google-analytics', 'wordads', 'search' );
break;
case 'jetpack_premium':
case 'jetpack_premium_monthly':
$to_deactivate = array( 'seo-tools', 'google-analytics' );
$to_deactivate = array( 'seo-tools', 'google-analytics', 'search' );
break;
}
$to_deactivate = array_intersect( $active, $to_deactivate );
Expand Down
7 changes: 6 additions & 1 deletion class.jetpack-admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,12 @@ static function is_module_available( $module ) {
if ( Jetpack::is_development_mode() ) {
return ! ( $module['requires_connection'] );
} else {
return Jetpack::is_active();
if ( ! Jetpack::is_active() ) {
return false;
}

$plan = Jetpack::get_active_plan();
return in_array( $module['module'], $plan['supports'] );
}
}

Expand Down
7 changes: 5 additions & 2 deletions class.jetpack-cli.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,11 @@ public function module( $args, $assoc_args ) {
case 'activate':
$module = Jetpack::get_module( $module_slug );
Jetpack::log( 'activate', $module_slug );
Jetpack::activate_module( $module_slug, false, false );
WP_CLI::success( sprintf( __( '%s has been activated.', 'jetpack' ), $module['name'] ) );
if ( Jetpack::activate_module( $module_slug, false, false ) ) {
WP_CLI::success( sprintf( __( '%s has been activated.', 'jetpack' ), $module['name'] ) );
} else {
WP_CLI::error( sprintf( __( '%s could not be activated.', 'jetpack' ), $module['name'] ) );
}
break;
case 'activate_all':
$modules = Jetpack::get_available_modules();
Expand Down
19 changes: 5 additions & 14 deletions class.jetpack-modules-list-table.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,6 @@ function filter_displayed_table_items( $modules ) {
return array_filter( $modules, array( $this, 'is_module_displayed' ) );
}

static function is_module_available( $module ) {
if ( ! is_array( $module ) || empty( $module ) )
return false;

if ( Jetpack::is_development_mode() ) {
return ! ( $module['requires_connection'] );
} else {
return Jetpack::is_active();
}
}

static function is_module_displayed( $module ) {
// Handle module tag based filtering.
if ( ! empty( $_REQUEST['module_tag'] ) ) {
Expand Down Expand Up @@ -231,8 +220,10 @@ function single_row( $item ) {
if ( ! empty( $item['activated'] ) )
$row_class .= ' active';

if ( ! $this->is_module_available( $item ) )
if ( ! Jetpack_Admin::is_module_available( $item ) ) {
error_log($item['module']." is NOT available");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

$row_class .= ' unavailable';
}

echo '<tr class="jetpack-module' . esc_attr( $row_class ) . '" id="' . esc_attr( $item['module'] ) . '">';
$this->single_row_columns( $item );
Expand All @@ -244,7 +235,7 @@ function get_table_classes() {
}

function column_cb( $item ) {
if ( ! $this->is_module_available( $item ) )
if ( ! Jetpack_Admin::is_module_available( $item ) )
return '';

return sprintf( '<input type="checkbox" name="modules[]" value="%s" />', $item['module'] );
Expand Down Expand Up @@ -273,7 +264,7 @@ function column_name( $item ) {
$actions['configure'] = $item['configurable'];
}

if ( empty( $item['activated'] ) && $this->is_module_available( $item ) ) {
if ( empty( $item['activated'] ) && Jetpack_Admin::is_module_available( $item ) ) {
$url = wp_nonce_url(
Jetpack::admin_url( array(
'page' => 'jetpack',
Expand Down
66 changes: 43 additions & 23 deletions class.jetpack.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1452,6 +1452,13 @@ public static function refresh_active_plan_from_wpcom() {
* @return array Active Jetpack plan details
*/
public static function get_active_plan() {
global $active_plan_cache;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any better way to do this than introducing another global?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I couldn't think of one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear: We could refactor the whole way that we list out modules, and then we'd only need to call this one - but that requires a lot more time than "hack week" necessarily allows.


// this can be expensive to compute so we cache for the duration of a request
if ( $active_plan_cache ) {
return $active_plan_cache;
}

$plan = get_option( 'jetpack_active_plan', array() );

// Set the default options
Expand All @@ -1463,6 +1470,8 @@ public static function get_active_plan() {
) );
}

$supports = array();

// Define what paid modules are supported by personal plans
$personal_plans = array(
'jetpack_personal',
Expand All @@ -1471,9 +1480,8 @@ public static function get_active_plan() {
);

if ( in_array( $plan['product_slug'], $personal_plans ) ) {
$plan['supports'] = array(
'akismet',
);
// special support value, not a module but a separate plugin
$supports[] = 'akismet';
$plan['class'] = 'personal';
}

Expand All @@ -1485,12 +1493,8 @@ public static function get_active_plan() {
);

if ( in_array( $plan['product_slug'], $premium_plans ) ) {
$plan['supports'] = array(
'videopress',
'akismet',
'vaultpress',
'wordads',
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to remove videopress and wordads from here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - they are modules, the others are plugins. Modules are automatically added to the list based on metadata.

$supports[] = 'akismet';
$supports[] = 'vaultpress';
$plan['class'] = 'premium';
}

Expand All @@ -1503,23 +1507,23 @@ public static function get_active_plan() {
);

if ( in_array( $plan['product_slug'], $business_plans ) ) {
$plan['supports'] = array(
'videopress',
'akismet',
'vaultpress',
'seo-tools',
'google-analytics',
'wordads',
'search',
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, did you mean to remove some items from this list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above - yes ;)

$supports[] = 'akismet';
$supports[] = 'vaultpress';
$plan['class'] = 'business';
}

// Make sure we have an array here in the event database data is stale
if ( ! isset( $plan['supports'] ) ) {
$plan['supports'] = array();
// get available features
foreach( self::get_available_modules() as $module_slug ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing -- foreach (

$module = self::get_module( $module_slug );
if ( in_array( 'free', $module['plan_classes'] ) || in_array( $plan['class'], $module['plan_classes'] ) ) {
$supports[] = $module_slug;
}
}

$plan['supports'] = $supports;

$active_plan_cache = $plan;

return $plan;
}

Expand Down Expand Up @@ -2411,6 +2415,7 @@ public static function get_module( $module ) {
'module_tags' => 'Module Tags',
'feature' => 'Feature',
'additional_search_queries' => 'Additional Search Queries',
'plan_classes' => 'Plans',
);

$file = Jetpack::get_module_path( Jetpack::get_module_slug( $module ) );
Expand Down Expand Up @@ -2440,6 +2445,13 @@ public static function get_module( $module ) {
$mod['module_tags'] = array( self::translate_module_tag( 'Other' ) );
}

if ( $mod['plan_classes'] ) {
$mod['plan_classes'] = explode( ',', $mod['plan_classes'] );
$mod['plan_classes'] = array_map( 'strtolower', array_map( 'trim', $mod['plan_classes'] ) );
} else {
$mod['plan_classes'] = array( 'free' );
}

if ( $mod['feature'] ) {
$mod['feature'] = explode( ',', $mod['feature'] );
$mod['feature'] = array_map( 'trim', $mod['feature'] );
Expand Down Expand Up @@ -2840,6 +2852,12 @@ public static function activate_module( $module, $exit = true, $redirect = true
}
}

$plan = Jetpack::get_active_plan();

if ( ! in_array( $module, $plan['supports'] ) ) {
return false;
}

// Check the file for fatal errors, a la wp-admin/plugins.php::activate
Jetpack::state( 'module', $module );
Jetpack::state( 'error', 'module_activation_failed' ); // we'll override this later if the plugin can be included without fatal error
Expand Down Expand Up @@ -2876,7 +2894,7 @@ public static function activate_module( $module, $exit = true, $redirect = true
}

function activate_module_actions( $module ) {
_deprecated_function( __METHOD__, 'jeptack-4.2' );
_deprecated_function( __METHOD__, 'jetpack-4.2' );
}

public static function deactivate_module( $module ) {
Expand Down Expand Up @@ -4033,7 +4051,9 @@ function admin_page_load() {
$module = stripslashes( $_GET['module'] );
check_admin_referer( "jetpack_activate-$module" );
Jetpack::log( 'activate', $module );
Jetpack::activate_module( $module );
if ( ! Jetpack::activate_module( $module ) ) {
Jetpack::state( 'error', sprintf( __( 'Could not activated %s', 'jetpack' ), $module ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: Could not activate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

}
// The following two lines will rarely happen, as Jetpack::activate_module normally exits at the end.
wp_safe_redirect( Jetpack::admin_url( 'page=jetpack' ) );
exit;
Expand Down
1 change: 1 addition & 0 deletions modules/google-analytics.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* Auto Activate: No
* Feature: Engagement
* Additional Search Queries: webmaster, google, analytics, console
* Plans: business
*/

include dirname( __FILE__ ) . "/google-analytics/wp-google-analytics.php";
1 change: 0 additions & 1 deletion modules/module-headings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ function jetpack_get_module_i18n( $key ) {
'seo-tools' => array(
'name' => _x( 'SEO Tools', 'Module Name', 'jetpack' ),
'description' => _x( 'Better results on search engines and social media.', 'Module Description', 'jetpack' ),
'recommended description' => _x( 'Better results on search engines and social media.', 'Jumpstart Description', 'jetpack' ),
),

'sharedaddy' => array(
Expand Down
1 change: 1 addition & 0 deletions modules/search.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Auto Activate: No
* Feature: Search
* Additional Search Queries: search
* Plans: business
*/

require_once( dirname( __FILE__ ) . '/search/class.jetpack-search.php' );
Expand Down
1 change: 1 addition & 0 deletions modules/seo-tools.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Module Tags: Social, Appearance
* Feature: Traffic
* Additional Search Queries: search engine optimization, social preview, meta description, custom title format
* Plans: business
*/

include dirname( __FILE__ ) . '/seo-tools/jetpack-seo.php';
Expand Down
1 change: 1 addition & 0 deletions modules/videopress.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* Module Tags: Photos and Videos
* Feature: Writing
* Additional Search Queries: video, videos, videopress
* Plans: business, premium
*/

include_once dirname( __FILE__ ) . '/videopress/utility-functions.php';
Expand Down
1 change: 1 addition & 0 deletions modules/wordads.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Auto Activate: No
* Module Tags: Traffic, Appearance
* Additional Search Queries: advertising, ad codes, ads
* Plans: premium, business
*/

function jetpack_load_wordads() {
Expand Down