From 1ad38258aa07e6cf267721862d40a081c605c228 Mon Sep 17 00:00:00 2001 From: Lukas Hirt Date: Tue, 7 Jul 2020 20:49:47 +0200 Subject: [PATCH] Add copy action Fix promise and properly handle errors Add acceptance tests Merge move and copy in locationPicker tests Add batch copy Add copy file action in tests Fix copy error message Add permissions check for copy in public links Add permissions check for batch copy action in public links Fix copy file action in tests Fix error message in copy tests Add changelog Add translations context and adjust guides for copy Skip public links test in oCIS Remove target from move --- apps/files/src/components/FilesAppBar.vue | 25 +++++- .../LocationPicker/CopySidebarMainContent.vue | 22 +++++ .../LocationPicker/LocationPicker.vue | 89 +++++++++++++------ apps/files/src/fileactions.js | 19 ++++ changelog/unreleased/copy | 7 ++ .../features/webUIFiles/copy.feature | 66 ++++++++++++++ .../FilesPageElement/fileActionsMenu.js | 16 +++- .../pageObjects/FilesPageElement/filesList.js | 14 ++- tests/acceptance/pageObjects/filesPage.js | 13 ++- .../acceptance/pageObjects/locationPicker.js | 2 +- .../stepDefinitions/filesContext.js | 18 +++- 11 files changed, 255 insertions(+), 36 deletions(-) create mode 100644 apps/files/src/components/LocationPicker/CopySidebarMainContent.vue create mode 100644 changelog/unreleased/copy create mode 100644 tests/acceptance/features/webUIFiles/copy.feature diff --git a/apps/files/src/components/FilesAppBar.vue b/apps/files/src/components/FilesAppBar.vue index d9c1f046ba9..5ccf9e76ec0 100644 --- a/apps/files/src/components/FilesAppBar.vue +++ b/apps/files/src/components/FilesAppBar.vue @@ -127,11 +127,22 @@ key="move-selected-btn" icon="folder-move" :disabled="!canMove" - @click.native="moveResources" + @click.native="triggerLocationPicker('move')" > Move selected +
+ + Copy selected + +
@@ -312,6 +323,14 @@ export default { }) return insufficientPermissions === false + }, + + canCopy() { + if (this.publicPage()) { + return this.currentFolder.canCreate() + } + + return true } }, methods: { @@ -610,14 +629,14 @@ export default { this.setHighlightedFile(null) }, - moveResources() { + triggerLocationPicker(action) { const resources = cloneStateObject(this.selectedFiles) const parent = pathUtil.dirname(this.currentFolder.path) this.$router.push({ name: 'location-picker', query: { - action: 'move', + action, target: parent, resource: resources.map(resource => { return resource.path diff --git a/apps/files/src/components/LocationPicker/CopySidebarMainContent.vue b/apps/files/src/components/LocationPicker/CopySidebarMainContent.vue new file mode 100644 index 00000000000..c0d508ba7a6 --- /dev/null +++ b/apps/files/src/components/LocationPicker/CopySidebarMainContent.vue @@ -0,0 +1,22 @@ + + + diff --git a/apps/files/src/components/LocationPicker/LocationPicker.vue b/apps/files/src/components/LocationPicker/LocationPicker.vue index 79834ddb6ec..19179acad78 100644 --- a/apps/files/src/components/LocationPicker/LocationPicker.vue +++ b/apps/files/src/components/LocationPicker/LocationPicker.vue @@ -112,6 +112,7 @@ import FileList from '../FileList.vue' import FileItem from '../FileItem.vue' import SortableColumnHeader from '../FilesLists/SortableColumnHeader.vue' import NoContentMessage from '../NoContentMessage.vue' +import CopySidebarMainContent from './CopySidebarMainContent.vue' export default { name: 'LocationPicker', @@ -217,26 +218,33 @@ export default { title() { const count = this.resourcesCount + let title = '' if (this.currentAction === 'move') { - const title = this.$ngettext( + title = this.$ngettext( 'Selected %{ count } resource to move into:', 'Selected %{ count } resources to move into:', count ) - - return this.$gettextInterpolate(title, { count: count }) + } else if (this.currentAction === 'copy') { + title = this.$ngettext( + 'Selected %{ count } resource to copy into:', + 'Selected %{ count } resources to copy into:', + count + ) } - return null + return this.$gettextInterpolate(title, { count: count }, false) }, confirmBtnText() { if (this.currentAction === 'move') { return this.$pgettext('Confirm action in the location picker for move', 'Move here') + } else if (this.currentAction === 'copy') { + return this.$pgettext('Confirm action in the location picker for copy', 'Copy here') } - return null + return this.$gettext('Confirm') } }, @@ -252,6 +260,8 @@ export default { if (this.currentAction === 'move') { this.SET_MAIN_CONTENT_COMPONENT(MoveSidebarMainContent) + } else if (this.currentAction === 'copy') { + this.SET_MAIN_CONTENT_COMPONENT(CopySidebarMainContent) } }, @@ -306,10 +316,17 @@ export default { this.$router.push({ name: 'files-list', params: { item: target || '/' } }) }, - async moveResources() { + isRowDisabled(resource) { + const isBeingMoved = this.resources.some(item => item === resource.path) + + return resource.type !== 'folder' || !resource.canCreate() || isBeingMoved + }, + + async confirmAction() { const errors = [] - const promise = this.isPublicPage ? this.$client.publicFiles : this.$client.files + let promise = null + // Execute move or copy for (const resource of this.resources) { let target = this.target || '/' const resourceName = basename(resource) @@ -328,14 +345,39 @@ export default { continue } - await promise - .move(resource, (target += '/' + resourceName), this.publicLinkPassword) - .catch(error => errors.push(error)) + if (this.currentAction === 'move') { + promise = this.isPublicPage + ? this.$client.publicFiles.move( + resource, + (target += '/' + resourceName), + this.publicLinkPassword + ) + : this.$client.files.move(resource, (target += '/' + resourceName)) + } else if (this.currentAction === 'copy') { + promise = this.isPublicPage + ? this.$client.publicFiles.copy( + resource, + (target += '/' + resourceName), + this.publicLinkPassword + ) + : this.$client.files.copy(resource, (target += '/' + resourceName)) + } + + await promise.catch(error => { + error.resource = resourceName + errors.push(error) + }) } // Display error message if (errors.length === 1) { - const title = this.$gettext('An error occurred while moving %{resource}') + let title = '' + + if (this.currentAction === 'move') { + title = this.$gettext('An error occurred while moving %{resource}') + } else if (this.currentAction === 'copy') { + title = this.$gettext('An error occurred while copying %{resource}') + } this.showMessage({ title: this.$gettextInterpolate(title, { resource: errors[0].resource }, true), @@ -347,10 +389,19 @@ export default { } if (errors.length > 1) { - const desc = this.$gettext('%{count} resources could not be moved') + let title = '' + let desc = '' + + if (this.currentAction === 'move') { + title = this.$gettext('An error occurred while moving several resources') + desc = this.$gettext('%{count} resources could not be moved') + } else if (this.currentAction === 'copy') { + title = this.$gettext('An error occurred while copying several resources') + desc = this.$gettext('%{count} resources could not be copied') + } this.showMessage({ - title: this.$gettext('An error occurred while moving several resources'), + title, desc: this.$gettextInterpolate(desc, { count: errors.length }, false), status: 'danger' }) @@ -360,18 +411,6 @@ export default { } this.leaveLocationPicker(this.target) - }, - - isRowDisabled(resource) { - const isBeingMoved = this.resources.some(item => item === resource.path) - - return resource.type !== 'folder' || !resource.canCreate() || isBeingMoved - }, - - confirmAction() { - if (this.currentAction === 'move') { - return this.moveResources() - } } } } diff --git a/apps/files/src/fileactions.js b/apps/files/src/fileactions.js index fe75b622653..eccd8272831 100644 --- a/apps/files/src/fileactions.js +++ b/apps/files/src/fileactions.js @@ -55,6 +55,25 @@ export default { return item.canRename() } }, + { + icon: 'file_copy', + handler: resource => { + // Parent of the resource selected for copy used as a default target location + const parent = dirname(resource.path) + this.$router.push({ + name: 'location-picker', + query: { action: 'copy', target: parent, resource: resource.path } + }) + }, + ariaLabel: () => this.$gettext('Copy'), + isEnabled: () => { + if (this.publicPage()) { + return this.currentFolder.canCreate() + } + + return true + } + }, { icon: 'folder-move', handler: resource => { diff --git a/changelog/unreleased/copy b/changelog/unreleased/copy new file mode 100644 index 00000000000..67856193ed2 --- /dev/null +++ b/changelog/unreleased/copy @@ -0,0 +1,7 @@ +Enhancement: Add ability to copy files and folders into a different location + +We've added copy action to the files list. The copy action is executed via a new page called location picker. + +https://github.com/owncloud/product/issues/102 +https://github.com/owncloud/product/issues/108 +https://github.com/owncloud/phoenix/pull/3749 \ No newline at end of file diff --git a/tests/acceptance/features/webUIFiles/copy.feature b/tests/acceptance/features/webUIFiles/copy.feature new file mode 100644 index 00000000000..a9b771e6c68 --- /dev/null +++ b/tests/acceptance/features/webUIFiles/copy.feature @@ -0,0 +1,66 @@ +Feature: copy files and folders + As a user + I want to copy files and folders + So that I can work safely on a copy without changing the original + + Background: + Given user "user1" has been created with default attributes + + @smokeTest + Scenario: copy a file and a folder into a folder + Given user "user1" has logged in using the webUI + And the user has browsed to the files page + When the user copies file "data.zip" into folder "simple-empty-folder" using the webUI + Then breadcrumb for folder "simple-empty-folder" should be displayed on the webUI + And file "data.zip" should be listed on the webUI + When the user browses to the files page + And the user copies folder "simple-folder" into folder "strängé नेपाली folder empty" using the webUI + Then breadcrumb for folder "strängé नेपाली folder empty" should be displayed on the webUI + And folder "simple-folder" should be listed on the webUI + + Scenario: copy a file into a folder where a file with the same name already exists + Given user "user1" has logged in using the webUI + And the user has browsed to the files page + When the user copies file "strängé filename (duplicate #2 &).txt" into folder "strängé नेपाली folder" using the webUI + Then the error message with header 'An error occurred while copying strängé filename (duplicate #2 &).txt' should be displayed on the webUI + + @smokeTest + Scenario: Copy multiple files at once + Given user "user1" has logged in using the webUI + And the user has browsed to the files page + When the user batch copies these files into folder "simple-empty-folder" using the webUI + | name | + | data.zip | + | lorem.txt | + | testapp.zip | + Then breadcrumb for folder "simple-empty-folder" should be displayed on the webUI + And the following file should be listed on the webUI + | name-parts | + | data.zip | + | lorem.txt | + | testapp.zip | + + Scenario Outline: copy a file into a folder (problematic characters) + Given user "user1" has logged in using the webUI + And the user has browsed to the files page + When the user renames file "lorem.txt" to using the webUI + And the user renames folder "simple-empty-folder" to using the webUI + And the user copies file into folder using the webUI + Then breadcrumb for folder should be displayed on the webUI + And file should be listed on the webUI + Examples: + | file_name | folder_name | + | "'single'" | "folder-with-'single'" | + # | "\"double\" quotes" | "folder-with\"double\" quotes" | FIXME: Needs a way to access breadcrumbs with double quotes issue-3734 + | "question?" | "folder-with-question?" | + | "&and#hash" | "folder-with-&and#hash" | + + @skipOnOCIS @issue-3755 + Scenario: copy files on a public share + Given user "user1" has shared folder "simple-folder" with link with "read, update, create, delete" permissions + And the public uses the webUI to access the last public link created by user "user1" + And the user copies file "data.zip" into folder "simple-empty-folder" using the webUI + Then breadcrumb for folder "simple-empty-folder" should be displayed on the webUI + And file "data.zip" should be listed on the webUI + And as "user1" file "simple-folder/simple-empty-folder/data.zip" should exist + And as "user1" file "simple-folder/data.zip" should exist diff --git a/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js b/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js index 1bc7c56cc0a..1ee0d40fad6 100644 --- a/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js +++ b/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js @@ -12,7 +12,8 @@ module.exports = { restore: 'restore', rename: 'rename', deleteImmediately: 'deleteImmediately', - move: 'move' + move: 'move', + copy: 'copy' }), /** @@ -135,10 +136,15 @@ module.exports = { }, /** * Trigger the move of a resource via its file action - * @param {String} target Folder into which the resource should be moved */ - move: function(target) { + move: function() { this.performFileAction(this.FileAction.move) + }, + /** + * Trigger the copy of a resource via its file action + */ + copy: function() { + this.performFileAction(this.FileAction.copy) } }, elements: { @@ -190,6 +196,10 @@ module.exports = { moveButtonInFileRow: { selector: '//button[@aria-label="Move"]', locateStrategy: 'xpath' + }, + copyButtonInFileRow: { + selector: '//button[@aria-label="Copy"]', + locateStrategy: 'xpath' } } } diff --git a/tests/acceptance/pageObjects/FilesPageElement/filesList.js b/tests/acceptance/pageObjects/FilesPageElement/filesList.js index 11d9f599645..cb550d136c6 100644 --- a/tests/acceptance/pageObjects/FilesPageElement/filesList.js +++ b/tests/acceptance/pageObjects/FilesPageElement/filesList.js @@ -634,7 +634,19 @@ module.exports = { await filesRow.openFileActionsMenu(resource).move() // Execute move - await client.page.locationPicker().move(target) + await client.page.locationPicker().selectFolderAndConfirm(target) + + return this + }, + + copyResource: async function(resource, target) { + await this.waitForFileVisible(resource) + + // Trigger copy + await filesRow.openFileActionsMenu(resource).copy() + + // Execute copy + await client.page.locationPicker().selectFolderAndConfirm(target) return this } diff --git a/tests/acceptance/pageObjects/filesPage.js b/tests/acceptance/pageObjects/filesPage.js index ef205304ea1..7d34ae3863a 100644 --- a/tests/acceptance/pageObjects/filesPage.js +++ b/tests/acceptance/pageObjects/filesPage.js @@ -302,7 +302,15 @@ module.exports = { this.click('@moveSelectedBtn') // Execute move - return client.page.locationPicker().move(target) + return client.page.locationPicker().selectFolderAndConfirm(target) + }, + + copyMultipleResources: function(target) { + // Trigger copy + this.click('@copySelectedBtn') + + // Execute copy + return client.page.locationPicker().selectFolderAndConfirm(target) } }, elements: { @@ -419,6 +427,9 @@ module.exports = { }, moveSelectedBtn: { selector: '#move-selected-btn' + }, + copySelectedBtn: { + selector: '#copy-selected-btn' } } } diff --git a/tests/acceptance/pageObjects/locationPicker.js b/tests/acceptance/pageObjects/locationPicker.js index f18b137af45..eb1f6b7228b 100644 --- a/tests/acceptance/pageObjects/locationPicker.js +++ b/tests/acceptance/pageObjects/locationPicker.js @@ -2,7 +2,7 @@ const { client } = require('nightwatch-api') module.exports = { commands: { - move: async function(target) { + selectFolderAndConfirm: async function(target) { await client.page.FilesPageElement.filesList().navigateToFolder(target) return this.waitForElementVisible('@confirmBtn').click('@confirmBtn') diff --git a/tests/acceptance/stepDefinitions/filesContext.js b/tests/acceptance/stepDefinitions/filesContext.js index fd730407741..d7cedb272fe 100644 --- a/tests/acceptance/stepDefinitions/filesContext.js +++ b/tests/acceptance/stepDefinitions/filesContext.js @@ -1100,6 +1100,20 @@ When( } ) -Then('the moved elements should be listed on the webUI', function(resources) { - console.log(resources) +When('the user copies file/folder {string} into folder {string} using the webUI', function( + resource, + target +) { + return client.page.FilesPageElement.filesList().copyResource(resource, target) }) + +When( + 'the user batch copies these files/folders into folder {string} using the webUI', + async function(target, resources) { + for (const item of resources.rows()) { + await client.page.FilesPageElement.filesList().toggleFileOrFolderCheckbox('enable', item[0]) + } + + return client.page.filesPage().copyMultipleResources(target) + } +)