Search 3.0: backend support for experience field in settings API (RSM-2291)

[Copilot]

Fixes RSM-2291: Search dashboard feature selection: backend support for experience field

Why

Site admins using the new feature-selector UI on the Search dashboard (shipped behind jetpack_search_blocks_enabled in #48500) currently can't actually save their choice — the existing settings API only models two booleans (module_active, instant_search_enabled) which can't distinguish the new "Embedded" experience from "Theme search," so picking Embedded silently degrades to Theme search after a save. This PR adds the back-end persistence and read path the new UI needs, so the dashboard reflects and persists the admin's actual choice.

Proposed changes

This branch is rebased on #48500, so the diff includes both the frontend feature-selector and the back-end work below.

• Data model. The wire format exposes experience as one of embedded | overlay | inline | off. Storage is narrower than the wire format — each fact lives where it's already owned:
  • 'off' is read from jetpack_active_modules (the global Jetpack module-active state) and not stored in this package's option, so flipping Search on/off via the global Jetpack dashboard, wp jetpack module …, or Jetpack::activate_module() doesn't drift.
  • 'inline' is the absence of an opt-in; saving inline deletes jetpack_search_experience rather than writing the value, so existing sites that have never engaged with the new UI are already correctly classified.
  • 'embedded' and 'overlay' are the only values actually written to the new option.
  • Legacy module_active / instant_search_enabled are kept in lockstep so unmigrated readers (Initializer, Options, sidebar registration) keep working until they're migrated to consult get_experience() in a follow-up.
• Module_Control::get_experience() resolves the wire value: off if the module isn't active, the saved embedded / overlay value if present, otherwise a legacy-boolean fallback (instant_search_enabled=true → 'overlay', else 'inline') for sites that have never saved via the new UI.
• Module_Control::update_experience() translates each wire value onto the storage shape. Adds the EXPERIENCE_OVERLAY / EXPERIENCE_EMBEDDED / EXPERIENCE_INLINE / EXPERIENCE_OFF constants and the SEARCH_MODULE_EXPERIENCE_OPTION_KEY (jetpack_search_experience) option key. 'off' deactivates the module and leaves both instant_search_enabled and the experience option untouched, so re-enabling later restores the prior preference (matches legacy ModuleControl behaviour).
• REST endpoint. POST /jetpack/v4/search/settings accepts an experience-only request body and delegates to update_experience(). validate_search_settings() allows experience-only requests without requiring the legacy booleans. GET /jetpack/v4/search/settings returns the resolved experience value alongside the existing fields.
• Initial state. Dashboard initial state includes experience so the React app can highlight the active row immediately at boot.
• Tests. Adds Module_Control_Test cases covering off-when-inactive (with stale option present), saved-value reads, legacy fallback, inline-deletes-option (seeded with 'embedded' to prove the switch clears it), off-preserves-other-state, and invalid-value rejection. Adds matching REST-controller integration tests for each wire value plus an invalid case.

Related product discussion/links

• peKye1-1Z1-p2
• Companion / parent front-end PR: Search 3.0: dashboard feature-selection UI (RSM-2116) #48500

Does this pull request change what data or activity we track or use?

No.

Testing instructions

The new UI is behind add_filter( 'jetpack_search_blocks_enabled', '__return_true' ); — set that on a test site so the new dashboard renders alongside the legacy ModuleControl.

1. Write path — POST /jetpack/v4/search/settings with each wire value:
   • { "experience": "overlay" } → response: module_active=true, instant_search_enabled=true, experience: "overlay". Confirm jetpack_search_experience option is 'overlay'.
   • { "experience": "embedded" } → response: module_active=true, instant_search_enabled=false, experience: "embedded". Confirm jetpack_search_experience option is 'embedded'.
   • { "experience": "inline" } → same booleans as embedded but experience: "inline". Confirm jetpack_search_experience option is deleted (not present in wp_options).
   • { "experience": "off" } → module_active=false, experience: "off". Confirm instant_search_enabled is unchanged from before the call, and the jetpack_search_experience option is preserved (so re-enabling later restores the prior overlay-vs-embedded choice).
2. Read path — GET /jetpack/v4/search/settings on a fresh site (no experience option saved):
   • Module inactive → experience: "off".
   • Module active + instant_search_enabled=true → experience: "overlay" (legacy fallback).
   • Module active + instant_search_enabled=false → experience: "inline" (legacy fallback).
3. After saving via the new UI, GET again — confirm the persisted value is returned, not derived.
4. Send an invalid experience (e.g. "foo") → confirm 400 response.
5. Confirm legacy requests (sending only module_active / instant_search_enabled) still work as before.
6. Toggle Search off via the global Jetpack dashboard or wp jetpack module deactivate search, then GET /search/settings → experience: "off" even if the jetpack_search_experience option still has a value (the global on/off bit wins).

[adamwoodnz]

@copilot follow the PR template please

[Copilot]

@copilot follow the PR template please

Updated the PR description to follow the template.

[github-actions]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the copilot/add-backend-support-experience-field branch.
• To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack copilot/add-backend-support-experience-field

bin/jetpack-downloader test jetpack-mu-wpcom-plugin copilot/add-backend-support-experience-field

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

[adamwoodnz]

@claude review please

[jp-launch-control]

Code Coverage Summary

Coverage changed in 4 files.

File	Coverage	Δ%	Δ Uncovered
projects/packages/search/src/class-rest-controller.php	194/231 (83.98%)	1.27%	0 💚
projects/packages/search/src/dashboard/class-initial-state.php	66/77 (85.71%)	0.19%	0 💚
projects/packages/search/src/class-module-control.php	67/74 (90.54%)	12.76%	-1 💚
projects/packages/search/src/classic-search/class-classic-search.php	39/870 (4.48%)	0.23%	-2 💚

Full summary · PHP report · JS report

[adamwoodnz]

@claude fresh review please

[claude]

Claude finished @adamwoodnz's task in 2m 27s —— View job

---

Review

• Gather context and read changed files
• Review class-module-control.php
• Review class-rest-controller.php
• Review class-initial-state.php
• Review tests
• Post review findings

---

Overall, this is a well-designed implementation. The storage model (off in module state, inline as absence of option, only embedded/overlay written affirmatively) is clean and the legacy fallback path is handled correctly. A few things worth addressing:

---

Bug / Logic Issues

1. Partial failure leaves inconsistent state in update_experience('overlay')

class-module-control.php lines 270–280

If activate() succeeds but enable_instant_search() returns a WP_Error (e.g., plan doesn't support instant search), the function returns the error without writing the experience option. The module is now active but the experience option is absent, so a subsequent get_experience() falls through to the legacy fallback — it reads instant_search_enabled=false and returns 'inline' rather than surfacing that overlay failed. A caller that expects rollback on error won't get it.

This is a pre-existing pattern in this codebase (other callers also don't roll back), but the new update_experience path is the first place where two side effects need to be atomic. Worth a note or a WP_Error path that at least deactivates the module on failure.

---

Minor Issues

2. Dead return true after exhaustive switch

class-module-control.php line 283

        }

        return true;  // <-- unreachable: all valid values have case branches, invalid rejected above
    }

The in_array guard at the top ensures only the four known values reach the switch, and all four have return statements. The trailing return true is dead code. Fix this →

3. Comment typo: "classic" should be "inline"

class-rest-controller.php line 259

// which encapsulates the storage shape (off → module deactivate, classic → delete option,

classic was renamed to inline in a prior commit. Should read inline → delete option. Fix this →

4. Silent field-dropping when experience and legacy fields are mixed

class-rest-controller.php lines 260–266

Sending { "experience": "overlay", "module_active": false } silently ignores module_active. The validation short-circuits on experience !== null and the legacy fields are never applied. A consumer could accidentally include both and be surprised. At minimum a doc note on the endpoint behaviour (experience takes full precedence) would be helpful, or the endpoint could return a 400 if legacy booleans are mixed with experience.

---

Test Coverage Notes

5. test_update_experience_off_preserves_other_state deactivation assertion is trivially true

Module_Control_Test.php lines 281–297

The filter return_search_active_array only intercepts option reads; the module is never actually written as active in the DB. So is_active() → false after the filter is removed regardless of whether deactivate() ran. The meaningful assertions (options preserved) are correct, but the assertFalse(is_active()) doesn't prove the module was deactivated.

6. No test for mixed experience + legacy boolean request

There's no REST test asserting that { "experience": "overlay", "module_active": false } returns overlay (i.e., module_active is ignored). Worth adding to document the behaviour explicitly.

---

Nits

• class-module-control.php line 44: SEARCH_MODULE_EXPERIENCE_OPTION_KEY = 'jetpack_search_experience' — the other keys in this class (SEARCH_MODULE_INSTANT_SEARCH_OPTION_KEY, etc.) are bare names without a jetpack_ prefix. This one is the full wp_options key rather than a bare slug. Inconsistent naming convention, though it's a constant so the usage is identical. Worth a comment noting the difference, or normalising to just search_experience and prefixing at call sites.
• get_experience() lines 199–204: checks explicitly for EMBEDDED and OVERLAY but silently falls through for any other stored value (e.g., a corrupt or externally written option). Consider adding a strict check with a fallback comment for clarity.

---

Summary

The implementation logic is sound. The storage design is well-reasoned and the legacy fallback correctly handles pre-existing sites. Three things worth fixing before merge: the stale "classic" comment typo, the dead return true, and ideally a note or guard on the mixed-field silent-drop behaviour. The partial-failure rollback in the overlay case is the only substantive concern but it's a known pattern in this codebase.

[adamwoodnz]

@kangzj — addressed in ed61648. Added jetpack_search_experience to $options_to_sync in projects/packages/sync/src/modules/class-search.php.

Flagging for the next reviewer / for WPcom coordination: per projects/packages/sync/AGENTS.md, whitelisting in this package only controls sending — WPcom's shadow replicastore filters independently and must also whitelist jetpack_search_experience for the value to be retained on the cache site. A separate WPcom-side diff is required before this option will actually be available to readers on the cache site (e.g. for the experience-derivation logic on WPcom).

Tracked as a follow-up: RSM-2535.

[kangzj]

should we return what the previous step's result rather than always successful return?

Similar applies the the following statements as well.

[adamwoodnz]

Partially addressed in caa3678 — the OFF branch now propagates Modules::deactivate()'s bool instead of hard-returning true. Added test_update_experience_off_when_module_already_inactive_returns_false to cover the no-op path (3a5e726 stabilizes its setup against state leakage from earlier tests in the class).

For the INLINE / EMBEDDED / OVERLAY branches I left the explicit return true in place. The remaining inner steps are update_option() / delete_option() / disable_instant_search() calls — all of which return false for benign no-change writes (the option already holds the target value). Propagating those would falsely report failure when a caller re-saves the same experience. The actual failure paths in those branches (activate() and enable_instant_search() returning WP_Error) are already short-circuited above. Happy to revisit if I'm reading the intent wrong.

[kangzj]

Two suggestions on the new update_settings flow — both quality nits, neither blocking. See inline.

[kangzj]

sanitize_text_field() is applied without a type check. If a client sends "experience": [] or "experience": {}, this triggers an array-to-string conversion warning (and a deprecation notice on PHP 8.1+). The downstream in_array check in update_experience() catches the bad value, but the warning is unnecessary noise.

Guard with is_string() (or cast (string)) before sanitizing, e.g.:

$experience = isset( $request_body['experience'] ) && is_string( $request_body['experience'] )
    ? sanitize_text_field( $request_body['experience'] )
    : null;

[adamwoodnz]

Fixed in 24b58b4 — added an is_string() guard up front so non-string experience payloads resolve to null and skip the experience branch. The downstream in_array() guard in update_experience() is left as defence-in-depth.

[kangzj]

swap_classic_to_inline_search is silently dropped when combined with experience.

validate_search_settings() rejects mixing experience with module_active / instant_search_enabled (good), but swap_classic_to_inline_search is allowed through — and then this early return ignores it without applying the value or surfacing an error. The validation error message also doesn't mention this field, so a caller debugging a missing swap update would have no signal.

Today's only caller is the new front-end (which sends { experience } alone), so impact is theoretical. But it's a footgun for any external API consumer that bundles fields, and the asymmetry contradicts the validation's stated intent ("don't silently lose those fields").

Fix options:

• Extend the validation rejection to also block swap_classic_to_inline_search when experience is set, or
• Process swap_classic_to_inline_search before the early return (the existing branch below already handles a swap-only update path).

[adamwoodnz]

Fixed in 4af42ed — validate_search_settings() now also rejects mixing experience with swap_classic_to_inline_search (returning 400 rest_invalid_arguments). Updated the error message and extended test_update_settings_experience_rejects_mixed_legacy_fields to cover the new combination.

[adamwoodnz]

All feedback addressed @kangzj, will merge after tests complete. Let me know if you have follow ups.