fix(rejection): notify the next signing order only after the commit

lib/Service/SequentialSigningService.php

@@ -9,9 +9,11 @@
 namespace OCA\Libresign\Service;
 
 use OCA\Libresign\Db\File as FileEntity;
+use OCA\Libresign\Db\SignRequest as SignRequestEntity;
 use OCA\Libresign\Db\SignRequestMapper;
 use OCA\Libresign\Enum\SignatureFlow;
 use OCA\Libresign\Enum\SignRequestStatus;
+use Psr\Log\LoggerInterface;
 
 class SequentialSigningService {
 	private int $currentOrder = 1;
@@ -20,6 +22,7 @@ class SequentialSigningService {
 	public function __construct(
 		private SignRequestMapper $signRequestMapper,
 		private IdentifyMethodService $identifyMethodService,
+		private LoggerInterface $logger,
 	) {
 	}
 
@@ -69,22 +72,77 @@ public function determineSigningOrder(?int $userProvidedOrder): int {
 	 * @param int $completedOrder The order that was just completed
 	 */
 	public function releaseNextOrder(int $fileId, int $completedOrder): void {
+		$this->notifyActivatedSigners($this->activateNextOrder($fileId, $completedOrder));
+	}
+
+	/**
+	 * Same as releaseNextOrder(), but it only persists the activation and returns
+	 * the signers it activated instead of notifying them.
+	 *
+	 * A caller running inside a transaction must collect the returned signers and
+	 * call notifyActivatedSigners() only after the commit: a notification sent
+	 * before the commit is already delivered when a rollback reverts the
+	 * activation it announced.
+	 *
+	 * @return list<SignRequestEntity> The signers moved to ABLE_TO_SIGN.
+	 */
+	public function activateNextOrder(int $fileId, int $completedOrder): array {
 		if (!$this->isOrderedNumericFlow()) {
-			return;
+			return [];
 		}
 
 		$allSignRequests = $this->signRequestMapper->getByFileId($fileId);
 
 		if (!$this->isOrderFullyCompleted($allSignRequests, $completedOrder)) {
-			return;
+			return [];
 		}
 
 		$nextOrder = $this->findNextOrder($allSignRequests, $completedOrder);
 		if ($nextOrder === null) {
+			return [];
+		}
+
+		return $this->activateSignersForOrder($allSignRequests, $nextOrder);
+	}
+
+	/**
+	 * @param list<SignRequestEntity> $signers
+	 */
+	public function notifyActivatedSigners(array $signers): void {
+		foreach ($signers as $signer) {
+			$this->notifySigner($signer);
+		}
+	}
+
+	/**
+	 * Isolate recoverable lookup and delivery failures so other signers can proceed.
+	 * Channel exceptions vary; programming errors must still propagate.
+	 */
+	private function notifySigner(SignRequestEntity $signer): void {
+		try {
+			$identifyMethods = $this->identifyMethodService->getIdentifyMethodsFromSignRequestId($signer->getId());
+		} catch (\Exception $e) {
+			$this->logger->error('Error loading identification methods for an activated signer: ' . $e->getMessage(), [
+				'exception' => $e,
+				'signRequestId' => $signer->getId(),
+			]);
 			return;
 		}
 
-		$this->activateSignersForOrder($allSignRequests, $nextOrder);
+		foreach ($identifyMethods as $methodGroup) {
+			foreach ($methodGroup as $identifyMethod) {
+				try {
+					$identifyMethod->willNotifyUser(true);
+					$identifyMethod->notify();
+				} catch (\Exception $e) {
+					$this->logger->error('Error notifying an activated signer: ' . $e->getMessage(), [
+						'exception' => $e,
+						'signRequestId' => $signer->getId(),
+						'identifyMethod' => $identifyMethod->getName(),
+					]);
+				}
+			}
+		}
 	}
 
 	/**
@@ -111,7 +169,7 @@ public function reorderAfterDeletion(int $fileId, int $deletedOrder): void {
 			if ($previousOrder > 0 && $this->isOrderFullyCompleted($allSignRequests, $previousOrder)) {
 				$nextOrder = $this->findNextOrder($allSignRequests, $deletedOrder);
 				if ($nextOrder !== null) {
-					$this->activateSignersForOrder($allSignRequests, $nextOrder);
+					$this->notifyActivatedSigners($this->activateSignersForOrder($allSignRequests, $nextOrder));
 				}
 			}
 		}
@@ -151,7 +209,12 @@ private function findNextOrder(array $signRequests, int $completedOrder): ?int {
 		return null;
 	}
 
-	private function activateSignersForOrder(array $signRequests, int $order): void {
+	/**
+	 * @param list<SignRequestEntity> $signRequests
+	 * @return list<SignRequestEntity> The signers moved to ABLE_TO_SIGN.
+	 */
+	private function activateSignersForOrder(array $signRequests, int $order): array {
+		$activated = [];
 		$signersToActivate = array_filter(
 			$signRequests,
 			fn ($sr) => $this->isSigningParticipant($sr)
@@ -162,16 +225,11 @@ private function activateSignersForOrder(array $signRequests, int $order): void
 			if ($signer->getStatusEnum() === SignRequestStatus::DRAFT) {
 				$signer->setStatusEnum(SignRequestStatus::ABLE_TO_SIGN);
 				$this->signRequestMapper->update($signer);
-
-				$identifyMethods = $this->identifyMethodService->getIdentifyMethodsFromSignRequestId($signer->getId());
-				foreach ($identifyMethods as $methodGroup) {
-					foreach ($methodGroup as $identifyMethod) {
-						$identifyMethod->willNotifyUser(true);
-						$identifyMethod->notify();
-					}
-				}
+				$activated[] = $signer;
 			}
 		}
+
+		return $activated;
 	}
 
 	private function getSignatureFlow(): SignatureFlow {

lib/Service/SignatureRejection/SignatureRejectionService.php

@@ -76,8 +76,10 @@ public function reject(
 		$commentIsPrivate = $normalizedComment !== null && $privateComment;
 
 		$workflowCanceled = $config->cancelsWorkflow();
-		$this->persistRejection($libreSignFile, $signRequest, $normalizedComment, $commentIsPrivate, $workflowCanceled);
+		$activatedSigners = $this->persistRejection($libreSignFile, $signRequest, $normalizedComment, $commentIsPrivate, $workflowCanceled);
+		// Record the committed rejection before notification errors can interrupt this flow.
 		$this->dispatchRejectedEvent($signRequest, $libreSignFile, $workflowCanceled);
+		$this->sequentialSigningService->notifyActivatedSigners($activatedSigners);
 
 		return $signRequest;
 	}
@@ -94,14 +96,16 @@ public function isWorkflowCanceled(FileEntity $libreSignFile): bool {
 	 * Marking the signer as rejected and closing the workflow describe a single
 	 * decision, so they are written together: a failure must never leave a signer
 	 * who already rejected on a document that stays open for everybody else.
+	 *
+	 * @return list<SignRequestEntity> Signers activated by the committed rejection.
 	 */
 	private function persistRejection(
 		FileEntity $libreSignFile,
 		SignRequestEntity $signRequest,
 		?string $comment,
 		bool $commentIsPrivate,
 		bool $workflowCanceled,
-	): void {
+	): array {
 		$signRequests = $this->collectSignRequestsToReject($libreSignFile, $signRequest);
 		$previousSignerStatuses = array_map(
 			static fn (SignRequestEntity $each): int => $each->getStatus(),
@@ -122,8 +126,9 @@ private function persistRejection(
 
 			if ($workflowCanceled) {
 				$this->cancelWorkflow($libreSignFile);
+				$activatedSigners = [];
 			} else {
-				$this->releaseNextSigningOrders($libreSignFile, $signRequests);
+				$activatedSigners = $this->releaseNextSigningOrders($libreSignFile, $signRequests);
 			}
 
 			$this->db->commit();
@@ -138,6 +143,8 @@ private function persistRejection(
 			// TRANSLATORS Error shown when the rejection could not be saved and nothing was changed.
 			throw new LibresignException($this->l10n->t('It was not possible to register the rejection. Nothing was changed.'));
 		}
+
+		return $activatedSigners;
 	}
 
 	/**
@@ -187,19 +194,26 @@ private function collectSignRequestsToReject(FileEntity $libreSignFile, SignRequ
 	 * forever.
 	 *
 	 * @param list<SignRequestEntity> $rejectedSignRequests
+	 * @return list<SignRequestEntity> The signers activated on every document.
 	 */
-	private function releaseNextSigningOrders(FileEntity $libreSignFile, array $rejectedSignRequests): void {
+	private function releaseNextSigningOrders(FileEntity $libreSignFile, array $rejectedSignRequests): array {
+		$activatedSigners = [];
+
 		foreach ($rejectedSignRequests as $rejected) {
 			$document = $rejected->getFileId() === $libreSignFile->getId()
 				? $libreSignFile
 				: $this->fileMapper->getById($rejected->getFileId());
-			$this->sequentialSigningService
+			foreach ($this->sequentialSigningService
 				->setFile($document)
-				->releaseNextOrder(
+				->activateNextOrder(
 					$rejected->getFileId(),
 					$rejected->getSigningOrder()
-				);
+				) as $activated) {
+				$activatedSigners[] = $activated;
+			}
 		}
+
+		return $activatedSigners;
 	}
 
 	/**