[stable32] fix(files): improve handling of copy-move action

apps/files/src/actions/moveOrCopyAction.ts

@@ -8,7 +8,7 @@
 import type { MoveCopyResult } from './moveOrCopyActionUtils'
 
 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, NodeStatus, davGetClient, davRootPath, davResultToNode, davGetDefaultPropfind, getUniqueName, Permission } from '@nextcloud/files'
 import { translate as t } from '@nextcloud/l10n'
@@ -23,6 +23,202 @@
 import { getContents } from '../services/Files'
 import logger from '../logger'
 
+/**
+ * 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<void, void, never>} 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<void, void, never> {
+	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<string, string>()
+	// 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<FileStat>
+							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
  * @param {Node[]} nodes The nodes to check against
@@ -40,10 +236,10 @@
 	return MoveCopyAction.COPY
 }
 
 /**
  * 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 {() => void} Function to hide the notification
  */

apps/files/src/services/DropService.ts

@@ -185,9 +185,18 @@ export const onDropInternalFiles = async (nodes: Node[], destination: Folder, co
 		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

cypress/e2e/files/files-copy-move.cy.ts

@@ -108,23 +108,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', () => {
@@ -135,7 +135,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 */

cypress/e2e/files/live_photos.cy.ts

@@ -55,24 +55,24 @@ 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', () => {
 			copyFile(`${randomFileName}.mov`, '.')
 			clickOnBreadcrumbs('All files')
 
 			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)
@@ -94,7 +94,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', () => {