Closed (fixed)
Project:
Drupal core
Version:
10.1.x-dev
Component:
Umami demo
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
21 Jun 2021 at 19:55 UTC
Updated:
21 Feb 2023 at 14:44 UTC
Jump to comment: Most recent
Comments
Comment #2
bernardm28 commentedComment #5
bernardm28 commentedComment #6
bernardm28 commentedComment #7
bernardm28 commentedComment #9
benjifisherThose permissions were added in #2914486: Add granular permissions to the Layout Builder. I agree that the Author and Editor roles in Umami should have these permissions.
I checked all three content types: Article, Basic page, Recipe. In all three, the Default view mode does not use Layout Builder, but the "Full content" view mode does: for example,
/en/admin/structure/types/manage/article/display/full. In that view mode, all three content types allow individual nodes to override the default layout.I reviewed the merge request, and it looks good, but I did not test.
A manual test should include at least the following:
/en/admin/people/permissions/module/layout_builder./en/node/18/layout.I think the testing is a Novice task, so I am adding the issue tag for that.
Comment #12
gxleano commentedComment #13
gxleano commentedComment #14
gxleano commentedI'll test it!
Comment #15
benjifisher@gxleano:
Thanks for helping with this issue.
Whether or not the testing goes as expected, please give details about how you tested. A few screenshots would help: show the URL and the logged-in user, and perhaps the confirmation message after making changes.
Comment #16
gxleano commentedTested in version 9.2.1-dev, which is the branch the merge request is created against.
TEST RESULT (Step by step)
1. As an Admin user, go to
/admin/people/permissionsand check Layout builder permissions for Author and Editor roles.The permissions below are automatically enabled to the Author and Editor roles:
2. As an Admin user, go to
/admin/structure/types/manage/article/displayand enable "Use Layout Builder" option over Layout options.3. As an Author and Editor user, try to edit the layout of his/him/their existing content (Article, Recipe or Basic Page).
Edited Layout of content
/node/11/edit?destination=/en/admin/content, adding a Basic block (custom block) and everything is working as expected.To sum up, the code into the MRs looks good to me, and it's working as expected.
Comment #17
benjifisher@gxleano:
Thanks for the testing. Your Comment #16 is just what I had in mind.
Since 9.2.x is not even supported anymore, this issue should really target 10.1.x. The core committers may decide to back-port it to 10.0.x and 9.5.x since it is not a disruptive change. (It affects only new installations of the Umami demo profile.)
I checked, and the patch (or diff) from the merge request applies cleanly to the 10.1.x branch.
I also noticed that, after applying the patch, the permissions are not in alphabetical order. I am sorry I missed that in #9. I am setting the status back to NW for that. I think that fixing the order is a Novice task, so I am leaving that issue tag. I am removing the tag for manual testing. After #16, I do not think we have to re-test.
Comment #19
gxleano commentedI've created a merge request to
10.1.xadding the permissions ordered alphabetically.Comment #20
lauriiiIt looks like CI is failing for the MR.
Comment #21
joachim commentedThese are dependencies that are missing in the role config. Permissions now have dependencies (https://www.drupal.org/node/3055548) and so giving a role a permission for a node type means the role config entity has a dependency on the node type config entity.
(Aside: AssertConfigTrait::assertConfigDiff() should be converted to a PHPUnit custom assertion, and uuuuurgh the Drupal\Component\Diff component has no documentation!!!!)
Comment #22
gxleano commentedComment #23
gxleano commentedComment #24
gxleano commentedI've fixed the Demo Umami test, but now it seems to be unstable with FunctionalJavascript test in CKEditor 5.
Comment #25
gxleano commentedComment #26
joachim commentedLGTM!
Comment #27
longwaveUpdating title, and saving issue credit.
Comment #28
longwaveBackported to 10.0.x/9.5.x as a low risk bug fix; the config change only affects newly installed demo sites.
Committed and pushed f0269a4896 to 10.1.x and 6f2b88b8dc to 10.0.x and 3d51469370 to 9.5.x. Thanks!