From 38ba64af77f9c58f13b59d3470465fce1a7f1ff2 Mon Sep 17 00:00:00 2001 From: Roland Tapken Date: Tue, 6 Feb 2018 16:44:04 +0100 Subject: [PATCH] Split move and copy operations The new 'Move and copy' operation from #6040 requires UPDATE permissions on the selected files. However, READ would be sufficient to create a copy of a file (if not viewed as a public share). For this reason this patch: - changes the permission of the 'MoveCopy' action to PERMISSION_READ - changes the label of the action depending on the permissions - changes the available buttons in the Move/Copy dialog depending on the permissions. The same changes are done to the filelist view for bulk actions. Signed-off-by: Roland Tapken --- apps/files/js/fileactions.js | 17 +++++++++-- apps/files/js/filelist.js | 34 ++++++++++++++++++---- apps/files/templates/list.php | 2 +- apps/files/tests/js/fileactionsmenuSpec.js | 3 ++ apps/files/tests/js/filelistSpec.js | 9 +++++- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 2fb7dfba29..a6d376aa2a 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -625,12 +625,23 @@ this.registerAction({ name: 'MoveCopy', - displayName: t('files', 'Move or copy'), + displayName: function(context) { + var permissions = context.fileInfoModel.attributes.permissions; + if (permissions & OC.PERMISSION_UPDATE) { + return t('files', 'Move or copy'); + } + return t('files', 'Copy'); + }, mime: 'all', order: -25, - permissions: OC.PERMISSION_UPDATE, + permissions: $('#isPublic').val() ? OC.PERMISSION_UPDATE : OC.PERMISSION_READ, iconClass: 'icon-external', actionHandler: function (filename, context) { + var permissions = context.fileInfoModel.attributes.permissions; + var actions = OC.dialogs.FILEPICKER_TYPE_COPY; + if (permissions & OC.PERMISSION_UPDATE) { + actions = OC.dialogs.FILEPICKER_TYPE_COPY_MOVE; + } OC.dialogs.filepicker(t('files', 'Target folder'), function(targetPath, type) { if (type === OC.dialogs.FILEPICKER_TYPE_COPY) { context.fileList.copy(filename, targetPath); @@ -638,7 +649,7 @@ if (type === OC.dialogs.FILEPICKER_TYPE_MOVE) { context.fileList.move(filename, targetPath); } - }, false, "httpd/unix-directory", true, OC.dialogs.FILEPICKER_TYPE_COPY_MOVE); + }, false, "httpd/unix-directory", true, actions); } }); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index b46db79267..4dc8a58e17 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -798,6 +798,7 @@ OCA.Files.FileActions.updateFileActionSpinner(moveFileAction, false); }; + var actions = this.isSelectedMovable() ? OC.dialogs.FILEPICKER_TYPE_COPY_MOVE : OC.dialogs.FILEPICKER_TYPE_COPY; OC.dialogs.filepicker(t('files', 'Target folder'), function(targetPath, type) { if (type === OC.dialogs.FILEPICKER_TYPE_COPY) { self.copy(files, targetPath, disableLoadingState); @@ -805,7 +806,7 @@ if (type === OC.dialogs.FILEPICKER_TYPE_MOVE) { self.move(files, targetPath, disableLoadingState); } - }, false, "httpd/unix-directory", true, OC.dialogs.FILEPICKER_TYPE_COPY_MOVE); + }, false, "httpd/unix-directory", true, actions); return false; }, @@ -2871,18 +2872,39 @@ this.$el.find('#headerName a.name>span:first').text(selection); this.$el.find('#modified a>span:first').text(''); this.$el.find('table').addClass('multiselect'); - this.$el.find('.selectedActions .copy-move').toggleClass('hidden', !this.isSelectedCopiableOrMovable()); this.$el.find('.selectedActions .download').toggleClass('hidden', !this.isSelectedDownloadable()); this.$el.find('.delete-selected').toggleClass('hidden', !this.isSelectedDeletable()); + + var $copyMove = this.$el.find('.selectedActions .copy-move'); + if (this.isSelectedCopiable()) { + $copyMove.toggleClass('hidden', false); + if (this.isSelectedMovable()) { + $copyMove.find('.label').text(t('files', 'Move or copy')); + } else { + $copyMove.find('.label').text(t('files', 'Copy')); + } + } else { + $copyMove.toggleClass('hidden', true); + } } }, /** - * Check whether all selected files are copiable or movable + * Check whether all selected files are copiable */ - isSelectedCopiableOrMovable: function() { - return _.reduce(this.getSelectedFiles(), function(copiableOrMovable, file) { - return copiableOrMovable && (file.permissions & OC.PERMISSION_UPDATE); + isSelectedCopiable: function() { + return _.reduce(this.getSelectedFiles(), function(copiable, file) { + var requiredPermission = $('#isPublic').val() ? OC.PERMISSION_UPDATE : OC.PERMISSION_READ; + return copiable && (file.permissions & requiredPermission); + }, true); + }, + + /** + * Check whether all selected files are movable + */ + isSelectedMovable: function() { + return _.reduce(this.getSelectedFiles(), function(movable, file) { + return movable && (file.permissions & OC.PERMISSION_UPDATE); }, true); }, diff --git a/apps/files/templates/list.php b/apps/files/templates/list.php index fd5423c334..e6b1e54d38 100644 --- a/apps/files/templates/list.php +++ b/apps/files/templates/list.php @@ -53,7 +53,7 @@ - t('Move or copy'))?> + t('Move or copy'))?> diff --git a/apps/files/tests/js/fileactionsmenuSpec.js b/apps/files/tests/js/fileactionsmenuSpec.js index 926516b304..c678d16615 100644 --- a/apps/files/tests/js/fileactionsmenuSpec.js +++ b/apps/files/tests/js/fileactionsmenuSpec.js @@ -271,6 +271,7 @@ describe('OCA.Files.FileActionsMenu tests', function() { $file: $tr, fileList: fileList, fileActions: fileActions, + fileInfoModel: new OCA.Files.FileInfoModel(fileData), dir: fileList.getCurrentDirectory() }; menu = new OCA.Files.FileActionsMenu(); @@ -304,6 +305,7 @@ describe('OCA.Files.FileActionsMenu tests', function() { $file: $tr, fileList: fileList, fileActions: fileActions, + fileInfoModel: new OCA.Files.FileInfoModel(fileData), dir: '/anotherpath/there' }; menu = new OCA.Files.FileActionsMenu(); @@ -336,6 +338,7 @@ describe('OCA.Files.FileActionsMenu tests', function() { $file: $tr, fileList: fileList, fileActions: fileActions, + fileInfoModel: new OCA.Files.FileInfoModel(fileData), dir: '/somepath/dir' }; menu = new OCA.Files.FileActionsMenu(); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 08da15b8a8..1b26a46817 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -94,7 +94,7 @@ describe('OCA.Files.FileList tests', function() { '' + 'Name' + '' + '' + @@ -2101,10 +2101,17 @@ describe('OCA.Files.FileList tests', function() { $('#permissions').val(OC.PERMISSION_READ | OC.PERMISSION_UPDATE); $('.select-all').click(); expect(fileList.$el.find('.selectedActions .copy-move').hasClass('hidden')).toEqual(false); + expect(fileList.$el.find('.selectedActions .copy-move .label').text()).toEqual('Move or copy'); testFiles[0].permissions = OC.PERMISSION_READ; $('.select-all').click(); fileList.setFiles(testFiles); $('.select-all').click(); + expect(fileList.$el.find('.selectedActions .copy-move').hasClass('hidden')).toEqual(false); + expect(fileList.$el.find('.selectedActions .copy-move .label').text()).toEqual('Copy'); + testFiles[0].permissions = OC.PERMISSION_NONE; + $('.select-all').click(); + fileList.setFiles(testFiles); + $('.select-all').click(); expect(fileList.$el.find('.selectedActions .copy-move').hasClass('hidden')).toEqual(true); }); it('show doesnt show the download action if one or more files are not downloadable', function () {