From 4321d7522e66f387351f6721a1380081ef2c78df Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 20 Nov 2014 16:34:33 +0100 Subject: [PATCH 1/3] Check if files are deletable before trying to delete them --- apps/files/ajax/delete.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/files/ajax/delete.php b/apps/files/ajax/delete.php index 323b70706ce..4d4232e872e 100644 --- a/apps/files/ajax/delete.php +++ b/apps/files/ajax/delete.php @@ -27,7 +27,9 @@ $success = true; //Now delete foreach ($files as $file) { if (\OC\Files\Filesystem::file_exists($dir . '/' . $file) && - !\OC\Files\Filesystem::unlink($dir . '/' . $file)) { + !(\OC\Files\Filesystem::isDeletable($dir . '/' . $file) && + \OC\Files\Filesystem::unlink($dir . '/' . $file)) + ) { $filesWithError .= $file . "\n"; $success = false; } From 9c86665ef400b82487f48374bf11bbe6628cbe28 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 20 Nov 2014 16:53:32 +0100 Subject: [PATCH 2/3] Dont show the delete button for selected files if one of the selected files is not deletable --- apps/files/js/filelist.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index bec0155e90e..6ffc10cdcbd 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -526,7 +526,8 @@ mimetype: $el.attr('data-mime'), type: $el.attr('data-type'), size: parseInt($el.attr('data-size'), 10), - etag: $el.attr('data-etag') + etag: $el.attr('data-etag'), + permissions: parseInt($el.attr('data-permissions'), 10) }; }, @@ -1636,7 +1637,7 @@ this.$el.find('.selectedActions').addClass('hidden'); } else { - canDelete = (this.getDirectoryPermissions() & OC.PERMISSION_DELETE); + canDelete = (this.getDirectoryPermissions() & OC.PERMISSION_DELETE) && this.isSelectedDeletable(); this.$el.find('.selectedActions').removeClass('hidden'); this.$el.find('#headerSize a>span:first').text(OC.Util.humanFileSize(summary.totalSize)); var selection = ''; @@ -1656,6 +1657,15 @@ } }, + /** + * Check whether all selected files are deletable + */ + isSelectedDeletable: function() { + return _.reduce(this.getSelectedFiles(), function(deletable, file) { + return deletable && (file.permissions & OC.PERMISSION_DELETE); + }, true); + }, + /** * Returns whether all files are selected * @return true if all files are selected, false otherwise From e8cbb8e2d8dfe9e6ddc1cb151b860719d983805b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 25 Nov 2014 17:16:23 +0100 Subject: [PATCH 3/3] Add js unit test --- apps/files/tests/js/filelistSpec.js | 34 +++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index a7fa14eb14a..21f8a12f4b5 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -97,7 +97,8 @@ describe('OCA.Files.FileList tests', function() { name: 'One.txt', mimetype: 'text/plain', size: 12, - etag: 'abc' + etag: 'abc', + permissions: OC.PERMISSION_ALL }, { id: 2, type: 'file', @@ -105,6 +106,7 @@ describe('OCA.Files.FileList tests', function() { mimetype: 'image/jpeg', size: 12049, etag: 'def', + permissions: OC.PERMISSION_ALL }, { id: 3, type: 'file', @@ -112,13 +114,15 @@ describe('OCA.Files.FileList tests', function() { mimetype: 'application/pdf', size: 58009, etag: '123', + permissions: OC.PERMISSION_ALL }, { id: 4, type: 'dir', name: 'somedir', mimetype: 'httpd/unix-directory', size: 250, - etag: '456' + etag: '456', + permissions: OC.PERMISSION_ALL }]; pageSizeStub = sinon.stub(OCA.Files.FileList.prototype, 'pageSize').returns(20); fileList = new OCA.Files.FileList($('#app-content-files')); @@ -1479,6 +1483,17 @@ describe('OCA.Files.FileList tests', function() { $('.select-all').click(); expect(fileList.$el.find('.delete-selected').hasClass('hidden')).toEqual(true); }); + it('show doesnt show the delete action if one or more files are not deletable', function () { + fileList.setFiles(testFiles); + $('#permissions').val(OC.PERMISSION_READ | OC.PERMISSION_DELETE); + $('.select-all').click(); + expect(fileList.$el.find('.delete-selected').hasClass('hidden')).toEqual(false); + testFiles[0].permissions = OC.PERMISSION_READ; + $('.select-all').click(); + fileList.setFiles(testFiles); + $('.select-all').click(); + expect(fileList.$el.find('.delete-selected').hasClass('hidden')).toEqual(true); + }); }); describe('Actions', function() { beforeEach(function() { @@ -1495,7 +1510,8 @@ describe('OCA.Files.FileList tests', function() { mimetype: 'text/plain', type: 'file', size: 12, - etag: 'abc' + etag: 'abc', + permissions: OC.PERMISSION_ALL }); expect(files[1]).toEqual({ id: 3, @@ -1503,7 +1519,8 @@ describe('OCA.Files.FileList tests', function() { name: 'Three.pdf', mimetype: 'application/pdf', size: 58009, - etag: '123' + etag: '123', + permissions: OC.PERMISSION_ALL }); expect(files[2]).toEqual({ id: 4, @@ -1511,7 +1528,8 @@ describe('OCA.Files.FileList tests', function() { name: 'somedir', mimetype: 'httpd/unix-directory', size: 250, - etag: '456' + etag: '456', + permissions: OC.PERMISSION_ALL }); }); it('Removing a file removes it from the selection', function() { @@ -1524,7 +1542,8 @@ describe('OCA.Files.FileList tests', function() { mimetype: 'text/plain', type: 'file', size: 12, - etag: 'abc' + etag: 'abc', + permissions: OC.PERMISSION_ALL }); expect(files[1]).toEqual({ id: 4, @@ -1532,7 +1551,8 @@ describe('OCA.Files.FileList tests', function() { name: 'somedir', mimetype: 'httpd/unix-directory', size: 250, - etag: '456' + etag: '456', + permissions: OC.PERMISSION_ALL }); }); describe('Download', function() {