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 bindings: Expose sources in the editor settings to consume them in the client #7020

Closed
Changes from 6 commits
Commits
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
17 changes: 17 additions & 0 deletions src/wp-includes/block-editor.php
Original file line number Diff line number Diff line change
@@ -648,6 +648,23 @@ function get_block_editor_settings( array $custom_settings, $block_editor_contex
$editor_settings['postContentAttributes'] = $post_content_block_attributes;
}

// Expose block bindings sources in the editor settings.
$registered_block_bindings_sources = get_all_registered_block_bindings_sources();
if ( ! empty( $registered_block_bindings_sources ) ) {
// Initialize array.
$editor_settings['blockBindingsSources'] = array();
foreach ( $registered_block_bindings_sources as $source_name => $source_properties ) {
// Add source with the label to editor settings.
$editor_settings['blockBindingsSources'][ $source_name ] = array(
'label' => $source_properties->label,
);
// Add `usesContext` property if exists.
if ( ! empty( $source_properties->uses_context ) ) {
$editor_settings['blockBindingsSources'][ $source_name ]['usesContext'] = $source_properties->uses_context;
}
}
}

/**
* Filters the settings to pass to the block editor for all editor type.
*
3 changes: 3 additions & 0 deletions tests/phpunit/tests/block-bindings/register.php
Original file line number Diff line number Diff line change
@@ -77,6 +77,9 @@ public function test_get_all_registered() {
);

$registered = get_all_registered_block_bindings_sources();
unregister_block_bindings_source( 'test/source-one' );
Copy link
Member

Choose a reason for hiding this comment

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

These lines are redundant as there is automated cleanup included in the tear_down lifecycle method:

public function tear_down() {
foreach ( get_all_registered_block_bindings_sources() as $source_name => $source_properties ) {
if ( str_starts_with( $source_name, 'test/' ) ) {
unregister_block_bindings_source( $source_name );
}
}

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense 🙂 And it explains why they weren't failing before. I made this change as part of this commit.

unregister_block_bindings_source( 'test/source-two' );
unregister_block_bindings_source( 'test/source-three' );
$this->assertEquals( $expected, $registered );
}

Original file line number Diff line number Diff line change
@@ -269,6 +269,9 @@ public function test_get_all_registered() {
);

$registered = $this->registry->get_all_registered();
$this->registry->unregister( 'test/source-one' );
Copy link
Member

Choose a reason for hiding this comment

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

This one is more nuanced, but also redundant. There is a new instance of the registry created for every test using the lifecycle method set_up:

$this->registry = new WP_Block_Bindings_Registry();

So, the instance with registered custom sources doesn't persist between tests.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense as well 🙂 I made this change as part of this commit.

$this->registry->unregister( 'test/source-two' );
$this->registry->unregister( 'test/source-three' );
$this->assertEquals( $expected, $registered );
}

@@ -311,6 +314,9 @@ public function test_get_registered() {

$expected = new WP_Block_Bindings_Source( $source_two_name, $source_two_properties );
$result = $this->registry->get_registered( 'test/source-two' );
$this->registry->unregister( 'test/source-one' );
$this->registry->unregister( 'test/source-two' );
$this->registry->unregister( 'test/source-three' );

$this->assertEquals(
$expected,
@@ -380,6 +386,8 @@ public function test_merging_uses_context_from_multiple_sources() {
);

$new_uses_context = $block_registry->get_registered( 'core/paragraph' )->uses_context;
unregister_block_bindings_source( 'test/source-one' );
SantosGuillamot marked this conversation as resolved.
Show resolved Hide resolved
unregister_block_bindings_source( 'test/source-two' );
// Checks that the resulting `uses_context` contains the values from both sources.
$this->assertContains( 'commonContext', $new_uses_context );
$this->assertContains( 'sourceOneContext', $new_uses_context );
34 changes: 34 additions & 0 deletions tests/phpunit/tests/blocks/editor.php
Original file line number Diff line number Diff line change
@@ -720,4 +720,38 @@ public function data_block_editor_rest_api_preload_adds_missing_leading_slash()
),
);
}

/**
* @ticket 61641
*/
public function test_get_block_editor_settings_block_bindings_sources() {
$block_editor_context = new WP_Block_Editor_Context();
register_block_bindings_source(
'test/source-one',
array(
'label' => 'Source One',
'get_value_callback' => function () {},
'uses_context' => array( 'postId' ),
)
);
register_block_bindings_source(
'test/source-two',
array(
'label' => 'Source Two',
'get_value_callback' => function () {},
)
);
$settings = get_block_editor_settings( array(), $block_editor_context );
$exposed_sources = $settings['blockBindingsSources'];
SantosGuillamot marked this conversation as resolved.
Show resolved Hide resolved
unregister_block_bindings_source( 'test/source-one' );
unregister_block_bindings_source( 'test/source-two' );
// It is expected to have 4 sources: the 2 registered sources in the test, and the 2 core sources.
$this->assertCount( 4, $exposed_sources );
$source_one = $exposed_sources['test/source-one'];
$this->assertSame( 'Source One', $source_one['label'] );
$this->assertSameSets( array( 'postId' ), $source_one['usesContext'] );
$source_two = $exposed_sources['test/source-two'];
$this->assertSame( 'Source Two', $source_two['label'] );
$this->assertArrayNotHasKey( 'usesContext', $source_two );
}
}

Unchanged files with check annotations Beta

await expect(
page,
'should redirect to the installation page'
).toHaveURL( /wp-admin\/install\.php$/ );

Check failure on line 40 in tests/e2e/specs/install.test.js

GitHub Actions / Test with SCRIPT_DEBUG enabled / Run E2E tests

[chromium] › install.test.js:34:6 › WordPress installation process › should install WordPress with pre-existing database credentials

1) [chromium] › install.test.js:34:6 › WordPress installation process › should install WordPress with pre-existing database credentials Error: should redirect to the installation page Timed out 5000ms waiting for expect(locator).toHaveURL(expected) Locator: locator(':root') Expected pattern: /wp-admin\/install\.php$/ Received string: "http://localhost:8889/" Call log: - should redirect to the installation page with timeout 5000ms - waiting for locator(':root') - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" 38 | page, 39 | 'should redirect to the installation page' > 40 | ).toHaveURL( /wp-admin\/install\.php$/ ); | ^ 41 | 42 | await expect( 43 | page.getByText( /WordPress database error/ ), at /home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/install.test.js:40:5

Check failure on line 40 in tests/e2e/specs/install.test.js

GitHub Actions / Test with SCRIPT_DEBUG disabled / Run E2E tests

[chromium] › install.test.js:34:6 › WordPress installation process › should install WordPress with pre-existing database credentials

1) [chromium] › install.test.js:34:6 › WordPress installation process › should install WordPress with pre-existing database credentials Error: should redirect to the installation page Timed out 5000ms waiting for expect(locator).toHaveURL(expected) Locator: locator(':root') Expected pattern: /wp-admin\/install\.php$/ Received string: "http://localhost:8889/" Call log: - should redirect to the installation page with timeout 5000ms - waiting for locator(':root') - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" - locator resolved to <html lang="en-US">…</html> - unexpected value "http://localhost:8889/" 38 | page, 39 | 'should redirect to the installation page' > 40 | ).toHaveURL( /wp-admin\/install\.php$/ ); | ^ 41 | 42 | await expect( 43 | page.getByText( /WordPress database error/ ), at /home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/install.test.js:40:5
await expect(
page.getByText( /WordPress database error/ ),
await applicationPasswords.create();
const [ app ] = await applicationPasswords.get();
expect( app['name']).toBe( TEST_APPLICATION_NAME );

Check failure on line 26 in tests/e2e/specs/profile/applications-passwords.test.js

GitHub Actions / Test with SCRIPT_DEBUG disabled / Run E2E tests

[chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password

2) [chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── TypeError: Cannot read properties of undefined (reading 'name') 24 | 25 | const [ app ] = await applicationPasswords.get(); > 26 | expect( app['name']).toBe( TEST_APPLICATION_NAME ); | ^ 27 | 28 | const successMessage = page.getByRole( 'alert' ); 29 | at /home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/profile/applications-passwords.test.js:26:14
const successMessage = page.getByRole( 'alert' );
}
async create(applicationName = TEST_APPLICATION_NAME) {
await this.admin.visitAdminPage( '/profile.php' );

Check failure on line 110 in tests/e2e/specs/profile/applications-passwords.test.js

GitHub Actions / Test with SCRIPT_DEBUG enabled / Run E2E tests

[chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password

2) [chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password Error: Not logged in 108 | 109 | async create(applicationName = TEST_APPLICATION_NAME) { > 110 | await this.admin.visitAdminPage( '/profile.php' ); | ^ 111 | 112 | const newPasswordField = this.page.getByRole( 'textbox', { name: 'New Application Password Name' } ); 113 | await expect( newPasswordField ).toBeVisible(); at Admin.visitAdminPage (/home/runner/work/wordpress-develop/wordpress-develop/node_modules/@wordpress/e2e-test-utils-playwright/src/admin/visit-admin-page.ts:36:9) at ApplicationPasswords.create (/home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/profile/applications-passwords.test.js:110:3) at /home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/profile/applications-passwords.test.js:23:3

Check failure on line 110 in tests/e2e/specs/profile/applications-passwords.test.js

GitHub Actions / Test with SCRIPT_DEBUG enabled / Run E2E tests

[chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password

2) [chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: Not logged in 108 | 109 | async create(applicationName = TEST_APPLICATION_NAME) { > 110 | await this.admin.visitAdminPage( '/profile.php' ); | ^ 111 | 112 | const newPasswordField = this.page.getByRole( 'textbox', { name: 'New Application Password Name' } ); 113 | await expect( newPasswordField ).toBeVisible(); at Admin.visitAdminPage (/home/runner/work/wordpress-develop/wordpress-develop/node_modules/@wordpress/e2e-test-utils-playwright/src/admin/visit-admin-page.ts:36:9) at ApplicationPasswords.create (/home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/profile/applications-passwords.test.js:110:3) at /home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/profile/applications-passwords.test.js:23:3

Check failure on line 110 in tests/e2e/specs/profile/applications-passwords.test.js

GitHub Actions / Test with SCRIPT_DEBUG disabled / Run E2E tests

[chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password

2) [chromium] › profile/applications-passwords.test.js:19:6 › Manage applications passwords › should correctly create a new application password Error: Not logged in 108 | 109 | async create(applicationName = TEST_APPLICATION_NAME) { > 110 | await this.admin.visitAdminPage( '/profile.php' ); | ^ 111 | 112 | const newPasswordField = this.page.getByRole( 'textbox', { name: 'New Application Password Name' } ); 113 | await expect( newPasswordField ).toBeVisible(); at Admin.visitAdminPage (/home/runner/work/wordpress-develop/wordpress-develop/node_modules/@wordpress/e2e-test-utils-playwright/src/admin/visit-admin-page.ts:36:9) at ApplicationPasswords.create (/home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/profile/applications-passwords.test.js:110:3) at /home/runner/work/wordpress-develop/wordpress-develop/tests/e2e/specs/profile/applications-passwords.test.js:23:3
const newPasswordField = this.page.getByRole( 'textbox', { name: 'New Application Password Name' } );
await expect( newPasswordField ).toBeVisible();