wordpress-develop icon indicating copy to clipboard operation
wordpress-develop copied to clipboard

Block Bindings: Add `canUpdateBlockBindings` editor setting

Open SantosGuillamot opened this issue 1 year ago • 14 comments

Needed for https://github.com/WordPress/gutenberg/pull/64570

Adds a canUpdateBlockBindings editor setting that will be read to decide if the user should be able to create and modify bindings through the UI. By default, only admin users can do it, but it can be overridden with block_editor_settings_all filter.

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


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

SantosGuillamot avatar Aug 28 '24 12:08 SantosGuillamot

Trac Ticket Missing

This pull request is missing a link to a Trac ticket. For a contribution to be considered, there must be a corresponding ticket in Trac.

To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. More information about contributing to WordPress on GitHub can be found in the Core Handbook.

github-actions[bot] avatar Aug 28 '24 12:08 github-actions[bot]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props santosguillamot, gziolo, jorbin, noisysocks, matveb, cbravobernal, youknowriad, mamaduka, timothyblynjacobs, peterwilsoncc, drivingralle.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions[bot] avatar Aug 28 '24 12:08 github-actions[bot]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance, it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

github-actions[bot] avatar Aug 28 '24 12:08 github-actions[bot]

I've posted a comment on the associated issue Core-61945 (comment) -- tl;dr is that I think this is solving the wrong problem given the earlier comment that the interface is too technical.

peterwilsoncc avatar Sep 09 '24 22:09 peterwilsoncc

Sorry folks, can I get some help on testing this. I followed the instructions in the GB PR but without success.

These are the steps I took:

  1. Set up:
    • WP 6.6.2
    • Gutenberg trunk @ https://github.com/wordpress/gutenberg/commit/8a74d31f28e4d5ddc6f21be1bc2134e675d2913f
  2. Log in as an admin
  3. Build GB
  4. Add new page
  5. Insert paragraph, unable to see attributes
  6. Insert second paragraph, still no luck
  7. Activate Jetpack after seeing a comment on the GB PR
  8. Add an image, no attributes.

I'm sure it's a problem at my end, can someone let me know what I am missing.

peterwilsoncc avatar Sep 11 '24 23:09 peterwilsoncc

Sorry folks, can I get some help on testing this. I followed the instructions in the GB PR but without success.

These are the steps I took:

1. Set up:
   
   * WP 6.6.2
   * Gutenberg trunk @ [WordPress/gutenberg@8a74d31](https://github.com/WordPress/gutenberg/commit/8a74d31f28e4d5ddc6f21be1bc2134e675d2913f)

2. Log in as an admin

3. Build GB

4. Add new page

5. Insert paragraph, unable to see attributes

6. Insert second paragraph, still no luck

7. Activate Jetpack after seeing a comment on the GB PR

8. Add an image, no attributes.

I'm sure it's a problem at my end, can someone let me know what I am missing.

You may need to register a meta data. Like this:

function add_bindings() {
register_meta(
		'post',
		'all_templates_key',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'default'           => 'all templates default value',
		)
	);
}

add_action( 'init', 'add_bindings');

If the meta is for a post it will appear in a post. If the meta is for a page, then you need to register with the page key.

Jetpack was adding a private custom field, that's the reason why it made the selector to appear before. We fix that, that's the reason why it didn't work for you anymore.

Here is a small video demo:

https://github.com/user-attachments/assets/82a8c7b9-c285-4460-8f7f-78e13e67d3bd

cbravobernal avatar Sep 12 '24 09:09 cbravobernal

Thanks @cbravobernal I knew I must have been missing something so I appreciate the help.

peterwilsoncc avatar Sep 12 '24 23:09 peterwilsoncc

I am circling back to the Dev Chat, September 11, 2024 that I couldn't attend. I want to share more context.

My main concern here is that the approach is to hide the UI from users with low permissions. I don’t feel that this is a great approach to handling a UI that is considered too technical, as I don’t think there is anything to suggest that an administrator will understand what an author does not.

So I’m of the view the interface ought to be improved and made less technical before it’s shipped in core.

Block Bindings UI for connecting sources with block attributes will be ready with optimized user experience on time for WP 6.7. We had some minor concerns initially about how to present post meta options in the UI in the sidebar and for editable fields, but we did several iterations and plan to extend post meta to offer human-friendly labels to show in the editor. I think that was the primary concern we had (related report) as seeing post meta keys like themeslug_book_rating will never be the most efficient for processing by human brain (despite being helpful for debugging in some cases).

I’d be fine with just updating this to use caps. The interface doesn’t strike me as being too technical. Can put it in the Advanced tab if we’re worried…

…The short of it is that I’m okay with fixing the cap issue (add a new cap, don’t check against a role) and shipping in 6.7 or leaving it in the plugin for more testing. Up to the team working on it. We have until beta 1 to decide.

The goal of this ticket/PR was to define the best strategy for handling the feature's exposure. We want to choose the best defaults that can be further optimized by the site owners. The panel for managing bindings would be only confusing when presented to all users, in particular those that only care about editing data. We also anticipated that on some sites, it will be disabled for nearly everyone as it is considered an advanced feature for site building. For example, when the intention is to expose block variations or patterns with bound attributes, the consumer should have a streamlined experience when editing values. However, the person editing the pattern’s design in the editor should be able to connect these attributes with binding sources. That’s basically what we want to offer with proper permissions, and that’s why we contemplated a custom capability.

gziolo avatar Sep 13 '24 12:09 gziolo

I think a custom capability is the best way. With that projects can decide what role can do the work. No matter what the core decides. I personal would limit this feature to the role of admin and editors. For my understanding an editor is a role for handling content but not structure.

Drivingralle avatar Sep 16 '24 12:09 Drivingralle

Given that the UI is being improved in other pull requests to make it more user-friendly, I agree we could focus this ticket/PR on discussing the best strategy to decide when to show or not the UI to create and modify bindings.

I still believe that adding a manage_block_bindings capability makes sense and, according to the feedback, it seems that mapping it to other capabilities like edit_posts or edit_other_posts could be enough. That way, it'd be available for admins and editors by default, but site owners can override this behavior if wanted.

SantosGuillamot avatar Sep 16 '24 17:09 SantosGuillamot

I think meta capabilities is the way to go.

Similar to post types, we probably want multiple caps for editing own, editing others, etc. This will allow developers maximum flexibility.

To account for different permissions with CPTs, we'd probably want them to require the post ID, eg current_user_can( 'edit_block_binding', $post_id ) mapping to $post_type->caps->edit_others_posts. The edit_post meta capability will provide a guide.

Naming things is hard but typically meta capabilities are singular while primitive capabilities are plural.

peterwilsoncc avatar Sep 16 '24 22:09 peterwilsoncc

Thanks for the guidance 🙂

To account for different permissions with CPTs, we'd probably want them to require the post ID

The main issue with this is that bindings can be edited in the site editor in templates, where a post ID doesn't exist. I checked and it seems in the site editor we are checking the edit_theme_options capability: link.

Would it make sense to fallback to that one when there is no post ID and we are in the context of the site editor? I made a commit trying to show what I mean: https://github.com/WordPress/wordpress-develop/pull/7258/commits/24951bcde7d5c2f8f756d5325c1e7e049c58a27d

SantosGuillamot avatar Sep 17 '24 09:09 SantosGuillamot

Hey @SantosGuillamot. JavaScript packages were updated in r59072 so you might wish to rebase this for easier testing. The deadline to commit this backport is 6.7 Beta 1 which is scheduled for 1 October.

noisysocks avatar Sep 20 '24 02:09 noisysocks

Rebased done 🙂 I believe I already have an idea of what needs to be done once we agree on the path forward. Let's see if we can agree on the mapping capabilities.

SantosGuillamot avatar Sep 20 '24 07:09 SantosGuillamot

I've addressed the changes suggested and added a unit test to check the capabilities are mapped properly. Let's see if the unit tests pass.

With these changes, this is how it should work:

  • If it is in the context of a post, and post->ID exists, it maps to edit_post.
  • If it is in the context of a template, and post->IDDOES NOT exist, it maps to edit_theme_options.

Let me know if that makes sense, or if I should change anything.

SantosGuillamot avatar Sep 26 '24 15:09 SantosGuillamot

What's clear here is that the right solution here is still unclear and there are still diverging opinions. If It helps unblock this PR, I wouldn't mind personally this is landing as a block editor setting.

Now, my personal opinion is that whether this applies to the block editor only or not is irrelevant, we want to control the ability of the user to perform a task or not. So for me, this is a user capability (in the large sense) and the ideal for me is that we should have a unique way in core-data to access user capabilities. (I'm less opinionated about the backend and REST API, how user capabilities are managed there is not something I have expertise on)

youknowriad avatar Sep 27 '24 14:09 youknowriad

Committed with https://core.trac.wordpress.org/changeset/59122

cbravobernal avatar Sep 30 '24 06:09 cbravobernal

There has been some discussion about whether the ability to update block bindings in the editor should be a setting/preference or a capability.

This ability should be filterable by external developers, which suggests the editor setting approach. However, it is not a user setting, as the user cannot choose whether or not to update block bindings—it is only enabled for admins or filtered by third-party extensions.

Overuse of editor settings is not recommended by Gutenberg practices, so adding a capability could work. However, it has not been merged yet.

Considering there is no clear right solution, and the editor setting is theoretically a valid approach, I have committed it so it doesn't block the adoption of block bindings. Additionally, it is a helpful feature for site admins, as shown, for example, in this comment: https://github.com/WordPress/gutenberg/pull/64570#issuecomment-2338829939

cbravobernal avatar Sep 30 '24 09:09 cbravobernal