Block Hooks: Harmonize ignoredHookedBlocks metadata injection logic

[Bernhard Reiter]

As of [57790] and ​https://github.com/WordPress/gutenberg/pull/59561, both the Templates and the Navigation endpoints use the ​`rest_pre_insert_*` filter to inject the ignoredHookedBlocks metadata attribute into anchor blocks, prior to persisting a template, template part, or navigation menu to the database.

​It has been requested that these methods be harmonized, i.e. to use the inject_ignored_hooked_blocks_metadata_attributes filter for both templates/parts and navigation menus. In particular, this would allow removing some of the filters that are currently present in the Navigation block in favor of more "generic" functions.

Conceptually, the relevant filters are indeed quite similar; the major difference is that the one for the Navigation block (block_core_navigation_update_ignore_hooked_blocks_meta) injects a _wp_ignored_hooked_blocks post meta into the wp_navigation post object, in order to allow hooked blocks to be inserted as a Navigation block's first or last child (see #59743).

To harmonize the filters, we would thus have to add a conditional in inject_ignored_hooked_blocks_metadata_attributes to set the post meta if the post type is wp_navigation. (Later on, we might consider extending this to include template parts, where it might also be desirable to insert hooked blocks as a Template Part block's first or last child; however, this should be tackled separately, in order not to conflate refactoring and adding new functionality.)

---

Eventually, it might also be possible to harmonize injection of hooked blocks (which happens upon reading from the database, unlike the injection of the ignoredHookedBlocks attribute discussed above, which happens upon writing to it.)

Trac ticket: https://core.trac.wordpress.org/ticket/60759

@ockham interested to know your thoughts on this approach. I can see an argument against the new function (_build_block_template_object_from_database_object) but without it the function inject_ignored_hooked_blocks_metadata_attributes becomes somewhat unwieldy and potentially hard to navigation.

Hmm, looks like inject_ignored_hooked_blocks_metadata_attributes was previously _really_ template-specific. I'm second-guessing now if it makes sense to use it as a shared filter for both rest_pre_insert_wp_template (and _part) on the one hand, and for rest_pre_insert_wp_navigation on the other, when we have to accommodate vastly different logic for those cases under the roof of that function after all 😕

I'm starting to think that we should after all start with a distinct filter function for rest_pre_insert_wp_navigation (basically copying over block_core_navigation_update_ignore_hooked_blocks_meta), and then work backwards from there 🤔

I'm starting to think that we should after all start with a distinct filter function for rest_pre_insert_wp_navigation (basically copying over block_core_navigation_update_ignore_hooked_blocks_meta), and then work backwards from there 🤔

We could probably extract a helper function for the applying the Block Hooks algorithm itself (i.e. running get_hooked_blocks, creating the visitors, and running traverse_and_serialized_blocks) that could hopefully be used by both inject_ignored_hooked_blocks_metadata_attributes and block_core_navigation_update_ignore_hooked_blocks_meta (or whatever we'll rename the latter to).

Trac ticket: https://core.trac.wordpress.org/ticket/60759

We could probably extract a helper function for the applying the Block Hooks algorithm itself (i.e. running get_hooked_blocks, creating the visitors, and running traverse_and_serialized_blocks) that could hopefully be used by both inject_ignored_hooked_blocks_metadata_attributes and block_core_navigation_update_ignore_hooked_blocks_meta (or whatever we'll rename the latter to).

I think it makes sense to keep the two concerns (injecting the data into template markup vs updating it in wp_postmeta) separate.

I've experimented with this approach here ​https://github.com/WordPress/wordpress-develop/pull/6604

Keen to know how you feel it compares to this one based on your comments above.

cc @ockham

This is shaping up nicely! 😄

We have some test coverage in ​https://github.com/WordPress/gutenberg/blob/6108134aae75d1bd4826256490c609cb29044cd8/phpunit/blocks/block-navigation-block-hooks-test.php that we can probably copy over (with some small tweaks).

I've experimented with this approach here #6604

Keen to know how you feel it compares to this one based on your comments above.

Loving it! That one reads a lot more naturally IMO 😄

@ockham I've addressed all your comments aside from the one we're going to address in a separate PR. I've also had to fix some merge conflicts with ​https://github.com/WordPress/wordpress-develop/pull/6358

@ockham I've addressed all your comments aside from the one we're going to address in a separate PR. I've also had to fix some merge conflicts with #6358

Awesome, thank you very much!

We'll need to land the ​GB counterpart PR first; I've approved it, you wanna do the honors?

After that, we'll still need to wait for the code to be sync'd over to Core, so that the ​built Navigation block code in Core is updated and the "old" filter isn't still being added there if the new one is present. As always, the package sync will happen before Feature Freeze (i.e. next Tue, June 4), but probably only after the GB RC is released (this Friday). This means we'll likely land this PR on Monday 🙂

Should we close this one in favor of ​https://github.com/WordPress/wordpress-develop/pull/6604? 😊

We'll need to land the ​GB counterpart PR first; I've approved it, you wanna do the honors?

I remembered you won't be able to do so today, so I want ahead and merged it 😬 Hope that's okay!

Should we close this one in favor of ​https://github.com/WordPress/wordpress-develop/pull/6604? 😊

Closing 😄

Please only review ​https://github.com/ockham/wordpress-develop/pull/2/commits/b4c13c2b83555353d5935c5eb103fdc2651827ab; all other commits are from ​https://github.com/WordPress/wordpress-develop/pull/6604.

In a similar vein as @tjcafferkey's ​https://github.com/WordPress/wordpress-develop/pull/6604, this introduces a new insert_hooked_blocks_into_rest_response filter and adds it to the rest_prepare_wp_navigation hook. (This branch is based on Tom's; the reason is that it uses at least one function that's introduced there.)

This is part of an effort to move code from the Navigation block into Core. Specifically, insert_hooked_blocks_into_rest_response is based on ​`block_core_navigation_insert_hooked_blocks_into_rest_response`.

Testing instructions:

Preparation: Make sure you're using a Block Theme to test, ideally TT4. Add the following code to your site (e.g. to your theme's functions.php).

function register_logout_block_as_navigation_last_child( $hooked_blocks, $position, $anchor_block, $context ) {
        if ( $anchor_block === 'core/navigation' && $position === 'last_child' ) {
                $hooked_blocks[] = 'core/loginout';
        }

        return $hooked_blocks;
}

add_filter( 'hooked_block_types', 'register_logout_block_as_navigation_last_child', 10, 4 );

1. If there's an existing wp_navigation post object for you Navigation menu in your database, make sure that it doesn't have any _wp_ignored_hooked_blocks post meta set. (You can check the latter via npm run wp-env run cli wp post meta list <postId>.) Otherwise, you'll need to delete said post meta.
2. Go to the site editor and verify that the Login/out button block is added to the Navigation menu. You will probably see _two_ copies. The reason is that the Navigation block code still keeps inserting hooked blocks on its own.
3. To avoid the latter, apply the following patch, and reload the editor. You should now see only one copy of the Login/out block added.

src/wp-includes/blocks/navigation.php

@@ -1566 +1566 @@
  */
 $rest_prepare_wp_navigation_core_callback = 'block_core_navigation_' . 'insert_hooked_blocks_into_rest_response';
 
-/*
- * Injection of hooked blocks into the Navigation block relies on some functions present in WP >= 6.5
- * that are not present in Gutenberg's WP 6.5 compatibility layer.
- */
-if ( function_exists( 'set_ignored_hooked_blocks_metadata' ) && ! has_filter( 'rest_prepare_wp_navigation', $rest_prepare_wp_navigation_core_callback ) ) {
-       add_filter( 'rest_prepare_wp_navigation', 'block_core_navigation_insert_hooked_blocks_into_rest_response', 10, 3 );
-}

I will follow up with a Gutenberg PR to make the Navigation block's insertion of hooked blocks conditional. That other PR will have to get merged and sync'd to Core _before_ we can land this change.

Trac ticket: https://core.trac.wordpress.org/ticket/60759

[Bernhard Reiter]

In 58291:

Block Hooks: Move ignoredHookedBlocks metadata injection logic.

As of [57790], the Templates endpoint uses the rest_pre_insert_* filter to inject the ignoredHookedBlocks metadata attribute into anchor blocks, prior to persisting a template or template part to the database. The same principle was implemented for the Navigation endpoint (where additionally, first and last child blocks added at the top level are store in the wp_navigation post object's post meta). The required logic was added to the Navigation block's code, i.e. inside the Gutenberg code repository, and then synchronized to Core.

In order to harmonize the code between the two endpoints, this changeset introduces a new update_ignored_hooked_blocks_postmeta function, which is based on the Navigation block's block_core_navigation_update_ignore_hooked_blocks_meta, alongside a few helper functions, and hooks it to the rest_pre_insert_wp_navigation filter hook. (The Navigation block has been prepared in [58275] to add an additional conditional to check for the new update_ignored_hooked_blocks_postmeta filter so there won't be any collisions.)

Eventually, this will allow to deprecate block_core_navigation_update_ignore_hooked_blocks_meta (and some related functions), and remove the relevant code from the Navigation block. It also paves the way for some other future changes, such as inserting a hooked block as a Template Part block's first or last child (#60854).

Props tomjcafferkey, bernhard-reiter.
Fixes #60759.

Committed to Core in https://core.trac.wordpress.org/changeset/58291.

[Bernhard Reiter]

In 58292:

Block Hooks: Move Posts controller hooked blocks injection logic.

In a similar vein as [58291], this changeset introduces a new insert_hooked_blocks_into_rest_response function and hooks it to the rest_prepare_wp_navigation filter.

This is part of an ongoing effort to move Block Hooks related code out of the Navigation block. Specifically, insert_hooked_blocks_into_rest_response is based on block_core_navigation_insert_hooked_blocks_into_rest_response. Eventually, it will be possible to deprecate the latter.

Follow-up to [58291].

See #60759.

Committed to Core in https://core.trac.wordpress.org/changeset/58292.