Comment Template Block: Retain inner blocks inserted via render_block_data filter

[ockham]

What?

Add a unit test to verify that an inner block added to a given Comment Template block via the render_block_data filter is retained at render_block time. Then, change the block's code to actually retain those modifications from render_block_data.

Follow up to #50279.

Why?

See https://github.com/WordPress/gutenberg/pull/50279/files#r1184916607.

How?

The unit test: By adding a render_block_data filter to insert a Social Icons block, and a filter mock to the render_block hook to check the length of the Comment Template block's inner blocks.

The fix: By creating a new WP_Block instance from the parsed block (similar to how we did things before #50279; however, we continue to set block context via the render_block_context filter).

Testing Instructions

See CI. Locally, this can be tested via npm run test:unit:php -- --group=blocks.

[github-actions]

Flaky tests detected in 703e6c7.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5122178734
📝 Reported issues:

• [Flaky Test] can add submenus #50976 in /test/e2e/specs/editor/blocks/navigation.spec.js

[ockham]

Note that the test is currently failing (as expected, per this comment). (It passes however after reverting 4409ba4 and rebuilding blocks).

For a more permanent fix -- that will also retain the behavior encoded in #50879 -- I'm considering applying this patch to this PR:

diff --git a/packages/block-library/src/comment-template/index.php b/packages/block-library/src/comment-template/index.php
index 6a8698f60f..d3ffc67cf8 100644
--- a/packages/block-library/src/comment-template/index.php
+++ b/packages/block-library/src/comment-template/index.php
@@ -31,7 +31,7 @@ function block_core_comment_template_render_comments( $comments, $block ) {
                        return $context;
                };
                add_filter( 'render_block_context', $filter_block_context );
-               $block_content = $block->render( array( 'dynamic' => false ) );
+               $block_content = ( new WP_Block( $block->parsed_block ) )->render( array( 'dynamic' => false ) );
                remove_filter( 'render_block_context', $filter_block_context );
 
                $children = $comment->get_children();

[gziolo]

I believe that we still need to propagate the available context here to avoid issues like the one reported in https://core.trac.wordpress.org/ticket/58278 by @jdm-web. See also his comment in #50313 (comment).

I checked how we instantiate WP_Block in WordPress core and it both places the upper context gets passed down:

https://github.com/WordPress/wordpress-develop/blob/d46c47f0fdd43ad67743fb390187c4e32876d8a3/src/wp-includes/blocks.php#L1072

https://github.com/WordPress/wordpress-develop/blob/d46c47f0fdd43ad67743fb390187c4e32876d8a3/src/wp-includes/class-wp-block-list.php#L96

It would be great to investigate with some additional unit test. I guess it could be another PR, too.

[ockham]

Yeah, I'd prefer to tackle this separately. Since we're basically restoring the way we rendered the block from prior to #50279 here -- except for how we set block context -- I don't think this PR marks a regression compared to that state; if anything, it's a small improvement with regard to (parts of) block context. So I think it's fine to iterate separately 😊

[michalczaplinski]

LGTM but FWIW I've just learned from you about this need to construct a new WP_Block instance so that the render_block_data filter works correctly 🙂 .