Skip to content

Conversation

@MSchinwaldEl
Copy link
Contributor

@MSchinwaldEl MSchinwaldEl commented Oct 31, 2025

Changes in this pull request

image

@MSchinwaldEl MSchinwaldEl linked an issue Oct 31, 2025 that may be closed by this pull request
@MSchinwaldEl MSchinwaldEl marked this pull request as ready for review October 31, 2025 15:29
Copy link
Contributor

@markus-moser markus-moser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MSchinwaldEl
In general it LGTM, but there are at least the following lines missing:

useHandleKeyBindings(() => { if (element != null) rename(element.id, getElementKey(element as unknown as Element, elementType)) }, 'rename')
useHandleKeyBindings(() => { if (element != null) publishNode(element as unknown as Element) }, 'publish')
useHandleKeyBindings(() => { if (element != null && !isNull(elementType) && elementType !== 'asset') unpublishTreeNode(element as unknown as DataObject | Document) }, 'unpublish')

@MSchinwaldEl
Copy link
Contributor Author

@MSchinwaldEl In general it LGTM, but there are at least the following lines missing:

useHandleKeyBindings(() => { if (element != null) rename(element.id, getElementKey(element as unknown as Element, elementType)) }, 'rename')
useHandleKeyBindings(() => { if (element != null) publishNode(element as unknown as Element) }, 'publish')
useHandleKeyBindings(() => { if (element != null && !isNull(elementType) && elementType !== 'asset') unpublishTreeNode(element as unknown as DataObject | Document) }, 'unpublish')

I didn't implement the permission checks for these because, if I understand correctly, these keybindings can only be used in the element editor, which users can only open if they already have the appropriate permissions.
However, I can add the check. Which permissions would be the correct ones here? The element type (Documents/Objects/Assets)?

@markus-moser
Copy link
Contributor

@MSchinwaldEl You will find them in the related actions:

!isTreeActionAllowed(TreePermission.Publish) ||

return !checkElementPermission(node.permissions, 'unpublish') ||

hidden: !checkElementPermission(node.permissions, 'rename') || node.isLocked,

but please use the enum for all locations, thanks

@MSchinwaldEl
Copy link
Contributor Author

@MSchinwaldEl You will find them in the related actions:

!isTreeActionAllowed(TreePermission.Publish) ||

return !checkElementPermission(node.permissions, 'unpublish') ||

hidden: !checkElementPermission(node.permissions, 'rename') || node.isLocked,

but please use the enum for all locations, thanks

I added the missing permissions. As discussed in our call, there are no enums for elementPermissions.

@sonarqubecloud
Copy link

@markus-moser markus-moser changed the title 2357 keybindings respect permissions Keybindings respect permissions Nov 14, 2025
@markus-moser markus-moser merged commit d5dafb9 into 1.x Nov 14, 2025
1 check passed
@markus-moser markus-moser deleted the 2357-keybindings-respect-permissions branch November 14, 2025 16:01
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Keybindings] respect permissions

3 participants