Skip to content
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.

Sign up for GitHub

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

Block Hooks: Persist information about ignoredHookedBlocks in anchor block metadata #5712

Closed
wants to merge 19 commits into from
+114 −18
Closed

Block Hooks: Persist information about ignoredHookedBlocks in anchor block metadata #5712

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c74cdb2
Extract get_hooked_block_markup function
ockham Nov 28, 2023
552c66e
Set ignoredHookedBlocks metadata
ockham Nov 28, 2023
498dcf2
Rewrite a bit
ockham Nov 28, 2023
450c05f
Rewrite a bit more
ockham Nov 28, 2023
bef2301
Add ticket PHPDoc
ockham Nov 28, 2023
061cbd6
Inject hooked blocks into modified templates and parts
ockham Nov 28, 2023
acbd7f2
Relax test assertion a bit
ockham Nov 28, 2023
9e22a87
Get tests to pass
ockham Nov 28, 2023
ca6b474
Fix test
ockham Nov 28, 2023
3ad8dce
Relax tests
ockham Nov 28, 2023
df1fc75
Add some assertions for ignoredHookedBlocks
ockham Nov 28, 2023
9339ea2
Rename test
ockham Nov 28, 2023
5289f74
Add PHPDoc
ockham Nov 28, 2023
7dc3dfa
Tabs vs spaces
ockham Nov 28, 2023
dc35659
Missing .php file extension :facepalm:
ockham Nov 29, 2023
ef73917
Add test case
ockham Nov 29, 2023
41c2f1c
Coding Standards
ockham Nov 30, 2023
f3dbb7a
Revert "Inject hooked blocks into modified templates and parts"
ockham Dec 4, 2023
f4379e1
Mark get_hooked_block_markup() as private
ockham Dec 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/wp-includes/block-template-utils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,14 @@ function _build_block_template_result_from_post( $post ) {
}
}

$hooked_blocks = get_hooked_blocks();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we leave out the change in this file, we could land the rest at any point. So the question is whether we can think of any potential issue with making the edited templates hookable?

Do we have user-created patterns covered?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we leave out the change in this file, we could land the rest at any point. So the question is whether we can think of any potential issue with making the edited templates hookable?

We can give it some smoke testing, plus maybe some more unit test coverage (e.g. in tests/phpunit/tests/block-templates/buildBlockTemplateResultFromPost.php) for increased confidence.

Do we have user-created patterns covered?

I haven't checked yet! I'm assuming that yes (since AFAIK their only "entry point" is the Patterns Registry class), but it'll be good to verify 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that leaves only Synced Blocks (aka Reusable Blocks), which are loaded through code similar to:

$reusable_block = get_post( $attributes['ref'] );
$content = do_blocks( $reusable_block->post_content );

if ( ! empty( $hooked_blocks ) || has_filter( 'hooked_block_types' ) ) {
$before_block_visitor = make_before_block_visitor( $hooked_blocks, $template );
$after_block_visitor = make_after_block_visitor( $hooked_blocks, $template );
$blocks = parse_blocks( $template->content );
$template->content = traverse_and_serialize_blocks( $blocks, $before_block_visitor, $after_block_visitor );
}

return $template;
}

Expand Down
37 changes: 33 additions & 4 deletions src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,35 @@ function get_hooked_blocks() {
return $hooked_blocks;
}

/**
* Conditionally returns the markup for a given hooked block type.
*
* Accepts two arguments: A reference to an anchor block, and the name of a hooked block type.
* If the anchor block has already been processed, and the given hooked block type is in the list
* of ignored hooked blocks, an empty string is returned.
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other similar utilities, should it be marked as private, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f4379e1.

* @since 6.5.0
*
* @param array $anchor_block The anchor block. Passed by reference.
* @param string $hooked_block_type The name of the hooked block type.
* @return string The markup for the given hooked block type, or an empty string if the block is ignored.
*/
function get_hooked_block_markup( &$anchor_block, $hooked_block_type ) {
if ( ! isset( $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] ) ) {
$anchor_block['attrs']['metadata']['ignoredHookedBlocks'] = array();
}

if ( in_array( $hooked_block_type, $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] ) ) {
return '';
}

// The following is only needed for the REST API endpoint.
// However, its presence does not affect the frontend.
$anchor_block['attrs']['metadata']['ignoredHookedBlocks'][] = $hooked_block_type;

return get_comment_delimited_block_content( $hooked_block_type, array(), '' );
}

/**
* Returns a function that injects the theme attribute into, and hooked blocks before, a given block.
*
Expand Down Expand Up @@ -813,7 +842,7 @@ function make_before_block_visitor( $hooked_blocks, $context ) {
*/
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $parent_block, $hooked_block_type );
}
}

Expand All @@ -826,7 +855,7 @@ function make_before_block_visitor( $hooked_blocks, $context ) {
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $block, $hooked_block_type );
}

return $markup;
Expand Down Expand Up @@ -874,7 +903,7 @@ function make_after_block_visitor( $hooked_blocks, $context ) {
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $block, $hooked_block_type );
}

if ( $parent_block && ! $next ) {
Expand All @@ -888,7 +917,7 @@ function make_after_block_visitor( $hooked_blocks, $context ) {
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $parent_block, $hooked_block_type );
}
}

Expand Down
69 changes: 69 additions & 0 deletions tests/phpunit/tests/blocks/getHookedBlockMarkup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php
/**
* Tests for the get_hooked_block_markup function.
*
* @package WordPress
* @subpackage Blocks
*
* @since 6.5.0
*
* @group blocks
* @group block-hooks
*/
class Tests_Blocks_GetHookedBlockMarkup extends WP_UnitTestCase
{

Check failure on line 14 in tests/phpunit/tests/blocks/getHookedBlockMarkup.php

View workflow job for this annotation

GitHub Actions / PHP coding standards 

Opening brace should be on the same line as the declaration for class Tests_Blocks_GetHookedBlockMarkup
/**
* @ticket 59646
*
* @covers ::get_hooked_block_markup
*/
public function test_get_hooked_block_markup_adds_metadata() {
$anchor_block = array(
'blockName' => 'tests/anchor-block',
);

$actual = get_hooked_block_markup( $anchor_block, 'tests/hooked-block' );
$this->assertSame( array( 'tests/hooked-block' ), $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] );
$this->assertSame( '<!-- wp:tests/hooked-block /-->', $actual );
}

/**
* @ticket 59646
*
* @covers ::get_hooked_block_markup
*/
public function test_get_hooked_block_markup_if_block_is_already_hooked() {
$anchor_block = array(
'blockName' => 'tests/anchor-block',
'attrs' => array(
'metadata' => array(
'ignoredHookedBlocks' => array( 'tests/hooked-block' ),
),
)

Check failure on line 42 in tests/phpunit/tests/blocks/getHookedBlockMarkup.php

View workflow job for this annotation

GitHub Actions / PHP coding standards 

There should be a comma after the last array item in a multi-line array.
);

$actual = get_hooked_block_markup( $anchor_block, 'tests/hooked-block' );
$this->assertSame( array( 'tests/hooked-block' ), $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] );
$this->assertSame( '', $actual );
}

/**
* @ticket 59646
*
* @covers ::get_hooked_block_markup
*/
public function test_get_hooked_block_markup_adds_to_ignored_hooked_blocks() {
$anchor_block = array(
'blockName' => 'tests/anchor-block',
'attrs' => array(
'metadata' => array(
'ignoredHookedBlocks' => array( 'tests/hooked-block' ),
),
)

Check failure on line 62 in tests/phpunit/tests/blocks/getHookedBlockMarkup.php

View workflow job for this annotation

GitHub Actions / PHP coding standards 

There should be a comma after the last array item in a multi-line array.
);

$actual = get_hooked_block_markup( $anchor_block, 'tests/other-hooked-block' );
$this->assertSame( array( 'tests/hooked-block', 'tests/other-hooked-block' ), $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] );
$this->assertSame( '<!-- wp:tests/other-hooked-block /-->', $actual );
}
}
6 changes: 3 additions & 3 deletions tests/phpunit/tests/blocks/getHookedBlocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public function test_loading_template_with_hooked_blocks() {
$template->content
);
$this->assertStringContainsString(
'<!-- wp:post-content {"layout":{"type":"constrained"}} /-->'
'<!-- wp:post-content {"layout":{"type":"constrained"},"metadata":{"ignoredHookedBlocks":["tests/hooked-after"]}} /-->'
. '<!-- wp:tests/hooked-after /-->',
$template->content
);
Expand All @@ -176,7 +176,7 @@ public function test_loading_template_part_with_hooked_blocks() {

$this->assertStringContainsString(
'<!-- wp:tests/hooked-before /-->'
. '<!-- wp:navigation {"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right"}} /-->',
. '<!-- wp:navigation {"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right"},"metadata":{"ignoredHookedBlocks":["tests/hooked-before"]}} /-->',
$template->content
);
$this->assertStringNotContainsString(
Expand Down Expand Up @@ -215,7 +215,7 @@ public function test_loading_pattern_with_hooked_blocks() {
$pattern['content']
);
$this->assertStringContainsString(
'<!-- wp:comments -->'
'<!-- wp:comments {"metadata":{"ignoredHookedBlocks":["tests/hooked-first-child"]}} -->'
. '<div class="wp-block-comments">'
. '<!-- wp:tests/hooked-first-child /-->',
str_replace( array( "\n", "\t" ), '', $pattern['content'] )
Expand Down
18 changes: 7 additions & 11 deletions tests/phpunit/tests/blocks/wpBlockPatternsRegistry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,14 +381,12 @@ public function test_get_all_registered_includes_hooked_blocks() {
$pattern_three['name'] = 'test/three';
$pattern_three['content'] .= '<!-- wp:tests/my-block /-->';

$expected = array(
$pattern_one,
$pattern_two,
$pattern_three,
);

$registered = $this->registry->get_all_registered();
$this->assertSame( $expected, $registered );
$this->assertCount( 3, $registered );
$this->assertStringEndsWith( '<!-- wp:tests/my-block /-->', $registered[1]['content'] );
$this->assertStringContainsString( '"metadata":{"ignoredHookedBlocks":["tests/my-block"]}', $registered[1]['content'] );
$this->assertStringEndsWith( '<!-- wp:tests/my-block /-->', $registered[2]['content'] );
$this->assertStringContainsString( '"metadata":{"ignoredHookedBlocks":["tests/my-block"]}', $registered[2]['content'] );
}

/**
Expand Down Expand Up @@ -444,11 +442,9 @@ public function test_get_registered_includes_hooked_blocks() {
);
$this->registry->register( 'test/two', $pattern_two );

$pattern_one['name'] = 'test/one';
$pattern_one['content'] = '<!-- wp:tests/my-block /-->' . $pattern_one['content'];

$pattern = $this->registry->get_registered( 'test/one' );
$this->assertSame( $pattern_one, $pattern );
$this->assertStringStartsWith( '<!-- wp:tests/my-block /-->', $pattern['content'] );
$this->assertStringContainsString( '"metadata":{"ignoredHookedBlocks":["tests/my-block"]}', $pattern['content'] );
}

/**
Expand Down
Loading