Core data revisions: extend support to other post types #56353
Conversation
ramonjd
added
[Type] Enhancement
| 'wp_navigation', | ||
| 'wp_template', | ||
| 'wp_template_part', | ||
| ]; |
There was a problem hiding this comment.
The current thinking is to extend the view context's response with support information.
There was a problem hiding this comment.
Just double-checking what you mean here (sorry if I'm missing context — so to speak!): that we have a hard-coded list of types here because calling '/wp/v2/types?context=view' doesn't include a supports array in the response for each post type?
That makes sense to me, and from looking up wp-includes/post.php the list here corresponds with types that all opt-in to revisions under supports 👍
|
Size Change: +1.59 kB (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code-wise this is looking good to me 👍
One thing I noticed while testing with the provided function calls, is that revisions for wp_template and wp_template_part for me appear to return an array with each element having the same value (look at the wp_id of each). In the API response in the Network tab, each value in the array is unique, but not in what gets returned by .getRevisions():
Are you able to replicate that? I was wondering if it might be something to do with .id being the same with each of these items in the array, where wp_id is different? That's just a wild guess, though 🙂
| @@ -20,8 +20,16 @@ export const DEFAULT_ENTITY_KEY = 'id'; | |||
| const POST_RAW_ATTRIBUTES = [ 'title', 'excerpt', 'content' ]; | |||
|
|
|||
| // A hardcoded list of post types that support revisions. | |||
| // Reflects post types in Core's src/wp-includes/post.php. | |||
There was a problem hiding this comment.
Thanks for highlighting where these are set in core!
| 'wp_navigation', | ||
| 'wp_template', | ||
| 'wp_template_part', | ||
| ]; |
There was a problem hiding this comment.
Just double-checking what you mean here (sorry if I'm missing context — so to speak!): that we have a hard-coded list of types here because calling '/wp/v2/types?context=view' doesn't include a supports array in the response for each post type?
That makes sense to me, and from looking up wp-includes/post.php the list here corresponds with types that all opt-in to revisions under supports 👍
Nice find! 10 points! I can replicate AND the content is always the same in each revision 🤦🏻 I checked the database and the revisions look good however. Nevertheless, this is definitely not right, and indicates that something is up with the template revisions endpoint perhaps. In theory, we should just be able to get revisions using the real post id, e.g., Thank you for finding this! I'll look into it. |
Exactly that. It's available in the So the next step is to expose it somehow in the view context I believe. 😄 |
|
Converted to draft while I investigate |
Yes, it's related. So for revisions, we have to use the At the moment 
So we either need to create an exception for revisions, that is, store by the real "id", or do it for all of them. I tried the latter first and broke the site editor 🤣 |
|
I've got it storing the revisions the right way by sending the const isTemplate = [ 'wp_template', 'wp_template_part' ].includes( name );
return {
type: 'RECEIVE_ITEM_REVISIONS',
key: isTemplate ? 'wp_id' : DEFAULT_ENTITY_KEY,
items: Array.isArray( records ) ? records : [ records ],
recordKey,
meta,
query,
kind,
name,
invalidateCache,
};Though I think I need to find a way to define this in the I just need to crack removing them on 🤔 |
|
While testing this I found another bug: calling 
Added to: |
packages/core-data/src/actions.js
Outdated
| return { | ||
| type: 'RECEIVE_ITEM_REVISIONS', | ||
| key: isTemplate ? 'wp_id' : DEFAULT_ENTITY_KEY, |
There was a problem hiding this comment.
Why do we have a special case for templates?
There was a problem hiding this comment.
Good question.
Template records have ids that look like this: theme_name//template_slug.
This is fine for storing template entities themselves, since the ids are unique are usually unique:
{
'theme_name//template_slug_1': { ...data },
'theme_name//template_slug_2': { ...data },
}These template records have a property wp_id, whose value is the real post id (the unique key in the database).
The spicy bit is that templates revisions use the same the schema as their parents:
// a bunch of template revisions
[
{
id: 'theme_name//template_slug_1',
wp_id: 2 // revision id
},
{
id: 'theme_name//template_slug_1',
wp_id: 3 // revision id
},
]So we can't store them by the default key id as we do with other post types since it's not unique. We have to use wp_id.
What I've done is temporary, and I think the key should be defined somewhere in entityConfig - not sure how to do it cleanly yet as it's only required for revisions.
There was a problem hiding this comment.
AFAIK, the "id" is unique (I implemented that system)
There was a problem hiding this comment.
the "id" is unique
For the entities or their revisions?
Here's the raw output of /wp/v2/templates/twentytwentythree/wp-custom-template-roo/revisions, where wp-custom-template-roo is a template I created with some changes.
API response
[
{
"id": "twentytwentythree//wp-custom-template-roo",
"theme": "twentytwentythree",
"content": {
"raw": "<!-- wp:paragraph -->\n<p>roo12</p>\n<!-- /wp:paragraph -->"
},
"slug": "142-revision-v1",
"source": "custom",
"origin": null,
"type": "revision",
"description": "",
"title": {
"raw": "roo",
"rendered": "roo"
},
"status": "inherit",
"wp_id": 145,
"has_theme_file": false,
"author": 1,
"modified": "2023-11-22T04:02:31",
"parent": 142,
"_links": {
"self": [
{
"href": "http://localhost:8888/index.php?rest_route=/wp/v2/templates/twentytwentythree//wp-custom-template-roo/revisions/145"
}
],
"parent": [
{
"href": "http://localhost:8888/index.php?rest_route=/wp/v2/templates/twentytwentythree//wp-custom-template-roo"
}
]
}
},
{
"id": "twentytwentythree//wp-custom-template-roo",
"theme": "twentytwentythree",
"content": {
"raw": "<!-- wp:paragraph -->\n<p>roo1</p>\n<!-- /wp:paragraph -->"
},
"slug": "142-revision-v1",
"source": "custom",
"origin": null,
"type": "revision",
"description": "",
"title": {
"raw": "roo",
"rendered": "roo"
},
"status": "inherit",
"wp_id": 144,
"has_theme_file": false,
"author": 1,
"modified": "2023-11-22T04:02:28",
"parent": 142,
"_links": {
"self": [
{
"href": "http://localhost:8888/index.php?rest_route=/wp/v2/templates/twentytwentythree//wp-custom-template-roo/revisions/144"
}
],
"parent": [
{
"href": "http://localhost:8888/index.php?rest_route=/wp/v2/templates/twentytwentythree//wp-custom-template-roo"
}
]
}
},
{
"id": "twentytwentythree//wp-custom-template-roo",
"theme": "twentytwentythree",
"content": {
"raw": "<!-- wp:paragraph -->\n<p>roo</p>\n<!-- /wp:paragraph -->"
},
"slug": "142-revision-v1",
"source": "custom",
"origin": null,
"type": "revision",
"description": "",
"title": {
"raw": "roo",
"rendered": "roo"
},
"status": "inherit",
"wp_id": 143,
"has_theme_file": false,
"author": 1,
"modified": "2023-11-22T04:02:26",
"parent": 142,
"_links": {
"self": [
{
"href": "http://localhost:8888/index.php?rest_route=/wp/v2/templates/twentytwentythree//wp-custom-template-roo/revisions/143"
}
],
"parent": [
{
"href": "http://localhost:8888/index.php?rest_route=/wp/v2/templates/twentytwentythree//wp-custom-template-roo"
}
]
}
}
]There was a problem hiding this comment.
For the entities, the "templates" are not always in the database so wp_id is not always set (it's not set if you load a raw untouched theme template). By yes, revisions are only for the "saved" templates, so the REST API for revisions is probably using wp_id.
The same should apply to template parts.
So yeah, there's a special case here for sure, I think it's probably ok to generate the URL to fetch revisions using wp_id for templates and template parts but it seems all the rest (all the places in our reducers....) should continue using id.
Fixed in #56416 |
ramonjd
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
Adds wp_block and wp_navigation
ramonjd
force-pushed
the
update/add-core-data-revisions-support-to-templates
branch
from
1eb6e18
to
c12f97b
Compare
There was a problem hiding this comment.
This is testing great for me now, @ramonjd!
✅ Tested getRevisions and getRevision for post
✅ Tested getRevisions and getRevision for page
✅ Tested getRevisions and getRevision for wp_block
✅ Tested getRevisions and getRevision for wp_navigation
✅ Tested getRevisions and getRevision for wp_template
✅ Tested getRevisions and getRevision for wp_template_part
✅ Tested that pagination options per_page and page work as expected
LGTM! ✨
| const entityConfig = configs.find( | ||
| ( config ) => config.kind === kind && config.name === name | ||
| ); | ||
| const key = |
There was a problem hiding this comment.
For me this should continue to use the default entity key.
There was a problem hiding this comment.
Do you mean DEFAULT_ENTITY_KEY?
The value is id, which doesn't work for template/template parts. To get the unique record id we have to refer to the wp_id property.
… as mirth in a grave, a jest without jesters. Begone, thou witless motley, for even a jest doth surpass thy meager worth! Empty line: *whimper*
ramonjd
deleted the
update/add-core-data-revisions-support-to-templates
branch
* Initial commit: add revisions support for templates * Renames constant to be clearer about what these post types apply to. Adds wp_block and wp_navigation * Testing updating record.key in the action. * Moving `revisionKey` to the entity config and use it in the action and resolvers * Me: Pray, depart hence, thou knavish trifle! Thy utility be as absent as mirth in a grave, a jest without jesters. Begone, thou witless motley, for even a jest doth surpass thy meager worth! Empty line: *whimper*
Follow up to:
What?
While we decide how best to expose revisions support to the frontend from the server, let's allow revisions support for templates, template parts, patterns and navigation post types.
Also changing the name of the supports array to
POST_TYPE_ENTITIES_WITH_REVISIONS_SUPPORTto highlight that it's for "post entities", not root entities.Why?
Share the love.
How?
Adding the post type slugs to the
POST_TYPE_ENTITIES_WITH_REVISIONS_SUPPORTarray.Testing Instructions
Fire up a block theme, head to the site editor and make some changes to:
Here's how to grab revisions for each post type in the browser's console:
Templates
Replace
templateSlugwith the template slug, e.g.,homeorwp-custom-template-my-template.await wp.data.resolveSelect( 'core' ).getRevisions( 'postType', 'wp_template',twentytwentyfour//${ templateSlug })Template parts
Replace
templateSlugwith the template part slug, e.g.,header.await wp.data.resolveSelect( 'core' ).getRevisions( 'postType', 'wp_template_part',twentytwentyfour//${ templateSlug })Navigation
Replace
parentIdwith the ID of the navigation post.await wp.data.resolveSelect( 'core' ).getRevisions( 'postType', 'wp_navigation', parentId )Patterns
Replace
parentIdwith the ID of the pattern post.await wp.data.resolveSelect( 'core' ).getRevisions( 'postType', 'wp_block', parentId )Also check that the revisions response updates as you continue to edit these posts.