#65577 closed enhancement (fixed)
View Config: Add versioning handling
| Reported by: | ntsekouras | Owned by: | ntsekouras |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.1 |
| Component: | General | Version: | |
| Severity: | normal | Keywords: | has-patch gutenberg-merge commit has-unit-tests |
| Cc: | Focuses: |
Description (last modified by )
This is a continuation of https://core.trac.wordpress.org/ticket/65516 and will port the changes in view config from:
- View Config: Add versioning handling. GB PR: https://github.com/WordPress/gutenberg/pull/79809
Right now consumers of the filters have to customize the config by walking the raw payload (find the right index, foreach, unset, etc..), and imperative array manipulation can never be migrated. A later shape change could break existing callbacks in the wild. So even though there is only version 1 today and no migrations exist, this code is the safeguard to keep filters working in a possible shape change.
Change History (23)
This ticket was mentioned in PR #12391 on WordPress/wordpress-develop by @ntsekouras.
3 months ago
#2
- Keywords has-patch added
#5
@
3 months ago
- Keywords commit added
Reviewed and approved in both https://github.com/WordPress/gutenberg/pull/79809 and https://github.com/WordPress/wordpress-develop/pull/12391.
#7
@
3 months ago
- Resolution fixed
- Status closed → reopened
We ran into an issue at https://github.com/WordPress/gutenberg/pull/79809#issuecomment-4979962005 that we have addressed by https://github.com/WordPress/gutenberg/pull/80319. We need to backport that PR into core.
This ticket was mentioned in PR #12638 on WordPress/wordpress-develop by @oandregal.
3 months ago
#8
- Keywords has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/65577
Backports: https://github.com/WordPress/gutenberg/pull/80319
Follow-up to: https://github.com/WordPress/wordpress-develop/pull/12391
## What
Ports the latest iteration of the view configuration API from the Gutenberg update/view-config-api-versioning branch:
## Why
## How
- Rework the
WP_View_Config_Datawrite API aroundmerge(),replace(),set(), andremove(), all operating on patches of top-level keys and taking the schema version the change was authored against. - Make
get_data()private and move filter application into a new apply_filters() method so callbacks cannot read the materialized configuration and become coupled to its shape. - Update the default post type configuration callbacks to use the new single-patch
set()signature. - Adapt and expand the unit tests accordingly.
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code, Zed agent
Model(s): Opus, Fabble
Used for: Initial code skeleton and test suggestions; final implementation and tests were reviewed and edited by me.
This ticket was mentioned in PR #12641 on WordPress/wordpress-develop by @jorgefilipecosta.
3 months ago
#11
The get_entity_view_config_{$kind}_{$name} filter docblock told callbacks the container is mutated in place and there is "no need to return it; any returned value is ignored". Only the final return value is discarded: as with any filter, WP_Hook::apply_filters() passes each callback's return value to the next callback, so a callback that follows that advice and returns nothing hands null to every callback hooked at a later priority, which then fails with a fatal error the moment it calls a method on it.
This updates the docblock to document the standard filter contract — callbacks must return the container they received — and notes that the write methods return the container, so a callback can end with return $data->merge( $patch, $version );. Also fixes a doubled "the" in the same paragraph.
Documentation-only change; the dispatch itself is unchanged, and the existing filter tests already register callbacks that return the container.
This ticket was mentioned in PR #12644 on WordPress/wordpress-develop by @jorgefilipecosta.
3 months ago
#12
- Keywords has-unit-tests added
## What
Fixes three defects in WP_View_Config_Data's merge engine, each with a test that fails without the fix:
- An associative patch value landing on a list (or a non-empty list landing on an associative value) silently discarded the whole current value. It is now rejected with
_doing_it_wrong()and leaves the current value unchanged. - An empty array in a
merge()patch — classified as a list — wiped associative values while silently no-oping on lists. It is now a documented no-op for both shapes. - A list member appended by
merge()(no identity match) was stored verbatim, keeping nestednulls that every other write path consumes. Appended members now go throughstrip_nulls().
Port of the Gutenberg fix in https://github.com/WordPress/gutenberg/pull/80571; follow-up to https://github.com/WordPress/wordpress-develop/pull/12638.
## Why
merge_properties() classifies patch values by shape but let mismatches fall through the classification branches into silent data loss:
merge( array( 'view_list' => array( 'published' => array( 'title' => 'Live' ) ) ), 1 )(the pre-7.1 slug-keyed shape) dropped every default view and stored the slug-keyed map, which then serialized as a JSON object where the schema and the DataViews client expect an array — with no misuse notice.merge( array( 'default_view' => array( 'sort' => array() ) ) ), 1 )destroyed the sort map, while the same empty array overfiltersdid nothing — two inconsistent outcomes for one input, neither documented.- An appended
view_listmember carrying'filters' => nullstored the literalnullinto the REST response where the schema declares an array, violating the documented "a nestednulldeletes just the leaf it names in every case" contract.
These semantics harden into the plugin-facing version-1 patch contract when 7.1 ships, so they need to be right before the release.
## How
One rule in merge_properties(): a patch value only merges into a current value of the same shape. A non-empty mismatch in either direction is reported via _doing_it_wrong() (the public method name is threaded down for the notice) and keeps the current value, so a malformed patch cannot destroy configuration. An empty array is shape-ambiguous and merges nothing, so it is a no-op — clearing a list stays spelled replace() with an empty list, and resetting a key stays spelled null. Empty arrays are exempt from the mismatch guards, so an associative patch still creates a new nested map (the recursion hands array() to keys that do not exist yet). merge_list_by_identity() strips nulls from appended members, matching set() and the lists replace() swaps in. Docblocks (class, merge(), and the two helpers) document the rule.
## Testing
npm run test:php -- --filter 'Tests_View_Config_Data|Tests_View_Config_API'
New cases in tests/phpunit/tests/view-config-data.php, one per fix (plus the mirror guard), each verified to fail against the previous implementation:
test_merge_rejects_associative_patch_over_a_listtest_merge_rejects_list_patch_over_an_associative_valuetest_merge_empty_array_is_a_nooptest_merge_appended_member_drops_nested_nulls
## AI usage disclosure
The issues were found by an AI-assisted code review of https://github.com/WordPress/wordpress-develop/pull/12638, and the fix, tests, and this description were drafted with AI assistance (Claude Code). I reviewed and verified the changes, including running the new tests against the unfixed code to confirm they fail.
This ticket was mentioned in PR #12766 on WordPress/wordpress-develop by @oandregal.
2 months ago
#15
Trac ticket https://core.trac.wordpress.org/ticket/65577
Backports https://github.com/WordPress/gutenberg/pull/80832
Follow-up to https://github.com/WordPress/wordpress-develop/pull/12391
search and page are managed via the URL, which is their only source of truth, so they should not be persisted as part of a view configuration.
Removes both properties from the shared ViewBase schema in WP_REST_View_Config_Controller, updates the WP_View_Config_Data docblock example that used search, and adds a test asserting neither property is declared by any view in the item schema.
This ticket was mentioned in PR #12809 on WordPress/wordpress-develop by @oandregal.
2 months ago
#17
Trac ticket https://core.trac.wordpress.org/ticket/65577
Backports https://github.com/WordPress/gutenberg/pull/81068
Follow-up to https://github.com/WordPress/wordpress-develop/pull/12391
## What?
Lowercases the dynamic filter names for view config.
## Why?
Lowercase names match PHP practices, feedback.
## How?
Make the dynamic filter hook name lowercase.
## Testing Instructions
Load the Site Editor screens (Pages, Templates, Patterns&Parts) and verify they still work as before.
@oandregal commented on PR #12809:
2 months ago
#19
Backported at https://core.trac.wordpress.org/changeset/62970
This ticket was mentioned in PR #13330 on WordPress/wordpress-develop by @oandregal.
6 weeks ago
#21
Adds descriptions to the styles property of the table layout schema in the view config REST controller and to the ColumnStyle properties (width, maxWidth, minWidth, align). The styles description notes that the styles apply to the columns listed in the view fields and not to the primary column (title, media, and description fields), which takes the width the other columns leave over. No behavior changes.
# Backports
Gutenberg PRs:
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Fable
Used for: Porting the server-side change from the Gutenberg PR. Reviewed and edited by me.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
@ugyensupport commented on PR #13330:
6 weeks ago
#22
Tested against a local trunk (7.1) checkout: applied cleanly, php -l clean, and confirmed via OPTIONS /wp-json/wp/v2/view-config that the schema diff before/after is exactly the 4 new description fields (styles, width, maxWidth, minWidth, align), consistently across every place get_table_layout_schema()/get_column_style_schema() are reused (default_view, default_layouts.table, default_layouts.pickerTable, view_list[].view) — nothing else changed, confirming the "no behavior changes" claim.
One environment note unrelated to this PR itself: with Gutenberg active, its compat/wordpress-7.1/rest-api.php unconditionally re-registers this same route at PHP_INT_MAX, shadowing core's controller — so testing needs Gutenberg deactivated to see core's own schema.
Full test report and further discussion in the Trac ticket per the PR description: https://core.trac.wordpress.org/ticket/65577
@oandregal commented on PR #13330:
6 weeks ago
#23
Committed at https://core.trac.wordpress.org/changeset/63436
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
This is a placeholder PR for backporting https://github.com/WordPress/gutenberg/pull/79809 when it lands in GB first.
Trac ticket: https://core.trac.wordpress.org/ticket/65577
## Use of AI Tools