Honor postTypes property in theme's theme.json when getting templates from post #3131

Aljullu · 2022-08-24T18:01:33Z

When loading a block template from post (ie: the user had made some modifications to it), WP was not setting the post_types attribute for that template. That caused templates specific to some post types to show available for all.

Trac ticket: https://core.trac.wordpress.org/ticket/55881#ticket

Steps to test

With a block theme, create two templates and add them to theme.json:

{
	"version": 2,
	"customTemplates": [
		....
		{
			"name": "custom-single-post-template",
			"title": "Custom Single Post template",
			"postTypes": [
				"post"
			]
		},
		{
			"name": "custom-single-post-template-not-modified",
			"title": "Custom Single Post template (not modified)",
			"postTypes": [
				"post"
			]
		}
	],
	...
}

Go to Appearance > Editor > Templates and make a modification to Custom Single Post template.
Now, create a new page.
Verify in the Template panel in the sidebar neither Custom Single Post template nor Custom Single Post template (not modified) are available.

Before	After

github-actions · 2022-08-24T18:02:14Z

Hi @Aljullu! 👋

Thank you for your contribution to WordPress! 💖

It looks like this is your first pull request to wordpress-develop. Here are a few things to be aware of that may help you out!

No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description.

Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making.

More information about how GitHub pull requests can be used to contribute to WordPress can be found in this blog post.

Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook.

If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook.

The Developer Hub also documents the various coding standards that are followed:

Thank you,
The WordPress Project

ockham · 2022-09-06T12:14:28Z

Thanks a lot for your backport PR, @Aljullu!

Could you maybe add a bit of test coverage for the updated behavior to tests/phpunit/tests/block-template-utils.php? 😊

We already have some coverage for _build_block_template_result_from_post and get_block_templates there, so it should hopefully be fairly straight-forward to do 😄

ockham · 2022-09-08T13:46:30Z

Here's some basic unit test coverage:

commit 4ea8873d670bfd785e51d5414938ae02b63d960c
Author: Bernie Reiter <ockham@raz.or.at>
Date:   Thu Sep 8 15:44:59 2022 +0200

    Add basic test coverage

diff --git a/tests/phpunit/data/themedir1/block-theme/templates/custom-single-post-template.html b/tests/phpunit/data/themedir1/block-theme/templates/custom-single-post-template.html
new file mode 100644
index 0000000000..fc0a5f8a4b
--- /dev/null
+++ b/tests/phpunit/data/themedir1/block-theme/templates/custom-single-post-template.html
@@ -0,0 +1,3 @@
+<!-- wp:paragraph -->
+<p>Custom Single Post template</p>
+<!-- /wp:paragraph -->
diff --git a/tests/phpunit/data/themedir1/block-theme/theme.json b/tests/phpunit/data/themedir1/block-theme/theme.json
index 38fcb1d9dd..b4fc0500cc 100644
--- a/tests/phpunit/data/themedir1/block-theme/theme.json
+++ b/tests/phpunit/data/themedir1/block-theme/theme.json
@@ -59,6 +59,11 @@
 		{
 			"name": "page-home",
 			"title": "Homepage template"
+		},
+		{
+			"name": "custom-single-post-template",
+			"title": "Custom Single Post template",
+			"postTypes": ["post"]
 		}
 	],
 	"templateParts": [
diff --git a/tests/phpunit/tests/block-template-utils.php b/tests/phpunit/tests/block-template-utils.php
index 666edd020a..1c4adc66ff 100644
--- a/tests/phpunit/tests/block-template-utils.php
+++ b/tests/phpunit/tests/block-template-utils.php
@@ -319,6 +319,27 @@ class Tests_Block_Template_Utils extends WP_UnitTestCase {
 			$template_ids
 		);
 		*/
+
+		// Filter by post type.
+		$templates    = get_block_templates( array( 'post_type' => 'post' ), 'wp_template' );
+		$template_ids = get_template_ids( $templates );
+		$this->assertSame(
+			array(
+				get_stylesheet() . '//' . 'my_template',
+				get_stylesheet() . '//' . 'custom-single-post-template'
+			),
+			$template_ids
+		);
+
+		$templates    = get_block_templates( array( 'post_type' => 'page' ), 'wp_template' );
+		$template_ids = get_template_ids( $templates );
+		$this->assertSame(
+			array(
+				get_stylesheet() . '//' . 'my_template',
+				get_stylesheet() . '//' . 'page-home'
+			),
+			$template_ids
+		);
 	}
 
 	/**

This is currently passing both on this branch, and on trunk. I think this is expected; we now need to add logic to have a user-modified variation of that template, and test with that.

We probably need to do that in a similar way as we're doing here:

wordpress-develop/tests/phpunit/tests/block-template-utils.php

Lines 37 to 51 in cb6f447

    
           // Set up template post. 
        
           $args       = array( 
        
           	'post_type'    => 'wp_template', 
        
           	'post_name'    => 'my_template', 
        
           	'post_title'   => 'My Template', 
        
           	'post_content' => 'Content', 
        
           	'post_excerpt' => 'Description of my template', 
        
           	'tax_input'    => array( 
        
           		'wp_theme' => array( 
        
           			self::$test_theme, 
        
           		), 
        
           	), 
        
           ); 
        
           self::$post = self::factory()->post->create_and_get( $args ); 
        
           wp_set_post_terms( self::$post->ID, self::$test_theme, 'wp_theme' );

I haven't figured out how to set post_types for a template object that needs to be persisted in the DB yet, though 🤔

ockham · 2022-09-08T14:18:13Z

The following test fails on trunk but passes on this branch:

commit d526a41f1cfb994ca1d2876e985b94936f83f5f7
Author: Bernie Reiter <ockham@raz.or.at>
Date:   Thu Sep 8 15:44:59 2022 +0200

    Add basic test coverage

diff --git a/tests/phpunit/data/themedir1/block-theme/templates/custom-single-post-template.html b/tests/phpunit/data/themedir1/block-theme/templates/custom-single-post-template.html
new file mode 100644
index 0000000000..fc0a5f8a4b
--- /dev/null
+++ b/tests/phpunit/data/themedir1/block-theme/templates/custom-single-post-template.html
@@ -0,0 +1,3 @@
+<!-- wp:paragraph -->
+<p>Custom Single Post template</p>
+<!-- /wp:paragraph -->
diff --git a/tests/phpunit/data/themedir1/block-theme/theme.json b/tests/phpunit/data/themedir1/block-theme/theme.json
index 7676d642b2..4961a620a2 100644
--- a/tests/phpunit/data/themedir1/block-theme/theme.json
+++ b/tests/phpunit/data/themedir1/block-theme/theme.json
@@ -58,6 +58,11 @@
 		{
 			"name": "page-home",
 			"title": "Homepage template"
+		},
+		{
+			"name": "custom-single-post-template",
+			"title": "Custom Single Post template",
+			"postTypes": ["post"]
 		}
 	],
 	"templateParts": [
diff --git a/tests/phpunit/tests/block-template-utils.php b/tests/phpunit/tests/block-template-utils.php
index 666edd020a..6d16cc7260 100644
--- a/tests/phpunit/tests/block-template-utils.php
+++ b/tests/phpunit/tests/block-template-utils.php
@@ -12,6 +12,7 @@
  */
 class Tests_Block_Template_Utils extends WP_UnitTestCase {
 	private static $post;
+	private static $custom_single_post_template_post;
 	private static $template_part_post;
 	private static $test_theme = 'block-theme';
 
@@ -50,6 +51,22 @@ class Tests_Block_Template_Utils extends WP_UnitTestCase {
 		self::$post = self::factory()->post->create_and_get( $args );
 		wp_set_post_terms( self::$post->ID, self::$test_theme, 'wp_theme' );
 
+		// Set up template post.
+		$args       = array(
+			'post_type'    => 'wp_template',
+			'post_name'    => 'custom-single-post-template',
+			'post_title'   => 'Custom Single Post template (modified)',
+			'post_content' => 'Content',
+			'post_excerpt' => 'Description of custom single post template',
+			'tax_input'    => array(
+				'wp_theme' => array(
+					self::$test_theme,
+				),
+			),
+		);
+		self::$custom_single_post_template_post = self::factory()->post->create_and_get( $args );
+		wp_set_post_terms( self::$custom_single_post_template_post->ID, self::$test_theme, 'wp_theme' );
+
 		// Set up template part post.
 		$template_part_args       = array(
 			'post_type'    => 'wp_template_part',
@@ -319,6 +336,27 @@ class Tests_Block_Template_Utils extends WP_UnitTestCase {
 			$template_ids
 		);
 		*/
+
+		// Filter by post type.
+		$templates    = get_block_templates( array( 'post_type' => 'post' ), 'wp_template' );
+		$template_ids = get_template_ids( $templates );
+		$this->assertSame(
+			array(
+				get_stylesheet() . '//' . 'my_template',
+				get_stylesheet() . '//' . 'custom-single-post-template'
+			),
+			$template_ids
+		);
+
+		$templates    = get_block_templates( array( 'post_type' => 'page' ), 'wp_template' );
+		$template_ids = get_template_ids( $templates );
+		$this->assertSame(
+			array(
+				get_stylesheet() . '//' . 'my_template',
+				get_stylesheet() . '//' . 'page-home'
+			),
+			$template_ids
+		);
 	}
 
 	/**

(Formatting might still be off, phpcbf isn't working for me right now.)

Run with npm run test:php -- --group=block-templates 🙁

ockham

LGTM. Let's include some test coverage (this patch)

Aljullu · 2022-09-08T14:55:44Z

Thanks for looking into this, @ockham! Do you want to push the commit yourself, or do you want me to do it? (Both work for me, just want to make sure you get proper attribution for your work 🙂 )

ockham · 2022-09-08T15:00:46Z

Thanks for looking into this, @ockham! Do you want to push the commit yourself, or do you want me to do it? (Both work for me, just want to make sure you get proper attribution for your work 🙂 )

@Aljullu I don't think I have write access to your wordpress-develop fork, so it'd be great if you push the commit 😊 (Thanks for considering attribution, but no worries, I don't mind.)

… from post

Aljullu · 2022-09-09T08:44:54Z

Thanks @ockham! I applied your patch and rebased this PR. You will notice I had to make some small changes in some other tests as well, so I would appreciate if you can take a look.

ockham · 2022-09-12T09:55:09Z

Thanks @ockham! I applied your patch and rebased this PR. You will notice I had to make some small changes in some other tests as well, so I would appreciate if you can take a look.

Ah, thank you -- I should've run all tests at least once I was done updating the block template utils one 😅

It's a bit unfortunate that we have to add the new template to the tests in [wpThemeJsonResolver.php](https://github.com/WordPress/wordpress-develop/pull/3131/files#diff-8d71d763873e0209e6b7435705f6b9c87390afe4ec5a2a7002c72fe0dd75b179), since that template isn't really of any concern to what we're testing there (translation of template title and description). I'll have a quick look if I can find a solution where we don't have to include it.

ockham · 2022-09-12T13:16:48Z

Here's a new patch. It keeps the check for custom-single-post-template in test_merges_child_theme_json_into_parent_theme_json where it seemed to make some sense, but removes it from test_translations_are_applied where it doesn't (and changes the assertions accordingly).

Finally, I'd forgotten to remove the template post object during teardown in block-template-utils.php, so I've also added that.

diff --git a/tests/phpunit/tests/block-template-utils.php b/tests/phpunit/tests/block-template-utils.php
index 1f034fdc78..340bbfff4f 100644
--- a/tests/phpunit/tests/block-template-utils.php
+++ b/tests/phpunit/tests/block-template-utils.php
@@ -95,6 +95,7 @@ class Tests_Block_Template_Utils extends WP_UnitTestCase {
 
 	public static function wpTearDownAfterClass() {
 		wp_delete_post( self::$post->ID );
+		wp_delete_post( self::$custom_single_post_template_post->ID );
 	}
 
 	public function test_build_block_template_result_from_post() {
diff --git a/tests/phpunit/tests/theme/wpThemeJsonResolver.php b/tests/phpunit/tests/theme/wpThemeJsonResolver.php
index 7aabc1c599..a8a93f9b23 100644
--- a/tests/phpunit/tests/theme/wpThemeJsonResolver.php
+++ b/tests/phpunit/tests/theme/wpThemeJsonResolver.php
@@ -154,18 +154,15 @@ class Tests_Theme_wpThemeJsonResolver extends WP_UnitTestCase {
 			),
 			$theme_data->get_settings()
 		);
-		$this->assertSameSets(
+
+		$custom_templates = $theme_data->get_custom_templates();
+		$this->assertArrayHasKey( 'page-home', $custom_templates );
+		$this->assertSame(
+			$custom_templates['page-home'],
 			array(
-				'page-home'                   => array(
-					'title'     => 'Szablon strony głównej',
-					'postTypes' => array( 'page' ),
-				),
-				'custom-single-post-template' => array(
-					'title'     => 'Custom Single Post template',
-					'postTypes' => array( 'post' ),
-				),
-			),
-			$theme_data->get_custom_templates()
+				'title'     => 'Szablon strony głównej',
+				'postTypes' => array( 'page' ),
+			)
 		);
 		$this->assertSameSets(
 			array(

Aljullu · 2022-09-13T08:48:23Z

New patch applied. Thanks for following-up on this, @ockham!

Tests fail before the fix. Tests pass after the fix.

hellofromtonya · 2022-09-15T21:43:59Z

tests/phpunit/tests/block-template-utils.php

+	 * @param string $post_type Post type for query.
+	 * @param array  $expected  Expected template IDs.
+	 */
+	public function test_get_block_template_should_respect_posttypes_property( $post_type, $expected ) {


To isolate the tests for the fix, I've:

created each dataset for switching the post type

encapsulated the tests for validating that it respects the post type property

By isolating the tests, it makes it easier to identify the purpose of the test. It will also will make it easier when the test class is split into separate files one per function (this is out-of-scope for this PR).

The test class itself needs to be split into separate files, one for each function. But that's out-of-scope for this PR.

hellofromtonya

I am able to reproduce the reported issue.
I can confirm this PR does fix the reported issue.
And the tests do validate it is fixed by:
- Before applying the fix, the tests fail.
- After applying the fix, the tests pass ✅

This PR is ready for commit.

hellofromtonya · 2022-09-15T22:30:05Z

Great job @Aljullu and @ockham 👏 Thank you for your contributions! This PR has been committed via changeset https://core.trac.wordpress.org/changeset/54184.

Aljullu mentioned this pull request Aug 24, 2022

Block templates with specific postTypes that have been edited by the user can be selected for any other post type WordPress/gutenberg#43557

Closed

This was referenced Aug 25, 2022

Block Templates: Add the custom templates info for the template posts WordPress/gutenberg#43597

Merged

Plugin: Backport PHP changes for WordPress 6.1 release WordPress/gutenberg#43440

Closed

Aljullu force-pushed the honor-post-types-property-in-templates-from-post branch from 916828f to 551eeaf Compare September 7, 2022 10:51

ockham approved these changes Sep 8, 2022

View reviewed changes

Aljullu force-pushed the honor-post-types-property-in-templates-from-post branch 4 times, most recently from 4dc0582 to b86a12f Compare September 9, 2022 08:23

Aljullu added 2 commits September 9, 2022 10:27

Honor postTypes property in theme's theme.json when getting templates…

fbc7509

… from post

Code refactor to match gutenberg#43597

97c3b73

Aljullu force-pushed the honor-post-types-property-in-templates-from-post branch from b86a12f to e5df864 Compare September 9, 2022 08:28

Add tests

442ff41

Aljullu force-pushed the honor-post-types-property-in-templates-from-post branch from e5df864 to 442ff41 Compare September 13, 2022 08:39

hellofromtonya self-assigned this Sep 15, 2022

Isolates tests for this fix

6cb9b0a

Tests fail before the fix. Tests pass after the fix.

hellofromtonya reviewed Sep 15, 2022

View reviewed changes

hellofromtonya approved these changes Sep 15, 2022

View reviewed changes

hellofromtonya closed this Sep 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Honor postTypes property in theme's theme.json when getting templates from post #3131

Honor postTypes property in theme's theme.json when getting templates from post #3131

Aljullu commented Aug 24, 2022

github-actions bot commented Aug 24, 2022

ockham commented Sep 6, 2022

ockham commented Sep 8, 2022 •

edited

Loading

ockham commented Sep 8, 2022

ockham left a comment

Aljullu commented Sep 8, 2022

ockham commented Sep 8, 2022

Aljullu commented Sep 9, 2022

ockham commented Sep 12, 2022

ockham commented Sep 12, 2022

Aljullu commented Sep 13, 2022

hellofromtonya Sep 15, 2022

hellofromtonya left a comment

hellofromtonya commented Sep 15, 2022

Honor postTypes property in theme's theme.json when getting templates from post #3131

Honor postTypes property in theme's theme.json when getting templates from post #3131

Conversation

Aljullu commented Aug 24, 2022

Steps to test

github-actions bot commented Aug 24, 2022

ockham commented Sep 6, 2022

ockham commented Sep 8, 2022 • edited Loading

ockham commented Sep 8, 2022

ockham left a comment

Choose a reason for hiding this comment

Aljullu commented Sep 8, 2022

ockham commented Sep 8, 2022

Aljullu commented Sep 9, 2022

ockham commented Sep 12, 2022

ockham commented Sep 12, 2022

Aljullu commented Sep 13, 2022

hellofromtonya Sep 15, 2022

Choose a reason for hiding this comment

hellofromtonya left a comment

Choose a reason for hiding this comment

hellofromtonya commented Sep 15, 2022

ockham commented Sep 8, 2022 •

edited

Loading