fix(files): always ask for confirmation if trashbin app is disabled

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>

4 files changed, 353 insertions, 102 deletions

diff --git a/apps/files/src/actions/deleteAction.spec.ts b/apps/files/src/actions/deleteAction.spec.ts
index 936079f8597..328e05afd79 100644
--- a/apps/files/src/actions/deleteAction.spec.ts
+++ b/apps/files/src/actions/deleteAction.spec.ts
@@ -22,8 +22,9 @@
 import { action } from './deleteAction'
 import { expect } from '@jest/globals'
 import { File, Folder, Permission, View, FileAction } from '@nextcloud/files'
-import eventBus from '@nextcloud/event-bus'
+import * as capabilities from '@nextcloud/capabilities'
 import axios from '@nextcloud/axios'
+import eventBus from '@nextcloud/event-bus'
 
 import logger from '../logger'
 
@@ -111,6 +112,16 @@ describe('Delete action conditions tests', () => {
 		expect(action.displayName([file], trashbinView)).toBe('Delete permanently')
 	})
 
+	test('Trashbin disabled displayName', () => {
+		jest.spyOn(capabilities, 'getCapabilities').mockImplementation(() => {
+			return {
+				files: {},
+			}
+		})
+		expect(action.displayName([file], view)).toBe('Delete permanently')
+		expect(capabilities.getCapabilities).toBeCalledTimes(1)
+	})
+
 	test('Shared root node displayName', () => {
 		expect(action.displayName([file2], view)).toBe('Leave this share')
 		expect(action.displayName([folder2], view)).toBe('Leave this share')
@@ -181,6 +192,9 @@ describe('Delete action enabled tests', () => {
 })
 
 describe('Delete action execute tests', () => {
+	afterEach(() => {
+		jest.restoreAllMocks()
+	})
 	test('Delete action', async () => {
 		jest.spyOn(axios, 'delete')
 		jest.spyOn(eventBus, 'emit')
@@ -235,9 +249,123 @@ describe('Delete action execute tests', () => {
 		expect(eventBus.emit).toHaveBeenNthCalledWith(2, 'files:node:deleted', file2)
 	})
 
+	test('Delete action batch large set', async () => {
+		jest.spyOn(axios, 'delete')
+		jest.spyOn(eventBus, 'emit')
+
+		// Emulate the confirmation dialog to always confirm
+		const confirmMock = jest.fn().mockImplementation((a, b, c, resolve) => resolve(true))
+		window.OC = { dialogs: { confirmDestructive: confirmMock } }
+
+		const file1 = new File({
+			id: 1,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foo.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const file2 = new File({
+			id: 2,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/bar.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const file3 = new File({
+			id: 3,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/baz.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const file4 = new File({
+			id: 4,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/qux.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const file5 = new File({
+			id: 5,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/quux.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const exec = await action.execBatch!([file1, file2, file3, file4, file5], view, '/')
+
+		// Enough nodes to trigger a confirmation dialog
+		expect(confirmMock).toBeCalledTimes(1)
+
+		expect(exec).toStrictEqual([true, true, true, true, true])
+		expect(axios.delete).toBeCalledTimes(5)
+		expect(axios.delete).toHaveBeenNthCalledWith(1, 'https://cloud.domain.com/remote.php/dav/files/test/foo.txt')
+		expect(axios.delete).toHaveBeenNthCalledWith(2, 'https://cloud.domain.com/remote.php/dav/files/test/bar.txt')
+		expect(axios.delete).toHaveBeenNthCalledWith(3, 'https://cloud.domain.com/remote.php/dav/files/test/baz.txt')
+		expect(axios.delete).toHaveBeenNthCalledWith(4, 'https://cloud.domain.com/remote.php/dav/files/test/qux.txt')
+		expect(axios.delete).toHaveBeenNthCalledWith(5, 'https://cloud.domain.com/remote.php/dav/files/test/quux.txt')
+
+		expect(eventBus.emit).toBeCalledTimes(5)
+		expect(eventBus.emit).toHaveBeenNthCalledWith(1, 'files:node:deleted', file1)
+		expect(eventBus.emit).toHaveBeenNthCalledWith(2, 'files:node:deleted', file2)
+		expect(eventBus.emit).toHaveBeenNthCalledWith(3, 'files:node:deleted', file3)
+		expect(eventBus.emit).toHaveBeenNthCalledWith(4, 'files:node:deleted', file4)
+		expect(eventBus.emit).toHaveBeenNthCalledWith(5, 'files:node:deleted', file5)
+	})
+
+	test('Delete action batch trashbin disabled', async () => {
+		jest.spyOn(axios, 'delete')
+		jest.spyOn(eventBus, 'emit')
+		jest.spyOn(capabilities, 'getCapabilities').mockImplementation(() => {
+			return {
+				files: {},
+			}
+		})
+
+		// Emulate the confirmation dialog to always confirm
+		const confirmMock = jest.fn().mockImplementation((a, b, c, resolve) => resolve(true))
+		window.OC = { dialogs: { confirmDestructive: confirmMock } }
+
+		const file1 = new File({
+			id: 1,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foo.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const file2 = new File({
+			id: 2,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/bar.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const exec = await action.execBatch!([file1, file2], view, '/')
+
+		// Will trigger a confirmation dialog because trashbin app is disabled
+		expect(confirmMock).toBeCalledTimes(1)
+
+		expect(exec).toStrictEqual([true, true])
+		expect(axios.delete).toBeCalledTimes(2)
+		expect(axios.delete).toHaveBeenNthCalledWith(1, 'https://cloud.domain.com/remote.php/dav/files/test/foo.txt')
+		expect(axios.delete).toHaveBeenNthCalledWith(2, 'https://cloud.domain.com/remote.php/dav/files/test/bar.txt')
+
+		expect(eventBus.emit).toBeCalledTimes(2)
+		expect(eventBus.emit).toHaveBeenNthCalledWith(1, 'files:node:deleted', file1)
+		expect(eventBus.emit).toHaveBeenNthCalledWith(2, 'files:node:deleted', file2)
+	})
+
 	test('Delete fails', async () => {
 		jest.spyOn(axios, 'delete').mockImplementation(() => { throw new Error('Mock error') })
 		jest.spyOn(logger, 'error').mockImplementation(() => jest.fn())
+		jest.spyOn(eventBus, 'emit')
 
 		const file = new File({
 			id: 1,
@@ -256,4 +384,35 @@ describe('Delete action execute tests', () => {
 		expect(eventBus.emit).toBeCalledTimes(0)
 		expect(logger.error).toBeCalledTimes(1)
 	})
+
+	test('Delete is cancelled', async () => {
+		jest.spyOn(axios, 'delete')
+		jest.spyOn(eventBus, 'emit')
+		jest.spyOn(capabilities, 'getCapabilities').mockImplementation(() => {
+			return {
+				files: {},
+			}
+		})
+
+		// Emulate the confirmation dialog to always confirm
+		const confirmMock = jest.fn().mockImplementation((a, b, c, resolve) => resolve(false))
+		window.OC = { dialogs: { confirmDestructive: confirmMock } }
+
+		const file1 = new File({
+			id: 1,
+			source: 'https://cloud.domain.com/remote.php/dav/files/test/foo.txt',
+			owner: 'test',
+			mime: 'text/plain',
+			permissions: Permission.READ | Permission.UPDATE | Permission.DELETE,
+		})
+
+		const exec = await action.execBatch!([file1], view, '/')
+
+		expect(confirmMock).toBeCalledTimes(1)
+
+		expect(exec).toStrictEqual([null])
+		expect(axios.delete).toBeCalledTimes(0)
+
+		expect(eventBus.emit).toBeCalledTimes(0)
+	})
 })
diff --git a/apps/files/src/actions/deleteAction.ts b/apps/files/src/actions/deleteAction.ts
index 3453ad0e67d..872836ef324 100644
--- a/apps/files/src/actions/deleteAction.ts
+++ b/apps/files/src/actions/deleteAction.ts
@@ -19,109 +19,23 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
-import { emit } from '@nextcloud/event-bus'
-import { Permission, Node, View, FileAction, FileType } from '@nextcloud/files'
-import { translate as t, translatePlural as n } from '@nextcloud/l10n'
-import axios from '@nextcloud/axios'
+import { Permission, Node, View, FileAction } from '@nextcloud/files'
+import { showInfo } from '@nextcloud/dialogs'
+import { translate as t } from '@nextcloud/l10n'
+import PQueue from 'p-queue'
 
 import CloseSvg from '@mdi/svg/svg/close.svg?raw'
 import NetworkOffSvg from '@mdi/svg/svg/network-off.svg?raw'
 import TrashCanSvg from '@mdi/svg/svg/trash-can.svg?raw'
 
 import logger from '../logger.js'
-import PQueue from 'p-queue'
-
-const canUnshareOnly = (nodes: Node[]) => {
-	return nodes.every(node => node.attributes['is-mount-root'] === true
-		&& node.attributes['mount-type'] === 'shared')
-}
-
-const canDisconnectOnly = (nodes: Node[]) => {
-	return nodes.every(node => node.attributes['is-mount-root'] === true
-		&& node.attributes['mount-type'] === 'external')
-}
-
-const isMixedUnshareAndDelete = (nodes: Node[]) => {
-	if (nodes.length === 1) {
-		return false
-	}
-
-	const hasSharedItems = nodes.some(node => canUnshareOnly([node]))
-	const hasDeleteItems = nodes.some(node => !canUnshareOnly([node]))
-	return hasSharedItems && hasDeleteItems
-}
-
-const isAllFiles = (nodes: Node[]) => {
-	return !nodes.some(node => node.type !== FileType.File)
-}
-
-const isAllFolders = (nodes: Node[]) => {
-	return !nodes.some(node => node.type !== FileType.Folder)
-}
+import { askConfirmation, canDisconnectOnly, canUnshareOnly, deleteNode, displayName, isTrashbinEnabled } from './deleteUtils'
 
 const queue = new PQueue({ concurrency: 5 })
 
 export const action = new FileAction({
 	id: 'delete',
-	displayName(nodes: Node[], view: View) {
-		/**
-		 * If we're in the trashbin, we can only delete permanently
-		 */
-		if (view.id === 'trashbin') {
-			return t('files', 'Delete permanently')
-		}
-
-		/**
-		 * If we're in the sharing view, we can only unshare
-		 */
-		if (isMixedUnshareAndDelete(nodes)) {
-			return t('files', 'Delete and unshare')
-		}
-
-		/**
-		 * If those nodes are all the root node of a
-		 * share, we can only unshare them.
-		 */
-		if (canUnshareOnly(nodes)) {
-			if (nodes.length === 1) {
-				return t('files', 'Leave this share')
-			}
-			return t('files', 'Leave these shares')
-		}
-
-		/**
-		 * If those nodes are all the root node of an
-		 * external storage, we can only disconnect it.
-		 */
-		if (canDisconnectOnly(nodes)) {
-			if (nodes.length === 1) {
-				return t('files', 'Disconnect storage')
-			}
-			return t('files', 'Disconnect storages')
-		}
-
-		/**
-		 * If we're only selecting files, use proper wording
-		 */
-		if (isAllFiles(nodes)) {
-			if (nodes.length === 1) {
-				return t('files', 'Delete file')
-			}
-			return t('files', 'Delete files')
-		}
-
-		/**
-		 * If we're only selecting folders, use proper wording
-		 */
-		if (isAllFolders(nodes)) {
-			if (nodes.length === 1) {
-				return t('files', 'Delete folder')
-			}
-			return t('files', 'Delete folders')
-		}
-
-		return t('files', 'Delete')
-	},
+	displayName,
 	iconSvgInline: (nodes: Node[]) => {
 		if (canUnshareOnly(nodes)) {
 			return CloseSvg
@@ -140,14 +54,22 @@ export const action = new FileAction({
 			.every(permission => (permission & Permission.DELETE) !== 0)
 	},
 
-	async exec(node: Node, view: View, dir: string) {
+	async exec(node: Node, view: View) {
 		try {
-			await axios.delete(node.encodedSource)
+			let confirm = true
+
+			// If trashbin is disabled, we need to ask for confirmation
+			if (!isTrashbinEnabled()) {
+				confirm = await askConfirmation([node], view)
+			}
+
+			// If the user cancels the deletion, we don't want to do anything
+			if (confirm === false) {
+				showInfo(t('files', 'Deletion cancelled'))
+				return null
+			}
 
-			// Let's delete even if it's moved to the trashbin
-			// since it has been removed from the current view
-			// and changing the view will trigger a reload anyway.
-			emit('files:node:deleted', node)
+			await deleteNode(node)
 
 			return true
 		} catch (error) {
@@ -155,14 +77,35 @@ export const action = new FileAction({
 			return false
 		}
 	},
-	async execBatch(nodes: Node[], view: View, dir: string) {
+
+	async execBatch(nodes: Node[], view: View): Promise<(boolean | null)[]> {
+		let confirm = true
+
+		// If trashbin is disabled, we need to ask for confirmation
+		if (!isTrashbinEnabled()) {
+			confirm = await askConfirmation(nodes, view)
+		} else if (nodes.length >= 5 && !canUnshareOnly(nodes) && !canDisconnectOnly(nodes)) {
+			confirm = await askConfirmation(nodes, view)
+		}
+
+		// If the user cancels the deletion, we don't want to do anything
+		if (confirm === false) {
+			showInfo(t('files', 'Deletion cancelled'))
+			return Promise.all(nodes.map(() => null))
+		}
+
 		// Map each node to a promise that resolves with the result of exec(node)
 		const promises = nodes.map(node => {
 		    // Create a promise that resolves with the result of exec(node)
 		    const promise = new Promise<boolean>(resolve => {
 				queue.add(async () => {
-					const result = await this.exec(node, view, dir)
-					resolve(result !== null ? result : false)
+					try {
+						await deleteNode(node)
+						resolve(true)
+					} catch (error) {
+						logger.error('Error while deleting a file', { error, source: node.source, node })
+						resolve(false)
+					}
 				})
 			})
 			return promise
diff --git a/apps/files/src/actions/deleteUtils.ts b/apps/files/src/actions/deleteUtils.ts
new file mode 100644
index 00000000000..b781be0ff16
--- /dev/null
+++ b/apps/files/src/actions/deleteUtils.ts
@@ -0,0 +1,134 @@
+/**
+ * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+import type { Capabilities } from '../types'
+import type { Node, View } from '@nextcloud/files'
+
+import { emit } from '@nextcloud/event-bus'
+import { FileType } from '@nextcloud/files'
+import { getCapabilities } from '@nextcloud/capabilities'
+import { n, t } from '@nextcloud/l10n'
+import axios from '@nextcloud/axios'
+
+export const isTrashbinEnabled = () => (getCapabilities() as Capabilities)?.files?.undelete === true
+
+export const canUnshareOnly = (nodes: Node[]) => {
+	return nodes.every(node => node.attributes['is-mount-root'] === true
+		&& node.attributes['mount-type'] === 'shared')
+}
+
+export const canDisconnectOnly = (nodes: Node[]) => {
+	return nodes.every(node => node.attributes['is-mount-root'] === true
+		&& node.attributes['mount-type'] === 'external')
+}
+
+export const isMixedUnshareAndDelete = (nodes: Node[]) => {
+	if (nodes.length === 1) {
+		return false
+	}
+
+	const hasSharedItems = nodes.some(node => canUnshareOnly([node]))
+	const hasDeleteItems = nodes.some(node => !canUnshareOnly([node]))
+	return hasSharedItems && hasDeleteItems
+}
+
+export const isAllFiles = (nodes: Node[]) => {
+	return !nodes.some(node => node.type !== FileType.File)
+}
+
+export const isAllFolders = (nodes: Node[]) => {
+	return !nodes.some(node => node.type !== FileType.Folder)
+}
+
+export const displayName = (nodes: Node[], view: View) => {
+	/**
+	 * If we're in the trashbin, we can only delete permanently
+	 */
+	if (view.id === 'trashbin' || !isTrashbinEnabled()) {
+		return t('files', 'Delete permanently')
+	}
+
+	/**
+	 * If we're in the sharing view, we can only unshare
+	 */
+	if (isMixedUnshareAndDelete(nodes)) {
+		return t('files', 'Delete and unshare')
+	}
+
+	/**
+	 * If those nodes are all the root node of a
+	 * share, we can only unshare them.
+	 */
+	if (canUnshareOnly(nodes)) {
+		if (nodes.length === 1) {
+			return t('files', 'Leave this share')
+		}
+		return t('files', 'Leave these shares')
+	}
+
+	/**
+	 * If those nodes are all the root node of an
+	 * external storage, we can only disconnect it.
+	 */
+	if (canDisconnectOnly(nodes)) {
+		if (nodes.length === 1) {
+			return t('files', 'Disconnect storage')
+		}
+		return t('files', 'Disconnect storages')
+	}
+
+	/**
+	 * If we're only selecting files, use proper wording
+	 */
+	if (isAllFiles(nodes)) {
+		if (nodes.length === 1) {
+			return t('files', 'Delete file')
+		}
+		return t('files', 'Delete files')
+	}
+
+	/**
+	 * If we're only selecting folders, use proper wording
+	 */
+	if (isAllFolders(nodes)) {
+		if (nodes.length === 1) {
+			return t('files', 'Delete folder')
+		}
+		return t('files', 'Delete folders')
+	}
+
+	return t('files', 'Delete')
+}
+
+export const askConfirmation = async (nodes: Node[], view: View) => {
+	const message = view.id === 'trashbin' || !isTrashbinEnabled()
+		? n('files', 'You are about to permanently delete {count} item', 'You are about to permanently delete {count} items', nodes.length, { count: nodes.length })
+		: n('files', 'You are about to delete {count} item', 'You are about to delete {count} items', nodes.length, { count: nodes.length })
+
+	return new Promise<boolean>(resolve => {
+		// TODO: Use the new dialog API
+		window.OC.dialogs.confirmDestructive(
+			message,
+			t('files', 'Confirm deletion'),
+			{
+				type: window.OC.dialogs.YES_NO_BUTTONS,
+				confirm: displayName(nodes, view),
+				confirmClasses: 'error',
+				cancel: t('files', 'Cancel'),
+			},
+			(decision: boolean) => {
+				resolve(decision)
+			},
+		)
+	})
+}
+
+export const deleteNode = async (node: Node) => {
+	await axios.delete(node.encodedSource)
+
+	// Let's delete even if it's moved to the trashbin
+	// since it has been removed from the current view
+	// and changing the view will trigger a reload anyway.
+	emit('files:node:deleted', node)
+}
diff --git a/apps/files/src/types.ts b/apps/files/src/types.ts
index 1ee0d31a33f..e066b522556 100644
--- a/apps/files/src/types.ts
+++ b/apps/files/src/types.ts
@@ -121,3 +121,18 @@ export interface TemplateFile {
 	ratio?: number
 	templates?: Record<string, unknown>[]
 }
+
+export type Capabilities = {
+	files: {
+		bigfilechunking: boolean
+		blacklisted_files: string[]
+		forbidden_filename_basenames: string[]
+		forbidden_filename_characters: string[]
+		forbidden_filename_extensions: string[]
+		forbidden_filenames: string[]
+		undelete: boolean
+		version_deletion: boolean
+		version_labeling: boolean
+		versioning: boolean
+	}
+}