diff --git a/apps/files/src/actions/moveOrCopyAction.ts b/apps/files/src/actions/moveOrCopyAction.ts index 7827fc06b29..36a17a4f7b5 100644 --- a/apps/files/src/actions/moveOrCopyAction.ts +++ b/apps/files/src/actions/moveOrCopyAction.ts @@ -11,18 +11,214 @@ import type { MoveCopyResult } from './moveOrCopyActionUtils.ts' import FolderMoveSvg from '@mdi/svg/svg/folder-move-outline.svg?raw' import CopyIconSvg from '@mdi/svg/svg/folder-multiple-outline.svg?raw' import { isAxiosError } from '@nextcloud/axios' -import { FilePickerClosed, getFilePickerBuilder, showError, showInfo, TOAST_PERMANENT_TIMEOUT } from '@nextcloud/dialogs' +import { FilePickerClosed, getFilePickerBuilder, openConflictPicker, showError, showLoading } from '@nextcloud/dialogs' import { emit } from '@nextcloud/event-bus' import { FileAction, FileType, getUniqueName, NodeStatus, Permission } from '@nextcloud/files' import { defaultRootPath, getClient, getDefaultPropfind, resultToNode } from '@nextcloud/files/dav' import { t } from '@nextcloud/l10n' -import { hasConflict, openConflictPicker } from '@nextcloud/upload' +import { getConflicts } from '@nextcloud/upload' import { basename, join } from 'path' import Vue from 'vue' import logger from '../logger.ts' import { getContents } from '../services/Files.ts' import { canCopy, canMove, getQueue, MoveCopyAction } from './moveOrCopyActionUtils.ts' +/** + * Exception to hint the user about something. + * The message is intended to be shown to the user. + */ +export class HintException extends Error {} + +export const ACTION_COPY_MOVE = 'move-copy' + +export const action = new FileAction({ + id: ACTION_COPY_MOVE, + order: 15, + displayName({ nodes }) { + switch (getActionForNodes(nodes)) { + case MoveCopyAction.MOVE: + return t('files', 'Move') + case MoveCopyAction.COPY: + return t('files', 'Copy') + case MoveCopyAction.MOVE_OR_COPY: + return t('files', 'Move or copy') + } + }, + iconSvgInline: () => FolderMoveSvg, + enabled({ nodes, view }): boolean { + // We can not copy or move in single file shares + if (view.id === 'public-file-share') { + return false + } + // We only support moving/copying files within the user folder + if (!nodes.every((node) => node.root?.startsWith('/files/'))) { + return false + } + return nodes.length > 0 && (canMove(nodes) || canCopy(nodes)) + }, + + async exec(context) { + return this.execBatch!(context)[0] + }, + + async execBatch({ nodes, folder }) { + const action = getActionForNodes(nodes) + const target = await openFilePickerForAction(action, folder.path, nodes) + // Handle cancellation silently + if (target === false) { + return nodes.map(() => null) + } + + try { + const result = await Array.fromAsync(handleCopyMoveNodesTo(nodes, target.destination, target.action)) + return result.map(() => true) + } catch (error) { + logger.error(`Failed to ${target.action} node`, { nodes, error }) + if (error instanceof HintException && !!error.message) { + showError(error.message) + // Silent action as we handle the toast + return nodes.map(() => null) + } + // We need to keep the selection on error! + // So we do not return null, and for batch action + return nodes.map(() => false) + } + }, +}) + +/** + * Handle the copy/move of a node to a destination + * This can be imported and used by other scripts/components on server + * + * @param nodes The nodes to copy/move + * @param destination The destination to copy/move the nodes to + * @param method The method to use for the copy/move + * @param overwrite Whether to overwrite the destination if it exists + * @yields {AsyncGenerator} A promise that resolves when the copy/move is done + */ +export async function* handleCopyMoveNodesTo(nodes: INode[], destination: IFolder, method: MoveCopyAction.COPY | MoveCopyAction.MOVE, overwrite = false): AsyncGenerator { + if (!destination) { + return + } + + if (destination.type !== FileType.Folder) { + throw new Error(t('files', 'Destination is not a folder')) + } + + // Do not allow to MOVE a node to the same folder it is already located + if (method === MoveCopyAction.MOVE && nodes.some((node) => node.dirname === destination.path)) { + throw new Error(t('files', 'This file/folder is already in that directory')) + } + + /** + * Example: + * - node: /foo/bar/file.txt -> path = /foo/bar/file.txt, destination: /foo + * Allow move of /foo does not start with /foo/bar/file.txt so allow + * - node: /foo , destination: /foo/bar + * Do not allow as it would copy foo within itself + * - node: /foo/bar.txt, destination: /foo + * Allow copy a file to the same directory + * - node: "/foo/bar", destination: "/foo/bar 1" + * Allow to move or copy but we need to check with trailing / otherwise it would report false positive + */ + if (nodes.some((node) => `${destination.path}/`.startsWith(`${node.path}/`))) { + throw new Error(t('files', 'You cannot move a file/folder onto itself or into a subfolder of itself')) + } + + const nameMapping = new Map() + // Check for conflicts if we do not want to overwrite + if (!overwrite) { + const otherNodes = (await getContents(destination.path)).contents + const conflicts = getConflicts(nodes, otherNodes) as unknown as INode[] + const nodesToRename: INode[] = [] + if (conflicts.length > 0) { + if (method === MoveCopyAction.MOVE) { + // Let the user choose what to do with the conflicting files + const content = otherNodes.filter((n) => conflicts.some((c) => c.basename === n.basename)) + const result = await openConflictPicker(destination.path, conflicts, content) + if (!result) { + // User cancelled + return + } + + nodes = nodes.filter((n) => !result.skipped.includes(n as never)) + nodesToRename.push(...(result.renamed as unknown as INode[])) + } else { + // for COPY we always rename conflicting files + nodesToRename.push(...conflicts) + } + + const usedNames = [...otherNodes, ...nodes.filter((n) => !conflicts.includes(n))].map((n) => n.basename) + for (const node of nodesToRename) { + const newName = getUniqueName(node.basename, usedNames, { ignoreFileExtension: node.type === FileType.Folder }) + nameMapping.set(node.source, newName) + usedNames.push(newName) // add the new name to avoid duplicates for following re-namimgs + } + } + } + + const actionFinished = createLoadingNotification(method, nodes.map((node) => node.basename), destination.path) + const queue = getQueue() + try { + for (const node of nodes) { + // Set loading state + Vue.set(node, 'status', NodeStatus.LOADING) + yield queue.add(async () => { + try { + const client = getClient() + + const currentPath = join(defaultRootPath, node.path) + const destinationPath = join(defaultRootPath, destination.path, nameMapping.get(node.source) ?? node.basename) + + if (method === MoveCopyAction.COPY) { + await client.copyFile(currentPath, destinationPath) + // If the node is copied into current directory the view needs to be updated + if (node.dirname === destination.path) { + const { data } = await client.stat( + destinationPath, + { + details: true, + data: getDefaultPropfind(), + }, + ) as ResponseDataDetailed + emit('files:node:created', resultToNode(data)) + } + } else { + await client.moveFile(currentPath, destinationPath) + // Delete the node as it will be fetched again + // when navigating to the destination folder + emit('files:node:deleted', node) + } + } catch (error) { + logger.debug(`Error while trying to ${method === MoveCopyAction.COPY ? 'copy' : 'move'} node`, { node, error }) + if (isAxiosError(error)) { + if (error.response?.status === 412) { + throw new HintException(t('files', 'A file or folder with that name already exists in this folder')) + } else if (error.response?.status === 423) { + throw new HintException(t('files', 'The files are locked')) + } else if (error.response?.status === 404) { + throw new HintException(t('files', 'The file does not exist anymore')) + } else if ('response' in error && error.response) { + const parser = new DOMParser() + const text = await (error as WebDAVClientError).response!.text() + const message = parser.parseFromString(text ?? '', 'text/xml') + .querySelector('message')?.textContent + if (message) { + throw new HintException(message) + } + } + } + throw error + } finally { + Vue.set(node, 'status', undefined) + } + }) + } + } finally { + actionFinished() + } +} + /** * Return the action that is possible for the given nodes * @@ -45,168 +241,25 @@ function getActionForNodes(nodes: INode[]): MoveCopyAction { * Create a loading notification toast * * @param mode The move or copy mode - * @param source Name of the node that is copied / moved + * @param sources Names of the nodes that are copied / moved * @param destination Destination path * @return Function to hide the notification */ -function createLoadingNotification(mode: MoveCopyAction, source: string, destination: string): () => void { - const text = mode === MoveCopyAction.MOVE ? t('files', 'Moving "{source}" to "{destination}" …', { source, destination }) : t('files', 'Copying "{source}" to "{destination}" …', { source, destination }) +function createLoadingNotification(mode: MoveCopyAction, sources: string[], destination: string): () => void { + const text = mode === MoveCopyAction.MOVE + ? (sources.length === 1 + ? t('files', 'Moving "{source}" to "{destination}" …', { source: sources[0], destination }) + : t('files', 'Moving {count} files to "{destination}" …', { count: sources.length, destination }) + ) + : (sources.length === 1 + ? t('files', 'Copying "{source}" to "{destination}" …', { source: sources[0], destination }) + : t('files', 'Copying {count} files to "{destination}" …', { count: sources.length, destination }) + ) - let toast: ReturnType | undefined - toast = showInfo( - ` ${text}`, - { - isHTML: true, - timeout: TOAST_PERMANENT_TIMEOUT, - onRemove() { - toast?.hideToast() - toast = undefined - }, - }, - ) + const toast = showLoading(text) return () => toast && toast.hideToast() } -/** - * Handle the copy/move of a node to a destination - * This can be imported and used by other scripts/components on server - * - * @param node The node to copy/move - * @param destination The destination to copy/move the node to - * @param method The method to use for the copy/move - * @param overwrite Whether to overwrite the destination if it exists - * @return A promise that resolves when the copy/move is done - */ -export async function handleCopyMoveNodeTo(node: INode, destination: IFolder, method: MoveCopyAction.COPY | MoveCopyAction.MOVE, overwrite = false) { - if (!destination) { - return - } - - if (destination.type !== FileType.Folder) { - throw new Error(t('files', 'Destination is not a folder')) - } - - // Do not allow to MOVE a node to the same folder it is already located - if (method === MoveCopyAction.MOVE && node.dirname === destination.path) { - throw new Error(t('files', 'This file/folder is already in that directory')) - } - - /** - * Example: - * - node: /foo/bar/file.txt -> path = /foo/bar/file.txt, destination: /foo - * Allow move of /foo does not start with /foo/bar/file.txt so allow - * - node: /foo , destination: /foo/bar - * Do not allow as it would copy foo within itself - * - node: /foo/bar.txt, destination: /foo - * Allow copy a file to the same directory - * - node: "/foo/bar", destination: "/foo/bar 1" - * Allow to move or copy but we need to check with trailing / otherwise it would report false positive - */ - if (`${destination.path}/`.startsWith(`${node.path}/`)) { - throw new Error(t('files', 'You cannot move a file/folder onto itself or into a subfolder of itself')) - } - - // Set loading state - Vue.set(node, 'status', NodeStatus.LOADING) - const actionFinished = createLoadingNotification(method, node.basename, destination.path) - - const queue = getQueue() - return await queue.add(async () => { - const copySuffix = (index: number) => { - if (index === 1) { - return t('files', '(copy)') // TRANSLATORS: Mark a file as a copy of another file - } - return t('files', '(copy %n)', undefined, index) // TRANSLATORS: Meaning it is the n'th copy of a file - } - - try { - const client = getClient() - const currentPath = join(defaultRootPath, node.path) - const destinationPath = join(defaultRootPath, destination.path) - - if (method === MoveCopyAction.COPY) { - let target = node.basename - // If we do not allow overwriting then find an unique name - if (!overwrite) { - const otherNodes = await client.getDirectoryContents(destinationPath) as FileStat[] - target = getUniqueName( - node.basename, - otherNodes.map((n) => n.basename), - { - suffix: copySuffix, - ignoreFileExtension: node.type === FileType.Folder, - }, - ) - } - await client.copyFile(currentPath, join(destinationPath, target)) - // If the node is copied into current directory the view needs to be updated - if (node.dirname === destination.path) { - const { data } = await client.stat( - join(destinationPath, target), - { - details: true, - data: getDefaultPropfind(), - }, - ) as ResponseDataDetailed - emit('files:node:created', resultToNode(data)) - } - } else { - // show conflict file popup if we do not allow overwriting - if (!overwrite) { - const otherNodes = await getContents(destination.path) - if (hasConflict([node], otherNodes.contents)) { - try { - // Let the user choose what to do with the conflicting files - const { selected, renamed } = await openConflictPicker(destination.path, [node], otherNodes.contents) - // two empty arrays: either only old files or conflict skipped -> no action required - if (!selected.length && !renamed.length) { - return - } - } catch { - // User cancelled - return - } - } - } - // getting here means either no conflict, file was renamed to keep both files - // in a conflict, or the selected file was chosen to be kept during the conflict - try { - await client.moveFile(currentPath, join(destinationPath, node.basename)) - } catch (error) { - const parser = new DOMParser() - const text = await (error as WebDAVClientError).response?.text() - const message = parser.parseFromString(text ?? '', 'text/xml') - .querySelector('message')?.textContent - if (message) { - showError(message) - } - throw error - } - // Delete the node as it will be fetched again - // when navigating to the destination folder - emit('files:node:deleted', node) - } - } catch (error) { - if (isAxiosError(error)) { - if (error.response?.status === 412) { - throw new Error(t('files', 'A file or folder with that name already exists in this folder')) - } else if (error.response?.status === 423) { - throw new Error(t('files', 'The files are locked')) - } else if (error.response?.status === 404) { - throw new Error(t('files', 'The file does not exist anymore')) - } else if (error.message) { - throw new Error(error.message) - } - } - logger.debug(error as Error) - throw new Error() - } finally { - Vue.set(node, 'status', '') - actionFinished() - } - }) -} - /** * Open a file picker for the given action * @@ -302,82 +355,3 @@ async function openFilePickerForAction( return promise } - -export const ACTION_COPY_MOVE = 'move-copy' -export const action = new FileAction({ - id: ACTION_COPY_MOVE, - displayName({ nodes }) { - switch (getActionForNodes(nodes)) { - case MoveCopyAction.MOVE: - return t('files', 'Move') - case MoveCopyAction.COPY: - return t('files', 'Copy') - case MoveCopyAction.MOVE_OR_COPY: - return t('files', 'Move or copy') - } - }, - iconSvgInline: () => FolderMoveSvg, - enabled({ nodes, view }): boolean { - // We can not copy or move in single file shares - if (view.id === 'public-file-share') { - return false - } - // We only support moving/copying files within the user folder - if (!nodes.every((node) => node.root?.startsWith('/files/'))) { - return false - } - return nodes.length > 0 && (canMove(nodes) || canCopy(nodes)) - }, - - async exec({ nodes, folder }) { - const action = getActionForNodes([nodes[0]]) - let result - try { - result = await openFilePickerForAction(action, folder.path, [nodes[0]]) - } catch (e) { - logger.error(e as Error) - return false - } - if (result === false) { - return null - } - - try { - await handleCopyMoveNodeTo(nodes[0], result.destination, result.action) - return true - } catch (error) { - if (error instanceof Error && !!error.message) { - showError(error.message) - // Silent action as we handle the toast - return null - } - return false - } - }, - - async execBatch({ nodes, folder }) { - const action = getActionForNodes(nodes) - const result = await openFilePickerForAction(action, folder.path, nodes) - // Handle cancellation silently - if (result === false) { - return nodes.map(() => null) - } - - const promises = nodes.map(async (node) => { - try { - await handleCopyMoveNodeTo(node, result.destination, result.action) - return true - } catch (error) { - logger.error(`Failed to ${result.action} node`, { node, error }) - return false - } - }) - - // We need to keep the selection on error! - // So we do not return null, and for batch action - // we let the front handle the error. - return await Promise.all(promises) - }, - - order: 15, -}) diff --git a/apps/files/src/services/DropService.ts b/apps/files/src/services/DropService.ts index 7542c57d5ff..096ae48ca1f 100644 --- a/apps/files/src/services/DropService.ts +++ b/apps/files/src/services/DropService.ts @@ -8,12 +8,10 @@ import type { Upload } from '@nextcloud/upload' import type { RootDirectory } from './DropServiceUtils.ts' import { showError, showInfo, showSuccess, showWarning } from '@nextcloud/dialogs' -import { NodeStatus } from '@nextcloud/files' import { t } from '@nextcloud/l10n' import { join } from '@nextcloud/paths' import { getUploader, hasConflict } from '@nextcloud/upload' -import Vue from 'vue' -import { handleCopyMoveNodeTo } from '../actions/moveOrCopyAction.ts' +import { handleCopyMoveNodesTo, HintException } from '../actions/moveOrCopyAction.ts' import { MoveCopyAction } from '../actions/moveOrCopyActionUtils.ts' import logger from '../logger.ts' import { createDirectoryIfNotExists, Directory, resolveConflict, traverseTree } from './DropServiceUtils.ts' @@ -178,8 +176,6 @@ export async function onDropExternalFiles(root: RootDirectory, destination: IFol * @param isCopy - Whether the operation is a copy */ export async function onDropInternalFiles(nodes: INode[], destination: IFolder, contents: INode[], isCopy = false) { - const queue = [] as Promise[] - // Check for conflicts on root elements if (await hasConflict(nodes, contents)) { nodes = await resolveConflict(nodes, destination, contents) @@ -191,23 +187,17 @@ export async function onDropInternalFiles(nodes: INode[], destination: IFolder, return } - for (const node of nodes) { - Vue.set(node, 'status', NodeStatus.LOADING) - queue.push(handleCopyMoveNodeTo(node, destination, isCopy ? MoveCopyAction.COPY : MoveCopyAction.MOVE, true)) + try { + const promises = Array.fromAsync(handleCopyMoveNodesTo(nodes, destination, isCopy ? MoveCopyAction.COPY : MoveCopyAction.MOVE)) + await promises + logger.debug('Files copy/move successful') + showSuccess(isCopy ? t('files', 'Files copied successfully') : t('files', 'Files moved successfully')) + } catch (error) { + logger.error('Error while processing dropped files', { error }) + if (error instanceof HintException) { + showError(error.message) + } else { + showError(isCopy ? t('files', 'Some files could not be copied') : t('files', 'Some files could not be moved')) + } } - - // Wait for all promises to settle - const results = await Promise.allSettled(queue) - nodes.forEach((node) => Vue.set(node, 'status', undefined)) - - // Check for errors - const errors = results.filter((result) => result.status === 'rejected') - if (errors.length > 0) { - logger.error('Error while copying or moving files', { errors }) - showError(isCopy ? t('files', 'Some files could not be copied') : t('files', 'Some files could not be moved')) - return - } - - logger.debug('Files copy/move successful') - showSuccess(isCopy ? t('files', 'Files copied successfully') : t('files', 'Files moved successfully')) } diff --git a/cypress/e2e/files/files-copy-move.cy.ts b/cypress/e2e/files/files-copy-move.cy.ts index 2a688e47785..abd0b9598cb 100644 --- a/cypress/e2e/files/files-copy-move.cy.ts +++ b/cypress/e2e/files/files-copy-move.cy.ts @@ -107,23 +107,23 @@ describe('Files: Move or copy files', { testIsolation: true }, () => { copyFile('original.txt', '.') getRowForFile('original.txt').should('be.visible') - getRowForFile('original (copy).txt').should('be.visible') + getRowForFile('original (1).txt').should('be.visible') }) it('Can copy a file multiple times to same folder', () => { cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original.txt') - cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original (copy).txt') + cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original (1).txt') cy.login(currentUser) cy.visit('/apps/files') copyFile('original.txt', '.') getRowForFile('original.txt').should('be.visible') - getRowForFile('original (copy 2).txt').should('be.visible') + getRowForFile('original (2).txt').should('be.visible') }) /** - * Test that a copied folder with a dot will be renamed correctly ('foo.bar' -> 'foo.bar (copy)') + * Test that a copied folder with a dot will be renamed correctly ('foo.bar' -> 'foo.bar (1)') * Test for: https://github.com/nextcloud/server/issues/43843 */ it('Can copy a folder to same folder', () => { @@ -134,7 +134,7 @@ describe('Files: Move or copy files', { testIsolation: true }, () => { copyFile('foo.bar', '.') getRowForFile('foo.bar').should('be.visible') - getRowForFile('foo.bar (copy)').should('be.visible') + getRowForFile('foo.bar (1)').should('be.visible') }) /** Test for https://github.com/nextcloud/server/issues/43329 */ diff --git a/cypress/e2e/files/live_photos.cy.ts b/cypress/e2e/files/live_photos.cy.ts index b8514de4bc1..2e7556f676b 100644 --- a/cypress/e2e/files/live_photos.cy.ts +++ b/cypress/e2e/files/live_photos.cy.ts @@ -56,8 +56,8 @@ describe('Files: Live photos', { testIsolation: true }, () => { getRowForFile(`${randomFileName}.jpg`).should('have.length', 1) getRowForFile(`${randomFileName}.mov`).should('have.length', 1) - getRowForFile(`${randomFileName} (copy).jpg`).should('have.length', 1) - getRowForFile(`${randomFileName} (copy).mov`).should('have.length', 1) + getRowForFile(`${randomFileName} (1).jpg`).should('have.length', 1) + getRowForFile(`${randomFileName} (1).mov`).should('have.length', 1) }) it('Copies both files when copying the .mov', () => { @@ -65,15 +65,15 @@ describe('Files: Live photos', { testIsolation: true }, () => { reloadCurrentFolder() getRowForFile(`${randomFileName}.mov`).should('have.length', 1) - getRowForFile(`${randomFileName} (copy).jpg`).should('have.length', 1) - getRowForFile(`${randomFileName} (copy).mov`).should('have.length', 1) + getRowForFile(`${randomFileName} (1).jpg`).should('have.length', 1) + getRowForFile(`${randomFileName} (1).mov`).should('have.length', 1) }) it('Keeps live photo link when copying folder', () => { createFolder('folder') moveFile(`${randomFileName}.jpg`, 'folder') copyFile('folder', '.') - navigateToFolder('folder (copy)') + navigateToFolder('folder (1)') getRowForFile(`${randomFileName}.jpg`).should('have.length', 1) getRowForFile(`${randomFileName}.mov`).should('have.length', 1) @@ -95,7 +95,7 @@ describe('Files: Live photos', { testIsolation: true }, () => { cy.get('[data-cy-files-list-row-fileid]').should('have.length', 1) getRowForFile(`${randomFileName}.mov`).should('have.length', 1) getRowForFile(`${randomFileName}.jpg`).should('have.length', 0) - getRowForFile(`${randomFileName} (copy).jpg`).should('have.length', 0) + getRowForFile(`${randomFileName} (1).jpg`).should('have.length', 0) }) it('Moves files when moving the .jpg', () => {