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