We allow editors on the website to change styles on panes, but not for the region or display. With the current permission model, this isn't possible unless we entirely replace the panels_renderer_editor class.
Add more granular permissions that are broken down by display, region and pane.
| Task | Novice task? | Contributor instructions | Complete? |
|---|---|---|---|
| Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions |
n/a
Additional permissions are created and existing permission is migrated.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | panels-style_permissions_not-2329419-13.patch | 5.38 KB | heddn |
Comments
Comment #1
heddnComment #3
heddnDidn't notice the duplicate hook_update. Let's try this again.
Comment #4
mglamanPerfect for our needs. We don't utilize region styles (yet) and it'd be great to simplify UX until we work it into product. One problem with patch is lack of love for IPE - which is only renderer we expose.
Comment #6
michelleI tested #4 and it applies cleanly and the permissions work as expected. There was one minor thing that I don't know if it matters so I still set it RTBC:
function panels_stylizer_pane_add_style(&$renderer, $plugin, &$conf, $type, $pid, $step = NULL) {
- if (!user_access('administer panels styles')) {
+ if (!user_access("administer panels $type styles")) {
Why double quotes when all the others are single?
One other consideration is that if #1699432: Add IPE permissions for changing pane settings or deleting panes gets in as well, there's going to be an awful lot of IPE permissions.
Comment #7
mglamanI think the use of double quotes was to simplify variable substitution. There is going to be a lot of permissions, but I think this might be a great time for them. When I think of the IPE I think of customers, not developers. This allows site builders to give a tailored experience without having to make and extend their own renderer.
Comment #8
michelleOh, good catch. I missed the variable in there. I'm not sure I agree on having so many perms, though. Isn't it going to be confusing having, for example, "administer panels pane styles" which works in IPE and also "administer pane styles in place editing"? I think they're going to need to be merged/simplified if they both go in.
Comment #9
mglamanI think this is the expected workflow. There is the comment #15 from #1699432: Add IPE permissions for changing pane settings or deleting panes. The todo mentions to move permissions into pipelines
Comment #10
heddnbump
Comment #11
heddnThis has been in RTBC for a while now. Any chance of a commit?
Comment #12
heddnpanels_update_7304 is already taken.
Comment #13
heddnAnd back to RTBC. Since it was such a simple re-roll I don't see why I cannot mark as RTBC. Re-slotted the hook update into 7307.
Comment #14
japerryCommitted. I did make a slight change upon commit in that the old permission still remains, in case any other modules that use it don't break.
Comment #17
sgdev commentedFYI, I just posted a new patch that fixes a problem missed by this patch. Might want to get this added.
https://www.drupal.org/project/panels/issues/3007506