diff --git a/lib/Db/ActionStepMapper.php b/lib/Db/ActionStepMapper.php index 892da8ab7d..d670ecabd2 100644 --- a/lib/Db/ActionStepMapper.php +++ b/lib/Db/ActionStepMapper.php @@ -76,4 +76,21 @@ public function findHighestOrderStep(int $actionId): ?ActionStep { return $this->findEntity($qb); } + + /** + * @return ActionStep[] + */ + public function findStepsFromOrder(int $actionId, int $order): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->andX( + $qb->expr()->eq('action_id', $qb->createNamedParameter($actionId, IQueryBuilder::PARAM_INT)), + $qb->expr()->gte('order', $qb->createNamedParameter($order, IQueryBuilder::PARAM_INT)) + ) + ); + + return $this->findEntities($qb); + } } diff --git a/lib/Service/QuickActionsService.php b/lib/Service/QuickActionsService.php index 7f11e9a5db..18809bc627 100644 --- a/lib/Service/QuickActionsService.php +++ b/lib/Service/QuickActionsService.php @@ -91,15 +91,16 @@ public function findActionStep(int $stepId, string $userId): ActionStep { } /** - * @param string $name - * @param int $order - * @param int $actionId - * @param string $parameter If the steps needs a parameter - * @return ActionStep + * @param ?int $tagId Required if $name is 'applyTag' + * @param ?int $mailboxId Required if $name is 'moveThread' * @throws ServiceException */ public function createActionStep(string $name, int $order, int $actionId, ?int $tagId = null, ?int $mailboxId = null): ActionStep { $this->validateActionStep($name, $order, $actionId, $tagId, $mailboxId); + foreach ($this->actionStepMapper->findStepsFromOrder($actionId, $order) as $stepToShift) { + $stepToShift->setOrder($stepToShift->getOrder() + 1); + $this->actionStepMapper->update($stepToShift); + } $action = new ActionStep(); $action->setName($name); $action->setOrder($order); @@ -146,12 +147,18 @@ private function validateActionStep(string $name, int $order, int $actionId, ?in throw new ServiceException('Invalid action step order'); } - if ($highestOrderForAction && in_array($highestOrderForAction->getName(), ['deleteThread', 'moveThread', 'markAsSpam'], true)) { - throw new ServiceException('Cant perform actions after ' . $highestOrderForAction->getName()); - } - - if ($highestOrderForAction !== null && $order !== $highestOrderForAction->getOrder() + 1) { - throw new ServiceException('Invalid action step order'); + if ($highestOrderForAction !== null) { + // A terminal step (delete/move/spam) always has to stay last. Inserting a + // step in front of it (i.e. at or before its current order) is fine, since + // createActionStep() shifts it and any steps after it up by one. + if (in_array($highestOrderForAction->getName(), ['deleteThread', 'moveThread', 'markAsSpam'], true) + && $order > $highestOrderForAction->getOrder()) { + throw new ServiceException('Cant perform actions after ' . $highestOrderForAction->getName()); + } + + if ($order > $highestOrderForAction->getOrder() + 1) { + throw new ServiceException('Invalid action step order'); + } } } catch (DoesNotExistException $th) { if ($order > 1) { diff --git a/src/components/quickActions/Settings.vue b/src/components/quickActions/Settings.vue index 29a6283365..75bd23b4a6 100644 --- a/src/components/quickActions/Settings.vue +++ b/src/components/quickActions/Settings.vue @@ -270,9 +270,13 @@ export default { showError(t('mail', 'Failed to update step in quick action')) } } else { - const createdStep = await createActionStep(action.name, action.order, quickAction.id, action?.tagId, action?.mailboxId) - if (createdStep) { - this.actions[index] = createdStep + try { + this.actions[index] = await createActionStep(action.name, action.order, quickAction.id, action?.tagId, action?.mailboxId) + } catch (error) { + logger.error('Could not add step to quick action', { + error, + }) + showError(t('mail', 'Failed to add step to quick action')) } } this.localAction = quickAction