diff --git a/lib/Controller/ApprovalController.php b/lib/Controller/ApprovalController.php index a696d91d..944d4fed 100644 --- a/lib/Controller/ApprovalController.php +++ b/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,12 +79,19 @@ 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); + } } /** @@ -89,12 +99,19 @@ public function approve(int $fileId, ?string $message = ''): DataResponse { * * @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); + } } /** diff --git a/lib/Exceptions/OutdatedEtagException.php b/lib/Exceptions/OutdatedEtagException.php new file mode 100644 index 00000000..aa3523de --- /dev/null +++ b/lib/Exceptions/OutdatedEtagException.php @@ -0,0 +1,13 @@ +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(); diff --git a/src/files/actions/approveAction.js b/src/files/actions/approveAction.js index d0c5f712..bf707d98 100644 --- a/src/files/actions/approveAction.js +++ b/src/files/actions/approveAction.js @@ -25,7 +25,7 @@ export const approveAction = new FileAction({ order: 0, async exec(node) { try { - await approve(node.fileid, node.basename, node) + await approve(node) } catch (error) { console.debug('Approve action failed') } @@ -34,7 +34,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') }, diff --git a/src/files/actions/rejectAction.js b/src/files/actions/rejectAction.js index 2cd77faf..c4a90c17 100644 --- a/src/files/actions/rejectAction.js +++ b/src/files/actions/rejectAction.js @@ -25,7 +25,7 @@ export const rejectAction = new FileAction({ order: 0, async exec(node) { try { - await reject(node.fileid, node.basename, node) + await reject(node) } catch (error) { console.debug('Reject action failed') } @@ -34,7 +34,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') }, diff --git a/src/files/helpers.js b/src/files/helpers.js index e15464df..9dec91a7 100644 --- a/src/files/helpers.js +++ b/src/files/helpers.js @@ -83,39 +83,37 @@ 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 = '', updateNode = true) { + 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 })) + showSuccess(t('approval', 'You approved {name}', { name: node.basename })) } - if (node) { + if (updateNode) { 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 } diff --git a/src/files/modals.js b/src/files/modals.js index fabc521d..d0fe2350 100644 --- a/src/files/modals.js +++ b/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) diff --git a/src/views/ApprovalTab.vue b/src/views/ApprovalTab.vue index ed59d8bd..49713272 100644 --- a/src/views/ApprovalTab.vue +++ b/src/views/ApprovalTab.vue @@ -58,6 +58,13 @@ export default { }, computed: { + node() { + const result = {} + result.fileid = this.fileInfo.id + result.basename = this.fileInfo.basename + result.attributes = this.fileInfo.attributes + return result + }, }, watch: { @@ -93,16 +100,12 @@ export default { }, async onApprove(message) { this.state = null - const fileId = this.fileInfo.id - const fileName = this.fileInfo.name - await approve(fileId, fileName, null, true, message) + await approve(this.node, true, message, false) this.update(this.fileInfo) }, async onReject(message) { this.state = null - const fileId = this.fileInfo.id - const fileName = this.fileInfo.name - await reject(fileId, fileName, null, true, message) + await reject(this.node, true, message, false) this.update(this.fileInfo) }, async onRequestSubmit(ruleId, createShares) {