Skip to content

Commit 25e6961

Browse files
authored
Merge pull request #6172 from LibreSign/fix/parallel-flow-signer-status
fix: parallel flow signer status
2 parents 48f330e + 9053636 commit 25e6961

File tree

2 files changed

+94
-17
lines changed

2 files changed

+94
-17
lines changed

lib/Service/RequestSignatureService.php

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -333,26 +333,13 @@ private function determineInitialStatus(
333333
?\OCA\Libresign\Enum\SignRequestStatus $currentStatus = null,
334334
?int $fileId = null,
335335
): \OCA\Libresign\Enum\SignRequestStatus {
336-
if ($signerStatus !== null) {
337-
$desiredStatus = \OCA\Libresign\Enum\SignRequestStatus::from($signerStatus);
338-
if ($currentStatus !== null && !$this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
339-
return $currentStatus;
340-
}
341-
342-
// Validate status transition based on signing order
343-
if ($fileId !== null) {
344-
return $this->sequentialSigningService->validateStatusByOrder($desiredStatus, $signingOrder, $fileId);
345-
}
346-
347-
return $desiredStatus;
348-
}
349-
350336
// If fileStatus is explicitly DRAFT (0), keep signer as DRAFT
351337
// This allows adding new signers in DRAFT mode even when file is not in DRAFT status
352338
if ($fileStatus === FileEntity::STATUS_DRAFT) {
353339
return \OCA\Libresign\Enum\SignRequestStatus::DRAFT;
354340
}
355341

342+
// If file status is ABLE_TO_SIGN, apply flow-based logic
356343
if ($fileStatus === FileEntity::STATUS_ABLE_TO_SIGN) {
357344
if ($this->sequentialSigningService->isOrderedNumericFlow()) {
358345
// In ordered flow, only first signer (order 1) should be ABLE_TO_SIGN
@@ -361,10 +348,26 @@ private function determineInitialStatus(
361348
? \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN
362349
: \OCA\Libresign\Enum\SignRequestStatus::DRAFT;
363350
}
364-
// In parallel flow, all can sign
351+
// In parallel flow, all can sign - ignore individual signer status if file is ABLE_TO_SIGN
365352
return \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN;
366353
}
367354

355+
// Handle explicit signer status when file status is not DRAFT or ABLE_TO_SIGN
356+
if ($signerStatus !== null) {
357+
$desiredStatus = \OCA\Libresign\Enum\SignRequestStatus::from($signerStatus);
358+
if ($currentStatus !== null && !$this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
359+
return $currentStatus;
360+
}
361+
362+
// Validate status transition based on signing order
363+
if ($fileId !== null) {
364+
return $this->sequentialSigningService->validateStatusByOrder($desiredStatus, $signingOrder, $fileId);
365+
}
366+
367+
return $desiredStatus;
368+
}
369+
370+
// Default fallback based on flow type
368371
if (!$this->sequentialSigningService->isOrderedNumericFlow()) {
369372
return \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN;
370373
}

tests/php/Unit/Service/RequestSignatureServiceTest.php

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public function setUp(): void {
7878
$this->appConfig = $this->createMock(IAppConfig::class);
7979
}
8080

81-
private function getService(): RequestSignatureService {
81+
private function getService(?SequentialSigningService $sequentialSigningService = null): RequestSignatureService {
8282
return new RequestSignatureService(
8383
$this->l10n,
8484
$this->identifyMethodService,
@@ -95,7 +95,7 @@ private function getService(): RequestSignatureService {
9595
$this->client,
9696
$this->docMdpHandler,
9797
$this->loggerInterface,
98-
$this->sequentialSigningService,
98+
$sequentialSigningService ?? $this->sequentialSigningService,
9999
$this->appConfig,
100100
);
101101
}
@@ -245,4 +245,78 @@ public static function dataSaveVisibleElements():array {
245245
[[['uid' => 1], ['uid' => 1]]],
246246
];
247247
}
248+
249+
/**
250+
* Test that parallel flow correctly sets ABLE_TO_SIGN status for all signers
251+
* even when frontend sends status 0 (DRAFT) for individual signers,
252+
* as long as file status is ABLE_TO_SIGN (1)
253+
*/
254+
public function testParallelFlowIgnoresSignerDraftStatusWhenFileIsAbleToSign(): void {
255+
$sequentialSigningService = $this->createMock(SequentialSigningService::class);
256+
$sequentialSigningService
257+
->method('isOrderedNumericFlow')
258+
->willReturn(false); // Parallel flow
259+
260+
// File status is ABLE_TO_SIGN (1)
261+
$fileStatus = \OCA\Libresign\Db\File::STATUS_ABLE_TO_SIGN;
262+
263+
// Signer status is DRAFT (0) - as sent by frontend
264+
$signerStatus = \OCA\Libresign\Enum\SignRequestStatus::DRAFT->value;
265+
266+
$result = self::invokePrivate(
267+
$this->getService($sequentialSigningService),
268+
'determineInitialStatus',
269+
[
270+
1, // signingOrder
271+
$fileStatus,
272+
$signerStatus,
273+
null, // currentStatus
274+
null, // fileId
275+
]
276+
);
277+
278+
// In parallel flow with ABLE_TO_SIGN file status, signer should be ABLE_TO_SIGN
279+
$this->assertEquals(
280+
\OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN,
281+
$result,
282+
'Parallel flow should set all signers to ABLE_TO_SIGN when file status is ABLE_TO_SIGN'
283+
);
284+
}
285+
286+
/**
287+
* Test that ordered flow respects signing order when file is ABLE_TO_SIGN
288+
*/
289+
public function testOrderedFlowRespectsSigningOrderWhenFileIsAbleToSign(): void {
290+
$sequentialSigningService = $this->createMock(SequentialSigningService::class);
291+
$sequentialSigningService
292+
->method('isOrderedNumericFlow')
293+
->willReturn(true); // Ordered flow
294+
295+
$fileStatus = \OCA\Libresign\Db\File::STATUS_ABLE_TO_SIGN;
296+
$signerStatus = \OCA\Libresign\Enum\SignRequestStatus::DRAFT->value;
297+
298+
// First signer (order 1) should be ABLE_TO_SIGN
299+
$result1 = self::invokePrivate(
300+
$this->getService($sequentialSigningService),
301+
'determineInitialStatus',
302+
[1, $fileStatus, $signerStatus, null, null]
303+
);
304+
$this->assertEquals(
305+
\OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN,
306+
$result1,
307+
'First signer in ordered flow should be ABLE_TO_SIGN'
308+
);
309+
310+
// Second signer (order 2) should remain DRAFT
311+
$result2 = self::invokePrivate(
312+
$this->getService($sequentialSigningService),
313+
'determineInitialStatus',
314+
[2, $fileStatus, $signerStatus, null, null]
315+
);
316+
$this->assertEquals(
317+
\OCA\Libresign\Enum\SignRequestStatus::DRAFT,
318+
$result2,
319+
'Second signer in ordered flow should remain DRAFT until first signs'
320+
);
321+
}
248322
}

0 commit comments

Comments
 (0)