Skip to content

Commit 7a542b3

Browse files
authored
Merge pull request #6184 from LibreSign/refactor/extract-status-services
refactor: extract status services
2 parents e784c39 + 3d96524 commit 7a542b3

8 files changed

Lines changed: 452 additions & 109 deletions

lib/Service/FileStatusService.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Libresign\Service;
10+
11+
use OCA\Libresign\Db\File as FileEntity;
12+
use OCA\Libresign\Db\FileMapper;
13+
14+
class FileStatusService {
15+
public function __construct(
16+
private FileMapper $fileMapper,
17+
) {
18+
}
19+
20+
public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus): FileEntity {
21+
$currentStatus = $file->getStatus();
22+
if ($newStatus > $currentStatus) {
23+
$file->setStatus($newStatus);
24+
$this->fileMapper->update($file);
25+
}
26+
return $file;
27+
}
28+
29+
public function canNotifySigners(?int $fileStatus): bool {
30+
return $fileStatus === FileEntity::STATUS_ABLE_TO_SIGN;
31+
}
32+
}

lib/Service/RequestSignatureService.php

Lines changed: 12 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ public function __construct(
5454
protected SequentialSigningService $sequentialSigningService,
5555
protected IAppConfig $appConfig,
5656
protected IEventDispatcher $eventDispatcher,
57+
protected FileStatusService $fileStatusService,
58+
protected SignRequestStatusService $signRequestStatusService,
5759
) {
5860
}
5961

@@ -76,7 +78,7 @@ public function save(array $data): FileEntity {
7678
public function saveFile(array $data): FileEntity {
7779
if (!empty($data['uuid'])) {
7880
$file = $this->fileMapper->getByUuid($data['uuid']);
79-
return $this->updateStatus($file, $data['status'] ?? 0);
81+
return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0);
8082
}
8183
$fileId = null;
8284
if (isset($data['file']['fileNode']) && $data['file']['fileNode'] instanceof Node) {
@@ -87,7 +89,7 @@ public function saveFile(array $data): FileEntity {
8789
if (!is_null($fileId)) {
8890
try {
8991
$file = $this->fileMapper->getByFileId($fileId);
90-
return $this->updateStatus($file, $data['status'] ?? 0);
92+
return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0);
9193
} catch (\Throwable) {
9294
}
9395
}
@@ -136,15 +138,6 @@ private function setSignatureFlowFromGlobalConfig(FileEntity $file): void {
136138
$file->setSignatureFlowEnum($globalFlow);
137139
}
138140

139-
private function updateStatus(FileEntity $file, int $status): FileEntity {
140-
if ($status > $file->getStatus()) {
141-
$file->setStatus($status);
142-
/** @var FileEntity */
143-
return $this->fileMapper->update($file);
144-
}
145-
return $file;
146-
}
147-
148141
private function getFileMetadata(\OCP\Files\Node $node): array {
149142
$metadata = [];
150143
if ($extension = strtolower($node->getExtension())) {
@@ -244,7 +237,7 @@ private function associateToSigners(array $data, int $fileId): array {
244237
],
245238
displayName: $user['displayName'] ?? '',
246239
description: $user['description'] ?? '',
247-
notify: empty($user['notify']) && $this->isStatusAbleToNotify($fileStatus),
240+
notify: empty($user['notify']),
248241
fileId: $fileId,
249242
signingOrder: $signingOrder,
250243
fileStatus: $fileStatus,
@@ -256,7 +249,7 @@ private function associateToSigners(array $data, int $fileId): array {
256249
identifyMethods: $user['identify'],
257250
displayName: $user['displayName'] ?? '',
258251
description: $user['description'] ?? '',
259-
notify: empty($user['notify']) && $this->isStatusAbleToNotify($fileStatus),
252+
notify: empty($user['notify']),
260253
fileId: $fileId,
261254
signingOrder: $signingOrder,
262255
fileStatus: $fileStatus,
@@ -268,13 +261,6 @@ private function associateToSigners(array $data, int $fileId): array {
268261
return $return;
269262
}
270263

271-
private function isStatusAbleToNotify(?int $status): bool {
272-
return in_array($status, [
273-
FileEntity::STATUS_ABLE_TO_SIGN,
274-
FileEntity::STATUS_PARTIAL_SIGNED,
275-
]);
276-
}
277-
278264
private function associateToSigner(
279265
array $identifyMethods,
280266
string $displayName,
@@ -302,13 +288,16 @@ private function associateToSigner(
302288
$currentStatus = $signRequest->getStatusEnum();
303289

304290
if ($isNewSignRequest || $currentStatus === \OCA\Libresign\Enum\SignRequestStatus::DRAFT) {
305-
$desiredStatus = $this->determineInitialStatus($signingOrder, $fileStatus, $signerStatus, $currentStatus, $fileId);
306-
$this->updateStatusIfAllowed($signRequest, $currentStatus, $desiredStatus, $isNewSignRequest);
291+
$desiredStatus = $this->signRequestStatusService->determineInitialStatus($signingOrder, $fileId, $fileStatus, $signerStatus, $currentStatus);
292+
$this->signRequestStatusService->updateStatusIfAllowed($signRequest, $currentStatus, $desiredStatus, $isNewSignRequest);
307293
}
308294

309295
$this->saveSignRequest($signRequest);
310296

311-
$shouldNotify = $notify && $signRequest->getStatusEnum() === \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN;
297+
$shouldNotify = $notify && $this->signRequestStatusService->shouldNotifySignRequest(
298+
$signRequest->getStatusEnum(),
299+
$fileStatus
300+
);
312301

313302
foreach ($identifyMethodsIncances as $identifyMethod) {
314303
$identifyMethod->getEntity()->setSignRequestId($signRequest->getId());
@@ -318,68 +307,6 @@ private function associateToSigner(
318307
return $signRequest;
319308
}
320309

321-
private function updateStatusIfAllowed(
322-
SignRequestEntity $signRequest,
323-
\OCA\Libresign\Enum\SignRequestStatus $currentStatus,
324-
\OCA\Libresign\Enum\SignRequestStatus $desiredStatus,
325-
bool $isNewSignRequest,
326-
): void {
327-
if ($isNewSignRequest || $this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
328-
$signRequest->setStatusEnum($desiredStatus);
329-
}
330-
}
331-
332-
private function determineInitialStatus(
333-
int $signingOrder,
334-
?int $fileStatus = null,
335-
?int $signerStatus = null,
336-
?\OCA\Libresign\Enum\SignRequestStatus $currentStatus = null,
337-
?int $fileId = null,
338-
): \OCA\Libresign\Enum\SignRequestStatus {
339-
// If fileStatus is explicitly DRAFT (0), keep signer as DRAFT
340-
// This allows adding new signers in DRAFT mode even when file is not in DRAFT status
341-
if ($fileStatus === FileEntity::STATUS_DRAFT) {
342-
return \OCA\Libresign\Enum\SignRequestStatus::DRAFT;
343-
}
344-
345-
// If file status is ABLE_TO_SIGN, apply flow-based logic
346-
if ($fileStatus === FileEntity::STATUS_ABLE_TO_SIGN) {
347-
if ($this->sequentialSigningService->isOrderedNumericFlow()) {
348-
// In ordered flow, only first signer (order 1) should be ABLE_TO_SIGN
349-
// Others remain DRAFT until their turn
350-
return $signingOrder === 1
351-
? \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN
352-
: \OCA\Libresign\Enum\SignRequestStatus::DRAFT;
353-
}
354-
// In parallel flow, all can sign - ignore individual signer status if file is ABLE_TO_SIGN
355-
return \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN;
356-
}
357-
358-
// Handle explicit signer status when file status is not DRAFT or ABLE_TO_SIGN
359-
if ($signerStatus !== null) {
360-
$desiredStatus = \OCA\Libresign\Enum\SignRequestStatus::from($signerStatus);
361-
if ($currentStatus !== null && !$this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
362-
return $currentStatus;
363-
}
364-
365-
// Validate status transition based on signing order
366-
if ($fileId !== null) {
367-
return $this->sequentialSigningService->validateStatusByOrder($desiredStatus, $signingOrder, $fileId);
368-
}
369-
370-
return $desiredStatus;
371-
}
372-
373-
// Default fallback based on flow type
374-
if (!$this->sequentialSigningService->isOrderedNumericFlow()) {
375-
return \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN;
376-
}
377-
378-
return $signingOrder === 1
379-
? \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN
380-
: \OCA\Libresign\Enum\SignRequestStatus::DRAFT;
381-
}
382-
383310
/**
384311
* @param IIdentifyMethod[] $identifyMethodsIncances
385312
* @param string $displayName

lib/Service/SequentialSigningService.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@
1212
use OCA\Libresign\Db\SignRequestMapper;
1313
use OCA\Libresign\Enum\SignatureFlow;
1414
use OCA\Libresign\Enum\SignRequestStatus;
15-
use OCP\IAppConfig;
1615

1716
class SequentialSigningService {
1817
private int $currentOrder = 1;
1918
private ?FileEntity $file = null;
2019

2120
public function __construct(
22-
private IAppConfig $appConfig,
2321
private SignRequestMapper $signRequestMapper,
2422
private IdentifyMethodService $identifyMethodService,
2523
) {
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Libresign\Service;
10+
11+
use OCA\Libresign\Db\File as FileEntity;
12+
use OCA\Libresign\Db\SignRequest as SignRequestEntity;
13+
use OCA\Libresign\Enum\SignRequestStatus;
14+
15+
class SignRequestStatusService {
16+
public function __construct(
17+
private SequentialSigningService $sequentialSigningService,
18+
private FileStatusService $fileStatusService,
19+
) {
20+
}
21+
22+
public function shouldNotifySignRequest(SignRequestStatus $signRequestStatus, ?int $fileStatus): bool {
23+
return $this->fileStatusService->canNotifySigners($fileStatus)
24+
&& $this->canNotifySignRequest($signRequestStatus);
25+
}
26+
27+
public function canNotifySignRequest(SignRequestStatus $status): bool {
28+
return $status === SignRequestStatus::ABLE_TO_SIGN;
29+
}
30+
31+
public function updateStatusIfAllowed(
32+
SignRequestEntity $signRequest,
33+
SignRequestStatus $currentStatus,
34+
SignRequestStatus $desiredStatus,
35+
bool $isNewSignRequest,
36+
): void {
37+
if ($isNewSignRequest || $this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
38+
$signRequest->setStatusEnum($desiredStatus);
39+
}
40+
}
41+
42+
public function determineInitialStatus(
43+
int $signingOrder,
44+
int $fileId,
45+
?int $fileStatus = null,
46+
?int $signerStatus = null,
47+
?SignRequestStatus $currentStatus = null,
48+
): SignRequestStatus {
49+
if ($fileStatus === FileEntity::STATUS_DRAFT) {
50+
return SignRequestStatus::DRAFT;
51+
}
52+
53+
if ($fileStatus === FileEntity::STATUS_ABLE_TO_SIGN) {
54+
return $this->determineStatusForAbleToSignFile($signingOrder);
55+
}
56+
57+
if ($signerStatus !== null) {
58+
return $this->handleExplicitSignerStatus($signerStatus, $signingOrder, $fileId, $currentStatus);
59+
}
60+
61+
return $this->getDefaultStatusByFlow($signingOrder);
62+
}
63+
64+
private function determineStatusForAbleToSignFile(int $signingOrder): SignRequestStatus {
65+
if ($this->sequentialSigningService->isOrderedNumericFlow()) {
66+
return $signingOrder === 1 ? SignRequestStatus::ABLE_TO_SIGN : SignRequestStatus::DRAFT;
67+
}
68+
return SignRequestStatus::ABLE_TO_SIGN;
69+
}
70+
71+
private function handleExplicitSignerStatus(
72+
int $signerStatus,
73+
int $signingOrder,
74+
int $fileId,
75+
?SignRequestStatus $currentStatus,
76+
): SignRequestStatus {
77+
$desiredStatus = SignRequestStatus::from($signerStatus);
78+
79+
if ($currentStatus !== null && !$this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
80+
return $currentStatus;
81+
}
82+
83+
return $this->sequentialSigningService->validateStatusByOrder($desiredStatus, $signingOrder, $fileId);
84+
}
85+
86+
private function getDefaultStatusByFlow(int $signingOrder): SignRequestStatus {
87+
if (!$this->sequentialSigningService->isOrderedNumericFlow()) {
88+
return SignRequestStatus::ABLE_TO_SIGN;
89+
}
90+
91+
return $signingOrder === 1 ? SignRequestStatus::ABLE_TO_SIGN : SignRequestStatus::DRAFT;
92+
}
93+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Libresign\Tests\Unit\Service;
10+
11+
use OCA\Libresign\Db\File as FileEntity;
12+
use OCA\Libresign\Db\FileMapper;
13+
use OCA\Libresign\Service\FileStatusService;
14+
use PHPUnit\Framework\Attributes\DataProvider;
15+
use PHPUnit\Framework\TestCase;
16+
17+
class FileStatusServiceTest extends TestCase {
18+
private FileMapper $fileMapper;
19+
private FileStatusService $service;
20+
21+
protected function setUp(): void {
22+
parent::setUp();
23+
$this->fileMapper = $this->createMock(FileMapper::class);
24+
$this->service = new FileStatusService($this->fileMapper);
25+
}
26+
27+
#[DataProvider('fileStatusUpgradeScenarios')]
28+
public function testUpdateFileStatusIfUpgrade(int $currentStatus, int $newStatus, bool $shouldUpdate): void {
29+
$file = new FileEntity();
30+
$file->setStatus($currentStatus);
31+
32+
if ($shouldUpdate) {
33+
$this->fileMapper->expects($this->once())
34+
->method('update')
35+
->with($file)
36+
->willReturn($file);
37+
} else {
38+
$this->fileMapper->expects($this->never())->method('update');
39+
}
40+
41+
$result = $this->service->updateFileStatusIfUpgrade($file, $newStatus);
42+
43+
$expectedStatus = $shouldUpdate ? $newStatus : $currentStatus;
44+
$this->assertEquals($expectedStatus, $result->getStatus());
45+
}
46+
47+
public static function fileStatusUpgradeScenarios(): array {
48+
$draft = FileEntity::STATUS_DRAFT;
49+
$able = FileEntity::STATUS_ABLE_TO_SIGN;
50+
$partial = FileEntity::STATUS_PARTIAL_SIGNED;
51+
$signed = FileEntity::STATUS_SIGNED;
52+
$deleted = FileEntity::STATUS_DELETED;
53+
54+
return [
55+
[$draft, $able, true],
56+
[$draft, $partial, true],
57+
[$draft, $signed, true],
58+
[$draft, $deleted, true],
59+
[$able, $partial, true],
60+
[$able, $signed, true],
61+
[$able, $deleted, true],
62+
[$partial, $signed, true],
63+
[$partial, $deleted, true],
64+
[$signed, $deleted, true],
65+
[$able, $draft, false],
66+
[$partial, $draft, false],
67+
[$partial, $able, false],
68+
[$signed, $draft, false],
69+
[$signed, $able, false],
70+
[$signed, $partial, false],
71+
[$deleted, $draft, false],
72+
];
73+
}
74+
75+
#[DataProvider('fileStatusNotificationScenarios')]
76+
public function testCanNotifySigners(?int $fileStatus, bool $expected): void {
77+
$result = $this->service->canNotifySigners($fileStatus);
78+
$this->assertEquals($expected, $result);
79+
}
80+
81+
public static function fileStatusNotificationScenarios(): array {
82+
return [
83+
[FileEntity::STATUS_DRAFT, false],
84+
[FileEntity::STATUS_ABLE_TO_SIGN, true],
85+
[FileEntity::STATUS_PARTIAL_SIGNED, false],
86+
[FileEntity::STATUS_SIGNED, false],
87+
[FileEntity::STATUS_DELETED, false],
88+
[null, false],
89+
];
90+
}
91+
}

0 commit comments

Comments
 (0)