Add better error message on users side for failed approval

lib/Controller/ApprovalController.php

@@ -8,12 +8,14 @@
 namespace OCA\Approval\Controller;
 
 use OCA\Approval\AppInfo\Application;
+use OCA\Approval\Exceptions\OutdatedEtagException;
 use OCA\Approval\Service\ApprovalService;
 use OCA\Approval\Service\RuleService;
-
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
+use OCP\IL10N;
 use OCP\IRequest;
 
 class ApprovalController extends OCSController {
@@ -23,6 +25,7 @@ public function __construct(
 		IRequest $request,
 		private ApprovalService $approvalService,
 		private RuleService $ruleService,
+		private IL10N $l10n,
 		private ?string $userId,
 	) {
 		parent::__construct($appName, $request);
@@ -76,25 +79,39 @@ public function getPendingNodes(?int $since = null): DataResponse {
 	 *
 	 * @param int $fileId
 	 * @param string|null $message
+	 * @param string|null $etag
 	 * @return DataResponse
 	 */
 	#[NoAdminRequired]
-	public function approve(int $fileId, ?string $message = ''): DataResponse {
-		$this->approvalService->approve($fileId, $this->userId, $message);
-		return new DataResponse(1);
+	public function approve(int $fileId, ?string $message = '', ?string $etag = ''): DataResponse {
+		try {
+			if ($this->approvalService->approve($fileId, $this->userId, $message, $etag)) {
+				return new DataResponse([]);
+			}
+			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		} catch (OutdatedEtagException) {
+			return new DataResponse(['error' => $this->l10n->t('The file/folder you tried to approve has an outdated content, please reload and review it again')], Http::STATUS_BAD_REQUEST);
+		}
 	}
 
 	/**
 	 * Reject a file
 	 *
 	 * @param int $fileId
 	 * @param string|null $message
+	 * @param string|null $etag
 	 * @return DataResponse
 	 */
 	#[NoAdminRequired]
-	public function reject(int $fileId, ?string $message = ''): DataResponse {
-		$this->approvalService->reject($fileId, $this->userId, $message);
-		return new DataResponse(1);
+	public function reject(int $fileId, ?string $message = '', ?string $etag = ''): DataResponse {
+		try {
+			if ($this->approvalService->reject($fileId, $this->userId, $message, $etag)) {
+				return new DataResponse([]);
+			}
+			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		} catch (OutdatedEtagException) {
+			return new DataResponse(['error' => $this->l10n->t('The file/folder you tried to reject has an outdated content, please reload and review it again')], Http::STATUS_BAD_REQUEST);
+		}
 	}
 
 	/**

lib/Exceptions/OutdatedEtagException.php

@@ -0,0 +1,13 @@
+<?php
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Approval\Exceptions;
+
+use Exception;
+
+class OutdatedEtagException extends Exception {
+}

lib/Service/ApprovalService.php

@@ -10,6 +10,7 @@
 use DateTime;
 use OCA\Approval\Activity\ActivityManager;
 use OCA\Approval\AppInfo\Application;
+use OCA\Approval\Exceptions\OutdatedEtagException;
 use OCA\DAV\Connector\Sabre\Node as SabreNode;
 use OCP\App\IAppManager;
 use OCP\Files\FileInfo;
@@ -19,15 +20,12 @@
 use OCP\IL10N;
 use OCP\IUser;
 use OCP\IUserManager;
-
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Share\IManager as IShareManager;
-
 use OCP\Share\IShare;
 use OCP\SystemTag\ISystemTagObjectMapper;
 use OCP\SystemTag\TagNotFoundException;
 use Psr\Log\LoggerInterface;
-
 use Sabre\DAV\INode;
 use Sabre\DAV\PropFind;
 
@@ -270,6 +268,18 @@ public function getPendingNodes(string $userId, ?int $since = null): array {
 		return $result;
 	}
 
+	/**
+	 * @param int $fileId
+	 * @return string
+	 */
+	public function getEtag(int $fileId): string {
+		$file = $this->root->getFirstNodeById($fileId);
+		if ($file !== null) {
+			return $file->getEtag();
+		}
+		return '';
+	}
+
 	/**
 	 * Get approval state of a given file for a given user
 	 * @param int $fileId
@@ -339,10 +349,15 @@ public function getApprovalState(int $fileId, ?string $userId, bool $userHasAcce
 	 * @param int $fileId
 	 * @param string|null $userId
 	 * @param string $message
+	 * @param string $etag optional etag of the file to check if it has changed since approval was requested
 	 * @return bool success
+	 * @throws OutdatedEtagException
 	 */
-	public function approve(int $fileId, ?string $userId, string $message = ''): bool {
+	public function approve(int $fileId, ?string $userId, string $message = '', string $etag = ''): bool {
 		$fileState = $this->getApprovalState($fileId, $userId);
+		if ($etag !== '' && $etag !== $this->getEtag($fileId)) {
+			throw new OutdatedEtagException();
+		}
 		// if file has pending tag and user is authorized to approve it
 		if ($fileState['state'] === Application::STATE_APPROVABLE) {
 			$rules = $this->ruleService->getRules();
@@ -377,10 +392,15 @@ public function approve(int $fileId, ?string $userId, string $message = ''): boo
 	 * @param int $fileId
 	 * @param string|null $userId
 	 * @param string $message
+	 * @param string $etag optional etag of the file to check if it has changed since approval was requested
 	 * @return bool success
+	 * @throws OutdatedEtagException
 	 */
-	public function reject(int $fileId, ?string $userId, string $message = ''): bool {
+	public function reject(int $fileId, ?string $userId, string $message = '', string $etag = ''): bool {
 		$fileState = $this->getApprovalState($fileId, $userId);
+		if ($etag !== '' && $etag !== $this->getEtag($fileId)) {
+			throw new OutdatedEtagException();
+		}
 		// if file has pending tag and user is authorized to approve it
 		if ($fileState['state'] === Application::STATE_APPROVABLE) {
 			$rules = $this->ruleService->getRules();

src/files/actions/approveAction.js

@@ -26,7 +26,7 @@ export const approveAction = new FileAction({
 	async exec({ nodes }) {
 		const node = nodes[0]
 		try {
-			await approve(node.fileid, node.basename, node)
+			await approve(node)
 		} catch (error) {
 			console.debug('Approve action failed')
 		}
@@ -35,7 +35,7 @@ export const approveAction = new FileAction({
 	async execBatch({ nodes }) {
 		const promises = nodes
 			.filter(node => node.attributes['approval-state'] === states.APPROVABLE)
-			.map(node => approve(node.fileid, node.basename, node, false))
+			.map(node => approve(node, false))
 		const results = await Promise.allSettled(promises)
 		return results.map(promise => promise.status === 'fulfilled')
 	},

src/files/actions/rejectAction.js

@@ -26,7 +26,7 @@ export const rejectAction = new FileAction({
 	async exec({ nodes }) {
 		const node = nodes[0]
 		try {
-			await reject(node.fileid, node.basename, node)
+			await reject(node)
 		} catch (error) {
 			console.debug('Reject action failed')
 		}
@@ -35,7 +35,7 @@ export const rejectAction = new FileAction({
 	async execBatch({ nodes }) {
 		const promises = nodes
 			.filter(node => node.attributes['approval-state'] === states.APPROVABLE)
-			.map(node => reject(node.fileid, node.basename, node, false))
+			.map(node => reject(node, false))
 		const results = await Promise.allSettled(promises)
 		return results.map(promise => promise.status === 'fulfilled')
 	},

src/files/helpers.js

@@ -83,39 +83,35 @@ export async function requestAfterShareCreation(fileId, fileName, ruleId, node =
 	}
 }
 
-export async function approve(fileId, fileName, node = null, notify = true, message = '') {
-	const url = generateOcsUrl('apps/approval/api/v1/approve/{fileId}', { fileId })
+export async function approve(node, notify = true, message = '') {
+	const url = generateOcsUrl('apps/approval/api/v1/approve/{fileId}', { fileId: node.fileid })
 	try {
-		await axios.put(url, { message })
+		await axios.put(url, { message, etag: node.attributes.etag })
 		if (notify) {
-			showSuccess(t('approval', 'You approved {name}', { name: fileName }))
-		}
-		if (node) {
-			await updateNodeApprovalState(node)
+			showSuccess(t('approval', 'You approved {name}', { name: node.basename }))
 		}
+		await updateNodeApprovalState(node)
 	} catch (error) {
 		console.error(error)
 		if (notify) {
-			showError(t('approval', 'Failed to approve {name}', { name: fileName }))
+			showError(error.response.data?.ocs?.data?.error ?? t('approval', 'Failed to approve {name}', { name: node.basename }))
 		}
 		throw error
 	}
 }
 
-export async function reject(fileId, fileName, node = null, notify = true, message = '') {
-	const url = generateOcsUrl('apps/approval/api/v1/reject/{fileId}', { fileId })
+export async function reject(node, notify = true, message = '') {
+	const url = generateOcsUrl('apps/approval/api/v1/reject/{fileId}', { fileId: node.fileid })
 	try {
-		await axios.put(url, { message })
+		await axios.put(url, { message, etag: node.attributes.etag })
 		if (notify) {
-			showSuccess(t('approval', 'You rejected {name}', { name: fileName }))
-		}
-		if (node) {
-			await updateNodeApprovalState(node)
+			showSuccess(t('approval', 'You rejected {name}', { name: node.basename }))
 		}
+		await updateNodeApprovalState(node)
 	} catch (error) {
 		console.error(error)
 		if (notify) {
-			showError(t('approval', 'Failed to reject {name}', { name: fileName }))
+			showError(error.response.data?.ocs?.data?.error ?? t('approval', 'Failed to reject {name}', { name: node.basename }))
 		}
 		throw error
 	}

src/files/modals.js

@@ -39,10 +39,10 @@ export function createInfoModal() {
 			console.debug('[Approval] modal closed')
 		},
 		onApprove: (node, message) => {
-			approve(node.fileid, node.basename, node, true, message)
+			approve(node, true, message)
 		},
 		onReject: (node, message) => {
-			reject(node.fileid, node.basename, node, true, message)
+			reject(node, true, message)
 		},
 		onRequest: (node) => {
 			onRequestFileAction(node)

src/views/ApprovalTab.vue

@@ -113,12 +113,12 @@ export default {
 		},
 		async onApprove(message) {
 			this.state = null
-			await approve(this.fileId, this.fileName, null, true, message)
+			await approve(this.node, true, message)
 			this.update()
 		},
 		async onReject(message) {
 			this.state = null
-			await reject(this.fileId, this.fileName, null, true, message)
+			await reject(this.node, true, message)
 			this.update()
 		},
 		async onRequestSubmit(ruleId, createShares) {