fix(quick-actions): add step before pinned action steps

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);
+	}
 }

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) {

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