Search: Filter_Static helper + REST endpoint (SEARCH-219, 2/3)#49039
Merged
Conversation
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Package] Search
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
github-actions
Bot
added
the
[Status] Needs Author Reply
This was referenced May 21, 2026
Code Coverage SummaryCoverage changed in 1 file.
1 file is newly checked for coverage.
|
kangzj
force-pushed
the
search-219-static-filter-store-foundation
branch
from
3abcffa to
dbca1bf
Compare
kangzj
force-pushed
the
search-219-static-filter-helper
branch
from
a45f65d to
ea9574c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…ing tests (PR #49039) - _doing_it_wrong call now matches the package convention: 'jetpack-search-pkg 7.0.0' (was 'jetpack-search 0.1.0'), and the message routes through esc_html__() + esc_html() with a phpcs:ignore comment — same pattern as Custom_Taxonomy_Slot_Mapping. (The earlier esc_html() wrapper around the already-translated message had the right intent but the wrong layer; switching to esc_html__() escapes at translation time, which is what the WP CS sniff wants.) - Drop the over-defensive is_object/method_exists duck-type in the REST callback — register_rest_route() always passes a WP_REST_Request. Type-hint the parameter explicitly to match the JSDoc. - Add 'validate_callback' => 'rest_validate_request_arg' to the route's variation arg so the enum check actually fires; without it, the enum declaration is only schema metadata. - Tests: - Filter_Static_Test now extends Search_TestCase so REST cases can dispatch through a real WP_REST_Server with admin/editor users. - Add test for the legacy + new hook union — the new jetpack_search_static_filters callback receives the legacy jetpack_instant_search_options.staticFilters list as input and can override entries by filter_id. - Add REST endpoint cases: anonymous request returns 401; authenticated editor returns the variation-scoped subset; invalid variation enum value returns 400; omitted variation defaults to sidebar. Addresses claude[bot]'s and copilot-swe-agent's reviews on PR #49039. <!-- jp-loop -->
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…criber test (PR #49039) Two non-blocking observations from claude[bot]'s re-review: - Remove the function_exists('register_rest_route') guard in register_rest_routes(). The callback only runs from the rest_api_init hook, which by definition fires after REST is loaded — the guard can't be true in production. - Add a test for a logged-in subscriber: REST should return 403 (not 401) so callers can tell 'not authenticated' from 'authenticated but unauthorised'. The 401 anonymous case already existed; this fills in the role-without-edit_posts side. <!-- jp-loop -->
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…ing tests (PR #49039) - _doing_it_wrong call now matches the package convention: 'jetpack-search-pkg 7.0.0' (was 'jetpack-search 0.1.0'), and the message routes through esc_html__() + esc_html() with a phpcs:ignore comment — same pattern as Custom_Taxonomy_Slot_Mapping. (The earlier esc_html() wrapper around the already-translated message had the right intent but the wrong layer; switching to esc_html__() escapes at translation time, which is what the WP CS sniff wants.) - Drop the over-defensive is_object/method_exists duck-type in the REST callback — register_rest_route() always passes a WP_REST_Request. Type-hint the parameter explicitly to match the JSDoc. - Add 'validate_callback' => 'rest_validate_request_arg' to the route's variation arg so the enum check actually fires; without it, the enum declaration is only schema metadata. - Tests: - Filter_Static_Test now extends Search_TestCase so REST cases can dispatch through a real WP_REST_Server with admin/editor users. - Add test for the legacy + new hook union — the new jetpack_search_static_filters callback receives the legacy jetpack_instant_search_options.staticFilters list as input and can override entries by filter_id. - Add REST endpoint cases: anonymous request returns 401; authenticated editor returns the variation-scoped subset; invalid variation enum value returns 400; omitted variation defaults to sidebar. Addresses claude[bot]'s and copilot-swe-agent's reviews on PR #49039. <!-- jp-loop -->
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…criber test (PR #49039) Two non-blocking observations from claude[bot]'s re-review: - Remove the function_exists('register_rest_route') guard in register_rest_routes(). The callback only runs from the rest_api_init hook, which by definition fires after REST is loaded — the guard can't be true in production. - Add a test for a logged-in subscriber: REST should return 403 (not 401) so callers can tell 'not authenticated' from 'authenticated but unauthorised'. The 401 anonymous case already existed; this fills in the role-without-edit_posts side. <!-- jp-loop -->
kangzj
force-pushed
the
search-219-static-filter-helper
branch
from
308ad57 to
7da8d9d
Compare
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…ing tests (PR #49039) - _doing_it_wrong call now matches the package convention: 'jetpack-search-pkg 7.0.0' (was 'jetpack-search 0.1.0'), and the message routes through esc_html__() + esc_html() with a phpcs:ignore comment — same pattern as Custom_Taxonomy_Slot_Mapping. (The earlier esc_html() wrapper around the already-translated message had the right intent but the wrong layer; switching to esc_html__() escapes at translation time, which is what the WP CS sniff wants.) - Drop the over-defensive is_object/method_exists duck-type in the REST callback — register_rest_route() always passes a WP_REST_Request. Type-hint the parameter explicitly to match the JSDoc. - Add 'validate_callback' => 'rest_validate_request_arg' to the route's variation arg so the enum check actually fires; without it, the enum declaration is only schema metadata. - Tests: - Filter_Static_Test now extends Search_TestCase so REST cases can dispatch through a real WP_REST_Server with admin/editor users. - Add test for the legacy + new hook union — the new jetpack_search_static_filters callback receives the legacy jetpack_instant_search_options.staticFilters list as input and can override entries by filter_id. - Add REST endpoint cases: anonymous request returns 401; authenticated editor returns the variation-scoped subset; invalid variation enum value returns 400; omitted variation defaults to sidebar. Addresses claude[bot]'s and copilot-swe-agent's reviews on PR #49039. <!-- jp-loop -->
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…criber test (PR #49039) Two non-blocking observations from claude[bot]'s re-review: - Remove the function_exists('register_rest_route') guard in register_rest_routes(). The callback only runs from the rest_api_init hook, which by definition fires after REST is loaded — the guard can't be true in production. - Add a test for a logged-in subscriber: REST should return 403 (not 401) so callers can tell 'not authenticated' from 'authenticated but unauthorised'. The 401 anonymous case already existed; this fills in the role-without-edit_posts side. <!-- jp-loop -->
kangzj
force-pushed
the
search-219-static-filter-helper
branch
from
7da8d9d to
350ab3d
Compare
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…ing tests (PR #49039) - _doing_it_wrong call now matches the package convention: 'jetpack-search-pkg 7.0.0' (was 'jetpack-search 0.1.0'), and the message routes through esc_html__() + esc_html() with a phpcs:ignore comment — same pattern as Custom_Taxonomy_Slot_Mapping. (The earlier esc_html() wrapper around the already-translated message had the right intent but the wrong layer; switching to esc_html__() escapes at translation time, which is what the WP CS sniff wants.) - Drop the over-defensive is_object/method_exists duck-type in the REST callback — register_rest_route() always passes a WP_REST_Request. Type-hint the parameter explicitly to match the JSDoc. - Add 'validate_callback' => 'rest_validate_request_arg' to the route's variation arg so the enum check actually fires; without it, the enum declaration is only schema metadata. - Tests: - Filter_Static_Test now extends Search_TestCase so REST cases can dispatch through a real WP_REST_Server with admin/editor users. - Add test for the legacy + new hook union — the new jetpack_search_static_filters callback receives the legacy jetpack_instant_search_options.staticFilters list as input and can override entries by filter_id. - Add REST endpoint cases: anonymous request returns 401; authenticated editor returns the variation-scoped subset; invalid variation enum value returns 400; omitted variation defaults to sidebar. Addresses claude[bot]'s and copilot-swe-agent's reviews on PR #49039. <!-- jp-loop -->
kangzj
added a commit
that referenced
this pull request
May 21, 2026
…criber test (PR #49039) Two non-blocking observations from claude[bot]'s re-review: - Remove the function_exists('register_rest_route') guard in register_rest_routes(). The callback only runs from the rest_api_init hook, which by definition fires after REST is loaded — the guard can't be true in production. - Add a test for a logged-in subscriber: REST should return 403 (not 401) so callers can tell 'not authenticated' from 'authenticated but unauthorised'. The 401 anonymous case already existed; this fills in the role-without-edit_posts side. <!-- jp-loop -->
kangzj
force-pushed
the
search-219-static-filter-helper
branch
from
350ab3d to
0c55d44
Compare
kangzj
removed
the
[Status] Needs Author Reply
Contributor
Author
🤖 Review-cycle summary —
|
| Source | Item | Resolution |
|---|---|---|
claude[bot] + copilot-swe-agent |
_doing_it_wrong version string 'jetpack-search 0.1.0' → 'jetpack-search-pkg 7.0.0' |
Matched package convention (d8e106578e). |
claude[bot] |
esc_html() wrapper layer wrong | Switched to esc_html__() + esc_html() arg-escape + phpcs:ignore (d8e106578e). |
claude[bot] + copilot-swe-agent |
Over-defensive is_object/method_exists guard in REST callback | Dropped guard, added type hint (d8e106578e). |
claude[bot] |
Missing REST endpoint tests | Added 4 cases + validate_callback wiring (d8e106578e). |
claude[bot] |
Missing hook-union test | Added test_new_hook_receives_legacy_payload_as_input (d8e106578e). |
claude[bot] |
Dead function_exists('register_rest_route') guard | Removed (308ad575c7). |
claude[bot] |
No 403 test for authenticated-without-edit_posts | Added test_rest_endpoint_forbids_user_without_edit_posts (308ad575c7). |
Cascade-only commits (no content changes in this PR's own diff):
- Multiple cascade-rebases as PR Search: store foundation for static-filter URL contract (SEARCH-219, 1/3) #49038 absorbed additional review fixes.
Reviewer outcome (last review):
claude[bot]: "No new issues. All five original bugs are fixed, all five missing tests are in place, and both round-2 observations are addressed. Ready to merge."copilot-swe-agent: "Looks good — no blockers, no suggestions. Ready to merge."
CI: 54 functional checks pass; 1 informational Code coverage requirement pending — non-blocking.
Base automatically changed from
search-219-static-filter-store-foundation
to
trunk
May 22, 2026 00:58
Server-side foundation for the upcoming jetpack-search/filter-static
block. Inert until the block is registered (part 3/3) — exposes a
helper class and REST route that the block's render.php and editor
inspector will consume.
- New `Filter_Static` class (block helpers package) reads the
site-configured static-filter list, dedupes by `filter_id`, and
exposes:
- `get_static_filters_config()` — memoised lookup. Resolves the union
of `jetpack_instant_search_options` (legacy options blob, used by
the overlay today) and `jetpack_search_static_filters` (narrower
sibling for blocks-only callers). Sites already wired up for the
overlay pick up the new block with zero migration.
- `filters_for_variation()` — narrow by `sidebar` vs `tabbed`
variation + optional filter_id scoping.
- `normalize_variation` / `normalize_entry` / `normalize_values` /
`build_config` / `derive_label` / `parse_url_selections`.
- Last-wins on duplicate `filter_id`; surfaces collisions via
`_doing_it_wrong()`.
- `Search_Blocks::register_rest_routes()` registers
`GET /jetpack-search/v1/static-filters?variation=<sidebar|tabbed>`
(permission `edit_posts`) — backs the block's editor inspector
picker. Falls under the existing `jetpack_search_blocks_enabled` gate.
- `Search_Blocks::build_initial_state()` seeds an empty
`staticFilterSelections` slot so JS readers see a defined shape on
pages without the block.
Test coverage: 15 new PHPUnit cases (`Filter_Static_Test`):
- Both hook surfaces (`jetpack_instant_search_options` legacy seam and
`jetpack_search_static_filters` narrower sibling).
- Memoisation + the reset_cache_for_testing() seam.
- Reserved-param `filter_id` rejection, empty-values rejection, per-value
sanitisation with `name` falling back to `value`.
- Last-wins dedupe on duplicate `filter_id`.
- `filters_for_variation` scoping (variation + optional filter_id).
- `normalize_variation` defaults, `build_config` shape lockdown
(the `kind:'static'` marker is what the JS store keys off).
- `derive_label` attribute override + fallback.
- `parse_url_selections` configured-keys-only gate, array-shape misuse
rejection, empty-string drop.
…ing tests (PR #49039) - _doing_it_wrong call now matches the package convention: 'jetpack-search-pkg 7.0.0' (was 'jetpack-search 0.1.0'), and the message routes through esc_html__() + esc_html() with a phpcs:ignore comment — same pattern as Custom_Taxonomy_Slot_Mapping. (The earlier esc_html() wrapper around the already-translated message had the right intent but the wrong layer; switching to esc_html__() escapes at translation time, which is what the WP CS sniff wants.) - Drop the over-defensive is_object/method_exists duck-type in the REST callback — register_rest_route() always passes a WP_REST_Request. Type-hint the parameter explicitly to match the JSDoc. - Add 'validate_callback' => 'rest_validate_request_arg' to the route's variation arg so the enum check actually fires; without it, the enum declaration is only schema metadata. - Tests: - Filter_Static_Test now extends Search_TestCase so REST cases can dispatch through a real WP_REST_Server with admin/editor users. - Add test for the legacy + new hook union — the new jetpack_search_static_filters callback receives the legacy jetpack_instant_search_options.staticFilters list as input and can override entries by filter_id. - Add REST endpoint cases: anonymous request returns 401; authenticated editor returns the variation-scoped subset; invalid variation enum value returns 400; omitted variation defaults to sidebar. Addresses claude[bot]'s and copilot-swe-agent's reviews on PR #49039. <!-- jp-loop -->
…criber test (PR #49039) Two non-blocking observations from claude[bot]'s re-review: - Remove the function_exists('register_rest_route') guard in register_rest_routes(). The callback only runs from the rest_api_init hook, which by definition fires after REST is loaded — the guard can't be true in production. - Add a test for a logged-in subscriber: REST should return 403 (not 401) so callers can tell 'not authenticated' from 'authenticated but unauthorised'. The 401 anonymous case already existed; this fills in the role-without-edit_posts side. <!-- jp-loop -->
kangzj
force-pushed
the
search-219-static-filter-helper
branch
from
0c55d44 to
20da295
Compare
The `/jetpack-search/v1/static-filters` route only existed to back a custom editor inspector picker for the upcoming `filter-static` block. The block's editor view will be a static placeholder (like the Post Type Scope block), so the route had no consumer — inert code. - Remove the `rest_api_init` hook, `register_rest_routes()` and `rest_get_static_filters()` from `Search_Blocks`. - Drop the REST dispatch tests and server scaffolding from `Filter_Static_Test` (helper-only coverage remains, 16 tests green). - `Filter_Static` helper is unchanged; `filters_for_variation()` still feeds part 3's front-end render. Front end never used REST — it reads the helper directly and round-trips via `?filter_id=value` params.
github-actions
Bot
removed
the
[Status] Needs Team Review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of three sequential PRs for SEARCH-219. Base branch: #49038 (review that one first; this one's diff is server-side only).
Filter_Statichelper +/wp-json/jetpack-search/v1/static-filtersREST endpoint + thestaticFilterSelectionsseed slot.jetpack-search/filter-staticblock + editor wiring + user-visible changelog entry.Proposed changes
Filter_Staticclass (block helpers package). Reads the site-configured static-filter list, dedupes byfilter_id, exposes:get_static_filters_config()— memoised. Resolves the union ofjetpack_instant_search_options(legacy options blob the overlay already uses) andjetpack_search_static_filters(narrower sibling for blocks-only callers). Sites already configured for the overlay pick up the new block with zero migration.filters_for_variation( $variation, $filter_id )— narrows by sidebar/tabbed + optional filter_id scoping.normalize_variation/normalize_entry/normalize_values/build_config/derive_label/parse_url_selections.filter_id; surfaces collisions via_doing_it_wrong().Search_Blocks::register_rest_routes()registersGET /jetpack-search/v1/static-filters?variation=<sidebar|tabbed>(permissionedit_posts). Backs the block's editor inspector picker in part 3.Search_Blocks::build_initial_state()adds an emptystaticFilterSelectionsslot so JS readers see a defined shape on pages without the block.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
This PR adds back-end surface; the block isn't registered yet (part 3).
vendor/bin/phpunit -c phpunit.11.xml.dist --filter='Filter_Static_Test'— 15 cases passing.jetpack_search_blocks_enabled=true, register a static filter viaadd_filter('jetpack_search_static_filters', fn () => [[...]] )and confirmwp eval "var_dump(Automattic\\Jetpack\\Search\\Filter_Static::get_static_filters_config());"returns the normalized entry./wp-json/jetpack-search/v1/static-filters?variation=sidebarreturns the filter list.