From ba2fbcdd7632f252651bd065359c10899ab1e44a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 22:02:14 +0000 Subject: [PATCH 01/11] feat(search): add backend support for experience field (RSM-2291) - Add SEARCH_MODULE_EXPERIENCE_OPTION_KEY constant and experience value constants to Module_Control - Add get_experience() method that returns persisted value or derives from legacy booleans - Add update_experience() method that writes experience and legacy booleans in lockstep - Update REST update_settings() to accept experience-only requests - Update REST get_settings() to include last_saved_experience in response - Update validate_search_settings() to allow experience-only requests - Update Initial_State to include last_saved_experience - Update existing tests to include last_saved_experience in expected responses - Add new tests for experience read/write paths - Add changelog entry" Agent-Logs-Url: https://github.com/Automattic/jetpack/sessions/40f5e762-e4d0-4932-b4ac-5bd0ec01f26b Co-authored-by: adamwoodnz <1017872+adamwoodnz@users.noreply.github.com> --- .../add-rsm-2291-experience-field-backend | 4 + .../search/src/class-module-control.php | 85 +++++++++++++ .../search/src/class-rest-controller.php | 27 +++- .../src/dashboard/class-initial-state.php | 1 + .../search/tests/php/Module_Control_Test.php | 119 +++++++++++++++++ .../search/tests/php/REST_Controller_Test.php | 120 +++++++++++++++++- 6 files changed, 349 insertions(+), 7 deletions(-) create mode 100644 projects/packages/search/changelog/add-rsm-2291-experience-field-backend diff --git a/projects/packages/search/changelog/add-rsm-2291-experience-field-backend b/projects/packages/search/changelog/add-rsm-2291-experience-field-backend new file mode 100644 index 000000000000..52f0a52d3acc --- /dev/null +++ b/projects/packages/search/changelog/add-rsm-2291-experience-field-backend @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Search: Add backend support for the `experience` field in the search settings REST endpoint. The `POST /jetpack/v4/search/settings` endpoint now accepts `experience` (`embedded`, `overlay`, `classic`, or `off`) and persists it. The `GET /jetpack/v4/search/settings` response now includes `last_saved_experience`, derived from legacy settings for sites that have not yet saved via the new UI. diff --git a/projects/packages/search/src/class-module-control.php b/projects/packages/search/src/class-module-control.php index 4044765be1cc..62e39f47dc22 100644 --- a/projects/packages/search/src/class-module-control.php +++ b/projects/packages/search/src/class-module-control.php @@ -41,6 +41,15 @@ class Module_Control { const JETPACK_SEARCH_MODULE_SLUG = 'search'; const SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY = 'instant_search_enabled'; const SEARCH_MODULE_SWAP_CLASSIC_TO_INLINE_OPTION_KEY = 'swap_classic_to_inline_search'; + const SEARCH_MODULE_EXPERIENCE_OPTION_KEY = 'jetpack_search_experience'; + + /** + * Valid experience values. + */ + const EXPERIENCE_OVERLAY = 'overlay'; + const EXPERIENCE_EMBEDDED = 'embedded'; + const EXPERIENCE_CLASSIC = 'classic'; + const EXPERIENCE_OFF = 'off'; /** * Contructor @@ -170,6 +179,82 @@ public function update_swap_classic_to_inline_search( bool $swap_classic_to_inli return update_option( self::SEARCH_MODULE_SWAP_CLASSIC_TO_INLINE_OPTION_KEY, $swap_classic_to_inline_search ); } + /** + * Get the active search experience. + * + * Returns the persisted experience if one has been saved via update_experience(), + * otherwise derives from the legacy module_active / instant_search_enabled booleans. + * + * @return string One of 'overlay', 'embedded', 'classic', 'off'. + */ + public function get_experience() { + $saved = get_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, false ); + if ( $saved ) { + return $saved; + } + + // Derive from legacy booleans. + if ( ! $this->is_active() ) { + return self::EXPERIENCE_OFF; + } + + if ( $this->is_instant_search_enabled() ) { + return self::EXPERIENCE_OVERLAY; + } + + // Cannot distinguish embedded from classic without a saved value; default to classic. + return self::EXPERIENCE_CLASSIC; + } + + /** + * Update the search experience and keep legacy booleans in lockstep. + * + * @param string $experience One of 'overlay', 'embedded', 'classic', 'off'. + * @return bool|WP_Error True on success, WP_Error on failure. + */ + public function update_experience( string $experience ) { + $valid_values = array( self::EXPERIENCE_OVERLAY, self::EXPERIENCE_EMBEDDED, self::EXPERIENCE_CLASSIC, self::EXPERIENCE_OFF ); + if ( ! in_array( $experience, $valid_values, true ) ) { + return new WP_Error( + 'invalid_experience', + esc_html__( 'Invalid experience value.', 'jetpack-search-pkg' ), + array( 'status' => 400 ) + ); + } + + switch ( $experience ) { + case self::EXPERIENCE_OVERLAY: + $result = $this->activate(); + if ( is_wp_error( $result ) ) { + return $result; + } + $result = $this->enable_instant_search(); + if ( is_wp_error( $result ) ) { + return $result; + } + break; + + case self::EXPERIENCE_EMBEDDED: + case self::EXPERIENCE_CLASSIC: + $result = $this->activate(); + if ( is_wp_error( $result ) ) { + return $result; + } + $this->disable_instant_search(); + break; + + case self::EXPERIENCE_OFF: + // Only deactivate the module; leave instant_search_enabled unchanged so the + // user's overlay-vs-embedded preference is preserved if they re-enable later. + ( new Modules() )->deactivate( self::JETPACK_SEARCH_MODULE_SLUG ); + break; + } + + update_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, $experience ); + + return true; + } + /** * Get a list of activated modules as an array of module slugs. * diff --git a/projects/packages/search/src/class-rest-controller.php b/projects/packages/search/src/class-rest-controller.php index 0b5b144bb3b6..d96769610d2d 100644 --- a/projects/packages/search/src/class-rest-controller.php +++ b/projects/packages/search/src/class-rest-controller.php @@ -246,13 +246,24 @@ public function update_settings( $request ) { $module_active = isset( $request_body['module_active'] ) ? (bool) $request_body['module_active'] : null; $instant_search_enabled = isset( $request_body['instant_search_enabled'] ) ? (bool) $request_body['instant_search_enabled'] : null; $swap_classic_to_inline_search = isset( $request_body['swap_classic_to_inline_search'] ) ? (bool) $request_body['swap_classic_to_inline_search'] : null; + $experience = isset( $request_body['experience'] ) ? sanitize_text_field( $request_body['experience'] ) : null; - $error = $this->validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search ); + $error = $this->validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search, $experience ); if ( is_wp_error( $error ) ) { return $error; } + // If an experience value was provided, delegate to Module_Control::update_experience() + // which also writes the legacy booleans in lockstep. + if ( $experience !== null ) { + $result = $this->search_module->update_experience( $experience ); + if ( is_wp_error( $result ) ) { + return $result; + } + return rest_ensure_response( $this->get_settings() ); + } + // Enabling instant search should enable the module too. if ( true === $instant_search_enabled && true !== $module_active ) { $module_active = true; @@ -298,11 +309,16 @@ public function update_settings( $request ) { /** * Validate $module_active and $instant_search_enabled. Returns an WP_Error instance if invalid. * - * @param boolean $module_active - Module status. - * @param boolean $instant_search_enabled - Instant Search status. - * @param boolean $swap_classic_to_inline_search - New inline search status. + * @param boolean $module_active - Module status. + * @param boolean $instant_search_enabled - Instant Search status. + * @param boolean $swap_classic_to_inline_search - New inline search status. + * @param string|null $experience - Experience value. */ - protected function validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search ) { + protected function validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search, $experience = null ) { + // An experience-only request is always valid; update_experience() validates the value. + if ( $experience !== null ) { + return true; + } if ( $module_active === null && $instant_search_enabled === null && $swap_classic_to_inline_search !== null ) { // allow updating 'swap_classic_to_inline_search' without updating/validating other settings. return true; @@ -326,6 +342,7 @@ public function get_settings() { 'module_active' => $this->search_module->is_active(), 'instant_search_enabled' => $this->search_module->is_instant_search_enabled(), 'swap_classic_to_inline_search' => $this->search_module->is_swap_classic_to_inline_search(), + 'last_saved_experience' => $this->search_module->get_experience(), ) ); } diff --git a/projects/packages/search/src/dashboard/class-initial-state.php b/projects/packages/search/src/dashboard/class-initial-state.php index 0235176af93a..0d92f58d4192 100644 --- a/projects/packages/search/src/dashboard/class-initial-state.php +++ b/projects/packages/search/src/dashboard/class-initial-state.php @@ -92,6 +92,7 @@ public function get_initial_state() { 'jetpackSettings' => array( 'search' => $this->module_control->is_active(), 'instant_search_enabled' => $this->module_control->is_instant_search_enabled(), + 'last_saved_experience' => $this->module_control->get_experience(), ), 'features' => array_map( 'sanitize_text_field', diff --git a/projects/packages/search/tests/php/Module_Control_Test.php b/projects/packages/search/tests/php/Module_Control_Test.php index 32e4bc184e2f..11c972473cbe 100644 --- a/projects/packages/search/tests/php/Module_Control_Test.php +++ b/projects/packages/search/tests/php/Module_Control_Test.php @@ -167,6 +167,125 @@ public function test_disable_instant_search() { $this->assertFalse( get_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ) ); } + /** + * Test get_experience() derivation from legacy booleans when no experience option is saved. + */ + public function test_get_experience_derived_off() { + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + // Module is inactive → 'off'. + $this->assertEquals( Module_Control::EXPERIENCE_OFF, static::$search_module->get_experience() ); + } + + /** + * Test get_experience() derivation: module active + instant search enabled → 'overlay'. + */ + public function test_get_experience_derived_overlay() { + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); + update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, true ); + $this->assertEquals( Module_Control::EXPERIENCE_OVERLAY, static::$search_module->get_experience() ); + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); + } + + /** + * Test get_experience() derivation: module active + instant search disabled → 'classic'. + */ + public function test_get_experience_derived_classic() { + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); + update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, false ); + $this->assertEquals( Module_Control::EXPERIENCE_CLASSIC, static::$search_module->get_experience() ); + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); + } + + /** + * Test get_experience() returns persisted value over derived. + */ + public function test_get_experience_returns_persisted() { + update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_EMBEDDED ); + $this->assertEquals( Module_Control::EXPERIENCE_EMBEDDED, static::$search_module->get_experience() ); + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + } + + /** + * Test update_experience() with 'overlay'. + */ + public function test_update_experience_overlay() { + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + // Use the filter that includes search so that is_active() returns true during + // enable_instant_search(), which requires the module to be active. + add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); + static::$search_module->update_experience( Module_Control::EXPERIENCE_OVERLAY ); + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + + $this->assertTrue( static::$search_module->is_instant_search_enabled() ); + $this->assertEquals( Module_Control::EXPERIENCE_OVERLAY, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + } + + /** + * Test update_experience() with 'embedded'. + */ + public function test_update_experience_embedded() { + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + add_filter( 'jetpack_options', array( $this, 'return_active_modules_array_without_search' ), 10, 2 ); + static::$search_module->update_experience( Module_Control::EXPERIENCE_EMBEDDED ); + $active_modules = get_option( 'jetpack_' . Module_Control::JETPACK_ACTIVE_MODULES_OPTION_KEY, array() ); + remove_filter( 'jetpack_options', array( $this, 'return_active_modules_array_without_search' ) ); + + $this->assertContains( Module_Control::JETPACK_SEARCH_MODULE_SLUG, $active_modules ); + $this->assertFalse( static::$search_module->is_instant_search_enabled() ); + $this->assertEquals( Module_Control::EXPERIENCE_EMBEDDED, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + } + + /** + * Test update_experience() with 'classic'. + */ + public function test_update_experience_classic() { + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + add_filter( 'jetpack_options', array( $this, 'return_active_modules_array_without_search' ), 10, 2 ); + static::$search_module->update_experience( Module_Control::EXPERIENCE_CLASSIC ); + $active_modules = get_option( 'jetpack_' . Module_Control::JETPACK_ACTIVE_MODULES_OPTION_KEY, array() ); + remove_filter( 'jetpack_options', array( $this, 'return_active_modules_array_without_search' ) ); + + $this->assertContains( Module_Control::JETPACK_SEARCH_MODULE_SLUG, $active_modules ); + $this->assertFalse( static::$search_module->is_instant_search_enabled() ); + $this->assertEquals( Module_Control::EXPERIENCE_CLASSIC, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + } + + /** + * Test update_experience() with 'off' deactivates module but preserves instant_search_enabled. + */ + public function test_update_experience_off_preserves_instant_search() { + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + // Start with module active and instant search enabled. + add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); + update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, true ); + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + + static::$search_module->update_experience( Module_Control::EXPERIENCE_OFF ); + + $this->assertFalse( static::$search_module->is_active() ); + // instant_search_enabled should remain true (preserved for later re-enable). + $this->assertTrue( (bool) get_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ) ); + $this->assertEquals( Module_Control::EXPERIENCE_OFF, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); + } + + /** + * Test update_experience() with an invalid value returns WP_Error. + */ + public function test_update_experience_invalid_value() { + $result = static::$search_module->update_experience( 'invalid_value' ); + $this->assertInstanceOf( \WP_Error::class, $result ); + $this->assertEquals( 'invalid_experience', $result->get_error_code() ); + } + /** * Returns an empty array */ diff --git a/projects/packages/search/tests/php/REST_Controller_Test.php b/projects/packages/search/tests/php/REST_Controller_Test.php index a95c2cbd143f..dd58b84821e5 100644 --- a/projects/packages/search/tests/php/REST_Controller_Test.php +++ b/projects/packages/search/tests/php/REST_Controller_Test.php @@ -119,13 +119,14 @@ public function test_update_search_settings_success_both_enable() { 'instant_search_enabled' => true, 'swap_classic_to_inline_search' => false, ); + $expected = array_merge( $new_settings, array( 'last_saved_experience' => 'overlay' ) ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); $request->set_header( 'content-type', 'application/json' ); $request->set_body( wp_json_encode( $new_settings, JSON_UNESCAPED_SLASHES ) ); $response = $this->server->dispatch( $request ); $this->assertEquals( 200, $response->get_status() ); - $this->assertEquals( $new_settings, $response->get_data() ); + $this->assertEquals( $expected, $response->get_data() ); } /** @@ -170,13 +171,14 @@ public function test_update_search_settings_success_both_disable() { 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => false, ); + $expected = array_merge( $new_settings, array( 'last_saved_experience' => 'off' ) ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); $request->set_header( 'content-type', 'application/json' ); $request->set_body( wp_json_encode( $new_settings, JSON_UNESCAPED_SLASHES ) ); $response = $this->server->dispatch( $request ); $this->assertEquals( 200, $response->get_status() ); - $this->assertEquals( $new_settings, $response->get_data() ); + $this->assertEquals( $expected, $response->get_data() ); } /** @@ -191,6 +193,7 @@ public function test_update_search_settings_success_disable_module_only() { 'module_active' => false, 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => false, + 'last_saved_experience' => 'off', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -213,6 +216,7 @@ public function test_update_search_settings_success_disable_instant_only() { 'module_active' => true, 'instant_search_enabled' => true, 'swap_classic_to_inline_search' => false, + 'last_saved_experience' => 'overlay', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -235,6 +239,7 @@ public function test_update_search_settings_success_enable_inline_search() { 'module_active' => false, 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => true, + 'last_saved_experience' => 'off', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -257,6 +262,7 @@ public function test_update_search_settings_success_disable_inline_search() { 'module_active' => false, 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => false, + 'last_saved_experience' => 'off', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -292,6 +298,7 @@ public function test_get_search_settings_success() { $this->assertEquals( 200, $response->get_status() ); $this->assertArrayHasKey( 'module_active', $response->get_data() ); $this->assertArrayHasKey( 'instant_search_enabled', $response->get_data() ); + $this->assertArrayHasKey( 'last_saved_experience', $response->get_data() ); } /** @@ -332,6 +339,115 @@ public function test_get_search_results_success_admin() { $this->assertEquals( 6, $response->get_data()['total'] ); } + /** + * Testing the `POST /jetpack/v4/search/settings` with experience=overlay. + */ + public function test_update_settings_experience_overlay() { + wp_set_current_user( $this->admin_id ); + + $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( array( 'experience' => 'overlay' ), JSON_UNESCAPED_SLASHES ) ); + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertTrue( $data['module_active'] ); + $this->assertTrue( $data['instant_search_enabled'] ); + $this->assertEquals( 'overlay', $data['last_saved_experience'] ); + } + + /** + * Testing the `POST /jetpack/v4/search/settings` with experience=embedded. + */ + public function test_update_settings_experience_embedded() { + wp_set_current_user( $this->admin_id ); + + $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( array( 'experience' => 'embedded' ), JSON_UNESCAPED_SLASHES ) ); + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertTrue( $data['module_active'] ); + $this->assertFalse( $data['instant_search_enabled'] ); + $this->assertEquals( 'embedded', $data['last_saved_experience'] ); + } + + /** + * Testing the `POST /jetpack/v4/search/settings` with experience=classic. + */ + public function test_update_settings_experience_classic() { + wp_set_current_user( $this->admin_id ); + + $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( array( 'experience' => 'classic' ), JSON_UNESCAPED_SLASHES ) ); + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertTrue( $data['module_active'] ); + $this->assertFalse( $data['instant_search_enabled'] ); + $this->assertEquals( 'classic', $data['last_saved_experience'] ); + } + + /** + * Testing the `POST /jetpack/v4/search/settings` with experience=off. + */ + public function test_update_settings_experience_off() { + wp_set_current_user( $this->admin_id ); + + // Pre-activate the module and enable instant search via the legacy path so we can verify + // that `experience=off` deactivates the module but preserves instant_search_enabled. + $activate_request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $activate_request->set_header( 'content-type', 'application/json' ); + $activate_request->set_body( wp_json_encode( array( 'module_active' => true, 'instant_search_enabled' => true ), JSON_UNESCAPED_SLASHES ) ); + $this->server->dispatch( $activate_request ); + + $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( array( 'experience' => 'off' ), JSON_UNESCAPED_SLASHES ) ); + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertFalse( $data['module_active'] ); + $this->assertEquals( 'off', $data['last_saved_experience'] ); + // instant_search_enabled should be preserved (not changed to false by deactivation). + $this->assertTrue( $data['instant_search_enabled'] ); + } + + /** + * Testing the `POST /jetpack/v4/search/settings` with an invalid experience value. + */ + public function test_update_settings_experience_invalid() { + wp_set_current_user( $this->admin_id ); + + $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( array( 'experience' => 'invalid_value' ), JSON_UNESCAPED_SLASHES ) ); + $response = $this->server->dispatch( $request ); + $this->assertEquals( 400, $response->get_status() ); + } + + /** + * Testing that the persisted experience is returned from `GET /jetpack/v4/search/settings`. + */ + public function test_get_settings_returns_persisted_experience() { + wp_set_current_user( $this->admin_id ); + + // Save experience=embedded. + $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( array( 'experience' => 'embedded' ), JSON_UNESCAPED_SLASHES ) ); + $this->server->dispatch( $request ); + + // Read back and check persisted value is returned. + $request = new WP_REST_Request( 'GET', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $response = $this->server->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $this->assertEquals( 'embedded', $response->get_data()['last_saved_experience'] ); + } + /** * Testing the `POST /jetpack/v4/search/plan/activate` endpoint with no user. */ From f4e9afacd519deb5cf4adba22c8cee5fd5535d8e Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Wed, 6 May 2026 13:26:15 +1200 Subject: [PATCH 02/11] Search: narrow `experience` storage shape (RSM-2291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Off and classic are no longer stored in `jetpack_search_experience`. Off is read from `Modules::is_active()` because that bit lives in the global `jetpack_active_modules` option; storing `'off'` here would drift the moment any non-Search path (Jetpack dashboard module toggle, WP-CLI, `Jetpack::activate_module()`) flipped the global. Classic is the absence of an opt-in — `update_experience('classic')` deletes the option so existing classic sites stay at zero writes and don't need a migration. The wire format still exposes all four values (`embedded | overlay | classic | off`), so JS callers and external consumers see a clean enum; `get_experience()` resolves them from the on/off bit + saved value + legacy `instant_search_enabled` fallback. Renames the REST/initial-state field from `last_saved_experience` to `experience` to match the JS selector (`getActiveExperience`) and the request-body field name. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../add-rsm-2291-experience-field-backend | 2 +- .../search/src/class-module-control.php | 82 ++++++++++++------ .../search/src/class-rest-controller.php | 15 ++-- .../src/dashboard/class-initial-state.php | 2 +- .../search/tests/php/Module_Control_Test.php | 83 ++++++++++++------- .../search/tests/php/REST_Controller_Test.php | 34 +++++--- 6 files changed, 137 insertions(+), 81 deletions(-) diff --git a/projects/packages/search/changelog/add-rsm-2291-experience-field-backend b/projects/packages/search/changelog/add-rsm-2291-experience-field-backend index 52f0a52d3acc..978417f3ff16 100644 --- a/projects/packages/search/changelog/add-rsm-2291-experience-field-backend +++ b/projects/packages/search/changelog/add-rsm-2291-experience-field-backend @@ -1,4 +1,4 @@ Significance: minor Type: added -Search: Add backend support for the `experience` field in the search settings REST endpoint. The `POST /jetpack/v4/search/settings` endpoint now accepts `experience` (`embedded`, `overlay`, `classic`, or `off`) and persists it. The `GET /jetpack/v4/search/settings` response now includes `last_saved_experience`, derived from legacy settings for sites that have not yet saved via the new UI. +Search: Add backend support for the `experience` field in the search settings REST endpoint. `POST /jetpack/v4/search/settings` accepts `experience` (`embedded`, `overlay`, `classic`, or `off`) and updates the package state in lockstep. `GET /jetpack/v4/search/settings` returns the active `experience`, derived from the legacy settings for sites that have not yet saved via the new UI. diff --git a/projects/packages/search/src/class-module-control.php b/projects/packages/search/src/class-module-control.php index 62e39f47dc22..41fcfe9026a2 100644 --- a/projects/packages/search/src/class-module-control.php +++ b/projects/packages/search/src/class-module-control.php @@ -182,34 +182,49 @@ public function update_swap_classic_to_inline_search( bool $swap_classic_to_inli /** * Get the active search experience. * - * Returns the persisted experience if one has been saved via update_experience(), - * otherwise derives from the legacy module_active / instant_search_enabled booleans. + * The wire format always resolves to one of the four values, but storage is narrower: + * `'off'` is read from the global Jetpack module-active state (not stored in this + * package's option), and `'classic'` is the absence of an opt-in (the option is + * deleted, not written as `'classic'`). Only `'embedded'` and `'overlay'` are + * actually written to `jetpack_search_experience`. * - * @return string One of 'overlay', 'embedded', 'classic', 'off'. + * @return string One of 'embedded', 'overlay', 'classic', 'off'. */ public function get_experience() { - $saved = get_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, false ); - if ( $saved ) { - return $saved; - } - - // Derive from legacy booleans. if ( ! $this->is_active() ) { return self::EXPERIENCE_OFF; } + $saved = get_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, false ); + if ( self::EXPERIENCE_EMBEDDED === $saved ) { + return self::EXPERIENCE_EMBEDDED; + } + if ( self::EXPERIENCE_OVERLAY === $saved ) { + return self::EXPERIENCE_OVERLAY; + } + + // Legacy fallback for sites that have never saved via the new UI: a true + // `instant_search_enabled` boolean reads as overlay; otherwise classic. if ( $this->is_instant_search_enabled() ) { return self::EXPERIENCE_OVERLAY; } - // Cannot distinguish embedded from classic without a saved value; default to classic. return self::EXPERIENCE_CLASSIC; } /** - * Update the search experience and keep legacy booleans in lockstep. + * Update the search experience. + * + * Storage is narrower than the wire format: `'off'` only deactivates the global + * module (no write to the experience option), and `'classic'` deletes the + * experience option (the absence of an opt-in *is* classic). Only `'embedded'` + * and `'overlay'` write affirmative values. * - * @param string $experience One of 'overlay', 'embedded', 'classic', 'off'. + * Legacy `module_active` / `instant_search_enabled` are kept in lockstep so + * unmigrated readers (Initializer, Options, sidebar registration) continue to + * see the right state until they're migrated to consult get_experience(). + * + * @param string $experience One of 'embedded', 'overlay', 'classic', 'off'. * @return bool|WP_Error True on success, WP_Error on failure. */ public function update_experience( string $experience ) { @@ -223,35 +238,48 @@ public function update_experience( string $experience ) { } switch ( $experience ) { - case self::EXPERIENCE_OVERLAY: + case self::EXPERIENCE_OFF: + // Off lives in the global jetpack_active_modules option, not in this + // package's experience option. Leave instant_search_enabled and the + // experience option untouched so re-enabling later restores the user's + // prior preference (matches legacy ModuleControl behaviour). + ( new Modules() )->deactivate( self::JETPACK_SEARCH_MODULE_SLUG ); + return true; + + case self::EXPERIENCE_CLASSIC: $result = $this->activate(); if ( is_wp_error( $result ) ) { return $result; } - $result = $this->enable_instant_search(); - if ( is_wp_error( $result ) ) { - return $result; - } - break; + $this->disable_instant_search(); + // Classic is the absence of an opt-in — delete the option rather than + // writing 'classic'. Pre-existing sites that have never saved are + // already in this state, so this also normalises after a switch. + delete_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + return true; case self::EXPERIENCE_EMBEDDED: - case self::EXPERIENCE_CLASSIC: $result = $this->activate(); if ( is_wp_error( $result ) ) { return $result; } $this->disable_instant_search(); - break; + update_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, self::EXPERIENCE_EMBEDDED ); + return true; - case self::EXPERIENCE_OFF: - // Only deactivate the module; leave instant_search_enabled unchanged so the - // user's overlay-vs-embedded preference is preserved if they re-enable later. - ( new Modules() )->deactivate( self::JETPACK_SEARCH_MODULE_SLUG ); - break; + case self::EXPERIENCE_OVERLAY: + $result = $this->activate(); + if ( is_wp_error( $result ) ) { + return $result; + } + $result = $this->enable_instant_search(); + if ( is_wp_error( $result ) ) { + return $result; + } + update_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, self::EXPERIENCE_OVERLAY ); + return true; } - update_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, $experience ); - return true; } diff --git a/projects/packages/search/src/class-rest-controller.php b/projects/packages/search/src/class-rest-controller.php index d96769610d2d..867f5f6231c8 100644 --- a/projects/packages/search/src/class-rest-controller.php +++ b/projects/packages/search/src/class-rest-controller.php @@ -254,8 +254,9 @@ public function update_settings( $request ) { return $error; } - // If an experience value was provided, delegate to Module_Control::update_experience() - // which also writes the legacy booleans in lockstep. + // If an experience value was provided, delegate to Module_Control::update_experience(), + // which encapsulates the storage shape (off → module deactivate, classic → delete option, + // embedded/overlay → write affirmative value) and keeps the legacy booleans in lockstep. if ( $experience !== null ) { $result = $this->search_module->update_experience( $experience ); if ( is_wp_error( $result ) ) { @@ -309,10 +310,10 @@ public function update_settings( $request ) { /** * Validate $module_active and $instant_search_enabled. Returns an WP_Error instance if invalid. * - * @param boolean $module_active - Module status. - * @param boolean $instant_search_enabled - Instant Search status. - * @param boolean $swap_classic_to_inline_search - New inline search status. - * @param string|null $experience - Experience value. + * @param boolean $module_active - Module status. + * @param boolean $instant_search_enabled - Instant Search status. + * @param boolean $swap_classic_to_inline_search - New inline search status. + * @param string|null $experience - Experience value. */ protected function validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search, $experience = null ) { // An experience-only request is always valid; update_experience() validates the value. @@ -342,7 +343,7 @@ public function get_settings() { 'module_active' => $this->search_module->is_active(), 'instant_search_enabled' => $this->search_module->is_instant_search_enabled(), 'swap_classic_to_inline_search' => $this->search_module->is_swap_classic_to_inline_search(), - 'last_saved_experience' => $this->search_module->get_experience(), + 'experience' => $this->search_module->get_experience(), ) ); } diff --git a/projects/packages/search/src/dashboard/class-initial-state.php b/projects/packages/search/src/dashboard/class-initial-state.php index 0d92f58d4192..f0393f3fc160 100644 --- a/projects/packages/search/src/dashboard/class-initial-state.php +++ b/projects/packages/search/src/dashboard/class-initial-state.php @@ -92,7 +92,7 @@ public function get_initial_state() { 'jetpackSettings' => array( 'search' => $this->module_control->is_active(), 'instant_search_enabled' => $this->module_control->is_instant_search_enabled(), - 'last_saved_experience' => $this->module_control->get_experience(), + 'experience' => $this->module_control->get_experience(), ), 'features' => array_map( 'sanitize_text_field', diff --git a/projects/packages/search/tests/php/Module_Control_Test.php b/projects/packages/search/tests/php/Module_Control_Test.php index 11c972473cbe..f21d402893d9 100644 --- a/projects/packages/search/tests/php/Module_Control_Test.php +++ b/projects/packages/search/tests/php/Module_Control_Test.php @@ -168,54 +168,68 @@ public function test_disable_instant_search() { } /** - * Test get_experience() derivation from legacy booleans when no experience option is saved. + * Inactive module always reads as 'off' regardless of any saved experience + * option — off lives in jetpack_active_modules, not in the package's option. */ - public function test_get_experience_derived_off() { + public function test_get_experience_off_when_module_inactive() { delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); - // Module is inactive → 'off'. $this->assertEquals( Module_Control::EXPERIENCE_OFF, static::$search_module->get_experience() ); + + // Even with a stale 'embedded' value in the option, an inactive module is off. + update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_EMBEDDED ); + $this->assertEquals( Module_Control::EXPERIENCE_OFF, static::$search_module->get_experience() ); + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); } /** - * Test get_experience() derivation: module active + instant search enabled → 'overlay'. + * Saved 'embedded' / 'overlay' values are returned when the module is active. */ - public function test_get_experience_derived_overlay() { - delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + public function test_get_experience_returns_saved_value() { add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); - update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, true ); + + update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_EMBEDDED ); + $this->assertEquals( Module_Control::EXPERIENCE_EMBEDDED, static::$search_module->get_experience() ); + + update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_OVERLAY ); $this->assertEquals( Module_Control::EXPERIENCE_OVERLAY, static::$search_module->get_experience() ); + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); - delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); + delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); } /** - * Test get_experience() derivation: module active + instant search disabled → 'classic'. + * Legacy fallback: active module + instant_search_enabled=true with no saved + * value resolves to 'overlay'. */ - public function test_get_experience_derived_classic() { + public function test_get_experience_legacy_fallback_overlay() { delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); - update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, false ); - $this->assertEquals( Module_Control::EXPERIENCE_CLASSIC, static::$search_module->get_experience() ); + update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, true ); + $this->assertEquals( Module_Control::EXPERIENCE_OVERLAY, static::$search_module->get_experience() ); remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); } /** - * Test get_experience() returns persisted value over derived. + * Active module + instant_search_enabled=false with no saved value resolves to + * 'classic' — classic is the absence of an opt-in. */ - public function test_get_experience_returns_persisted() { - update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_EMBEDDED ); - $this->assertEquals( Module_Control::EXPERIENCE_EMBEDDED, static::$search_module->get_experience() ); + public function test_get_experience_classic_when_no_opt_in() { delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); + update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, false ); + $this->assertEquals( Module_Control::EXPERIENCE_CLASSIC, static::$search_module->get_experience() ); + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); } /** - * Test update_experience() with 'overlay'. + * Overlay activates the module, enables instant search, and writes 'overlay' + * to the experience option. */ public function test_update_experience_overlay() { delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); - // Use the filter that includes search so that is_active() returns true during - // enable_instant_search(), which requires the module to be active. + // is_active() needs to return true during enable_instant_search(). add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); static::$search_module->update_experience( Module_Control::EXPERIENCE_OVERLAY ); remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); @@ -226,7 +240,8 @@ public function test_update_experience_overlay() { } /** - * Test update_experience() with 'embedded'. + * Embedded activates the module, disables instant search, and writes + * 'embedded' to the experience option. */ public function test_update_experience_embedded() { delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); @@ -242,10 +257,12 @@ public function test_update_experience_embedded() { } /** - * Test update_experience() with 'classic'. + * Classic activates the module, disables instant search, and deletes the + * experience option (classic is the absence of an opt-in). */ - public function test_update_experience_classic() { - delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + public function test_update_experience_classic_deletes_option() { + // Seed an existing 'embedded' to prove the switch to classic clears it. + update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_EMBEDDED ); add_filter( 'jetpack_options', array( $this, 'return_active_modules_array_without_search' ), 10, 2 ); static::$search_module->update_experience( Module_Control::EXPERIENCE_CLASSIC ); $active_modules = get_option( 'jetpack_' . Module_Control::JETPACK_ACTIVE_MODULES_OPTION_KEY, array() ); @@ -253,32 +270,34 @@ public function test_update_experience_classic() { $this->assertContains( Module_Control::JETPACK_SEARCH_MODULE_SLUG, $active_modules ); $this->assertFalse( static::$search_module->is_instant_search_enabled() ); - $this->assertEquals( Module_Control::EXPERIENCE_CLASSIC, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); - delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); + $this->assertFalse( get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, false ) ); } /** - * Test update_experience() with 'off' deactivates module but preserves instant_search_enabled. + * Off deactivates the module and leaves the experience option and + * instant_search_enabled untouched, so re-enabling later restores the user's + * prior preference. */ - public function test_update_experience_off_preserves_instant_search() { - delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); - // Start with module active and instant search enabled. + public function test_update_experience_off_preserves_other_state() { + // Start with module active, overlay saved, instant search on. add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); + update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_OVERLAY ); update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, true ); remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); static::$search_module->update_experience( Module_Control::EXPERIENCE_OFF ); $this->assertFalse( static::$search_module->is_active() ); - // instant_search_enabled should remain true (preserved for later re-enable). + // experience option preserved (still 'overlay' for later re-enable). + $this->assertEquals( Module_Control::EXPERIENCE_OVERLAY, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); + // instant_search_enabled preserved. $this->assertTrue( (bool) get_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ) ); - $this->assertEquals( Module_Control::EXPERIENCE_OFF, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); } /** - * Test update_experience() with an invalid value returns WP_Error. + * Invalid input returns WP_Error. */ public function test_update_experience_invalid_value() { $result = static::$search_module->update_experience( 'invalid_value' ); diff --git a/projects/packages/search/tests/php/REST_Controller_Test.php b/projects/packages/search/tests/php/REST_Controller_Test.php index dd58b84821e5..c6e70687ee33 100644 --- a/projects/packages/search/tests/php/REST_Controller_Test.php +++ b/projects/packages/search/tests/php/REST_Controller_Test.php @@ -119,7 +119,7 @@ public function test_update_search_settings_success_both_enable() { 'instant_search_enabled' => true, 'swap_classic_to_inline_search' => false, ); - $expected = array_merge( $new_settings, array( 'last_saved_experience' => 'overlay' ) ); + $expected = array_merge( $new_settings, array( 'experience' => 'overlay' ) ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); $request->set_header( 'content-type', 'application/json' ); @@ -171,7 +171,7 @@ public function test_update_search_settings_success_both_disable() { 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => false, ); - $expected = array_merge( $new_settings, array( 'last_saved_experience' => 'off' ) ); + $expected = array_merge( $new_settings, array( 'experience' => 'off' ) ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); $request->set_header( 'content-type', 'application/json' ); @@ -193,7 +193,7 @@ public function test_update_search_settings_success_disable_module_only() { 'module_active' => false, 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => false, - 'last_saved_experience' => 'off', + 'experience' => 'off', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -216,7 +216,7 @@ public function test_update_search_settings_success_disable_instant_only() { 'module_active' => true, 'instant_search_enabled' => true, 'swap_classic_to_inline_search' => false, - 'last_saved_experience' => 'overlay', + 'experience' => 'overlay', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -239,7 +239,7 @@ public function test_update_search_settings_success_enable_inline_search() { 'module_active' => false, 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => true, - 'last_saved_experience' => 'off', + 'experience' => 'off', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -262,7 +262,7 @@ public function test_update_search_settings_success_disable_inline_search() { 'module_active' => false, 'instant_search_enabled' => false, 'swap_classic_to_inline_search' => false, - 'last_saved_experience' => 'off', + 'experience' => 'off', ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -298,7 +298,7 @@ public function test_get_search_settings_success() { $this->assertEquals( 200, $response->get_status() ); $this->assertArrayHasKey( 'module_active', $response->get_data() ); $this->assertArrayHasKey( 'instant_search_enabled', $response->get_data() ); - $this->assertArrayHasKey( 'last_saved_experience', $response->get_data() ); + $this->assertArrayHasKey( 'experience', $response->get_data() ); } /** @@ -353,7 +353,7 @@ public function test_update_settings_experience_overlay() { $data = $response->get_data(); $this->assertTrue( $data['module_active'] ); $this->assertTrue( $data['instant_search_enabled'] ); - $this->assertEquals( 'overlay', $data['last_saved_experience'] ); + $this->assertEquals( 'overlay', $data['experience'] ); } /** @@ -370,7 +370,7 @@ public function test_update_settings_experience_embedded() { $data = $response->get_data(); $this->assertTrue( $data['module_active'] ); $this->assertFalse( $data['instant_search_enabled'] ); - $this->assertEquals( 'embedded', $data['last_saved_experience'] ); + $this->assertEquals( 'embedded', $data['experience'] ); } /** @@ -387,7 +387,7 @@ public function test_update_settings_experience_classic() { $data = $response->get_data(); $this->assertTrue( $data['module_active'] ); $this->assertFalse( $data['instant_search_enabled'] ); - $this->assertEquals( 'classic', $data['last_saved_experience'] ); + $this->assertEquals( 'classic', $data['experience'] ); } /** @@ -400,7 +400,15 @@ public function test_update_settings_experience_off() { // that `experience=off` deactivates the module but preserves instant_search_enabled. $activate_request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); $activate_request->set_header( 'content-type', 'application/json' ); - $activate_request->set_body( wp_json_encode( array( 'module_active' => true, 'instant_search_enabled' => true ), JSON_UNESCAPED_SLASHES ) ); + $activate_request->set_body( + wp_json_encode( + array( + 'module_active' => true, + 'instant_search_enabled' => true, + ), + JSON_UNESCAPED_SLASHES + ) + ); $this->server->dispatch( $activate_request ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); @@ -410,7 +418,7 @@ public function test_update_settings_experience_off() { $this->assertEquals( 200, $response->get_status() ); $data = $response->get_data(); $this->assertFalse( $data['module_active'] ); - $this->assertEquals( 'off', $data['last_saved_experience'] ); + $this->assertEquals( 'off', $data['experience'] ); // instant_search_enabled should be preserved (not changed to false by deactivation). $this->assertTrue( $data['instant_search_enabled'] ); } @@ -445,7 +453,7 @@ public function test_get_settings_returns_persisted_experience() { $request->set_header( 'content-type', 'application/json' ); $response = $this->server->dispatch( $request ); $this->assertEquals( 200, $response->get_status() ); - $this->assertEquals( 'embedded', $response->get_data()['last_saved_experience'] ); + $this->assertEquals( 'embedded', $response->get_data()['experience'] ); } /** From 0ea57f945e0cb5da57e7839a3fa029542ce40bd7 Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Wed, 6 May 2026 14:23:08 +1200 Subject: [PATCH 03/11] Search: rename `classic` experience to `inline` (RSM-2291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the frontend rename in PR #48500 — `classic` collided with the deprecated original Jetpack Search "classic search" terminology. Wire value is now `'inline'`; storage shape is unchanged (still deleted on write, since inline is the absence of an opt-in). - `Module_Control::EXPERIENCE_CLASSIC` → `EXPERIENCE_INLINE` (value `'inline'`) - `update_experience()` switch case + validation list - `get_experience()` legacy-fallback return value and docblock - Test names + values in `Module_Control_Test` and `REST_Controller_Test` - Changelog entry Co-Authored-By: Claude Opus 4.7 (1M context) --- .../add-rsm-2291-experience-field-backend | 2 +- .../search/src/class-module-control.php | 26 +++++++++---------- .../search/tests/php/Module_Control_Test.php | 16 ++++++------ .../search/tests/php/REST_Controller_Test.php | 8 +++--- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/projects/packages/search/changelog/add-rsm-2291-experience-field-backend b/projects/packages/search/changelog/add-rsm-2291-experience-field-backend index 978417f3ff16..ba4bd08e86ef 100644 --- a/projects/packages/search/changelog/add-rsm-2291-experience-field-backend +++ b/projects/packages/search/changelog/add-rsm-2291-experience-field-backend @@ -1,4 +1,4 @@ Significance: minor Type: added -Search: Add backend support for the `experience` field in the search settings REST endpoint. `POST /jetpack/v4/search/settings` accepts `experience` (`embedded`, `overlay`, `classic`, or `off`) and updates the package state in lockstep. `GET /jetpack/v4/search/settings` returns the active `experience`, derived from the legacy settings for sites that have not yet saved via the new UI. +Search: Add backend support for the `experience` field in the search settings REST endpoint. `POST /jetpack/v4/search/settings` accepts `experience` (`embedded`, `overlay`, `inline`, or `off`) and updates the package state in lockstep. `GET /jetpack/v4/search/settings` returns the active `experience`, derived from the legacy settings for sites that have not yet saved via the new UI. diff --git a/projects/packages/search/src/class-module-control.php b/projects/packages/search/src/class-module-control.php index 41fcfe9026a2..997b08b6b824 100644 --- a/projects/packages/search/src/class-module-control.php +++ b/projects/packages/search/src/class-module-control.php @@ -48,7 +48,7 @@ class Module_Control { */ const EXPERIENCE_OVERLAY = 'overlay'; const EXPERIENCE_EMBEDDED = 'embedded'; - const EXPERIENCE_CLASSIC = 'classic'; + const EXPERIENCE_INLINE = 'inline'; const EXPERIENCE_OFF = 'off'; /** @@ -184,11 +184,11 @@ public function update_swap_classic_to_inline_search( bool $swap_classic_to_inli * * The wire format always resolves to one of the four values, but storage is narrower: * `'off'` is read from the global Jetpack module-active state (not stored in this - * package's option), and `'classic'` is the absence of an opt-in (the option is - * deleted, not written as `'classic'`). Only `'embedded'` and `'overlay'` are + * package's option), and `'inline'` is the absence of an opt-in (the option is + * deleted, not written as `'inline'`). Only `'embedded'` and `'overlay'` are * actually written to `jetpack_search_experience`. * - * @return string One of 'embedded', 'overlay', 'classic', 'off'. + * @return string One of 'embedded', 'overlay', 'inline', 'off'. */ public function get_experience() { if ( ! $this->is_active() ) { @@ -204,31 +204,31 @@ public function get_experience() { } // Legacy fallback for sites that have never saved via the new UI: a true - // `instant_search_enabled` boolean reads as overlay; otherwise classic. + // `instant_search_enabled` boolean reads as overlay; otherwise inline. if ( $this->is_instant_search_enabled() ) { return self::EXPERIENCE_OVERLAY; } - return self::EXPERIENCE_CLASSIC; + return self::EXPERIENCE_INLINE; } /** * Update the search experience. * * Storage is narrower than the wire format: `'off'` only deactivates the global - * module (no write to the experience option), and `'classic'` deletes the - * experience option (the absence of an opt-in *is* classic). Only `'embedded'` + * module (no write to the experience option), and `'inline'` deletes the + * experience option (the absence of an opt-in *is* inline). Only `'embedded'` * and `'overlay'` write affirmative values. * * Legacy `module_active` / `instant_search_enabled` are kept in lockstep so * unmigrated readers (Initializer, Options, sidebar registration) continue to * see the right state until they're migrated to consult get_experience(). * - * @param string $experience One of 'embedded', 'overlay', 'classic', 'off'. + * @param string $experience One of 'embedded', 'overlay', 'inline', 'off'. * @return bool|WP_Error True on success, WP_Error on failure. */ public function update_experience( string $experience ) { - $valid_values = array( self::EXPERIENCE_OVERLAY, self::EXPERIENCE_EMBEDDED, self::EXPERIENCE_CLASSIC, self::EXPERIENCE_OFF ); + $valid_values = array( self::EXPERIENCE_OVERLAY, self::EXPERIENCE_EMBEDDED, self::EXPERIENCE_INLINE, self::EXPERIENCE_OFF ); if ( ! in_array( $experience, $valid_values, true ) ) { return new WP_Error( 'invalid_experience', @@ -246,14 +246,14 @@ public function update_experience( string $experience ) { ( new Modules() )->deactivate( self::JETPACK_SEARCH_MODULE_SLUG ); return true; - case self::EXPERIENCE_CLASSIC: + case self::EXPERIENCE_INLINE: $result = $this->activate(); if ( is_wp_error( $result ) ) { return $result; } $this->disable_instant_search(); - // Classic is the absence of an opt-in — delete the option rather than - // writing 'classic'. Pre-existing sites that have never saved are + // Inline is the absence of an opt-in — delete the option rather than + // writing 'inline'. Pre-existing sites that have never saved are // already in this state, so this also normalises after a switch. delete_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); return true; diff --git a/projects/packages/search/tests/php/Module_Control_Test.php b/projects/packages/search/tests/php/Module_Control_Test.php index f21d402893d9..19f6b52579f5 100644 --- a/projects/packages/search/tests/php/Module_Control_Test.php +++ b/projects/packages/search/tests/php/Module_Control_Test.php @@ -212,13 +212,13 @@ public function test_get_experience_legacy_fallback_overlay() { /** * Active module + instant_search_enabled=false with no saved value resolves to - * 'classic' — classic is the absence of an opt-in. + * 'inline' — inline is the absence of an opt-in. */ - public function test_get_experience_classic_when_no_opt_in() { + public function test_get_experience_inline_when_no_opt_in() { delete_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ); add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, false ); - $this->assertEquals( Module_Control::EXPERIENCE_CLASSIC, static::$search_module->get_experience() ); + $this->assertEquals( Module_Control::EXPERIENCE_INLINE, static::$search_module->get_experience() ); remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); } @@ -257,14 +257,14 @@ public function test_update_experience_embedded() { } /** - * Classic activates the module, disables instant search, and deletes the - * experience option (classic is the absence of an opt-in). + * Inline activates the module, disables instant search, and deletes the + * experience option (inline is the absence of an opt-in). */ - public function test_update_experience_classic_deletes_option() { - // Seed an existing 'embedded' to prove the switch to classic clears it. + public function test_update_experience_inline_deletes_option() { + // Seed an existing 'embedded' to prove the switch to inline clears it. update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_EMBEDDED ); add_filter( 'jetpack_options', array( $this, 'return_active_modules_array_without_search' ), 10, 2 ); - static::$search_module->update_experience( Module_Control::EXPERIENCE_CLASSIC ); + static::$search_module->update_experience( Module_Control::EXPERIENCE_INLINE ); $active_modules = get_option( 'jetpack_' . Module_Control::JETPACK_ACTIVE_MODULES_OPTION_KEY, array() ); remove_filter( 'jetpack_options', array( $this, 'return_active_modules_array_without_search' ) ); diff --git a/projects/packages/search/tests/php/REST_Controller_Test.php b/projects/packages/search/tests/php/REST_Controller_Test.php index c6e70687ee33..039c8142838b 100644 --- a/projects/packages/search/tests/php/REST_Controller_Test.php +++ b/projects/packages/search/tests/php/REST_Controller_Test.php @@ -374,20 +374,20 @@ public function test_update_settings_experience_embedded() { } /** - * Testing the `POST /jetpack/v4/search/settings` with experience=classic. + * Testing the `POST /jetpack/v4/search/settings` with experience=inline. */ - public function test_update_settings_experience_classic() { + public function test_update_settings_experience_inline() { wp_set_current_user( $this->admin_id ); $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); $request->set_header( 'content-type', 'application/json' ); - $request->set_body( wp_json_encode( array( 'experience' => 'classic' ), JSON_UNESCAPED_SLASHES ) ); + $request->set_body( wp_json_encode( array( 'experience' => 'inline' ), JSON_UNESCAPED_SLASHES ) ); $response = $this->server->dispatch( $request ); $this->assertEquals( 200, $response->get_status() ); $data = $response->get_data(); $this->assertTrue( $data['module_active'] ); $this->assertFalse( $data['instant_search_enabled'] ); - $this->assertEquals( 'classic', $data['experience'] ); + $this->assertEquals( 'inline', $data['experience'] ); } /** From 860d670ab52dce61c48ed23dc441a652844cd863 Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Wed, 6 May 2026 14:38:44 +1200 Subject: [PATCH 04/11] Search: address backend PR review feedback (RSM-2291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the bot review on #48540: - Drop the unreachable `return true` after the exhaustive switch in `update_experience()`. The four valid values each return inside their case and the in_array guard rejects everything else, so the trailing return was dead code. - Fix the stale "classic" mention in the REST controller's delegation comment — it should read "inline → delete option" after the rename. - Reject requests that mix `experience` with `module_active` or `instant_search_enabled` rather than silently dropping the legacy fields. `experience` writes the legacy booleans in lockstep, so there is no scenario where the caller needs both. Returns 400 `rest_invalid_arguments` with a clear message; new REST test covers the two mixing combinations. - Tighten `test_update_experience_off_preserves_other_state` so it proves deactivation actually ran. Previously the active-modules filter was removed before the assertion, so `is_active() === false` was trivially true regardless of whether `deactivate()` had been called. Now reads `jetpack_active_modules` directly while the filter is still active, matching the pattern in `test_deactivate_module`. Two items intentionally not changed: the partial-failure rollback in the overlay branch (acknowledged as a known package-wide pattern by the reviewer; "rolling back" by deactivating the module would itself be unrequested state mutation), and the `SEARCH_MODULE_EXPERIENCE_OPTION_KEY` prefix nit (the option name is what's stored in `wp_options` and prefixing it is the correct namespace for cross-plugin coexistence). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../search/src/class-module-control.php | 2 -- .../search/src/class-rest-controller.php | 13 +++++++-- .../search/tests/php/Module_Control_Test.php | 12 ++++++-- .../search/tests/php/REST_Controller_Test.php | 29 +++++++++++++++++++ 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/projects/packages/search/src/class-module-control.php b/projects/packages/search/src/class-module-control.php index 997b08b6b824..ea24a9e57f45 100644 --- a/projects/packages/search/src/class-module-control.php +++ b/projects/packages/search/src/class-module-control.php @@ -279,8 +279,6 @@ public function update_experience( string $experience ) { update_option( self::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, self::EXPERIENCE_OVERLAY ); return true; } - - return true; } /** diff --git a/projects/packages/search/src/class-rest-controller.php b/projects/packages/search/src/class-rest-controller.php index 867f5f6231c8..70f905980671 100644 --- a/projects/packages/search/src/class-rest-controller.php +++ b/projects/packages/search/src/class-rest-controller.php @@ -255,7 +255,7 @@ public function update_settings( $request ) { } // If an experience value was provided, delegate to Module_Control::update_experience(), - // which encapsulates the storage shape (off → module deactivate, classic → delete option, + // which encapsulates the storage shape (off → module deactivate, inline → delete option, // embedded/overlay → write affirmative value) and keeps the legacy booleans in lockstep. if ( $experience !== null ) { $result = $this->search_module->update_experience( $experience ); @@ -316,8 +316,17 @@ public function update_settings( $request ) { * @param string|null $experience - Experience value. */ protected function validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search, $experience = null ) { - // An experience-only request is always valid; update_experience() validates the value. + // `experience` is the canonical source of truth and writes the legacy booleans in lockstep. + // Reject requests that mix it with `module_active` / `instant_search_enabled` so callers + // don't silently lose those fields. if ( $experience !== null ) { + if ( $module_active !== null || $instant_search_enabled !== null ) { + return new WP_Error( + 'rest_invalid_arguments', + esc_html__( 'The `experience` field cannot be combined with `module_active` or `instant_search_enabled`.', 'jetpack-search-pkg' ), + array( 'status' => 400 ) + ); + } return true; } if ( $module_active === null && $instant_search_enabled === null && $swap_classic_to_inline_search !== null ) { diff --git a/projects/packages/search/tests/php/Module_Control_Test.php b/projects/packages/search/tests/php/Module_Control_Test.php index 19f6b52579f5..f4c8464b08fd 100644 --- a/projects/packages/search/tests/php/Module_Control_Test.php +++ b/projects/packages/search/tests/php/Module_Control_Test.php @@ -279,15 +279,21 @@ public function test_update_experience_inline_deletes_option() { * prior preference. */ public function test_update_experience_off_preserves_other_state() { - // Start with module active, overlay saved, instant search on. + // Start with module active, overlay saved, instant search on. The filter + // has to stay active across update_experience() so deactivate() has a + // real active-modules option to remove 'search' from — see test_deactivate_module + // for the same pattern. add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_OVERLAY ); update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, true ); - remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); static::$search_module->update_experience( Module_Control::EXPERIENCE_OFF ); - $this->assertFalse( static::$search_module->is_active() ); + // Read the actual option (not via the filter) to prove deactivate() ran. + $active_modules = get_option( 'jetpack_' . Module_Control::JETPACK_ACTIVE_MODULES_OPTION_KEY, array() ); + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + + $this->assertNotContains( Module_Control::JETPACK_SEARCH_MODULE_SLUG, $active_modules ); // experience option preserved (still 'overlay' for later re-enable). $this->assertEquals( Module_Control::EXPERIENCE_OVERLAY, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); // instant_search_enabled preserved. diff --git a/projects/packages/search/tests/php/REST_Controller_Test.php b/projects/packages/search/tests/php/REST_Controller_Test.php index 039c8142838b..9e9dc9bb0e4d 100644 --- a/projects/packages/search/tests/php/REST_Controller_Test.php +++ b/projects/packages/search/tests/php/REST_Controller_Test.php @@ -436,6 +436,35 @@ public function test_update_settings_experience_invalid() { $this->assertEquals( 400, $response->get_status() ); } + /** + * Mixing `experience` with `module_active` or `instant_search_enabled` is rejected + * so callers don't silently drop fields. `experience` writes the legacy booleans + * in lockstep — there's no scenario where the caller needs both. + */ + public function test_update_settings_experience_rejects_mixed_legacy_fields() { + wp_set_current_user( $this->admin_id ); + + foreach ( + array( + array( + 'experience' => 'overlay', + 'module_active' => false, + ), + array( + 'experience' => 'embedded', + 'instant_search_enabled' => true, + ), + ) as $body + ) { + $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); + $request->set_header( 'content-type', 'application/json' ); + $request->set_body( wp_json_encode( $body, JSON_UNESCAPED_SLASHES ) ); + $response = $this->server->dispatch( $request ); + $this->assertEquals( 400, $response->get_status() ); + $this->assertEquals( 'rest_invalid_arguments', $response->get_data()['code'] ); + } + } + /** * Testing that the persisted experience is returned from `GET /jetpack/v4/search/settings`. */ From b1e47a07e4d9754dbb47668fe1cd86fa6fccb630 Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Wed, 6 May 2026 19:47:09 +1200 Subject: [PATCH 05/11] Search: cover update_experience() error-propagation branches (RSM-2291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four `if ( is_wp_error( $result ) ) { return $result; }` branches in `update_experience()`'s INLINE / EMBEDDED / OVERLAY cases were the only new lines in this PR not exercised by tests (per the PR's coverage report). They're behavioral — a typo in any of them would silently swallow a partial-failure error and write the experience option in an inconsistent state. Adds: - A data-provider-driven test asserting each of the three experiences that calls activate() propagates the WP_Error from a plan-without- search and leaves the experience option unwritten. - A test asserting overlay propagates the WP_Error from enable_instant_search() (plan supports search but not instant search) and leaves the experience option unwritten. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../search/tests/php/Module_Control_Test.php | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/projects/packages/search/tests/php/Module_Control_Test.php b/projects/packages/search/tests/php/Module_Control_Test.php index f4c8464b08fd..a2d44311bcf8 100644 --- a/projects/packages/search/tests/php/Module_Control_Test.php +++ b/projects/packages/search/tests/php/Module_Control_Test.php @@ -311,6 +311,58 @@ public function test_update_experience_invalid_value() { $this->assertEquals( 'invalid_experience', $result->get_error_code() ); } + /** + * Each experience that calls activate() must propagate its WP_Error rather than + * fall through and write the experience option in an inconsistent state. + * + * @param string $experience One of 'inline', 'embedded', 'overlay'. + * @dataProvider experiences_requiring_activation + */ + #[\PHPUnit\Framework\Attributes\DataProvider( 'experiences_requiring_activation' )] + public function test_update_experience_propagates_activate_error( $experience ) { + $plan = $this->createStub( Plan::class ); + $plan->method( 'supports_search' )->willReturn( false ); + $plan->method( 'supports_instant_search' )->willReturn( false ); + $module = new Module_Control( $plan ); + + $result = $module->update_experience( $experience ); + + $this->assertInstanceOf( \WP_Error::class, $result ); + $this->assertEquals( 'not_supported', $result->get_error_code() ); + // On failure, the experience option must not be written. + $this->assertFalse( get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, false ) ); + } + + /** + * @return array> + */ + public static function experiences_requiring_activation() { + return array( + 'inline' => array( Module_Control::EXPERIENCE_INLINE ), + 'embedded' => array( Module_Control::EXPERIENCE_EMBEDDED ), + 'overlay' => array( Module_Control::EXPERIENCE_OVERLAY ), + ); + } + + /** + * Overlay propagates the WP_Error from enable_instant_search() (e.g. plan + * doesn't support instant search) and does not write the experience option. + */ + public function test_update_experience_overlay_propagates_enable_instant_search_error() { + // $search_module_no_instant has supports_search=true but supports_instant_search=false, + // so activate() succeeds and enable_instant_search() returns 'not_supported'. + // Filter is on so is_active() returns true inside enable_instant_search(). + add_filter( 'jetpack_options', array( $this, 'return_search_active_array' ), 10, 2 ); + + $result = static::$search_module_no_instant->update_experience( Module_Control::EXPERIENCE_OVERLAY ); + + remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + + $this->assertInstanceOf( \WP_Error::class, $result ); + $this->assertEquals( 'not_supported', $result->get_error_code() ); + $this->assertFalse( get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, false ) ); + } + /** * Returns an empty array */ From e15730a093154c4322782a6f281129b7ce0ed3fb Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Wed, 6 May 2026 20:19:23 +1200 Subject: [PATCH 06/11] Search dashboard: stop rendering ModuleControl when feature-selector is on (RSM-2291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontend PR (#48500) shipped a transitional state where ModuleControl rendered alongside FeatureSelector even with the feature flag on, so admins could still manage Search settings while the back-end `experience` field was unimplemented. With this PR adding that back end, the new selector can fully persist the user's choice on its own — the legacy toggles below it become redundant. Reverts the dashboard-page bottom-area to the either/or shape: when `jetpack_search_blocks_enabled` is on, render only ``; otherwise render only ``. Updates the branching test to assert ModuleControl is *absent* when the flag is on. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../components/pages/dashboard-page.jsx | 37 +++++++++---------- .../dashboard/pages/dashboard-page.test.jsx | 8 ++-- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/projects/packages/search/src/dashboard/components/pages/dashboard-page.jsx b/projects/packages/search/src/dashboard/components/pages/dashboard-page.jsx index 68a79fa80459..404c9d3d7bf8 100644 --- a/projects/packages/search/src/dashboard/components/pages/dashboard-page.jsx +++ b/projects/packages/search/src/dashboard/components/pages/dashboard-page.jsx @@ -170,7 +170,7 @@ export default function DashboardPage( { isLoading = false } ) { /> ) }
- { isSearchBlocksEnabled && ( + { isSearchBlocksEnabled ? (
@@ -178,26 +178,23 @@ export default function DashboardPage( { isLoading = false } ) {
+ ) : ( + ) } - { /* ModuleControl renders regardless of the feature flag for now — - until the back-end `experience` field lands (RSM-2291), the new - FeatureSelector can't actually persist changes. Keeping the legacy - toggles visible lets admins continue managing Search settings. */ } -
branch', () => { - test( 'renders FeatureSelector and ModuleControl when searchBlocksEnabled is true', () => { + test( 'renders FeatureSelector when searchBlocksEnabled is true', () => { renderWith( { searchBlocksEnabled: true, jetpackSettings: settings } ); expect( screen.getByRole( 'group', { name: /select a search experience for your visitors/i } ) ).toBeInTheDocument(); - // ModuleControl renders unconditionally so admins can still change - // settings until the back-end `experience` field lands. - expect( screen.getByTestId( 'module-control' ) ).toBeInTheDocument(); + expect( screen.queryByTestId( 'module-control' ) ).not.toBeInTheDocument(); } ); - test( 'renders only ModuleControl when searchBlocksEnabled is false', () => { + test( 'renders ModuleControl when searchBlocksEnabled is false', () => { renderWith( { searchBlocksEnabled: false, jetpackSettings: settings } ); expect( screen.queryByRole( 'group', { name: /select a search experience for your visitors/i } ) From ed61648a317db7bf75c6f4a6aae361d32b080647 Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Thu, 7 May 2026 09:27:49 +1200 Subject: [PATCH 07/11] Sync: whitelist jetpack_search_experience option (RSM-2291) Per review feedback on PR #48540, add the new `jetpack_search_experience` option to the Search sync module's options whitelist so writes propagate to the WPcom shadow replicastore. Without this, the dashboard saves locally but the WPcom-side experience derivation can't see the user's choice. Note: requires a coordinated WPcom-side whitelist entry for the option to actually be retained on the cache site. --- .../sync/changelog/add-rsm-2291-search-experience-option | 4 ++++ projects/packages/sync/src/modules/class-search.php | 1 + 2 files changed, 5 insertions(+) create mode 100644 projects/packages/sync/changelog/add-rsm-2291-search-experience-option diff --git a/projects/packages/sync/changelog/add-rsm-2291-search-experience-option b/projects/packages/sync/changelog/add-rsm-2291-search-experience-option new file mode 100644 index 000000000000..aa81ee636112 --- /dev/null +++ b/projects/packages/sync/changelog/add-rsm-2291-search-experience-option @@ -0,0 +1,4 @@ +Significance: patch +Type: added + +Sync: Whitelist the new jetpack_search_experience option so it propagates to WPcom. diff --git a/projects/packages/sync/src/modules/class-search.php b/projects/packages/sync/src/modules/class-search.php index 112da68a27ca..0691174bc5fe 100644 --- a/projects/packages/sync/src/modules/class-search.php +++ b/projects/packages/sync/src/modules/class-search.php @@ -1771,6 +1771,7 @@ public function __construct() { 'jetpack_search_enable_sort', 'jetpack_search_inf_scroll', 'jetpack_search_show_powered_by', + 'jetpack_search_experience', 'instant_search_enabled', ); // end options. From 24b58b444955f18db2ef9d3cc754e838cf422032 Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Thu, 7 May 2026 10:52:38 +1200 Subject: [PATCH 08/11] Search: type-guard `experience` before sanitize_text_field() (RSM-2291) Per PR #48540 review: passing an array or object payload through sanitize_text_field() triggers a PHP 8.1+ array-to-string deprecation notice. The downstream in_array() check in update_experience() already rejects non-string values, but the warning is unnecessary noise. Add an is_string() guard so non-string values resolve to null up front and skip the experience branch entirely. --- projects/packages/search/src/class-rest-controller.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/projects/packages/search/src/class-rest-controller.php b/projects/packages/search/src/class-rest-controller.php index 70f905980671..7931e75ae8ea 100644 --- a/projects/packages/search/src/class-rest-controller.php +++ b/projects/packages/search/src/class-rest-controller.php @@ -246,7 +246,9 @@ public function update_settings( $request ) { $module_active = isset( $request_body['module_active'] ) ? (bool) $request_body['module_active'] : null; $instant_search_enabled = isset( $request_body['instant_search_enabled'] ) ? (bool) $request_body['instant_search_enabled'] : null; $swap_classic_to_inline_search = isset( $request_body['swap_classic_to_inline_search'] ) ? (bool) $request_body['swap_classic_to_inline_search'] : null; - $experience = isset( $request_body['experience'] ) ? sanitize_text_field( $request_body['experience'] ) : null; + $experience = isset( $request_body['experience'] ) && is_string( $request_body['experience'] ) + ? sanitize_text_field( $request_body['experience'] ) + : null; $error = $this->validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search, $experience ); From 4af42edb42eef4fc861f27bbb1c0335c64fbc5a6 Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Thu, 7 May 2026 10:53:13 +1200 Subject: [PATCH 09/11] Search: reject `experience` + `swap_classic_to_inline_search` mixing (RSM-2291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR #48540 review: validate_search_settings() rejected mixing `experience` with `module_active` / `instant_search_enabled`, but let `swap_classic_to_inline_search` through. The early return in the `experience` branch of update_settings() then dropped that field silently — same footgun the existing rejection was meant to prevent. Extend the rejection to cover all three legacy fields and update the error message accordingly. Today's only caller (the new front end) sends `{ experience }` alone, so this is a defensive fix for external API consumers. Add the new mixing combination to the existing rejection test. --- projects/packages/search/src/class-rest-controller.php | 9 +++++---- .../packages/search/tests/php/REST_Controller_Test.php | 4 ++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/projects/packages/search/src/class-rest-controller.php b/projects/packages/search/src/class-rest-controller.php index 7931e75ae8ea..42c714bb50af 100644 --- a/projects/packages/search/src/class-rest-controller.php +++ b/projects/packages/search/src/class-rest-controller.php @@ -319,13 +319,14 @@ public function update_settings( $request ) { */ protected function validate_search_settings( $module_active, $instant_search_enabled, $swap_classic_to_inline_search, $experience = null ) { // `experience` is the canonical source of truth and writes the legacy booleans in lockstep. - // Reject requests that mix it with `module_active` / `instant_search_enabled` so callers - // don't silently lose those fields. + // Reject requests that mix it with any other settings field so callers don't silently + // lose those fields — the `experience` branch in update_settings() early-returns and + // would otherwise drop them. if ( $experience !== null ) { - if ( $module_active !== null || $instant_search_enabled !== null ) { + if ( $module_active !== null || $instant_search_enabled !== null || $swap_classic_to_inline_search !== null ) { return new WP_Error( 'rest_invalid_arguments', - esc_html__( 'The `experience` field cannot be combined with `module_active` or `instant_search_enabled`.', 'jetpack-search-pkg' ), + esc_html__( 'The `experience` field cannot be combined with `module_active`, `instant_search_enabled`, or `swap_classic_to_inline_search`.', 'jetpack-search-pkg' ), array( 'status' => 400 ) ); } diff --git a/projects/packages/search/tests/php/REST_Controller_Test.php b/projects/packages/search/tests/php/REST_Controller_Test.php index 9e9dc9bb0e4d..4a4c1b05ea56 100644 --- a/projects/packages/search/tests/php/REST_Controller_Test.php +++ b/projects/packages/search/tests/php/REST_Controller_Test.php @@ -454,6 +454,10 @@ public function test_update_settings_experience_rejects_mixed_legacy_fields() { 'experience' => 'embedded', 'instant_search_enabled' => true, ), + array( + 'experience' => 'inline', + 'swap_classic_to_inline_search' => true, + ), ) as $body ) { $request = new WP_REST_Request( 'POST', '/jetpack/v4/search/settings' ); From caa36785b78fe760c3758e2d65b1c5ac71f0a32f Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Thu, 7 May 2026 10:55:00 +1200 Subject: [PATCH 10/11] Search: propagate Modules::deactivate() result from update_experience('off') (RSM-2291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR #48540 review: the OFF branch was discarding Modules::deactivate()'s bool and hard-returning true. Propagate the bool instead — false signals the module was already inactive (a benign no-op), true signals it was removed from jetpack_active_modules. The REST controller only branches on is_wp_error(), so a propagated false still falls through to the success response — but direct callers of update_experience() can now distinguish the two cases. INLINE / EMBEDDED / OVERLAY are unchanged: their inner update_option() and delete_option() calls return false on no-change writes (option already at the target value), which can't be safely conflated with failure. WP_Error paths in those branches are already propagated. Update test_update_experience_off_preserves_other_state to assert the new return value, and add coverage for the no-op deactivate path. --- .../search/src/class-module-control.php | 8 +++++--- .../search/tests/php/Module_Control_Test.php | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/projects/packages/search/src/class-module-control.php b/projects/packages/search/src/class-module-control.php index ea24a9e57f45..8db94f1e9549 100644 --- a/projects/packages/search/src/class-module-control.php +++ b/projects/packages/search/src/class-module-control.php @@ -225,7 +225,10 @@ public function get_experience() { * see the right state until they're migrated to consult get_experience(). * * @param string $experience One of 'embedded', 'overlay', 'inline', 'off'. - * @return bool|WP_Error True on success, WP_Error on failure. + * @return bool|WP_Error WP_Error on failure; true on success for the affirmative + * branches; the bool from Modules::deactivate() for `'off'` + * (false signals the module was already inactive — a benign + * no-op the REST controller treats as success). */ public function update_experience( string $experience ) { $valid_values = array( self::EXPERIENCE_OVERLAY, self::EXPERIENCE_EMBEDDED, self::EXPERIENCE_INLINE, self::EXPERIENCE_OFF ); @@ -243,8 +246,7 @@ public function update_experience( string $experience ) { // package's experience option. Leave instant_search_enabled and the // experience option untouched so re-enabling later restores the user's // prior preference (matches legacy ModuleControl behaviour). - ( new Modules() )->deactivate( self::JETPACK_SEARCH_MODULE_SLUG ); - return true; + return ( new Modules() )->deactivate( self::JETPACK_SEARCH_MODULE_SLUG ); case self::EXPERIENCE_INLINE: $result = $this->activate(); diff --git a/projects/packages/search/tests/php/Module_Control_Test.php b/projects/packages/search/tests/php/Module_Control_Test.php index a2d44311bcf8..52bc86a27223 100644 --- a/projects/packages/search/tests/php/Module_Control_Test.php +++ b/projects/packages/search/tests/php/Module_Control_Test.php @@ -287,12 +287,14 @@ public function test_update_experience_off_preserves_other_state() { update_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY, Module_Control::EXPERIENCE_OVERLAY ); update_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, true ); - static::$search_module->update_experience( Module_Control::EXPERIENCE_OFF ); + $result = static::$search_module->update_experience( Module_Control::EXPERIENCE_OFF ); // Read the actual option (not via the filter) to prove deactivate() ran. $active_modules = get_option( 'jetpack_' . Module_Control::JETPACK_ACTIVE_MODULES_OPTION_KEY, array() ); remove_filter( 'jetpack_options', array( $this, 'return_search_active_array' ) ); + // Propagated from Modules::deactivate(): true when the module was actually removed. + $this->assertTrue( $result ); $this->assertNotContains( Module_Control::JETPACK_SEARCH_MODULE_SLUG, $active_modules ); // experience option preserved (still 'overlay' for later re-enable). $this->assertEquals( Module_Control::EXPERIENCE_OVERLAY, get_option( Module_Control::SEARCH_MODULE_EXPERIENCE_OPTION_KEY ) ); @@ -302,6 +304,20 @@ public function test_update_experience_off_preserves_other_state() { delete_option( Module_Control::SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY ); } + /** + * When the module is already inactive, Modules::deactivate() is a no-op and + * returns false. update_experience('off') propagates that bool — it's not an + * error, just a signal that nothing changed. The REST controller (which only + * branches on is_wp_error()) still treats it as success. + */ + public function test_update_experience_off_when_module_already_inactive_returns_false() { + // No filter installed → active modules option is empty → deactivate is a no-op. + $result = static::$search_module->update_experience( Module_Control::EXPERIENCE_OFF ); + + $this->assertFalse( $result ); + $this->assertNotInstanceOf( \WP_Error::class, $result ); + } + /** * Invalid input returns WP_Error. */ From 3a5e726f42f4334de993454df2b67f55575c6aa5 Mon Sep 17 00:00:00 2001 From: Adam Wood <1017872+adamwoodnz@users.noreply.github.com> Date: Thu, 7 May 2026 10:58:10 +1200 Subject: [PATCH 11/11] Search: stabilize update_experience('off') no-op test (RSM-2291) The new test added in caa36785b7 assumed an empty jetpack_active_modules option, but earlier tests in the same class activate the module via update_experience() and leak 'search' into the persisted option. Seed the option with an empty array up front so deactivate() truly is a no-op (update_option returns false when the stored value already matches). --- projects/packages/search/tests/php/Module_Control_Test.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/projects/packages/search/tests/php/Module_Control_Test.php b/projects/packages/search/tests/php/Module_Control_Test.php index 52bc86a27223..8d927880d3ae 100644 --- a/projects/packages/search/tests/php/Module_Control_Test.php +++ b/projects/packages/search/tests/php/Module_Control_Test.php @@ -311,7 +311,12 @@ public function test_update_experience_off_preserves_other_state() { * branches on is_wp_error()) still treats it as success. */ public function test_update_experience_off_when_module_already_inactive_returns_false() { - // No filter installed → active modules option is empty → deactivate is a no-op. + // Earlier tests in this class activate the search module via update_experience() + // and persist 'search' into the real jetpack_active_modules option. Set it to an + // empty array so deactivate() really is a no-op (`update_option` with the same + // value returns false). + update_option( 'jetpack_' . Module_Control::JETPACK_ACTIVE_MODULES_OPTION_KEY, array() ); + $result = static::$search_module->update_experience( Module_Control::EXPERIENCE_OFF ); $this->assertFalse( $result );