Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions apps/files/src/actions/openLocallyAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import IconWeb from '@mdi/svg/svg/web.svg?raw'
import { getCurrentUser } from '@nextcloud/auth'
import axios from '@nextcloud/axios'
import { DialogBuilder, showError } from '@nextcloud/dialogs'
import { FileAction, Permission } from '@nextcloud/files'
import { FileAction } from '@nextcloud/files'
import { translate as t } from '@nextcloud/l10n'
import { encodePath } from '@nextcloud/paths'
import { generateOcsUrl } from '@nextcloud/router'
import { isPublicShare } from '@nextcloud/sharing/public'
import logger from '../logger.ts'
import { isSyncable } from '../utils/permissions.ts'

export const action = new FileAction({
id: 'edit-locally',
Expand All @@ -34,7 +35,7 @@ export const action = new FileAction({
return false
}

return (nodes[0].permissions & Permission.UPDATE) !== 0
return isSyncable(nodes[0])
},

async exec(node: Node) {
Expand Down
31 changes: 31 additions & 0 deletions apps/files/src/utils/permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,34 @@ export function isDownloadable(node: Node): boolean {

return true
}


/**
* Check permissions on the node if it can be synced/open locally
*
* @param node The node to check
* @return True if syncable, false otherwise
*/
export function isSyncable(node: Node): boolean {
if ((node.permissions & Permission.UPDATE) === 0) {
return false
}

// 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
}
}
Comment on lines +52 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

Isnt this basically

Suggested change
// 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
}

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.

Copy link
Member Author

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

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?


return true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

4 changes: 2 additions & 2 deletions dist/files-init.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files-init.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files-main.js.map

Large diffs are not rendered by default.

Loading