-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add capabilities for reusable blocks #4725
Conversation
Adds capabilities for creating, reading, updating and deleting reusable blocks. The capabilities are mapped like so: | | Editors^ | Authors | Contributors | Subscribers* | | ------ | -------- | ------- | ------------ | ------------ | | Create | Yes | Yes | No | No | | Read | Yes | Yes | Yes | No | | Update | Yes | Own | Own | No | | Delete | Yes | Own | Own | No | ^ Includes administrators. * Includes visitors that are not logged in.
a82d73d
to
f4a8d92
Compare
lib/register.php
Outdated
$contributor->add_cap( 'read_blocks' ); | ||
$contributor->add_cap( 'delete_blocks' ); | ||
$contributor->add_cap( 'delete_published_blocks' ); | ||
$contributor->add_cap( 'edit_published_blocks' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a more suitable hook to do this work in? I read through the relevant WordPress docs but couldn't figure out a better place to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init
is fine, this will end up in schema.php
when it lands in Core.
That said, let's wrap it in a check for if the capabilities have already been added: each add_cap()
call causes a database write, which we don't really want to be doing every page load. 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a206f51.
@pento @brandonpayton—I think this works (well, the test cases I wrote definitely pass...) but this capabilities stuff is pretty hairy so I'd love a second pair of 👀s on this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality looks good, just needs the performance tweak I mentioned.
lib/register.php
Outdated
$contributor->add_cap( 'read_blocks' ); | ||
$contributor->add_cap( 'delete_blocks' ); | ||
$contributor->add_cap( 'delete_published_blocks' ); | ||
$contributor->add_cap( 'edit_published_blocks' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init
is fine, this will end up in schema.php
when it lands in Core.
That said, let's wrap it in a check for if the capabilities have already been added: each add_cap()
call causes a database write, which we don't really want to be doing every page load. 🙂
@noisysocks @pento The description and table says that Contributors can edit/delete blocks for which they're the post_author. But at the same time it says they can only see and use existing blocks and cannot create them. If they cannot create them, why can they edit/delete them? Here is what the permissions table is above:
Shouldn't it actually be:
Example:
Shouldn't the Contributor have strictly Read access to use blocks and no update/delete at all if they have no create capability? Even if the block was originally created by them while they had higher role access? |
Good catch, @carlhancock. The weirdness comes from the fact that there is an implicit publish action contained within the create action. This is because reusable blocks are immediately published when they are created. Contributors cannot publish posts or pages, and so I made it so that they cannot create reusable blocks. It seems totally reasonable to implement your suggestion and make it so that contributors cannot update or delete blocks. |
Avoid calling $role->add_cap() when the role already has the given capability. This avoids an unnecessary database write.
Make reusable block permissions more consistent by preventing contributors from *editing* blocks - not just creating.
I've updated this PR. Let me know what you think! 💃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕺🏻💯👍🏻
public function check_read_permission( $post ) { | ||
// Ensure that the user is logged in and has the read_blocks capability. | ||
$post_type = get_post_type_object( $post->post_type ); | ||
if ( ! current_user_can( $post_type->cap->read_post, $post->ID ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor point, but you should be able to provide the edit_post
meta capability here instead rather than looking up the primitive directly, since map_meta_cap()
will be doing it for you later anyway. So then this line can be simply:
if ( ! current_user_can( 'read_post', $post->ID ) ) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, this could be simply ! current_user_can( 'read_block', $post->ID )
. But I like that how this currently is means that if the name of the capability in lib/register.php
is changed, the controller code doesn't need to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use read_post
(not read_block
) then map_meta_cap()
should automatically map it over to read_block
(or whatever the registered post type cap gets changed to).
Hello, @noisysocks I'm a bit confused - It looks like this PR was merged, but I can't find these 'block' caps in WordPress core - how do I use these capabilities please? |
🔒 What this does
Fixes #3936.
Adds capabilities for the reusable block custom post type and maps them to the default WordPress roles. The capabilities are mapped as per #3936 (comment):
Here's a table to help illustrate:
✅ How I tested this (and how you could too)
I've added unit tests which run through the cases described by the table above.
You could also test this manually. First, I used WP-CLI to make sure I was working with a clean slate:
Then I created a bunch of users with different roles:
Then I tested various reusable block API operations while authenticated as different users. Here I'm using HTTPie, but any API client would work. Make sure your WordPress has the basic-auth plugin installed.