Changes for FSE backport in core #36201
Conversation
ntsekouras
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
| @@ -165,72 +165,6 @@ function _gutenberg_add_template_part_area_info( $template_info ) { | |||
| return $template_info; | |||
| } | |||
There was a problem hiding this comment.
I think you should be able to move all the functions from this file into the new block-template-utils.php file
| @@ -339,7 +339,7 @@ public function delete_item( $request ) { | |||
| * @return stdClass Changes to pass to wp_update_post. | |||
| */ | |||
| protected function prepare_item_for_database( $request ) { | |||
| $template = $request['id'] ? gutenberg_get_block_template( $request['id'], $this->post_type ) : null; | |||
| $template = $request['id'] ? get_block_template( $request['id'], $this->post_type ) : null; | |||
| $changes = new stdClass(); | |||
| if ( null === $template ) { | |||
| $changes->post_type = $this->post_type; | |||
There was a problem hiding this comment.
This endpoint can also be moved to the 5.9 folder
There was a problem hiding this comment.
I saw that the controller was introduced in 5.8. Should I put the whole controller under 5.9 folder? 🤔
There was a problem hiding this comment.
5.9 because it changed between the two versions, so it should only be removed after 5.9 is the minimum version.
There was a problem hiding this comment.
I'm not sure how to handle the endpoint here, as in load I guess we should have the check for if the class exists, but this will be true for 5.8 as well.
There was a problem hiding this comment.
Yeah, I think we could check something that only exists on 5.9 and have a comment there. Also the call registering the endpoint should use the same check and same comment.
There was a problem hiding this comment.
Also we should keep the class name different in order to be able to register it in 5.8 even if it already exists (Gutenberg prefix)
There was a problem hiding this comment.
If we keep the prefix, do we need a 5.9 check?
There was a problem hiding this comment.
do we need a 5.9 check?
Not necessarily because the versions are exactly the same, that said, I think keeping the check is important for one case: imagine if in the future, we need to make changes to the endpoint, this means the endpoint need to move to wordpress-6.0 folder but if we don't have the check in place, we won't notice it and we could update it but keep it in wordpress-5.9 which means we'll forget about it when it comes time to make backports for 6.0
There was a problem hiding this comment.
Hm.. okay. I think this is coupled with the registration of the post types (wp_template and wp_template_part) handling as well (rest_controller_class prop) and we can check it in a follow up? I guess a check for Gutenberg_REST_Global_Styles_Controller would be a good candidate when the patch makes it to core?
There was a problem hiding this comment.
If it complicates things too much, it might not be worth it.
|
For the unit tests, I actually think we should consider removing all the unit tests that were ported to Core once the patch lands. |
…ates, as they already exist from 5.8
|
I added back the I could see that we have similar handling in other |
Changes for the FSE back porting in core.
Related: WordPress/wordpress-develop#1796
It's still in progress and will have more changes..