Use core version of template and template part post types and REST endpoints for WP 5.9, with back compat for 5.8 #36898

talldan · 2021-11-26T05:57:58Z

Description

When the Gutenberg plugin is active, it's currently using it's own version of the template and template part post types and REST endpoints.

Usually the practice is to rely on the core version of PHP code when it exists.

register_post_type overwrites any previous registration and we don't want it to overwrite the core version of the post types. The fix here is to check whether the post types have already been registered before attempting to register them again.

This also solves the REST API issue, as that's also defined as part of the post type configuration.

How has this been tested?

Make sure you're running WordPress 5.9
Change something related to one of the post types in the Gutenberg codebase (i.e. one of the labels)
Load the relevant part of the UI and notice the change is not displayed because this code isn't active.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

youknowriad · 2021-11-26T09:00:46Z

Is it possible to move this code into compat/5.9 to know that it's only used on older WP versions?

talldan · 2021-11-26T09:06:32Z

@Mamaduka pointed out that this won't work for users of WordPress 5.8, because it'll cause Gutenberg to use the outdated REST APIs in 5.8.

Is it possible to move this code into compat/5.9 to know that it's only used on older WP versions?

@youknowriad Will that solve the problem mentioned above? I'll have a look at it. At the end of my day now, but I don't think there's a rush to do this, so will try to tackle it on Monday.

youknowriad · 2021-11-26T09:07:35Z

Will that solve the problem mentioned above?

No it won't but we can still achieve it by testing something specific to WP 5.9 before registering the CPT and endpoints.

talldan · 2021-11-26T09:15:27Z

No it won't but we can still achieve it by testing something specific to WP 5.9 before registering the CPT and endpoints.

Great, thanks for the advice. I'll make those changes on Monday 👍

…dpoints

…WP5.8.x is active

…ives improvements to it

talldan · 2021-12-01T05:39:50Z

lib/compat/wordpress-5.9/block-template-utils.php

-
-			return $new_content;
+		if ( ! $has_updated_content ) {
+			return $template_content;


There were some minor changes to this function in WordPress core, so I've replicated them for Gutenberg.

In this branch there is no foreach ( $template_blocks as $block ) which is something I see in the Core version. Is that right?

Yeah, oops. I obviously wasn't thorough enough. Fixed in #37137.

…ore implementation

talldan · 2021-12-01T07:44:20Z

lib/compat/wordpress-5.9/class-gutenberg-block-template.php

@@ -9,7 +9,7 @@
 /**
 * Class representing a block template.
 */
-class WP_Block_Template {
+class Gutenberg_Block_Template {


I've renamed this and the situation should be:

If Gutenberg is running on WordPress 5.8, Gutenberg_Block_Template is used instead of WP_Block_Template`, since the latter is outdated in 5.8.

If Gutenberg is running on WordPress 5.9, WP_Block_Template is used.

This should resolve any issues where author or origin might not be defined on 5.8.

talldan · 2021-12-01T07:59:43Z

phpunit/class-gutenberg-rest-global-styles-controller-test.php

@@ -33,7 +33,6 @@ public function set_up() {
 	 * @param WP_UnitTest_Factory $factory Helper that lets us create fake data.
 	 */
 	public static function wpSetupBeforeClass( $factory ) {
-		gutenberg_register_wp_theme_taxonomy();


This function is now not defined for Gutenberg running against WP 5.9, as the taxonomy is registered in core. The call here was making the test fail though. I deleted it and the tests still pass, so I'll call that a win 🤷

youknowriad

LGTM 👍

youknowriad · 2021-12-01T08:18:22Z

Technically there's no backport to be done into Core but we should sync the "wp/trunk" branch, so I added the label.

talldan · 2021-12-01T08:27:38Z

Thanks for the thumbs up. I'll do some testing against 5.8 tomorrow before merging just to make sure I didn't break anything.

I'll also follow up to fix some more little inconsistencies between core and Gutenberg.

talldan · 2021-12-02T08:51:31Z

Tested against 5.8.2 and can't see any issues, so moving ahead with merging this.

…dpoints for WP 5.9, with back compat for 5.8 (#36898) * Use core version of template and template part post types and REST endpoints * Move files and add explanation * Only register gutenberg version of templates and template parts when WP5.8.x is active * Rename WP_Block_Template to Gutenberg_Block_Template so that 5.8 receives improvements to it * Load moved files * Update references and sync some code * Fix filename * Remove templates tests since Gutenberg now defaults to using the WP core implementation * Linting and test fixes

noisysocks · 2021-12-06T04:18:22Z

lib/compat/wordpress-5.9/block-template-utils.php

-
-			return $new_content;
+		if ( ! $has_updated_content ) {
+			return $template_content;
 		}

 		return $template_content;


This if is now unnecessary. In both cases we return $template_content;. Is that right?

talldan added [Type] Bug An existing feature does not function as intended Core REST API Task Task for Core REST API efforts [Block] Template Part Affects the Template Parts Block labels Nov 26, 2021

talldan self-assigned this Nov 26, 2021

talldan requested review from noisysocks and youknowriad November 26, 2021 05:58

talldan mentioned this pull request Nov 26, 2021

Show theme, plugin or author in Added By column with appropriate icon or avatar #36763

Merged

7 tasks

This was referenced Nov 29, 2021

Add origin and author to template rest api #36896

Merged

Update template controller unit test now that author support is in WP core #36987

Merged

talldan added 6 commits December 1, 2021 12:52

Use core version of template and template part post types and REST en…

f6b3861

…dpoints

Move files and add explanation

d818541

Only register gutenberg version of templates and template parts when …

85f2cf7

…WP5.8.x is active

Rename WP_Block_Template to Gutenberg_Block_Template so that 5.8 rece…

d210a6c

…ives improvements to it

Load moved files

7452819

Update references and sync some code

b48f643

talldan force-pushed the fix/use-core-version-of-templates-rest-api branch from b03d527 to b48f643 Compare December 1, 2021 05:38

talldan requested review from spacedmonkey and TimothyBJacobs as code owners December 1, 2021 05:38

talldan commented Dec 1, 2021

View reviewed changes

talldan added 3 commits December 1, 2021 13:42

Fix filename

fbf6265

Remove templates tests since Gutenberg now defaults to using the WP c…

6d15485

…ore implementation

Linting and test fixes

293a96a

talldan marked this pull request as draft December 1, 2021 07:34

talldan commented Dec 1, 2021

View reviewed changes

talldan changed the title ~~Use core version of template and template part post types and REST endpoints~~ Use core version of template and template part post types and REST endpoints for WP 5.9, with back compat for 5.8 Dec 1, 2021

talldan commented Dec 1, 2021

View reviewed changes

youknowriad approved these changes Dec 1, 2021

View reviewed changes

youknowriad added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 1, 2021

talldan marked this pull request as ready for review December 2, 2021 08:50

talldan merged commit 8eb1f34 into trunk Dec 2, 2021

talldan deleted the fix/use-core-version-of-templates-rest-api branch December 2, 2021 08:51

github-actions bot added this to the Gutenberg 12.2 milestone Dec 2, 2021

noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 6, 2021

noisysocks reviewed Dec 6, 2021

View reviewed changes

talldan mentioned this pull request Dec 6, 2021

Fix mistake in _remove_theme_attribute_in_block_template_content #37137

Merged

7 tasks

noisysocks mentioned this pull request Dec 6, 2021

Update @wordpress packages WordPress/wordpress-develop#2012

Closed

youknowriad modified the milestones: Gutenberg 12.2, Gutenberg 12.1 Dec 8, 2021

Mamaduka mentioned this pull request Dec 8, 2021

Fix WP 5.9 check for conditionally running code #37235

Merged

7 tasks

ockham mentioned this pull request Jan 10, 2022

Template Controller: Make test use test theme #32233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use core version of template and template part post types and REST endpoints for WP 5.9, with back compat for 5.8 #36898

Use core version of template and template part post types and REST endpoints for WP 5.9, with back compat for 5.8 #36898

talldan commented Nov 26, 2021

youknowriad commented Nov 26, 2021

talldan commented Nov 26, 2021

youknowriad commented Nov 26, 2021

talldan commented Nov 26, 2021

talldan Dec 1, 2021

noisysocks Dec 6, 2021

talldan Dec 6, 2021

talldan Dec 1, 2021 •

edited

Loading

talldan Dec 1, 2021

youknowriad left a comment

youknowriad commented Dec 1, 2021

talldan commented Dec 1, 2021 •

edited

Loading

talldan commented Dec 2, 2021

noisysocks Dec 6, 2021

Use core version of template and template part post types and REST endpoints for WP 5.9, with back compat for 5.8 #36898

Use core version of template and template part post types and REST endpoints for WP 5.9, with back compat for 5.8 #36898

Conversation

talldan commented Nov 26, 2021

Description

How has this been tested?

Types of changes

Checklist:

youknowriad commented Nov 26, 2021

talldan commented Nov 26, 2021

youknowriad commented Nov 26, 2021

talldan commented Nov 26, 2021

talldan Dec 1, 2021

Choose a reason for hiding this comment

noisysocks Dec 6, 2021

Choose a reason for hiding this comment

talldan Dec 6, 2021

Choose a reason for hiding this comment

talldan Dec 1, 2021 • edited Loading

Choose a reason for hiding this comment

talldan Dec 1, 2021

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

youknowriad commented Dec 1, 2021

talldan commented Dec 1, 2021 • edited Loading

talldan commented Dec 2, 2021

noisysocks Dec 6, 2021

Choose a reason for hiding this comment

talldan Dec 1, 2021 •

edited

Loading

talldan commented Dec 1, 2021 •

edited

Loading