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.

Single test fail completely unrelated: Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLinkabilityTest.

@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 :)

dburiak → changed the visibility of the branch 3041174-adjust-layout-builder to hidden.

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.

Also, I don't think #26 changes the solution, just extending it.
The MR!8743 is updated to pass the code styles check. Please review.

@dburiak mind rebasing