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

Jump to bottom

Block Bindings: Add canUpdateBlockBindings editor setting #7258

Closed
wants to merge 9 commits into from
Closed

Block Bindings: Add canUpdateBlockBindings editor setting #7258

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fd1f520
Add `canUpdateBlockBindings` editor setting
SantosGuillamot Sep 20, 2024
fd36430
Map `manage_block_bindings` cap
SantosGuillamot Sep 20, 2024
1ceef06
Adapt unit test
SantosGuillamot Sep 20, 2024
d860fd9
Remove extra comma
SantosGuillamot Sep 20, 2024
5b4c551
Try: `edit_block_binding` cap with fallback to `edit_theme_options`
SantosGuillamot Sep 20, 2024
ae3b1be
Change old name
SantosGuillamot Sep 20, 2024
a6a8d21
Address changes
SantosGuillamot Sep 26, 2024
653e563
Add unit test
SantosGuillamot Sep 26, 2024
5f6e484
Remove unnecessary check
SantosGuillamot Sep 27, 2024
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
2 changes: 2 additions & 0 deletions src/wp-includes/block-editor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,8 @@ function get_block_editor_settings( array $custom_settings, $block_editor_contex
}
}

$editor_settings['canUpdateBlockBindings'] = current_user_can( 'edit_block_binding', $block_editor_context->post->ID );
Copy link
Author

Choose a reason for hiding this comment

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

This would need a safety check in case $block_editor_context->post doesn't exist.

Copy link
Member

@gziolo gziolo Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think it's going to be reliable to use post ID. It will only work correctly for the existing post in the post editor. It is impossible to correctly detect the post id for the site editor as everything happens dynamically on the client.

What I mean, exposing it here as a setting calculated on the server has limitations.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it seemed complicated when I was looking through the code and use cases. That's why, if we want to check against a post ID, I was adding a fallback to edit_theme_options capability: https://github.com/WordPress/wordpress-develop/pull/7258/files#diff-7b99cb99a8105f5254c9c282a8cb9eed9d9b48f2d974544ce6919fe124bd0427R813

That way, we don't try to detect the post id, if it isn't provided in the context and we are in the site editor, we map the edit_block_binding capability to edit_theme_options.

Having said that, maybe it is just easier to not pass a post ID and map it directly to other capabilities.

Copy link
Member

Choose a reason for hiding this comment

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

This would need a safety check in case $block_editor_context->post doesn't exist.

Confirmed:

wordpress-develop/src/wp-includes/class-wp-block-editor-context.php

Line 39 in 7f4fd30

public $post = null;

Copy link
Author

Choose a reason for hiding this comment

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

This should be solved in this commit: a6a8d21


/**
* Filters the settings to pass to the block editor for all editor type.
*
Expand Down
24 changes: 24 additions & 0 deletions src/wp-includes/capabilities.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,30 @@ function map_meta_cap( $cap, $user_id, ...$args ) {
case 'delete_app_password':
$caps = map_meta_cap( 'edit_user', $user_id, $args[0] );
break;
case 'edit_block_binding':
/*
* If the post ID is null, check if the context is the site editor.
* Fall back to the edit_theme_options in that case.
*/
if ( ! isset( $args[0] ) ) {
$screen = get_current_screen();
if ( ! isset( $screen->id ) || 'site-editor' !== $screen->id ) {
Copy link
Member

Choose a reason for hiding this comment

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

If the post id gets passed as an argument, then probably it's easier to pass also the name of the editor:

wordpress-develop/src/wp-includes/class-wp-block-editor-context.php

Line 30 in 7f4fd30

public $name = 'core/edit-post';

Or even the entire block editor context.

Copy link
Author

Choose a reason for hiding this comment

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

I've used the entire block editor context as suggested in this commit: a6a8d21

$caps[] = 'do_not_allow';
break;
}
$caps = map_meta_cap( 'edit_theme_options', $user_id );
break;
}

$object_id = (int) $args[0];
$object_subtype = get_object_subtype( 'post', $object_id );
if ( empty( $object_subtype ) ) {
$caps[] = 'do_not_allow';
break;
}

$caps = map_meta_cap( "edit_{$object_subtype}", $user_id, $object_id );
break;
default:
// Handle meta capabilities for custom post types.
global $post_type_meta_caps;
Expand Down
3 changes: 2 additions & 1 deletion tests/phpunit/tests/user/capabilities.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,8 @@ public function testMetaCapsTestsAreCorrect() {
$expected['read_app_password'],
$expected['edit_app_password'],
$expected['delete_app_passwords'],
$expected['delete_app_password']
$expected['delete_app_password'],
$expected['edit_block_binding']
);

$expected = array_keys( $expected );
Expand Down
Loading