#55881 closed defect (bug) (fixed)
Block templates with specific postTypes that have been edited by the user can be selected for any other post type
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Editor | Keywords: | has-patch |
Focuses: | Cc: |
Description
When a block template provided by a theme has been edited by the user, the postTypes
argument is ignored, so the template is available for the wrong post types.
Steps to reproduce
- With a block theme (I used Twenty Twenty Two), 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.
- Notice in the Template panel in the sidebar,
Custom Single Post template
is available whileCustom Single Post template (not modified)
isn't.
Expected behavior
If the user modifies a template in the Site Editor, that shouldn't affect which post types that template is available for.
More context
I think the issue is caused by `_build_block_template_result_from_post()` not setting a post_types
attribute to templates returned. _build_block_template_result_from_post()
is used by `get_block_template()`.
We identified that issue when working on WooCommerce templates. See woocommerce/woocommerce-blocks#6490.
Change History (18)
This ticket was mentioned in PR #3131 on WordPress/wordpress-develop by Aljullu.
3 years ago
#1
- Keywords has-patch added
3 years ago
#2
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 😄
3 years ago
#3
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
andget_block_templates
there, so it should hopefully be fairly straight-forward to do 😄
FWIW, I've now tried the following:
{{{diff
diff --git a/tests/phpunit/data/themedir1/block-theme/theme.json b/tests/phpunit/data/themedir1/block-theme/theme.json
index 38fcb1d9dd..666340d5ab 100644
--- a/tests/phpunit/data/themedir1/block-theme/theme.json
+++ b/tests/phpunit/data/themedir1/block-theme/theme.json
@@ -59,6 +59,16 @@
{
"name": "page-home",
"title": "Homepage template"
+ },
+ {
+ "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?
}
],
"templateParts": [
diff --git a/tests/phpunit/tests/block-template-utils.php b/tests/phpunit/tests/block-template-utils.php
index 666edd020a..8c930eeb30 100644
--- a/tests/phpunit/tests/block-template-utils.php
+++ b/tests/phpunit/tests/block-template-utils.php
@@ -319,6 +319,11 @@ 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() . '' . 'custom-single-post-template' ), $template_ids );
}
/
}}}
Unfortunately, this is currently failing:
There was 1 failure: 1) Tests_Block_Template_Utils::test_get_block_templates Failed asserting that two arrays are identical. --- Expected +++ Actual @@ @@ Array &0 ( - 0 => 'block-theme//custom-single-post-template' + 0 => 'block-theme//my_template' )
I have a few theories why that might be happening; will elaborate in a follow-up comment.
3 years ago
#4
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
andget_block_templates
there, so it should hopefully be fairly straight-forward to do 😄
FWIW, I've now tried the following:
{{{diff
diff --git a/tests/phpunit/data/themedir1/block-theme/theme.json b/tests/phpunit/data/themedir1/block-theme/theme.json
index 38fcb1d9dd..666340d5ab 100644
--- a/tests/phpunit/data/themedir1/block-theme/theme.json
+++ b/tests/phpunit/data/themedir1/block-theme/theme.json
@@ -59,6 +59,16 @@
{
"name": "page-home",
"title": "Homepage template"
+ },
+ {
+ "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?
}
],
"templateParts": [
diff --git a/tests/phpunit/tests/block-template-utils.php b/tests/phpunit/tests/block-template-utils.php
index 666edd020a..8c930eeb30 100644
--- a/tests/phpunit/tests/block-template-utils.php
+++ b/tests/phpunit/tests/block-template-utils.php
@@ -319,6 +319,11 @@ 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() . '' . 'custom-single-post-template' ), $template_ids );
}
/
}}}
Unfortunately, npm run test:php -- --group=block-templates
is currently failing:
There was 1 failure: 1) Tests_Block_Template_Utils::test_get_block_templates Failed asserting that two arrays are identical. --- Expected +++ Actual @@ @@ Array &0 ( - 0 => 'block-theme//custom-single-post-template' + 0 => 'block-theme//my_template' )
I have a few theories why that might be happening; will elaborate in a follow-up comment.
3 years ago
#5
Here's some basic unit test coverage:
{{{diff
commit 4ea8873d670bfd785e51d5414938ae02b63d960c
Author: Bernie Reiter <ockham@…>
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 @@
+
+<p>Custom Single Post template</p>
+
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
+ );
}
/
}}}
However, this is currently passing both on this branch, _and_ on trunk
. This means that something is off -- it shouldn't pass on trunk
.
3 years ago
#6
The following test fails on trunk
but passes on this branch:
{{{diff
commit d526a41f1cfb994ca1d2876e985b94936f83f5f7
Author: Bernie Reiter <ockham@…>
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 @@
+
+<p>Custom Single Post template</p>
+
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
🙁
3 years ago
#7
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.)
3 years ago
#8
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.
3 years ago
#9
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 [https://github.com/WordPress/wordpress-develop/pull/3131/files#diff-8d71d763873e0209e6b7435705f6b9c87390afe4ec5a2a7002c72fe0dd75b179 wpThemeJsonResolver.php]
, 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.
3 years ago
#10
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
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_templatespage-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(
}}}
#12
@
3 years ago
- Milestone changed from Awaiting Review to 6.1
- Owner set to hellofromTonya
- Status changed from new to reviewing
I can reproduce the reported issue. Moving the ticket into 6.1.
There is a patch ready for review, testing, and commit consideration. I'll work on that next.
hellofromtonya commented on PR #3131:
3 years ago
#13
I can confirm that the PR fixes the reported issue. However, the tests are not validating the issue. Why? Without the fix applied, the tests should fail. Then applying the fix, the tests should pass. However, currently the tests are passing without the fix applied.
I'm looking at the tests now.
hellofromtonya commented on PR #3131:
3 years ago
#14
I can confirm that the PR fixes the reported issue. However, the tests are not validating the issue. Why? Without the fix applied, the tests should fail. Then applying the fix, the tests should pass. However, currently the tests are passing without the fix applied.
I'm looking at the tests now.
hellofromtonya commented on PR #3131:
3 years ago
#16
Great job @Aljullu and @ockham 👏 Thank you for your contributions! This PR has been committed via changeset https://core.trac.wordpress.org/changeset/54184.
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
{{{JSON
{
}
}}}
Before | After
| 
--- | ---