From 7b6db841e20abcfd2d0f443198a7d722eb6af434 Mon Sep 17 00:00:00 2001 From: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:33:58 +0300 Subject: [PATCH 1/4] fix(rejection): notify the next signing order only after the commit A rejection that lets the workflow continue releases the next signing order inside its transaction. releaseNextOrder() persisted ABLE_TO_SIGN and dispatched the notification in the same method, so on an envelope with several documents the signers released for an earlier document were already notified when a later document failed and rolled the transaction back: the message was delivered for an activation that no longer existed. activateSignersForOrder() now only persists the state and returns the signers it activated. activateNextOrder() exposes that for callers that run inside a transaction and notifyActivatedSigners() dispatches the notifications, which SignatureRejectionService calls after commit(). releaseNextOrder() keeps activating and notifying in one step, so the non-transactional callers are unchanged, and a failing listener can no longer undo a recorded rejection. Signed-off-by: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> --- lib/Service/SequentialSigningService.php | 61 ++++++++++++++----- .../SignatureRejectionService.php | 10 +++ .../Service/SequentialSigningServiceTest.php | 56 +++++++++++++++++ 3 files changed, 113 insertions(+), 14 deletions(-) diff --git a/lib/Service/SequentialSigningService.php b/lib/Service/SequentialSigningService.php index e9b36b3fe5..45a13ef1ef 100644 --- a/lib/Service/SequentialSigningService.php +++ b/lib/Service/SequentialSigningService.php @@ -9,6 +9,7 @@ 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; @@ -69,22 +70,54 @@ 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 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); + return $this->activateSignersForOrder($allSignRequests, $nextOrder); + } + + /** + * Notify the signers returned by activateNextOrder(). + * + * @param list $signers + */ + public function notifyActivatedSigners(array $signers): void { + foreach ($signers as $signer) { + $identifyMethods = $this->identifyMethodService->getIdentifyMethodsFromSignRequestId($signer->getId()); + foreach ($identifyMethods as $methodGroup) { + foreach ($methodGroup as $identifyMethod) { + $identifyMethod->willNotifyUser(true); + $identifyMethod->notify(); + } + } + } } /** @@ -111,7 +144,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)); } } } @@ -140,7 +173,12 @@ private function findNextOrder(array $signRequests, int $completedOrder): ?int { return null; } - private function activateSignersForOrder(array $signRequests, int $order): void { + /** + * @param list $signRequests + * @return list The signers moved to ABLE_TO_SIGN. + */ + private function activateSignersForOrder(array $signRequests, int $order): array { + $activated = []; $signersToActivate = array_filter( $signRequests, fn ($sr) => $sr->getSigningOrder() === $order @@ -150,16 +188,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 { diff --git a/lib/Service/SignatureRejection/SignatureRejectionService.php b/lib/Service/SignatureRejection/SignatureRejectionService.php index f39fc7baff..42b69763f9 100644 --- a/lib/Service/SignatureRejection/SignatureRejectionService.php +++ b/lib/Service/SignatureRejection/SignatureRejectionService.php @@ -134,6 +134,16 @@ 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.')); } + + // The activation is committed at this point, so the signers it released + // are only told about it now. Notifying inside the transaction would + // deliver a message for an activation that a later failure rolls back, + // and a failing listener must not undo a recorded rejection. + try { + $this->sequentialSigningService->notifyActivatedSigners($activatedSigners); + } catch (\Throwable $e) { + $this->logger->error('Error notifying the signers released by the rejection: ' . $e->getMessage(), ['exception' => $e]); + } } /** diff --git a/tests/php/Unit/Service/SequentialSigningServiceTest.php b/tests/php/Unit/Service/SequentialSigningServiceTest.php index 84de2b3a7c..6d3e588fad 100644 --- a/tests/php/Unit/Service/SequentialSigningServiceTest.php +++ b/tests/php/Unit/Service/SequentialSigningServiceTest.php @@ -202,6 +202,62 @@ public function testReleaseNextOrderOrderedFlow(array $requests, int $completedO $this->service->releaseNextOrder(99, $completedOrder); } + public function testActivateNextOrderReturnsTheActivatedSignersWithoutNotifyingThem(): void { + $file = $this->createMock(FileEntity::class); + $file->method('getSignatureFlowEnum') + ->willReturn(SignatureFlow::ORDERED_NUMERIC); + $this->service->setFile($file); + + $signRequests = $this->buildSignRequests([ + [1, SignRequestStatus::SIGNED, 1], + [2, SignRequestStatus::DRAFT, 2], + [3, SignRequestStatus::DRAFT, 2], + ]); + $this->signRequestMapper->expects($this->once()) + ->method('getByFileId') + ->with(99) + ->willReturn($signRequests); + + $this->signRequestMapper->expects($this->exactly(2)) + ->method('update') + ->with($this->callback(fn (SignRequest $request): bool => $request->getStatusEnum() === SignRequestStatus::ABLE_TO_SIGN)); + + $this->identifyMethodService->expects($this->never())->method('getIdentifyMethodsFromSignRequestId'); + + $activated = $this->service->activateNextOrder(99, 1); + + $this->assertCount(2, $activated); + $this->assertSame([2, 3], array_map(fn (SignRequest $request): int => $request->getId(), $activated)); + } + + public function testNotifyActivatedSignersNotifiesEverySignerItReceives(): void { + $this->service->setFile($this->createMock(FileEntity::class)); + + $identifyMethod = $this->createMock(\OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod::class); + $identifyMethod->expects($this->exactly(2)) + ->method('willNotifyUser') + ->with(true); + $identifyMethod->expects($this->exactly(2)) + ->method('notify'); + + $this->identifyMethodService->expects($this->exactly(2)) + ->method('getIdentifyMethodsFromSignRequestId') + ->willReturn([[$identifyMethod]]); + + $this->service->notifyActivatedSigners($this->buildSignRequests([ + [2, SignRequestStatus::ABLE_TO_SIGN, 2], + [3, SignRequestStatus::ABLE_TO_SIGN, 2], + ])); + } + + public function testNotifyActivatedSignersIsANoOpWithoutActivatedSigners(): void { + $this->service->setFile($this->createMock(FileEntity::class)); + + $this->identifyMethodService->expects($this->never())->method('getIdentifyMethodsFromSignRequestId'); + + $this->service->notifyActivatedSigners([]); + } + public function testReorderAfterDeletionSkipsWhenNotOrdered(): void { $file = $this->createMock(FileEntity::class); $file->method('getSignatureFlowEnum') From 1a19306ad43ead3744602ab0ae8656ab8c3a2bd4 Mon Sep 17 00:00:00 2001 From: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:53:38 +0300 Subject: [PATCH 2/4] fix: isolate signer notification failures Every signer/identify-method notification is isolated: a failed delivery is logged with the sign request id and the identify method, and the remaining activated signers are still notified instead of being skipped. The post-commit catch in SignatureRejectionService now catches recoverable exceptions (\Exception) instead of \Throwable, so programming errors are not converted into a logged delivery failure; the log includes the sign request id. Adds regression tests for both behaviours. Signed-off-by: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com> --- lib/Service/SequentialSigningService.php | 22 +++++++- .../SignatureRejectionService.php | 14 ++++- .../Service/SequentialSigningServiceTest.php | 56 ++++++++++++++++++- 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/lib/Service/SequentialSigningService.php b/lib/Service/SequentialSigningService.php index 45a13ef1ef..e27fc952bc 100644 --- a/lib/Service/SequentialSigningService.php +++ b/lib/Service/SequentialSigningService.php @@ -13,6 +13,7 @@ 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; @@ -21,6 +22,7 @@ class SequentialSigningService { public function __construct( private SignRequestMapper $signRequestMapper, private IdentifyMethodService $identifyMethodService, + private LoggerInterface $logger, ) { } @@ -106,6 +108,16 @@ public function activateNextOrder(int $fileId, int $completedOrder): array { /** * Notify the signers returned by activateNextOrder(). * + * A delivery is best effort: one failing notification must not stop the + * remaining signers from being told that they can sign now, so every + * identify method is isolated from the others. + * + * Only recoverable failures (\Exception) are isolated. The concrete + * exception classes depend on the installed notification channels (mail, + * notifications, gateways), so the recoverable family is caught here per + * signer and identify method. A programming error (\Error) is not a + * delivery failure and stays visible instead of being logged as one. + * * @param list $signers */ public function notifyActivatedSigners(array $signers): void { @@ -114,7 +126,15 @@ public function notifyActivatedSigners(array $signers): void { foreach ($identifyMethods as $methodGroup) { foreach ($methodGroup as $identifyMethod) { $identifyMethod->willNotifyUser(true); - $identifyMethod->notify(); + try { + $identifyMethod->notify(); + } catch (\Exception $e) { + $this->logger->error('Error notifying an activated signer: ' . $e->getMessage(), [ + 'exception' => $e, + 'signRequestId' => $signer->getId(), + 'identifyMethod' => $identifyMethod->getName(), + ]); + } } } } diff --git a/lib/Service/SignatureRejection/SignatureRejectionService.php b/lib/Service/SignatureRejection/SignatureRejectionService.php index 42b69763f9..14cb0885ee 100644 --- a/lib/Service/SignatureRejection/SignatureRejectionService.php +++ b/lib/Service/SignatureRejection/SignatureRejectionService.php @@ -139,10 +139,20 @@ private function persistRejection( // are only told about it now. Notifying inside the transaction would // deliver a message for an activation that a later failure rolls back, // and a failing listener must not undo a recorded rejection. + // + // Sending a notification is a post-commit side effect: a recoverable + // delivery failure (\Exception) is logged instead of making the + // recorded rejection look failed for the caller. Programming errors + // (\Error) are not delivery failures and deliberately stay visible; + // failing notifications are already isolated per signer by + // SequentialSigningService::notifyActivatedSigners(). try { $this->sequentialSigningService->notifyActivatedSigners($activatedSigners); - } catch (\Throwable $e) { - $this->logger->error('Error notifying the signers released by the rejection: ' . $e->getMessage(), ['exception' => $e]); + } catch (\Exception $e) { + $this->logger->error('Error notifying the signers released by the rejection: ' . $e->getMessage(), [ + 'exception' => $e, + 'signRequestId' => $signRequest->getId(), + ]); } } diff --git a/tests/php/Unit/Service/SequentialSigningServiceTest.php b/tests/php/Unit/Service/SequentialSigningServiceTest.php index 6d3e588fad..c6f810d957 100644 --- a/tests/php/Unit/Service/SequentialSigningServiceTest.php +++ b/tests/php/Unit/Service/SequentialSigningServiceTest.php @@ -18,20 +18,24 @@ use OCA\Libresign\Tests\Unit\TestCase; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; final class SequentialSigningServiceTest extends TestCase { private SignRequestMapper&MockObject $signRequestMapper; private IdentifyMethodService&MockObject $identifyMethodService; + private LoggerInterface&MockObject $logger; private SequentialSigningService $service; public function setUp(): void { parent::setUp(); $this->signRequestMapper = $this->createMock(SignRequestMapper::class); $this->identifyMethodService = $this->createMock(IdentifyMethodService::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->service = new SequentialSigningService( $this->signRequestMapper, - $this->identifyMethodService + $this->identifyMethodService, + $this->logger ); } @@ -258,6 +262,56 @@ public function testNotifyActivatedSignersIsANoOpWithoutActivatedSigners(): void $this->service->notifyActivatedSigners([]); } + public function testNotifyActivatedSignersKeepsNotifyingTheRemainingSignersWhenADeliveryFails(): void { + $this->service->setFile($this->createMock(FileEntity::class)); + + $failing = $this->createMock(\OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod::class); + $failing->method('getName')->willReturn('email'); + $failing->expects($this->once())->method('willNotifyUser')->with(true); + $failing->expects($this->once())->method('notify') + ->willThrowException(new \RuntimeException('the mail server is unreachable')); + + $delivered = $this->createMock(\OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod::class); + $delivered->expects($this->once())->method('willNotifyUser')->with(true); + $delivered->expects($this->once())->method('notify'); + + $this->identifyMethodService->method('getIdentifyMethodsFromSignRequestId') + ->willReturnCallback(fn (int $signRequestId): array => $signRequestId === 2 + ? [[$failing]] + : [[$delivered]]); + + $this->logger->expects($this->once())->method('error')->with( + $this->stringContains('the mail server is unreachable'), + $this->callback(fn (array $context): bool => $context['signRequestId'] === 2 + && $context['identifyMethod'] === 'email'), + ); + + $this->service->notifyActivatedSigners($this->buildSignRequests([ + [2, SignRequestStatus::ABLE_TO_SIGN, 2], + [3, SignRequestStatus::ABLE_TO_SIGN, 2], + ])); + } + + public function testNotifyActivatedSignersDoesNotSwallowProgrammingErrors(): void { + $this->service->setFile($this->createMock(FileEntity::class)); + + $broken = $this->createMock(\OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod::class); + $broken->expects($this->once())->method('notify') + ->willThrowException(new \TypeError('a listener is calling the wrong method')); + + $this->identifyMethodService->method('getIdentifyMethodsFromSignRequestId') + ->willReturn([[$broken]]); + + $this->logger->expects($this->never())->method('error'); + + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('a listener is calling the wrong method'); + + $this->service->notifyActivatedSigners($this->buildSignRequests([ + [2, SignRequestStatus::ABLE_TO_SIGN, 2], + ])); + } + public function testReorderAfterDeletionSkipsWhenNotOrdered(): void { $file = $this->createMock(FileEntity::class); $file->method('getSignatureFlowEnum') From 1dfa2a094ac7a13e0c6ac46977294e2c2d23a680 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:13:20 +0000 Subject: [PATCH 3/4] fix(rejection): address notification isolation review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep recoverable notification failures in SequentialSigningService, extract per-signer delivery, and preserve rollback assertions. Adapt identification lookup isolation from PR #8948 while keeping rejection event ordering for that follow-up PR. Co-authored-by: André Maia Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/SequentialSigningService.php | 51 ++++++++++--------- .../SignatureRejectionService.php | 20 +------- .../Service/SequentialSigningServiceTest.php | 44 ++++++++++++++-- 3 files changed, 70 insertions(+), 45 deletions(-) diff --git a/lib/Service/SequentialSigningService.php b/lib/Service/SequentialSigningService.php index e27fc952bc..01a7e71c56 100644 --- a/lib/Service/SequentialSigningService.php +++ b/lib/Service/SequentialSigningService.php @@ -106,35 +106,40 @@ public function activateNextOrder(int $fileId, int $completedOrder): array { } /** - * Notify the signers returned by activateNextOrder(). - * - * A delivery is best effort: one failing notification must not stop the - * remaining signers from being told that they can sign now, so every - * identify method is isolated from the others. - * - * Only recoverable failures (\Exception) are isolated. The concrete - * exception classes depend on the installed notification channels (mail, - * notifications, gateways), so the recoverable family is caught here per - * signer and identify method. A programming error (\Error) is not a - * delivery failure and stays visible instead of being logged as one. - * * @param list $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()); - foreach ($identifyMethods as $methodGroup) { - foreach ($methodGroup as $identifyMethod) { + } catch (\Exception $e) { + $this->logger->error('Error loading identification methods for an activated signer: ' . $e->getMessage(), [ + 'exception' => $e, + 'signRequestId' => $signer->getId(), + ]); + return; + } + + foreach ($identifyMethods as $methodGroup) { + foreach ($methodGroup as $identifyMethod) { + try { $identifyMethod->willNotifyUser(true); - try { - $identifyMethod->notify(); - } catch (\Exception $e) { - $this->logger->error('Error notifying an activated signer: ' . $e->getMessage(), [ - 'exception' => $e, - 'signRequestId' => $signer->getId(), - 'identifyMethod' => $identifyMethod->getName(), - ]); - } + $identifyMethod->notify(); + } catch (\Exception $e) { + $this->logger->error('Error notifying an activated signer: ' . $e->getMessage(), [ + 'exception' => $e, + 'signRequestId' => $signer->getId(), + 'identifyMethod' => $identifyMethod->getName(), + ]); } } } diff --git a/lib/Service/SignatureRejection/SignatureRejectionService.php b/lib/Service/SignatureRejection/SignatureRejectionService.php index 14cb0885ee..5202d99db1 100644 --- a/lib/Service/SignatureRejection/SignatureRejectionService.php +++ b/lib/Service/SignatureRejection/SignatureRejectionService.php @@ -135,25 +135,7 @@ private function persistRejection( throw new LibresignException($this->l10n->t('It was not possible to register the rejection. Nothing was changed.')); } - // The activation is committed at this point, so the signers it released - // are only told about it now. Notifying inside the transaction would - // deliver a message for an activation that a later failure rolls back, - // and a failing listener must not undo a recorded rejection. - // - // Sending a notification is a post-commit side effect: a recoverable - // delivery failure (\Exception) is logged instead of making the - // recorded rejection look failed for the caller. Programming errors - // (\Error) are not delivery failures and deliberately stay visible; - // failing notifications are already isolated per signer by - // SequentialSigningService::notifyActivatedSigners(). - try { - $this->sequentialSigningService->notifyActivatedSigners($activatedSigners); - } catch (\Exception $e) { - $this->logger->error('Error notifying the signers released by the rejection: ' . $e->getMessage(), [ - 'exception' => $e, - 'signRequestId' => $signRequest->getId(), - ]); - } + $this->sequentialSigningService->notifyActivatedSigners($activatedSigners); } /** diff --git a/tests/php/Unit/Service/SequentialSigningServiceTest.php b/tests/php/Unit/Service/SequentialSigningServiceTest.php index c6f810d957..d68d6a5a1d 100644 --- a/tests/php/Unit/Service/SequentialSigningServiceTest.php +++ b/tests/php/Unit/Service/SequentialSigningServiceTest.php @@ -272,12 +272,12 @@ public function testNotifyActivatedSignersKeepsNotifyingTheRemainingSignersWhenA ->willThrowException(new \RuntimeException('the mail server is unreachable')); $delivered = $this->createMock(\OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod::class); - $delivered->expects($this->once())->method('willNotifyUser')->with(true); - $delivered->expects($this->once())->method('notify'); + $delivered->expects($this->exactly(2))->method('willNotifyUser')->with(true); + $delivered->expects($this->exactly(2))->method('notify'); $this->identifyMethodService->method('getIdentifyMethodsFromSignRequestId') ->willReturnCallback(fn (int $signRequestId): array => $signRequestId === 2 - ? [[$failing]] + ? [[$failing, $delivered]] : [[$delivered]]); $this->logger->expects($this->once())->method('error')->with( @@ -292,6 +292,44 @@ public function testNotifyActivatedSignersKeepsNotifyingTheRemainingSignersWhenA ])); } + public function testNotifyActivatedSignersContinuesAfterAnIdentificationLookupFails(): void { + $delivered = $this->createMock(\OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod::class); + $delivered->expects($this->once())->method('willNotifyUser')->with(true); + $delivered->expects($this->once())->method('notify'); + + $this->identifyMethodService->expects($this->exactly(2)) + ->method('getIdentifyMethodsFromSignRequestId') + ->willReturnCallback(function (int $signRequestId) use ($delivered): array { + if ($signRequestId === 2) { + throw new \RuntimeException('identification lookup failed'); + } + $this->assertSame(3, $signRequestId); + return [[$delivered]]; + }); + $this->logger->expects($this->once())->method('error')->with( + $this->stringContains('identification lookup failed'), + $this->callback(fn (array $context): bool => $context['signRequestId'] === 2 + && $context['exception'] instanceof \RuntimeException), + ); + + $this->service->notifyActivatedSigners($this->buildSignRequests([ + [2, SignRequestStatus::ABLE_TO_SIGN, 2], + [3, SignRequestStatus::ABLE_TO_SIGN, 2], + ])); + } + + public function testNotifyActivatedSignersDoesNotSwallowIdentificationLookupProgrammingErrors(): void { + $this->identifyMethodService->method('getIdentifyMethodsFromSignRequestId') + ->willThrowException(new \TypeError('invalid identification lookup')); + $this->logger->expects($this->never())->method('error'); + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('invalid identification lookup'); + + $this->service->notifyActivatedSigners($this->buildSignRequests([ + [2, SignRequestStatus::ABLE_TO_SIGN, 2], + ])); + } + public function testNotifyActivatedSignersDoesNotSwallowProgrammingErrors(): void { $this->service->setFile($this->createMock(FileEntity::class)); From 0923b76f221b29afa1489b65ab1102bf65a8748b Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:29:46 +0000 Subject: [PATCH 4/4] fix(rejection): dispatch rejection event before signer notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix(rejection): dispatch rejection event before signer notifications Return activated signers after committing the rejection and dispatch SignatureRejectedEvent before attempting their notifications. Preserve event delivery when a post-commit notification error propagates. Adapt the event-ordering correction from PR #8948 and strengthen the successful-order and notification-error regression assertions. Co-authored-by: André Maia Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> [skip ci] --- .../SignatureRejection/SignatureRejectionService.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/Service/SignatureRejection/SignatureRejectionService.php b/lib/Service/SignatureRejection/SignatureRejectionService.php index 5202d99db1..f4a0627121 100644 --- a/lib/Service/SignatureRejection/SignatureRejectionService.php +++ b/lib/Service/SignatureRejection/SignatureRejectionService.php @@ -76,6 +76,7 @@ public function reject( $workflowCanceled = $policy['cancel_workflow']; $this->persistRejection($libreSignFile, $signRequest, $normalizedComment, $commentIsPrivate, $workflowCanceled); $this->dispatchRejectedEvent($signRequest, $libreSignFile, $workflowCanceled); + $this->sequentialSigningService->notifyActivatedSigners($activatedSigners); return $signRequest; } @@ -92,6 +93,8 @@ 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 Signers activated by the committed rejection. */ private function persistRejection( FileEntity $libreSignFile, @@ -99,7 +102,7 @@ private function persistRejection( ?string $comment, bool $commentIsPrivate, bool $workflowCanceled, - ): void { + ): array { $signRequests = $this->collectSignRequestsToReject($libreSignFile, $signRequest); $previousSignerStatuses = array_map( static fn (SignRequestEntity $each): int => $each->getStatus(), @@ -135,7 +138,7 @@ private function persistRejection( throw new LibresignException($this->l10n->t('It was not possible to register the rejection. Nothing was changed.')); } - $this->sequentialSigningService->notifyActivatedSigners($activatedSigners); + return $activatedSigners; } /**