Skip to content
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

Feature/copy #3749

Merged
merged 1 commit into from
Jul 10, 2020
Merged
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
25 changes: 22 additions & 3 deletions apps/files/src/components/FilesAppBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,22 @@
key="move-selected-btn"
icon="folder-move"
:disabled="!canMove"
@click.native="moveResources"
@click.native="triggerLocationPicker('move')"
>
<translate>Move selected</translate>
</oc-button>
</div>
<div>
<oc-button
id="copy-selected-btn"
key="copy-selected-btn"
icon="file_copy"
:disabled="!canCopy"
@click.native="triggerLocationPicker('copy')"
>
<translate>Copy selected</translate>
</oc-button>
</div>
</oc-grid>
</oc-grid>
</div>
Expand Down Expand Up @@ -312,6 +323,14 @@ export default {
})

return insufficientPermissions === false
},

canCopy() {
if (this.publicPage()) {
return this.currentFolder.canCreate()
}

return true
PVince81 marked this conversation as resolved.
Show resolved Hide resolved
}
},
methods: {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<template functional>
<oc-grid gutter="small" child-width="1-1">
<translate translate-comment="Location picker guide on how to execute copy."
>Navigate into the desired folder and copy selected resources into it.</translate
>
<translate translate-comment="Location picker guide on how to navigate into a folder."
>You can navigate into a folder by clicking on its name.</translate
>
<translate translate-comment="Location picker guide on how to navigate to previous folders."
>To navigate back, you can click on the breadcrumbs.</translate
>
<translate translate-comment="Location picker guide on where the resources will be copied."
>Resources will be copied into the folder where you are currently located.</translate
>
</oc-grid>
</template>

<script>
export default {
name: 'CopySidebarMainContent'
}
</script>
89 changes: 64 additions & 25 deletions apps/files/src/components/LocationPicker/LocationPicker.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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')
}
},

Expand All @@ -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)
}
},

Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, so your are indeed checking the perm on the target folder 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

would be good to add a test for the case(s) where a target folder is not targettable due to missing permissions (received read-only share)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've opened a ticket for collecting the next scenarios to test for copy and move. Should I add this in there or implement it already in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

later is fine, please label the ticket "QA-team" and ping Artur

Copy link
Contributor

Choose a reason for hiding this comment

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

later... but within the sprint would be good because else I don't think we can consider the task "done"

},

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)
Expand All @@ -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),
Expand All @@ -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'
})
Expand All @@ -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()
}
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions apps/files/src/fileactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
7 changes: 7 additions & 0 deletions changelog/unreleased/copy
Original file line number Diff line number Diff line change
@@ -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
66 changes: 66 additions & 0 deletions tests/acceptance/features/webUIFiles/copy.feature
Original file line number Diff line number Diff line change
@@ -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 <file_name> using the webUI
And the user renames folder "simple-empty-folder" to <folder_name> using the webUI
And the user copies file <file_name> into folder <folder_name> using the webUI
Then breadcrumb for folder <folder_name> should be displayed on the webUI
And file <file_name> 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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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
16 changes: 13 additions & 3 deletions tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ module.exports = {
restore: 'restore',
rename: 'rename',
deleteImmediately: 'deleteImmediately',
move: 'move'
move: 'move',
copy: 'copy'
}),

/**
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -190,6 +196,10 @@ module.exports = {
moveButtonInFileRow: {
selector: '//button[@aria-label="Move"]',
locateStrategy: 'xpath'
},
copyButtonInFileRow: {
selector: '//button[@aria-label="Copy"]',
locateStrategy: 'xpath'
}
}
}
Loading