-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix(files_sharing): Hide 'Open locally' action #56060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
/compile |
This patch ensures that the "Open locally" context menu item is not displayed for files in a share where the "download and sync" permission has not been granted. This prevents user confusion, as the action would fail anyway. The fix adds a permission check before rendering the menu item, and adds a corresponding unit test to verify this behavior. Resolves: #54970 Signed-off-by: Fauzan <[email protected]>
35b709b to
096047a
Compare
szaimen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐘
| } | ||
|
|
||
| return true | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | |
| } | |
Signed-off-by: nextcloud-command <[email protected]>
| // check hide-download property of shares | ||
| if (node.attributes['hide-download'] === true | ||
| || node.attributes['hide-download'] === 'true' | ||
| ) { | ||
| return false | ||
| } | ||
|
|
||
| // If the mount type is a share, ensure it got download permissions. | ||
| if (node.attributes['share-attributes']) { | ||
| const shareAttributes = JSON.parse(node.attributes['share-attributes'] || '[]') as Array<ShareAttribute> | ||
| const downloadAttribute = shareAttributes.find(({ scope, key }: ShareAttribute) => scope === 'permissions' && key === 'download') | ||
| if (downloadAttribute !== undefined) { | ||
| return downloadAttribute.value === true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isnt this basically
| // check hide-download property of shares | |
| if (node.attributes['hide-download'] === true | |
| || node.attributes['hide-download'] === 'true' | |
| ) { | |
| return false | |
| } | |
| // If the mount type is a share, ensure it got download permissions. | |
| if (node.attributes['share-attributes']) { | |
| const shareAttributes = JSON.parse(node.attributes['share-attributes'] || '[]') as Array<ShareAttribute> | |
| const downloadAttribute = shareAttributes.find(({ scope, key }: ShareAttribute) => scope === 'permissions' && key === 'download') | |
| if (downloadAttribute !== undefined) { | |
| return downloadAttribute.value === true | |
| } | |
| } | |
| if (!isDownloadable(node)) { | |
| return false | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow me to jump into the discussion. Yes, it’s basically the same logic as the isDownloadable function. I just copied and pasted the logic. But doesn’t your suggestion make the node.permissions validation repetitive? It already checks the UPDATE permission, which means it automatically has READ permission.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It already checks the UPDATE permission, which means it automatically has READ permission
you could have UPDATE without READ per se :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad:), should i update it first? re-commit to my fork?
Manual push of #55992 from community
Closes #55992