-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add: Template and template_lock to post types endpoint. #6865
Add: Template and template_lock to post types endpoint. #6865
Conversation
Trac Ticket MissingThis pull request is missing a link to a Trac ticket. For a contribution to be considered, there must be a corresponding ticket in Trac. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. More information about contributing to WordPress on GitHub can be found in the Core Handbook. |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
ca56a2f
to
319b0aa
Compare
src/wp-includes/rest-api/endpoints/class-wp-rest-post-types-controller.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't typically completely omit properties if they don't have a value defined. If these don't have a natural default, we should set them to null
and add null
as an allowed type. See the icon
property for an example.
This also needs tests.
Hi @TimothyBJacobs, thank you for the review 😊 In the case of template_lock which is an enum with the following possible values array( 'all', 'insert', 'contentOnly' ) would it still be ok to return null when a template_lock does not exist? |
Yep! We just need to make sure to add it to the enum list as well. |
bbddc4c
to
0e76951
Compare
I quickly checked for this in my review, thinking specifically about the enum. I approved based on that we actually do this in some places: for example, see the |
Gotcha, yeah I don't think we should be doing that there either really. I think for the most part we should be trying to keep the actual properties still defined. |
df67697
to
9244871
Compare
Is an empty array a valid template? Just want to make sure it's not, otherwise the current code will convert it to |
9244871
to
5867d02
Compare
Hi @TimothyBJacobs, very good point, an empty template is probably not expected but someone may be doing that I replaced the condition with isset. |
fe4d5cc
to
4f67113
Compare
@@ -246,6 +246,14 @@ public function prepare_item_for_response( $item, $request ) { | |||
$data['rest_namespace'] = $namespace; | |||
} | |||
|
|||
if ( rest_is_field_included( 'template', $fields ) ) { | |||
$data['template'] = ! isset( $post_type->template ) ? null : $post_type->template; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$data['template'] = ! isset( $post_type->template ) ? null : $post_type->template; | |
$data['template'] = $post_type->template ) ?? null; |
Can we use here ??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I followed this logic 👍
19eb733
to
f11d95a
Compare
It seems like an empty array is even the default: https://github.com/jorgefilipecosta/wordpress-develop/blob/f11d95acf7e65c86733db21975a652820d4dd1fd/src/wp-includes/class-wp-post-type.php#L548-L549 I'm not too fond of these defaults, but they are already like that for a long time so I updated the API to match them. |
f11d95a
to
51f79de
Compare
@@ -77,6 +77,24 @@ public function test_get_item_cpt() { | |||
$this->check_post_type_object_response( 'view', $response, 'cpt' ); | |||
} | |||
|
|||
public function test_get_item_template_cpt() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ticket
annotation is missing.
51f79de
to
f5cc335
Compare
Squashed commits: [fe4d5cc] add test cases for template and template_lock
Committed at 5d41bda. |
Backports WordPress/gutenberg#62488 into the core.
Adds template and template_lock property of the post type to post types REST API endpoint.
This change allows us to fix a bug where the template of a page is not respected when creating a new page on the site editor.
Trac ticket:https://core.trac.wordpress.org/ticket/61477
Testing
With the Gutenberg plugin disabled.
I added the following test code to a PHP file:
I opened the site editor and pasted:
wp.data.select('core').getPostType( 'page' );
I verified the template and template_lock information was included.
I repeated the test with the Gutenberg plugin enabled to make sure things worked well together.