Skip to content

Commit f960e5b

Browse files
kraftbjkraftbj
authored
My Jetpack: Have the Protect card read the Protect page's module (#53727)
* My Jetpack: Have the Protect card read the Protect page's module The protect module is Brute force protection, so switching it off marked all of Protect inactive. Where the Jetpack plugin offers the protect-dashboard module, Protect now reads that one instead. * My Jetpack: Keep Protect on its module with the standalone plugin Activating the Protect page module would take over the standalone plugin's dashboard. Also cover deactivation with the page module on offer, and read the module through get_module_name() everywhere. * My Jetpack: Turn on the Protect page instead of installing the plugin With the Jetpack plugin offering the Protect page, activating Protect installed the standalone plugin and then switched on the protect module. Skip that install there, and resolve the module before deactivating the standalone plugin so deactivation acts on the module the user saw. * My Jetpack: Don't reactivate the standalone Protect plugin either Where the Jetpack plugin offers the Protect page, activating Protect turned an installed but inactive Jetpack Protect plugin back on. The hook now covers activation as well as installation. * My Jetpack: Rename the Protect standalone plugin test provider * My Jetpack: Check for the Protect page in one place Main_Features repeated Protect's protect-dashboard availability check, so the Features tab and the Protect card could drift apart. It now calls Protect::is_dashboard_module_available() and uses its module constant.
1 parent 51bd708 commit f960e5b

8 files changed

Lines changed: 281 additions & 36 deletions

‎projects/packages/my-jetpack/changelog/update-my-jetpack-protect-card-module-JETPACK-2884‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: patch
2+
Type: fixed
3+
4+
Protect: Keep the Protect card active when Brute force protection is off on sites with the Protect page.

‎projects/packages/my-jetpack/src/class-main-features.php‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
namespace Automattic\Jetpack\My_Jetpack;
99

10-
use Automattic\Jetpack\Modules;
1110
use Automattic\Jetpack\Plugins_Installer;
1211
use Automattic\Jetpack\Status\Host;
1312

@@ -365,7 +364,7 @@ public static function get_feature_definitions() {
365364
'icon' => 'shield',
366365
'product' => 'protect',
367366
// Not `protect`: that module is Brute Force Protection, which has its own row.
368-
'module' => 'protect-dashboard',
367+
'module' => Products\Protect::DASHBOARD_MODULE_NAME,
369368
'essential' => true,
370369
'interstitial' => '/add-protect',
371370
'free_highlights' => array(
@@ -379,9 +378,8 @@ public static function get_feature_definitions() {
379378
__( 'Instant email alerts when a threat is found', 'jetpack-my-jetpack' ),
380379
),
381380
'delivery' => array(
382-
// Only while the Jetpack plugin offers its flag-gated `protect-dashboard` module, and never on
383-
// WordPress.com Simple, whose partial bootstrap skips the flag. Not is_module(): it passes any slug.
384-
'jetpack' => ! ( new Host() )->is_wpcom_simple() && in_array( 'protect-dashboard', (array) ( new Modules() )->get_available(), true ),
381+
// Never on WordPress.com Simple, whose partial bootstrap skips the module's flag.
382+
'jetpack' => ! ( new Host() )->is_wpcom_simple() && Products\Protect::is_dashboard_module_available(),
385383
'plugin' => 'jetpack-protect',
386384
'plugin_name' => __( 'Jetpack Protect', 'jetpack-my-jetpack' ),
387385
'plugin_url' => 'https://wordpress.org/plugins/jetpack-protect/',
@@ -925,7 +923,7 @@ public static function get_hidden_modules() {
925923
$names = array_filter( array( $slug, $definition['product'] ?? '', $definition['module'] ?? '' ) );
926924
if ( array_intersect( $names, $hidden ) ) {
927925
$product_class = isset( $definition['product'] ) ? Products::get_product_class( $definition['product'] ) : null;
928-
$hidden[] = $definition['module'] ?? ( $product_class ? $product_class::$module_name : '' );
926+
$hidden[] = $definition['module'] ?? ( $product_class ? $product_class::get_module_name() : '' );
929927
}
930928
}
931929
return array_values( array_unique( array_filter( $hidden ) ) );

‎projects/packages/my-jetpack/src/products/class-hybrid-product.php‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public static function activate_plugin() {
8484
/*
8585
* Activate self-installed plugin if it's installed.
8686
*/
87-
if ( parent::is_plugin_installed() ) {
87+
if ( parent::is_plugin_installed() && static::should_use_standalone_plugin() ) {
8888
return activate_plugin( static::get_installed_plugin_filename() );
8989
}
9090

@@ -121,8 +121,9 @@ public static function do_product_specific_activation( $product_activation, $loc
121121
}
122122
}
123123

124-
if ( ! empty( static::$module_name ) ) {
125-
if ( $local && ! Main_Features::module_works_locally( static::$module_name ) ) {
124+
$module_name = static::get_module_name();
125+
if ( ! empty( $module_name ) ) {
126+
if ( $local && ! Main_Features::module_works_locally( $module_name ) ) {
126127
return true;
127128
}
128129

@@ -132,7 +133,7 @@ public static function do_product_specific_activation( $product_activation, $loc
132133
return true;
133134
}
134135

135-
$module_activation = ( new Modules() )->activate( static::$module_name, false, false );
136+
$module_activation = ( new Modules() )->activate( $module_name, false, false );
136137

137138
if ( ! $module_activation ) {
138139
return new WP_Error( 'module_activation_failed', __( 'Error activating Jetpack module', 'jetpack-my-jetpack' ) );
@@ -148,25 +149,27 @@ public static function do_product_specific_activation( $product_activation, $loc
148149
* Deactivates the product.
149150
*
150151
* In addition to the parent's plugin deactivation, also deactivates the
151-
* matching Jetpack module when `static::$module_name` is set and the
152+
* matching Jetpack module when `static::get_module_name()` returns one and the
152153
* module is saved and active — otherwise Hybrid products activated via
153154
* the Jetpack-module path stay half-on after "deactivation".
154155
*
155156
* @return bool|WP_Error True on success (matches Product::deactivate()),
156157
* WP_Error if the module deactivation failed.
157158
*/
158159
public static function deactivate() {
160+
// Before the parent call: deactivating the standalone plugin can change which module the product reads.
161+
$module_name = static::get_module_name();
159162
$result = parent::deactivate();
160163
$modules = new Modules();
161164
$saved_active_modules = (array) \Jetpack_Options::get_option( 'active_modules', array() );
162165

163166
// Modules::deactivate() can't remove a module only a plugin's filter forces on, nor a saved one that is unavailable.
164167
if (
165-
! empty( static::$module_name )
166-
&& in_array( static::$module_name, $saved_active_modules, true )
167-
&& $modules->is_active( static::$module_name )
168+
! empty( $module_name )
169+
&& in_array( $module_name, $saved_active_modules, true )
170+
&& $modules->is_active( $module_name )
168171
) {
169-
if ( ! $modules->deactivate( static::$module_name ) ) {
172+
if ( ! $modules->deactivate( $module_name ) ) {
170173
return new WP_Error(
171174
'module_deactivation_failed',
172175
__( 'Error deactivating Jetpack module', 'jetpack-my-jetpack' )
@@ -193,8 +196,9 @@ public static function install_and_activate_standalone() {
193196
* Activate the module as well, if the user has a plan
194197
* or the product does not require a plan to work
195198
*/
196-
if ( static::has_any_plan_for_product() && isset( static::$module_name ) ) {
197-
$module_activation = ( new Modules() )->activate( static::$module_name, false, false );
199+
$module_name = static::get_module_name();
200+
if ( static::has_any_plan_for_product() && isset( $module_name ) ) {
201+
$module_activation = ( new Modules() )->activate( $module_name, false, false );
198202

199203
if ( ! $module_activation ) {
200204
return new WP_Error( 'module_activation_failed', __( 'Error activating Jetpack module', 'jetpack-my-jetpack' ) );

‎projects/packages/my-jetpack/src/products/class-product.php‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ public static function get_status() {
906906
}
907907
// Check specifically for inactive modules, which will prevent a product from being active
908908
// Bundles have no module of their own, so they fall through to the plan checks below.
909-
} elseif ( static::$module_name && ! static::is_bundle_product() && ! static::is_module_active() ) {
909+
} elseif ( static::get_module_name() && ! static::is_bundle_product() && ! static::is_module_active() ) {
910910
$status = Products::STATUS_MODULE_DISABLED;
911911
// If there is not a plan associated with the disabled module, encourage a plan first
912912
// Getting a plan set up should help resolve any connection issues
@@ -995,12 +995,24 @@ public static function is_jetpack_plugin_active() {
995995
* @return bool
996996
*/
997997
public static function is_module_active() {
998-
if ( static::$module_name ) {
999-
return ( new Modules() )->is_active( static::$module_name );
998+
$module_name = static::get_module_name();
999+
if ( $module_name ) {
1000+
return ( new Modules() )->is_active( $module_name );
10001001
}
10011002
return true;
10021003
}
10031004

1005+
/**
1006+
* Get the Jetpack module that switches the product on, if any.
1007+
*
1008+
* @since $$next-version$$
1009+
*
1010+
* @return ?string
1011+
*/
1012+
public static function get_module_name() {
1013+
return static::$module_name;
1014+
}
1015+
10041016
/**
10051017
* Activates the plugin
10061018
*
@@ -1021,7 +1033,7 @@ private static function do_activation() {
10211033
}
10221034

10231035
// Default to installing the standalone plugin for the product
1024-
if ( ! self::is_plugin_installed() ) {
1036+
if ( ! self::is_plugin_installed() && static::should_use_standalone_plugin() ) {
10251037
$installed = Plugins_Installer::install_plugin( static::get_plugin_slug() );
10261038
if ( is_wp_error( $installed ) ) {
10271039
return $installed;
@@ -1040,6 +1052,17 @@ private static function do_activation() {
10401052
return true;
10411053
}
10421054

1055+
/**
1056+
* Whether activating the product installs and activates its standalone plugin, rather than relying on the Jetpack plugin.
1057+
*
1058+
* @since $$next-version$$
1059+
*
1060+
* @return bool
1061+
*/
1062+
public static function should_use_standalone_plugin() {
1063+
return true;
1064+
}
1065+
10431066
/**
10441067
* Activates the product by installing and activating its plugin
10451068
*

‎projects/packages/my-jetpack/src/products/class-protect.php‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Automattic\Jetpack\My_Jetpack\Products;
99

10+
use Automattic\Jetpack\Modules;
1011
use Automattic\Jetpack\My_Jetpack\Hybrid_Product;
1112
use Automattic\Jetpack\My_Jetpack\Wpcom_Products;
1213
use Automattic\Jetpack\Protect_Status\Status as Protect_Status;
@@ -86,12 +87,19 @@ class Protect extends Hybrid_Product {
8687
public static $slug = 'protect';
8788

8889
/**
89-
* The Jetpack module name
90+
* The Jetpack module name, used where the `protect-dashboard` module is not on offer.
9091
*
9192
* @var string
9293
*/
9394
public static $module_name = 'protect';
9495

96+
/**
97+
* The module that loads the Protect page.
98+
*
99+
* @var string
100+
*/
101+
const DASHBOARD_MODULE_NAME = 'protect-dashboard';
102+
95103
/**
96104
* The filename (id) of the plugin associated with this product.
97105
*
@@ -110,6 +118,46 @@ class Protect extends Hybrid_Product {
110118
*/
111119
public static $plugin_slug = 'jetpack-protect';
112120

121+
/**
122+
* Get the module that switches Protect on: the Protect page's, wherever the Jetpack plugin offers it.
123+
*
124+
* Not with the standalone plugin active: the page module would take over that plugin's dashboard (see JETPACK-3062).
125+
*
126+
* @since $$next-version$$
127+
*
128+
* @return string
129+
*/
130+
public static function get_module_name() {
131+
if ( ! static::is_standalone_plugin_active() && self::is_dashboard_module_available() ) {
132+
return self::DASHBOARD_MODULE_NAME;
133+
}
134+
return static::$module_name;
135+
}
136+
137+
/**
138+
* Leave the standalone plugin alone where the Jetpack plugin already delivers the Protect page.
139+
*
140+
* @since $$next-version$$
141+
*
142+
* @return bool
143+
*/
144+
public static function should_use_standalone_plugin() {
145+
return ! ( static::is_jetpack_plugin_active() && self::is_dashboard_module_available() );
146+
}
147+
148+
/**
149+
* Whether the Jetpack plugin offers the module that loads the Protect page, which only happens while its flag is on.
150+
*
151+
* Not Modules::is_module(): it passes any slug.
152+
*
153+
* @since $$next-version$$
154+
*
155+
* @return bool
156+
*/
157+
public static function is_dashboard_module_available() {
158+
return in_array( self::DASHBOARD_MODULE_NAME, (array) ( new Modules() )->get_available(), true );
159+
}
160+
113161
/**
114162
* Activate local product steps without scheduling connected modules offline.
115163
*

‎projects/packages/my-jetpack/tests/php/Hybrid_Product_Deactivate_Test.php‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,13 @@ class Hybrid_Product_Deactivate_Test extends TestCase {
4848
*/
4949
private $pre_deactivate_listener;
5050

51+
/**
52+
* Modules a single test adds to the available list.
53+
*
54+
* @var string[]
55+
*/
56+
private $extra_available_modules = array();
57+
5158
/**
5259
* Installs mock Jetpack plugin files, seeds the available-modules
5360
* option, and registers a scoped listener on
@@ -93,6 +100,7 @@ public function setUp(): void {
93100
*/
94101
public function tearDown(): void {
95102
remove_filter( 'jetpack_get_available_modules', array( $this, 'add_available_modules' ) );
103+
$this->extra_available_modules = array();
96104
if ( null !== $this->pre_deactivate_listener ) {
97105
remove_action( 'jetpack_pre_deactivate_module', $this->pre_deactivate_listener );
98106
$this->pre_deactivate_listener = null;
@@ -124,6 +132,18 @@ private function install_mock_plugins() {
124132
copy( __DIR__ . '/assets/videopress-mock-plugin.txt', WP_PLUGIN_DIR . '/jetpack-videopress/jetpack-videopress.php' );
125133
}
126134

135+
/**
136+
* Install and activate a mock of the standalone Protect plugin.
137+
*/
138+
private function activate_standalone_protect_plugin() {
139+
if ( ! file_exists( WP_PLUGIN_DIR . '/jetpack-protect' ) ) {
140+
mkdir( WP_PLUGIN_DIR . '/jetpack-protect', 0777, true );
141+
}
142+
copy( __DIR__ . '/assets/protect-mock-plugin.txt', WP_PLUGIN_DIR . '/jetpack-protect/jetpack-protect.php' );
143+
wp_cache_delete( 'plugins', 'plugins' );
144+
activate_plugins( 'jetpack-protect/jetpack-protect.php' );
145+
}
146+
127147
/**
128148
* Registers the modules the Hybrid_Product subclasses declare as available.
129149
*
@@ -140,7 +160,8 @@ public function add_available_modules( $modules ) {
140160
'protect' => '1.0',
141161
'publicize' => '1.0',
142162
'stats' => '1.0',
143-
)
163+
),
164+
array_fill_keys( $this->extra_available_modules, '1.0' )
144165
);
145166
}
146167

@@ -162,28 +183,36 @@ private function set_module_active( $module ) {
162183
* $module_name. Guards against future subclasses (or overrides) that
163184
* could silently drop the module-deactivation contract.
164185
*
165-
* @return array<string, array{0: class-string, 1: string}>
186+
* @return array<string, array{0: class-string, 1: string, 2?: string[], 3?: bool}>
166187
*/
167188
public static function provide_hybrid_subclasses_with_modules() {
168189
return array(
169-
'backup' => array( Backup::class, 'backup' ),
170-
'videopress' => array( Videopress::class, 'videopress' ),
171-
'search' => array( Search::class, 'search' ),
172-
'protect' => array( Protect::class, 'protect' ),
173-
'social' => array( Social::class, 'publicize' ),
190+
'backup' => array( Backup::class, 'backup' ),
191+
'videopress' => array( Videopress::class, 'videopress' ),
192+
'search' => array( Search::class, 'search' ),
193+
'protect' => array( Protect::class, 'protect' ),
194+
'protect page' => array( Protect::class, 'protect-dashboard', array( 'protect-dashboard' ) ),
195+
'protect plugin with the page on offer' => array( Protect::class, 'protect', array( 'protect-dashboard' ), true ),
196+
'social' => array( Social::class, 'publicize' ),
174197
);
175198
}
176199

177200
/**
178201
* Every Hybrid_Product subclass with a $module_name must, when its
179202
* module is active, deactivate the module as part of ::deactivate().
180203
*
181-
* @param class-string $class Hybrid_Product subclass under test.
182-
* @param string $module Jetpack module slug for the subclass.
204+
* @param class-string $class Hybrid_Product subclass under test.
205+
* @param string $module Jetpack module slug for the subclass.
206+
* @param string[] $offered Modules to add to the available list.
207+
* @param bool $standalone Whether the standalone Protect plugin is active.
183208
* @dataProvider provide_hybrid_subclasses_with_modules
184209
*/
185210
#[DataProvider( 'provide_hybrid_subclasses_with_modules' )]
186-
public function test_deactivate_turns_off_active_module( $class, $module ) {
211+
public function test_deactivate_turns_off_active_module( $class, $module, $offered = array(), $standalone = false ) {
212+
$this->extra_available_modules = $offered;
213+
if ( $standalone ) {
214+
$this->activate_standalone_protect_plugin();
215+
}
187216
$this->set_module_active( $module );
188217
$this->assertTrue( ( new Modules() )->is_active( $module ) );
189218

‎projects/packages/my-jetpack/tests/php/Plan_Matrix_Test.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -514,18 +514,19 @@ private function activate_standalone_plugin( string $class ) {
514514
* @param string $class Product class name.
515515
*/
516516
private function activate_module( string $class ) {
517-
if ( ! $class::$module_name ) {
517+
$module_name = $class::get_module_name();
518+
if ( ! $module_name ) {
518519
return;
519520
}
520521

521522
$active = Jetpack_Options::get_option( 'active_modules', array() );
522-
if ( ! in_array( $class::$module_name, $active, true ) ) {
523-
$active[] = $class::$module_name;
523+
if ( ! in_array( $module_name, $active, true ) ) {
524+
$active[] = $module_name;
524525
}
525526
Jetpack_Options::update_option( 'active_modules', $active );
526527

527528
// @phan-suppress-next-line PhanUndeclaredStaticProperty -- Declared on the mock in ./assets/jetpack-mock-plugin.txt
528-
\Jetpack::$active_modules[] = $class::$module_name;
529+
\Jetpack::$active_modules[] = $module_name;
529530
}
530531

531532
/**

0 commit comments

Comments
 (0)