Monitor: Store the downtime email setting per user

projects/plugins/jetpack/_inc/lib/class.core-rest-api-endpoints.php

@@ -3572,7 +3572,8 @@ public static function cast_value( $value, $definition ) {
 	 */
 	public static function get_remote_value( $module, $option ) {
 
-		if ( in_array( $module, array( 'post-by-email' ), true ) ) {
+		// These are per-user values on WordPress.com, so each user gets their own local copy.
+		if ( in_array( $module, array( 'monitor', 'post-by-email' ), true ) ) {
 			$option .= get_current_user_id();
 		}
 
@@ -3597,6 +3598,12 @@ public static function get_remote_value( $module, $option ) {
 					return false;
 				}
 				$value = Jetpack_Monitor::user_receives_notifications( false );
+				// Not saved: the user may not be connected yet, and would be stuck with this answer.
+				if ( is_wp_error( $value ) ) {
+					return false;
+				}
+				// update_option() won't create an option whose value is `false`.
+				$value = (int) (bool) $value;
 				break;
 
 			case 'post-by-email':
@@ -3611,11 +3618,6 @@ public static function get_remote_value( $module, $option ) {
 				break;
 		}
 
-		// Normalize value to boolean.
-		if ( is_wp_error( $value ) || $value === null ) {
-			$value = false;
-		}
-
 		// Save option to use it next time.
 		update_option( $option, $value );
 

projects/plugins/jetpack/changelog/fix-monitor-notifications-per-user

@@ -0,0 +1,4 @@
+Significance: patch
+Type: bugfix
+
+Monitor: Show each user their own downtime email setting instead of the one another admin saved last.

projects/plugins/jetpack/modules/monitor.php

@@ -41,6 +41,63 @@ class Jetpack_Monitor {
 	public function __construct() {
 		add_action( 'jetpack_modules_loaded', array( $this, 'jetpack_modules_loaded' ) );
 		add_action( 'jetpack_activate_module_monitor', array( $this, 'activate_module' ) );
+		add_action( 'jetpack_unlinked_user', array( $this, 'delete_notifications_copy' ) );
+		add_action( 'deleted_user', array( $this, 'delete_notifications_copy' ) );
+		add_action( 'jetpack_site_before_disconnected', array( $this, 'delete_all_notifications_copies' ) );
+	}
+
+	/**
+	 * Forgets every linked user's saved notification choice, while a site disconnect can still say who was linked.
+	 *
+	 * @since $$next-version$$
+	 *
+	 * @return void
+	 */
+	public function delete_all_notifications_copies() {
+		$user_tokens = Jetpack_Options::get_option( 'user_tokens' );
+		foreach ( array_keys( is_array( $user_tokens ) ? $user_tokens : array() ) as $user_id ) {
+			$this->delete_notifications_copy( $user_id );
+		}
+	}
+
+	/**
+	 * Forgets a user's saved notification choice, which belongs to the WordPress.com account they had linked.
+	 *
+	 * @since $$next-version$$
+	 *
+	 * @param int $user_id User ID.
+	 *
+	 * @return void
+	 */
+	public function delete_notifications_copy( $user_id ) {
+		delete_option( self::notifications_copy_option_name( $user_id ) );
+	}
+
+	/**
+	 * Name of the option holding a user's local copy of their notification choice.
+	 *
+	 * @since $$next-version$$
+	 *
+	 * @param int $user_id User ID.
+	 *
+	 * @return string
+	 */
+	public static function notifications_copy_option_name( $user_id ) {
+		return 'monitor_receive_notifications' . (int) $user_id;
+	}
+
+	/**
+	 * Saves the current user's notification choice where Jetpack_Core_Json_Api_Endpoints::get_remote_value reads it.
+	 *
+	 * @since $$next-version$$
+	 *
+	 * @param bool $enabled Whether the current user receives notifications.
+	 *
+	 * @return void
+	 */
+	public static function save_notifications_copy( $enabled ) {
+		// An integer: see Jetpack_Core_Json_Api_Endpoints::get_remote_value.
+		update_option( self::notifications_copy_option_name( get_current_user_id() ), (int) (bool) $enabled );
 	}
 
 	/**
@@ -82,8 +139,7 @@ public function update_option_receive_jetpack_monitor_notification( $value ) {
 			wp_die( sprintf( '%s: %s', esc_html( $xml->getErrorCode() ), esc_html( $xml->getErrorMessage() ) ) );
 		}
 
-		// To be used only in Jetpack_Core_Json_Api_Endpoints::get_remote_value.
-		update_option( 'monitor_receive_notifications', (bool) $value );
+		self::save_notifications_copy( $value );
 
 		return true;
 	}

projects/plugins/jetpack/src/abilities/class-monitor-abilities.php

@@ -13,6 +13,7 @@
 use Automattic\Jetpack\WP_Abilities\Registrar;
 use Jetpack;
 use Jetpack_IXR_Client;
+use Jetpack_Monitor;
 
 if ( ! defined( 'ABSPATH' ) ) {
 	exit( 0 );
@@ -225,10 +226,10 @@ public static function get_monitor_status( $input = null ) { // phpcs:ignore Var
 
 	/**
 	 * Execute: declarative state-setter. Idempotent — compares desired vs current
-	 * and returns changed=false when they match. Either way the local
-	 * `monitor_receive_notifications` option is synced to the remote value (after
-	 * the write on a change, and on the no-op path) so the legacy REST reader,
-	 * which trusts that option first, never reports a stale state.
+	 * and returns changed=false when they match. Either way the current user's
+	 * local copy of the value is synced to the remote one (after the write on a
+	 * change, and on the no-op path) so the legacy REST reader, which trusts that
+	 * copy first, never reports a stale state.
 	 *
 	 * @param array|null $input Input matching the ability's input_schema.
 	 * @return array|\WP_Error
@@ -270,14 +271,8 @@ public static function set_notifications( $input = null ) {
 		}
 
 		if ( $desired === $current ) {
-			// Sync the local `monitor_receive_notifications` option to the
-			// known-good remote value even on a no-op. The legacy
-			// `Jetpack_Core_Json_Api_Endpoints::get_remote_value` reader trusts
-			// this option before falling back to a remote read, so a stale local
-			// value would let it report the wrong state. The changed=true path
-			// below mirrors the option after a write; mirroring here keeps the
-			// unchanged path self-healing too.
-			update_option( 'monitor_receive_notifications', $current );
+			// Sync even on a no-op, so a stale local copy heals itself.
+			Jetpack_Monitor::save_notifications_copy( $current );
 
 			return array(
 				'enabled' => $current,
@@ -290,10 +285,8 @@ public static function set_notifications( $input = null ) {
 			return $applied;
 		}
 
-		// Mirror the write to the `monitor_receive_notifications` option so the
-		// legacy `Jetpack_Core_Json_Api_Endpoints::get_remote_value` reader — the
-		// only other reader of this option — stays in sync with the remote state.
-		update_option( 'monitor_receive_notifications', $desired );
+		// Keep the legacy REST reader's local copy in sync with the write.
+		Jetpack_Monitor::save_notifications_copy( $desired );
 
 		return array(
 			'enabled' => $desired,

projects/plugins/jetpack/tests/php/_inc/lib/Jetpack_REST_API_endpoints_Test.php

@@ -6,6 +6,7 @@
  * @package automattic/jetpack
  */
 
+use Automattic\Jetpack\Connection\Manager as Connection_Manager;
 use Automattic\Jetpack\Connection\REST_Connector;
 use Automattic\Jetpack\Status\Cache as StatusCache;
 use PHPUnit\Framework\Attributes\DataProvider;
@@ -1052,6 +1053,114 @@ public function test_settings_retrieve() {
 		$this->assertTrue( $response_data['carousel'] );
 	}
 
+	/**
+	 * Each admin reads their own Monitor email choice, not the one another admin saved.
+	 */
+	public function test_settings_retrieve_monitor_notifications_per_user() {
+		$subscribed   = $this->create_and_get_user( 'administrator' );
+		$unsubscribed = $this->create_and_get_user( 'administrator' );
+		update_option( 'monitor_receive_notifications' . $subscribed->ID, 1 );
+		update_option( 'monitor_receive_notifications' . $unsubscribed->ID, 0 );
+
+		wp_set_current_user( $subscribed->ID );
+		$this->assertTrue( $this->create_and_get_request( 'settings' )->get_data()['monitor_receive_notifications'] );
+
+		wp_set_current_user( $unsubscribed->ID );
+		$this->assertFalse( $this->create_and_get_request( 'settings' )->get_data()['monitor_receive_notifications'] );
+	}
+
+	/**
+	 * A first read saves what WordPress.com answered, and saves nothing when the lookup fails.
+	 *
+	 * @dataProvider provider_monitor_notification_lookups
+	 *
+	 * @param string|null $answer   XML-RPC boolean WordPress.com answers with, or null for a failed request.
+	 * @param string      $expected The saved copy afterwards.
+	 */
+	#[DataProvider( 'provider_monitor_notification_lookups' )]
+	public function test_settings_retrieve_monitor_notifications_remote_lookup( $answer, $expected ) {
+		$user = $this->create_and_get_user( 'administrator' );
+		wp_set_current_user( $user->ID );
+		Jetpack_Options::update_option( 'id', 1234 );
+		Jetpack_Options::update_option( 'blog_token', 'asd.qwe.1' );
+		Jetpack_Options::update_option( 'user_tokens', array( $user->ID => "honey.badger.$user->ID" ) );
+
+		add_filter(
+			'pre_http_request',
+			static function () use ( $answer ) {
+				if ( null === $answer ) {
+					return new WP_Error( 'http_request_failed', 'Could not connect.' );
+				}
+				return array(
+					'body'     => "<methodResponse><params><param><value><boolean>$answer</boolean></value></param></params></methodResponse>",
+					'response' => array( 'code' => 200 ),
+				);
+			}
+		);
+
+		$this->assertSame( '1' === $answer, $this->create_and_get_request( 'settings' )->get_data()['monitor_receive_notifications'] );
+		$this->assertSame( $expected, (string) get_option( 'monitor_receive_notifications' . $user->ID, 'not saved' ) );
+	}
+
+	public static function provider_monitor_notification_lookups() {
+		return array(
+			'subscribed'    => array( '1', '1' ),
+			'unsubscribed'  => array( '0', '0' ),
+			'failed lookup' => array( null, 'not saved' ),
+		);
+	}
+
+	/**
+	 * Unlinking or deleting a user forgets their saved Monitor email choice.
+	 *
+	 * @dataProvider provider_monitor_forgetting_hooks
+	 *
+	 * @param string $hook Action that fires with the user ID.
+	 */
+	#[DataProvider( 'provider_monitor_forgetting_hooks' )]
+	public function test_monitor_notifications_copy_is_forgotten( $hook ) {
+		require_once JETPACK__PLUGIN_DIR . 'modules/monitor.php';
+		new Jetpack_Monitor();
+		$user = $this->create_and_get_user( 'administrator' );
+		update_option( 'monitor_receive_notifications' . $user->ID, 1 );
+
+		// Core's `deleted_user` passes three arguments, and its own multisite callback requires them.
+		do_action( $hook, $user->ID, null, $user );
+
+		$this->assertFalse( get_option( 'monitor_receive_notifications' . $user->ID ) );
+	}
+
+	public static function provider_monitor_forgetting_hooks() {
+		return array(
+			'unlinked' => array( 'jetpack_unlinked_user' ),
+			'deleted'  => array( 'deleted_user' ),
+		);
+	}
+
+	/**
+	 * Disconnecting the site forgets the saved Monitor email choice of every linked user.
+	 */
+	public function test_monitor_notifications_copies_are_forgotten_on_site_disconnect() {
+		require_once JETPACK__PLUGIN_DIR . 'modules/monitor.php';
+		new Jetpack_Monitor();
+		$first  = $this->create_and_get_user( 'administrator' );
+		$second = $this->create_and_get_user( 'administrator' );
+		Jetpack_Options::update_option(
+			'user_tokens',
+			array(
+				$first->ID  => "honey.badger.$first->ID",
+				$second->ID => "honey.badger.$second->ID",
+			)
+		);
+		update_option( 'monitor_receive_notifications' . $first->ID, 1 );
+		update_option( 'monitor_receive_notifications' . $second->ID, 0 );
+
+		( new Connection_Manager( 'jetpack' ) )->disconnect_site( false );
+
+		$this->assertFalse( get_option( 'monitor_receive_notifications' . $first->ID ) );
+		$this->assertFalse( get_option( 'monitor_receive_notifications' . $second->ID ) );
+	}
+
 	/**
 	 * Test fetching milestone widget data.
 	 *

projects/plugins/jetpack/tests/php/src/Monitor_Abilities_Test.php

@@ -36,6 +36,9 @@ class Monitor_Abilities_Test extends WP_UnitTestCase {
 	public function set_up() {
 		parent::set_up();
 
+		// The abilities save through Jetpack_Monitor, which only loads with the module.
+		require_once JETPACK__PLUGIN_DIR . 'modules/monitor.php';
+
 		$this->admin_id      = wp_insert_user(
 			array(
 				'user_login' => 'monitor_abilities_admin_' . wp_generate_password( 8, false, false ),
@@ -79,7 +82,7 @@ public function tear_down() {
 		// registrar never registers and must never tear down.
 
 		delete_transient( 'monitor_last_downtime' );
-		delete_option( 'monitor_receive_notifications' );
+		delete_option( 'monitor_receive_notifications' . $this->admin_id );
 
 		parent::tear_down();
 	}
@@ -460,8 +463,13 @@ static function ( $mods ) {
 	 * `Monitor_Abilities_Test_Stub` test double (defined below this class) overrides
 	 * the protected seams so we can drive `set_notifications()` through its
 	 * end-to-end logic without standing up a Jetpack token fixture.
+	 *
+	 * @dataProvider provider_flipped_states
+	 *
+	 * @param bool $desired The state to flip to.
 	 */
-	public function test_set_notifications_returns_changed_true_when_state_flips() {
+	#[DataProvider( 'provider_flipped_states' )]
+	public function test_set_notifications_returns_changed_true_when_state_flips( bool $desired ) {
 		wp_set_current_user( $this->admin_id );
 
 		add_filter(
@@ -473,18 +481,24 @@ static function ( $mods ) {
 			}
 		);
 
-		Monitor_Abilities_Test_Stub::reset( false );
+		Monitor_Abilities_Test_Stub::reset( ! $desired );
 
-		$result = Monitor_Abilities_Test_Stub::set_notifications( array( 'enabled' => true ) );
+		$result = Monitor_Abilities_Test_Stub::set_notifications( array( 'enabled' => $desired ) );
 
 		$this->assertIsArray( $result );
-		$this->assertTrue( $result['enabled'] );
+		$this->assertSame( $desired, $result['enabled'] );
 		$this->assertTrue( $result['changed'] );
 		$this->assertSame( 1, Monitor_Abilities_Test_Stub::$apply_calls, 'IXR setNotifications should have been called exactly once.' );
-		$this->assertTrue( Monitor_Abilities_Test_Stub::$last_applied, 'apply_notifications_update should have been called with the desired value.' );
-		// `update_option` stores booleans as '1'/'' through the DB roundtrip; assert truthiness
-		// rather than the exact string so this test is robust to env differences.
-		$this->assertTrue( (bool) get_option( 'monitor_receive_notifications' ), 'monitor_receive_notifications option should mirror the new state.' );
+		$this->assertSame( $desired, Monitor_Abilities_Test_Stub::$last_applied, 'apply_notifications_update should have been called with the desired value.' );
+		$this->assertSame( $desired ? '1' : '0', (string) get_option( 'monitor_receive_notifications' . $this->admin_id, 'not stored' ), 'The user\'s monitor_receive_notifications option should mirror the new state.' );
+		$this->assertFalse( get_option( 'monitor_receive_notifications' ), 'The value must not be stored where other users would read it.' );
+	}
+
+	public static function provider_flipped_states(): array {
+		return array(
+			'off to on' => array( true ),
+			'on to off' => array( false ),
+		);
 	}
 
 	public function test_set_notifications_returns_changed_false_when_state_matches() {
@@ -503,7 +517,7 @@ static function ( $mods ) {
 
 		// Seed a stale local option (false) that disagrees with the remote state
 		// (true) to exercise the self-healing sync on the no-op path.
-		update_option( 'monitor_receive_notifications', false );
+		update_option( 'monitor_receive_notifications' . $this->admin_id, 0 );
 
 		$result = Monitor_Abilities_Test_Stub::set_notifications( array( 'enabled' => true ) );
 
@@ -514,7 +528,7 @@ static function ( $mods ) {
 		// Even on a no-op the local option is synced to the known-good remote
 		// value (true here) so the legacy REST reader, which trusts this option
 		// before going remote, can't surface a stale state.
-		$this->assertTrue( (bool) get_option( 'monitor_receive_notifications' ), 'No-op should still sync the mirrored option to the remote value.' );
+		$this->assertSame( '1', (string) get_option( 'monitor_receive_notifications' . $this->admin_id ), 'No-op should still sync the mirrored option to the remote value.' );
 	}
 
 	/**