Adjust Layout Builder permission checking for inline blocks once more granular block permissions exist

I think it would be disruptive to change things so that the current LB permission is no longer enough to create and edit inline blocks (of any type). If we were going to do that, we should have done it when the new block permissions were added, in 10.1.0. Now I think the right thing to do is to add a new permission that needs one of the more granular block permissions to be useful.

Currently, in layout_builder.permissions.yml:

create and edit custom blocks:
  title: 'Create and edit content blocks'
  description: 'Manage the single-use blocks within the Layout Builder'

I think we can tweak the description: change "the" to "all".

The new permission would be something like

create and edit certain custom blocks:
  title: 'Create and edit certain content blocks'
  description: 'Use Layout Builder to manage single-use blocks that the user can add or edit'

In the description, I am trying to be consistent with the labels of some existing LB permissions, like "Content - Article: Configure layout overrides for content items that the user can edit".

Tried to address the #10.

@_utsavsharma :

Thanks for starting this issue.

We should keep the old permission and add a new one. The patch in #11 replaces the old permission with a new one.

Once we have the permission, we will have to update the access controls so that the permission does what it says. That will be the hard part of this issue.

We also need to expand the issue summary. So far, it is just a stub.

No point having 2 permissions ultimately since the old one will allow bypassing access of granular block permissions. I was thinking maybe a follow-up with an update hook in the next major to leave only 1 permission so site owners will not end up not having access unexpectedly until they update to that next major with a CR of course?

Also.. I'll work on it.

Added some initial work on this. Definitely needs more work.

Passing to the first review and testing.

Thank you for checking this Alec! Included your remarks except those about catching exceptions. I think if any of those exceptions happen, it indicates a serious issue with the project that should be caught on a local / dev environment or as early as possible in any case (invalid custom code or data corruption) and not allowed to stay there much longer and eventually be noticed resulting in a completely different bug.

Just to be sure, I checked other calls of those methods in the module - they are not wrapped in a try-catch anywhere so those exceptions would be thrown elsewhere anyway. Setting those threads as resolved.

Single test fail completely unrelated: Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLinkabilityTest. OK, all green again.

@Graber

Could you please add some details to the IS Problems/Motivations section?

Thank you for checking this @plach, all resolved, if you have concerns about the Kernel test, please provide more details, I don't think we need to go for a functional in such a simple case and at a big performance cost. Layout Builder tests are very heavy already :(

Looks good to me, I'll manually test this for a while and then report back.

Issue summary appears to have been updated previously.

Tested locally the new permission and an editor role. Things appear to be working as expected but will leave in review for more eyes +1

Most entity types rely on an "update" operation instead of the "edit" operation. Paragraphs will check the "update" operation on it's parent entity. If the parent entity is a block, we'll need to convert "update" to "edit".

Here's a proposal for changing getBundlePermission:

  public static function getBundlePermission(string $block_bundle, string $operation): string {
    if ($operation === 'create') {
      return 'create ' . $block_bundle . ' block content';
    }
    elseif ($operation === 'update') {
      $operation = 'edit';
    }
    return $operation . ' any ' . $block_bundle . ' block content';
  }

To try out #26

@alecsmrekar feel free to commit that, it'll be your change after all :)

Combined the changes from MR!6125 and #26 to MR!8743.
The MR!6125 is hidden as redundant.

If we are changing solution issue summary should be updated.

6125 was passing the pipeline but new MR is not so moving to NW for that.

The #26 doesn't change the solution; it is just an adjustment.
The MR!8743 is updated to pass the code styles check. Please review.

@dburiak mind rebasing

Works fine on D10. Adding a dedicated patch file for D10.
The merge request should still be rebased for D11.

The MR is rebased. Review is needed.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The code style is fixed.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rebasing MR8743 gave too much errors, fixing it became a lot of work.
I've created a clean new merge request, starting from a fresh 11.x.

Attached a static D11 patch file.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

11.3.1 patch

Fixed D11.3.1 patch

Hi, in Drupal core changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies. Thanks.