From 5d33e0b1f41012b8face28a1ae9e5b9250408eba Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Sat, 13 Dec 2025 10:06:38 -0300 Subject: [PATCH 1/2] fix: Prioritize file status over signer status in parallel flow When file status is ABLE_TO_SIGN (1) in parallel flow, all signers should be set to ABLE_TO_SIGN regardless of individual signer status sent by frontend. This fixes the issue where signers remained in DRAFT status (0) even after clicking 'Request signatures'. The logic now: 1. Check file status DRAFT first - keep all signers as DRAFT 2. Check file status ABLE_TO_SIGN - apply flow-based logic: - Parallel: All signers get ABLE_TO_SIGN - Ordered: Only first signer gets ABLE_TO_SIGN 3. Fallback to individual signer status for other cases Resolves issue where sign_request records stayed at status 0 while libresign_file was updated to status 1. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/RequestSignatureService.php | 33 ++++++++++++++----------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/Service/RequestSignatureService.php b/lib/Service/RequestSignatureService.php index d65d900c83..16ec243584 100644 --- a/lib/Service/RequestSignatureService.php +++ b/lib/Service/RequestSignatureService.php @@ -333,26 +333,13 @@ private function determineInitialStatus( ?\OCA\Libresign\Enum\SignRequestStatus $currentStatus = null, ?int $fileId = null, ): \OCA\Libresign\Enum\SignRequestStatus { - if ($signerStatus !== null) { - $desiredStatus = \OCA\Libresign\Enum\SignRequestStatus::from($signerStatus); - if ($currentStatus !== null && !$this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) { - return $currentStatus; - } - - // Validate status transition based on signing order - if ($fileId !== null) { - return $this->sequentialSigningService->validateStatusByOrder($desiredStatus, $signingOrder, $fileId); - } - - return $desiredStatus; - } - // If fileStatus is explicitly DRAFT (0), keep signer as DRAFT // This allows adding new signers in DRAFT mode even when file is not in DRAFT status if ($fileStatus === FileEntity::STATUS_DRAFT) { return \OCA\Libresign\Enum\SignRequestStatus::DRAFT; } + // If file status is ABLE_TO_SIGN, apply flow-based logic if ($fileStatus === FileEntity::STATUS_ABLE_TO_SIGN) { if ($this->sequentialSigningService->isOrderedNumericFlow()) { // In ordered flow, only first signer (order 1) should be ABLE_TO_SIGN @@ -361,10 +348,26 @@ private function determineInitialStatus( ? \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN : \OCA\Libresign\Enum\SignRequestStatus::DRAFT; } - // In parallel flow, all can sign + // In parallel flow, all can sign - ignore individual signer status if file is ABLE_TO_SIGN return \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN; } + // Handle explicit signer status when file status is not DRAFT or ABLE_TO_SIGN + if ($signerStatus !== null) { + $desiredStatus = \OCA\Libresign\Enum\SignRequestStatus::from($signerStatus); + if ($currentStatus !== null && !$this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) { + return $currentStatus; + } + + // Validate status transition based on signing order + if ($fileId !== null) { + return $this->sequentialSigningService->validateStatusByOrder($desiredStatus, $signingOrder, $fileId); + } + + return $desiredStatus; + } + + // Default fallback based on flow type if (!$this->sequentialSigningService->isOrderedNumericFlow()) { return \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN; } From 90536362d8109342802f8d2fddda8e3e932e2dbc Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Sat, 13 Dec 2025 10:06:49 -0300 Subject: [PATCH 2/2] test: Add tests for parallel and ordered flow status determination Added two unit tests to validate status determination logic: 1. testParallelFlowIgnoresSignerDraftStatusWhenFileIsAbleToSign: Validates that in parallel flow, all signers are set to ABLE_TO_SIGN when file status is ABLE_TO_SIGN, ignoring individual DRAFT status sent by frontend. 2. testOrderedFlowRespectsSigningOrderWhenFileIsAbleToSign: Validates that in ordered flow, only the first signer (order 1) gets ABLE_TO_SIGN status, while subsequent signers remain DRAFT until their turn. Also updated getService() method to accept optional SequentialSigningService parameter for testing different flows. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Service/RequestSignatureServiceTest.php | 78 ++++++++++++++++++- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/tests/php/Unit/Service/RequestSignatureServiceTest.php b/tests/php/Unit/Service/RequestSignatureServiceTest.php index a7b9d6c2f9..0e8f0f5d01 100644 --- a/tests/php/Unit/Service/RequestSignatureServiceTest.php +++ b/tests/php/Unit/Service/RequestSignatureServiceTest.php @@ -78,7 +78,7 @@ public function setUp(): void { $this->appConfig = $this->createMock(IAppConfig::class); } - private function getService(): RequestSignatureService { + private function getService(?SequentialSigningService $sequentialSigningService = null): RequestSignatureService { return new RequestSignatureService( $this->l10n, $this->identifyMethodService, @@ -95,7 +95,7 @@ private function getService(): RequestSignatureService { $this->client, $this->docMdpHandler, $this->loggerInterface, - $this->sequentialSigningService, + $sequentialSigningService ?? $this->sequentialSigningService, $this->appConfig, ); } @@ -245,4 +245,78 @@ public static function dataSaveVisibleElements():array { [[['uid' => 1], ['uid' => 1]]], ]; } + + /** + * Test that parallel flow correctly sets ABLE_TO_SIGN status for all signers + * even when frontend sends status 0 (DRAFT) for individual signers, + * as long as file status is ABLE_TO_SIGN (1) + */ + public function testParallelFlowIgnoresSignerDraftStatusWhenFileIsAbleToSign(): void { + $sequentialSigningService = $this->createMock(SequentialSigningService::class); + $sequentialSigningService + ->method('isOrderedNumericFlow') + ->willReturn(false); // Parallel flow + + // File status is ABLE_TO_SIGN (1) + $fileStatus = \OCA\Libresign\Db\File::STATUS_ABLE_TO_SIGN; + + // Signer status is DRAFT (0) - as sent by frontend + $signerStatus = \OCA\Libresign\Enum\SignRequestStatus::DRAFT->value; + + $result = self::invokePrivate( + $this->getService($sequentialSigningService), + 'determineInitialStatus', + [ + 1, // signingOrder + $fileStatus, + $signerStatus, + null, // currentStatus + null, // fileId + ] + ); + + // In parallel flow with ABLE_TO_SIGN file status, signer should be ABLE_TO_SIGN + $this->assertEquals( + \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN, + $result, + 'Parallel flow should set all signers to ABLE_TO_SIGN when file status is ABLE_TO_SIGN' + ); + } + + /** + * Test that ordered flow respects signing order when file is ABLE_TO_SIGN + */ + public function testOrderedFlowRespectsSigningOrderWhenFileIsAbleToSign(): void { + $sequentialSigningService = $this->createMock(SequentialSigningService::class); + $sequentialSigningService + ->method('isOrderedNumericFlow') + ->willReturn(true); // Ordered flow + + $fileStatus = \OCA\Libresign\Db\File::STATUS_ABLE_TO_SIGN; + $signerStatus = \OCA\Libresign\Enum\SignRequestStatus::DRAFT->value; + + // First signer (order 1) should be ABLE_TO_SIGN + $result1 = self::invokePrivate( + $this->getService($sequentialSigningService), + 'determineInitialStatus', + [1, $fileStatus, $signerStatus, null, null] + ); + $this->assertEquals( + \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN, + $result1, + 'First signer in ordered flow should be ABLE_TO_SIGN' + ); + + // Second signer (order 2) should remain DRAFT + $result2 = self::invokePrivate( + $this->getService($sequentialSigningService), + 'determineInitialStatus', + [2, $fileStatus, $signerStatus, null, null] + ); + $this->assertEquals( + \OCA\Libresign\Enum\SignRequestStatus::DRAFT, + $result2, + 'Second signer in ordered flow should remain DRAFT until first signs' + ); + } }