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

Full Site Editing: Map features and block edit-ability to capabilities and update the UI where necessary #26573

Closed
jameskoster opened this issue Oct 29, 2020 · 12 comments
Labels
Needs Design Needs design efforts.

Comments

@jameskoster
Copy link
Contributor

For the most part, the Site Editor == Customizer + Theme Editor. These two existing core features map largely to two different capabilities, namely edit_theme_options (Customizer) and edit_themes (Theme Editor).

We need to clarify how these capabilities map to features and functionality within the Site Editor, and ensure the UI adapts accordingly.

The table below details a list of Site Editing features for consideration, along with a first pass at which capability is required for them to be exposed in the UI.

  Required capability Design
Template
View edit_theme_options
Edit contents of blocks edit_theme_options
Add / duplicate edit_themes
Manipulate block tools (position, width, etc) edit_theme_options
Edit details (scope, name) edit_themes
Revert edit_themes
Load and edit content edit_theme_options
Delete edit_themes
Template part
View edit_theme_options
Edit contents of blocks inner blocks edit_theme_options
Add / duplicate edit_themes
Edit details (name) edit_themes
Revert edit_themes
Delete edit_themes
Import / export
Import templates install_themes
Export templates install_themes

The Design column should link out to the relevant UI work pertaining to the associated feature, as and when it becomes available. In some cases this will be simple – e.g. if a user in not permitted to export templates, that menu item would be hidden in the UI. In other cases the design may need to be more nuanced – e.g. when the full template is loaded in to the regular editor, it should be possible for an Author to view the Site Title in the Header Template Part, but not edit it.

Considering the example above (and #18760), in addition to accommodating Site Editor features, we'll also need to explore how capabilities affect the interactivity of site editing / dynamic content blocks:

Block capability required to edit
Navigation edit_theme_options
Site Title edit_theme_options
Site Tagline edit_theme_options
Site Logo edit_theme_options
Featured Image edit_posts
Post Content edit_posts
Post Title edit_posts
Post Tags edit_posts
Post Categories edit_posts
Post Author edit_posts
Post Date edit_posts
Post Excerpt edit_posts
Post Comments moderate_comments
Post Comments Form edit_themes
Template Part edit_themes

If the capability required to edit these blocks is not met, they would likely render in a read-only or "locked" format. Once again, this list is not exhaustive and will likely evolve over time.


We should keep this issue updated as new features or relevant blocks become available, and use it to keep track of how the UI respects the appropriate capabilities.

@skorasaurus
Copy link
Member

skorasaurus commented Dec 24, 2020

ref #27597 has a proposal for this.

@carolinan
Copy link
Contributor

Adding this to the 5.8 must haves for the site blocks.

@carlomanf
Copy link

I haven't tested, but I would guess that the rest API controller would already prevent any unauthorised edits, so if this was added as a "must-have" for security reasons then it probably needn't be.

Whether the UI makes this clear to the user, or misleads them into thinking they can edit when they actually can't, is another question.

However, I think you need to be extremely careful when disabling UI elements based on capabilities. If you don't get the capabilities exactly right (and some in the table above are certainly not right), you run the risk of making the problem worse by disabling the UI for users who should not have had it disabled. There are precedents of this kind of thing going wrong, see https://core.trac.wordpress.org/ticket/52043 for an example. The reverse could also happen, where the UI might not get disabled for users for whom it should. Many sites have unusual capability configurations and that needs to be accounted for.

The proposal in #27597 couldn't be further away from being a solution to this issue. It's actually exactly the kind of tweak that could cause a premature and fragile solution to this issue to break.

@jameskoster
Copy link
Contributor Author

Yes this is a particularly gnarly issue. The trickiest part seems to be how the UI should present site/template blocks when they added to posts / pages by user roles like Author and Contributor. The are varying degrees to that... we might allow them to insert the blocks but lock down some/all edit-ability. Or we might not allow them to insert those blocks at all. None of this feels particularly clear right now.

For 5.8 I wonder if it would be simpler to restrict the site/template blocks to the template editing context, and only provide access to that context to users with the appropriate capabilities?

@carlomanf
Copy link

I just tested this out quickly, by editing a post as an Author role and testing out some of the new blocks. The experience is exactly as I predicted it would be. The UI kind of invites me to edit things, but when I click on Save, nothing gets saved, except in cases where I actually do have the capability to edit (such as my own posts.)

The trickiest part seems to be how the UI should present site/template blocks when they added to posts / pages by user roles like Author and Contributor.

That's not the tricky part at all. Just remove the editability of the block and render it as if it were being rendered on the front-end.

The tricky part is correctly specifying when to do this and when not to do this. It needs to be correctly aligned with the existing capabilities set by the REST API controller, to prevent anyone from getting a broken experience like is currently the case, and also to avoid disabling the editability for anyone who would otherwise have been able to edit it freely.

Most of the post-related blocks inherit their context from the query block, in other words, you can't change the post ID of a post-related block on-demand, so a simple solution like this would work. The context of the site-related blocks is also fixed. Either someone has the capability to edit the site title or they don't. It's already set by the REST API controller.

The only case I can think of where this gets more complicated is for reusable blocks and template parts, where a user might have the capability to edit one reusable block but not another. Therefore, if the block is set to one they can't edit, the editability of the inner blocks should be disabled, but their ability to change the setting of the outer block to one they can edit should remain.

The solution is really nothing more than aligning the UI capability with the REST API controller capability, and it's kind of obvious when you think about it. Overthinking it is probably the biggest risk with this task.

@jameskoster
Copy link
Contributor Author

Just remove the editability of the block and render it as if it were being rendered on the front-end

Not quite? If an Author can insert a Site Title block, they should also be able move it around, adjust the block attributes etc. But not edit the title itself.

So we probably need a design for blocks that are content-locked. Co-incidentally we're exploring something similar in #31461

@carlomanf
Copy link

carlomanf commented May 4, 2021

Just remove the editability of the block and render it as if it were being rendered on the front-end

Not quite? If an Author can insert a Site Title block, they should also be able move it around, adjust the block attributes etc. But not edit the title itself.

Yes, you're right. Another way to think about it is anything that simply manipulates the block markup should always be allowed (as they would have been able to do it with code view anyway) but anything that modifies a foreign entity should be restricted in alignment with the REST API controller for that entity.

@youknowriad
Copy link
Contributor

@WordPress/gutenberg-core Would anyone be able to audit the existing FSE blocks that landed in 5.8 to ensure they guarantee that #26573 (comment)

@jameskoster
Copy link
Contributor Author

Noting: We can potentially use the design pattern suggested in #31461 (comment) as a solution here. The only difference being that appropriate blocks will not exhibit the unlocking affordance at all.

@youknowriad
Copy link
Contributor

I think with #32817 we audited all blocks to check the right capabilities. Should we close this one @ntsekouras

@ntsekouras
Copy link
Contributor

we audited all blocks to check the right capabilities. Should we close this one @ntsekouras

Not yet. I want to take a closer look at the blocks again and Site Logo need changes for sure. I'll have a PR about it soon.

@youknowriad
Copy link
Contributor

Looks like these are fixed now. Thanks @ntsekouras for the great work here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Needs design efforts.
Projects
None yet
Development

No branches or pull requests

6 participants