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.

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: Add controller to expose hooked blocks for a given anchor block #58622

Closed
wants to merge 24 commits into from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Feb 2, 2024

More details to come!

What?

Add a REST API controller for a new /hooked-blocks route that exposes hooked blocks for a given anchor block, when used in a certain context (e.g. within a template, a pattern, or a navigation menu).

This allows obtaining a list of blocks that are hooked to the given anchor within the current context. This takes blocks into account that are added (or removed) via the hooked_block_types filter -- unlike the block-types endpoint, which is only aware of the context-agnostic blockHooks field in block.json.

Why?

See https://core.trac.wordpress.org/ticket/59574 and #58553.

How?

TBD

Testing Instructions

TBD, but largely: Add a hooked block via the hooked_block_types filter, and verify that the endpoint includes it.
E.g. in the browser console:

let result = await wp.apiFetch( { path: '/wp/v2/hooked-blocks/core/post-content?entity=template&id=twentytwentythree/single' } );

Screenshots or screencast

TODO

Work is largely inspired by the block-types endpoint

  • Test with conditionally added blocks (i.e. via hooked_block_types filter) and verify that they're present in the endpoint's response; add unit test coverage for that.
  • Build $context from query args, pass to hooked_block_types filter.
    • Template
    • Template Part
    • Pattern?
    • Navigation
  • Add test coverage.
  • Add schema
  • Get the /hooked-blocks/ "root" to work.
  • Filter fields (after, before, ... )
  • Re-write getHookedBlocks selector (introduced by Block Hooks: Set ignoredHookedBlocks metada attr upon insertion #58553) to use endpoint as source of truth.

@ockham ockham added [Type] Enhancement A suggestion for improvement. [Feature] Block API API that allows to express the block paradigm. labels Feb 2, 2024
@ockham ockham self-assigned this Feb 2, 2024
Copy link

github-actions bot commented Feb 2, 2024

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/compat/wordpress-6.5/class-gutenberg-rest-hooked-blocks-controller-6-5.php
❔ phpunit/class-gutenberg-rest-hooked-blocks-controller-gutenberg-test.php
❔ lib/compat/wordpress-6.5/rest-api.php
❔ lib/load.php

@tjcafferkey tjcafferkey self-assigned this Feb 8, 2024
@ockham
Copy link
Contributor Author

ockham commented Feb 8, 2024

I'll quickly rebase to get tests to pass 🙂

@ockham ockham force-pushed the add/hooked-blocks-controller branch from 9ecbd18 to e81aa4c Compare February 8, 2024 17:15
@ockham ockham force-pushed the add/hooked-blocks-controller branch from e81aa4c to 39b65b8 Compare February 8, 2024 17:32
@ockham
Copy link
Contributor Author

ockham commented Feb 9, 2024

Update: @tjcafferkey and I have gotten the hooked-blocks endpoint close to ready, but it’s still not quite there yet. At this point, it would feel too rushed to try and squeeze it into the GB RC (plus it would require some more work to wire the getHookedBlocks selector to it).

Given that all this work is needed only to fine-tune the behavior of a newly inserted anchor block that has a hooked block added via a filter, it doesn't seem worth rushing this, especially when it adds a whole new endpoint (that hasn't undergone a lot of review).

As a consequence, we won't try to include this in GB 17.7 RC1 -- which means it also won't land in WP 6.5.


(Note that enabling hooked blocks insertion into modified templates is still on track -- I'm planning to commit that change on Monday, in time before Feature Freeze on Tuesday.)

@ockham
Copy link
Contributor Author

ockham commented Feb 9, 2024

As a consequence, we won't try to include this in GB 17.7 RC1 -- which means it also won't land in WP 6.5.


(Note that enabling hooked blocks insertion into modified templates is still on track -- I'm planning to commit that change on Monday, in time before Feature Freeze on Tuesday.)

I had one idea. I will land the insert-into-modified-templates PR on Monday, and will then file a new Trac ticket to capture the issue. I will then run it by Core folks and ask if we might be granted "blessed task" status, to continue working on it past Feature Freeze. This way, it's at least on people's radar and they can decide if they prefer to have this (arguably minor) bug in 6.5, or to allow for a bit more time to have it fixed.

@ockham
Copy link
Contributor Author

ockham commented Feb 13, 2024

Closing in favor of WordPress/wordpress-develop#6087.

We might come back to this at some point in the future (in order to make the toggle work properly for hooked blocks added via the filter), but for WP 6.5, we won't need this.

@ockham ockham closed this Feb 13, 2024
@ockham ockham deleted the add/hooked-blocks-controller branch February 13, 2024 15:43
@ockham
Copy link
Contributor Author

ockham commented Feb 13, 2024

Here's some additional unit test code I had still lying around locally. Might come in handy in case we ever reopen this.

diff --git a/phpunit/class-gutenberg-rest-hooked-blocks-controller-gutenberg-test.php b/phpunit/class-gutenberg-rest-hooked-blocks-controller-gutenberg-test.php
index 9e49036ae19..86180047395 100644
--- a/phpunit/class-gutenberg-rest-hooked-blocks-controller-gutenberg-test.php
+++ b/phpunit/class-gutenberg-rest-hooked-blocks-controller-gutenberg-test.php
@@ -31,6 +31,11 @@ class Gutenberg_REST_Hooked_Blocks_Controller_Test extends WP_Test_REST_Controll
 	 */
 	protected static $subscriber_id;
 
+	/**
+	 * @var int
+	 */
+	private static $template_post;
+
 	/**
 	 * Create fake data before our tests run.
 	 *
@@ -71,9 +76,28 @@ class Gutenberg_REST_Hooked_Blocks_Controller_Test extends WP_Test_REST_Controll
 
 		register_block_type( 'fake/other-anchor-block', array() );
 		register_block_type( 'fake/other-hooked-block', $other_hooked_block_settings );
+
+		// Set up template post.
+		$args = array(
+			'post_type'    => 'wp_template',
+			'post_name'    => 'single',
+			'post_title'   => 'Single Posts',
+			'post_content' => '<!-- wp:fake/anchor-block -->Some content<!-- /wp:fake/anchor-block -->',
+			'post_excerpt' => 'Description of my template.',
+			'tax_input'    => array(
+				'wp_theme' => array(
+					get_stylesheet(),
+				),
+			),
+		);
+
+		self::$template_post = self::factory()->post->create_and_get( $args );
+		wp_set_post_terms( self::$template_post->ID, get_stylesheet(), 'wp_theme' );
 	}
 
 	public static function wpTearDownAfterClass() {
+		wp_delete_post( self::$template_post->ID );
+
 		self::delete_user( self::$admin_id );
 		self::delete_user( self::$subscriber_id );
 		unregister_block_type( 'fake/anchor-block' );
@@ -163,6 +187,62 @@ class Gutenberg_REST_Hooked_Blocks_Controller_Test extends WP_Test_REST_Controll
 		) );
 	}
 
+
+	public function test_get_item_with_hooked_block_added_by_filter_with_mismatched_template_context() {
+		$add_hooked_block = function( $hooked_block_types, $relative_position, $anchor_block_type, $context ) {
+			if ( ! $context instanceof WP_Block_Template || ! property_exists( $context, 'slug' ) || 'single' !== $context->slug ) {
+				return $hooked_block_types;
+			}
+
+			if ( 'last_child' === $relative_position && 'fake/anchor-block' === $anchor_block_type ) {
+				$hooked_block_types[] = 'fake/hooked-block-added-by-filter';
+			}
+			return $hooked_block_types;
+		};
+		add_filter( 'hooked_block_types', $add_hooked_block, 10, 4 );
+
+		wp_set_current_user( self::$admin_id );
+		$request  = new WP_REST_Request( 'GET', '/wp/v2/hooked-blocks/fake/anchor-block' );
+		$response = rest_get_server()->dispatch( $request );
+		$data	  = $response->get_data();
+
+		remove_filter( 'hooked_block_types', $add_hooked_block, 10 );
+		$this->assertSame( $data, array(
+			'after'      => array( 'fake/hooked-block' ),
+		) );
+	}
+
+	public function test_get_item_with_hooked_block_added_by_filter_with_matching_template_context() {
+		$add_hooked_block = function( $hooked_block_types, $relative_position, $anchor_block_type, $context ) {
+			if ( ! $context instanceof WP_Block_Template || ! property_exists( $context, 'slug' ) || 'single' !== $context->slug ) {
+				return $hooked_block_types;
+			}
+
+			if ( 'last_child' === $relative_position && 'fake/anchor-block' === $anchor_block_type ) {
+				$hooked_block_types[] = 'fake/hooked-block-added-by-filter';
+			}
+			return $hooked_block_types;
+		};
+		add_filter( 'hooked_block_types', $add_hooked_block, 10, 4 );
+
+		wp_set_current_user( self::$admin_id );
+		//$this->assertSame( get_stylesheet(), 'default' );
+		$theme = get_the_terms( self::$template_post, 'wp_theme' )[0]->name;
+		$template = get_block_template( get_stylesheet() . '//single', 'wp_template' );
+		$this->assertSame( $template->slug, 'single' );
+		$this->assertSame( $template->id, 'default//single' );
+		$request  = new WP_REST_Request( 'GET', '/wp/v2/hooked-blocks/fake/anchor-block' );
+		$request  = new WP_REST_Request( 'GET', '/wp/v2/hooked-blocks/fake/anchor-block?entity=wp_template&id=' . get_stylesheet() . '//single' );
+		$response = rest_get_server()->dispatch( $request );
+		$data	  = $response->get_data();
+
+		remove_filter( 'hooked_block_types', $add_hooked_block, 10 );
+		$this->assertSame( $data, array(
+			'after'      => array( 'fake/hooked-block' ),
+			'last_child' => array( 'fake/hooked-block-added-by-filter' )
+		) );
+	}
+
 	public function test_get_block_invalid_name() {
 		$block_type = 'fake/block';
 		wp_set_current_user( self::$admin_id );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants