refactor: extract status services

lib/Service/FileStatusService.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Libresign\Service;
+
+use OCA\Libresign\Db\File as FileEntity;
+use OCA\Libresign\Db\FileMapper;
+
+class FileStatusService {
+	public function __construct(
+		private FileMapper $fileMapper,
+	) {
+	}
+
+	public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus): FileEntity {
+		$currentStatus = $file->getStatus();
+		if ($newStatus > $currentStatus) {
+			$file->setStatus($newStatus);
+			$this->fileMapper->update($file);
+		}
+		return $file;
+	}
+
+	public function canNotifySigners(?int $fileStatus): bool {
+		return $fileStatus === FileEntity::STATUS_ABLE_TO_SIGN;
+	}
+}

lib/Service/RequestSignatureService.php

@@ -54,6 +54,8 @@ public function __construct(
 		protected SequentialSigningService $sequentialSigningService,
 		protected IAppConfig $appConfig,
 		protected IEventDispatcher $eventDispatcher,
+		protected FileStatusService $fileStatusService,
+		protected SignRequestStatusService $signRequestStatusService,
 	) {
 	}
 
@@ -76,7 +78,7 @@ public function save(array $data): FileEntity {
 	public function saveFile(array $data): FileEntity {
 		if (!empty($data['uuid'])) {
 			$file = $this->fileMapper->getByUuid($data['uuid']);
-			return $this->updateStatus($file, $data['status'] ?? 0);
+			return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0);
 		}
 		$fileId = null;
 		if (isset($data['file']['fileNode']) && $data['file']['fileNode'] instanceof Node) {
@@ -87,7 +89,7 @@ public function saveFile(array $data): FileEntity {
 		if (!is_null($fileId)) {
 			try {
 				$file = $this->fileMapper->getByFileId($fileId);
-				return $this->updateStatus($file, $data['status'] ?? 0);
+				return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0);
 			} catch (\Throwable) {
 			}
 		}
@@ -136,15 +138,6 @@ private function setSignatureFlowFromGlobalConfig(FileEntity $file): void {
 		$file->setSignatureFlowEnum($globalFlow);
 	}
 
-	private function updateStatus(FileEntity $file, int $status): FileEntity {
-		if ($status > $file->getStatus()) {
-			$file->setStatus($status);
-			/** @var FileEntity */
-			return $this->fileMapper->update($file);
-		}
-		return $file;
-	}
-
 	private function getFileMetadata(\OCP\Files\Node $node): array {
 		$metadata = [];
 		if ($extension = strtolower($node->getExtension())) {
@@ -244,7 +237,7 @@ private function associateToSigners(array $data, int $fileId): array {
 							],
 							displayName: $user['displayName'] ?? '',
 							description: $user['description'] ?? '',
-							notify: empty($user['notify']) && $this->isStatusAbleToNotify($fileStatus),
+							notify: empty($user['notify']),
 							fileId: $fileId,
 							signingOrder: $signingOrder,
 							fileStatus: $fileStatus,
@@ -256,7 +249,7 @@ private function associateToSigners(array $data, int $fileId): array {
 						identifyMethods: $user['identify'],
 						displayName: $user['displayName'] ?? '',
 						description: $user['description'] ?? '',
-						notify: empty($user['notify']) && $this->isStatusAbleToNotify($fileStatus),
+						notify: empty($user['notify']),
 						fileId: $fileId,
 						signingOrder: $signingOrder,
 						fileStatus: $fileStatus,
@@ -268,13 +261,6 @@ private function associateToSigners(array $data, int $fileId): array {
 		return $return;
 	}
 
-	private function isStatusAbleToNotify(?int $status): bool {
-		return in_array($status, [
-			FileEntity::STATUS_ABLE_TO_SIGN,
-			FileEntity::STATUS_PARTIAL_SIGNED,
-		]);
-	}
-
 	private function associateToSigner(
 		array $identifyMethods,
 		string $displayName,
@@ -302,13 +288,16 @@ private function associateToSigner(
 		$currentStatus = $signRequest->getStatusEnum();
 
 		if ($isNewSignRequest || $currentStatus === \OCA\Libresign\Enum\SignRequestStatus::DRAFT) {
-			$desiredStatus = $this->determineInitialStatus($signingOrder, $fileStatus, $signerStatus, $currentStatus, $fileId);
-			$this->updateStatusIfAllowed($signRequest, $currentStatus, $desiredStatus, $isNewSignRequest);
+			$desiredStatus = $this->signRequestStatusService->determineInitialStatus($signingOrder, $fileId, $fileStatus, $signerStatus, $currentStatus);
+			$this->signRequestStatusService->updateStatusIfAllowed($signRequest, $currentStatus, $desiredStatus, $isNewSignRequest);
 		}
 
 		$this->saveSignRequest($signRequest);
 
-		$shouldNotify = $notify && $signRequest->getStatusEnum() === \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN;
+		$shouldNotify = $notify && $this->signRequestStatusService->shouldNotifySignRequest(
+			$signRequest->getStatusEnum(),
+			$fileStatus
+		);
 
 		foreach ($identifyMethodsIncances as $identifyMethod) {
 			$identifyMethod->getEntity()->setSignRequestId($signRequest->getId());
@@ -318,68 +307,6 @@ private function associateToSigner(
 		return $signRequest;
 	}
 
-	private function updateStatusIfAllowed(
-		SignRequestEntity $signRequest,
-		\OCA\Libresign\Enum\SignRequestStatus $currentStatus,
-		\OCA\Libresign\Enum\SignRequestStatus $desiredStatus,
-		bool $isNewSignRequest,
-	): void {
-		if ($isNewSignRequest || $this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
-			$signRequest->setStatusEnum($desiredStatus);
-		}
-	}
-
-	private function determineInitialStatus(
-		int $signingOrder,
-		?int $fileStatus = null,
-		?int $signerStatus = null,
-		?\OCA\Libresign\Enum\SignRequestStatus $currentStatus = null,
-		?int $fileId = null,
-	): \OCA\Libresign\Enum\SignRequestStatus {
-		// 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
-				// Others remain DRAFT until their turn
-				return $signingOrder === 1
-					? \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN
-					: \OCA\Libresign\Enum\SignRequestStatus::DRAFT;
-			}
-			// 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;
-		}
-
-		return $signingOrder === 1
-			? \OCA\Libresign\Enum\SignRequestStatus::ABLE_TO_SIGN
-			: \OCA\Libresign\Enum\SignRequestStatus::DRAFT;
-	}
-
 	/**
 	 * @param IIdentifyMethod[] $identifyMethodsIncances
 	 * @param string $displayName

lib/Service/SequentialSigningService.php

@@ -12,14 +12,12 @@
 use OCA\Libresign\Db\SignRequestMapper;
 use OCA\Libresign\Enum\SignatureFlow;
 use OCA\Libresign\Enum\SignRequestStatus;
-use OCP\IAppConfig;
 
 class SequentialSigningService {
 	private int $currentOrder = 1;
 	private ?FileEntity $file = null;
 
 	public function __construct(
-		private IAppConfig $appConfig,
 		private SignRequestMapper $signRequestMapper,
 		private IdentifyMethodService $identifyMethodService,
 	) {

lib/Service/SignRequestStatusService.php

@@ -0,0 +1,93 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Libresign\Service;
+
+use OCA\Libresign\Db\File as FileEntity;
+use OCA\Libresign\Db\SignRequest as SignRequestEntity;
+use OCA\Libresign\Enum\SignRequestStatus;
+
+class SignRequestStatusService {
+	public function __construct(
+		private SequentialSigningService $sequentialSigningService,
+		private FileStatusService $fileStatusService,
+	) {
+	}
+
+	public function shouldNotifySignRequest(SignRequestStatus $signRequestStatus, ?int $fileStatus): bool {
+		return $this->fileStatusService->canNotifySigners($fileStatus)
+			&& $this->canNotifySignRequest($signRequestStatus);
+	}
+
+	public function canNotifySignRequest(SignRequestStatus $status): bool {
+		return $status === SignRequestStatus::ABLE_TO_SIGN;
+	}
+
+	public function updateStatusIfAllowed(
+		SignRequestEntity $signRequest,
+		SignRequestStatus $currentStatus,
+		SignRequestStatus $desiredStatus,
+		bool $isNewSignRequest,
+	): void {
+		if ($isNewSignRequest || $this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
+			$signRequest->setStatusEnum($desiredStatus);
+		}
+	}
+
+	public function determineInitialStatus(
+		int $signingOrder,
+		int $fileId,
+		?int $fileStatus = null,
+		?int $signerStatus = null,
+		?SignRequestStatus $currentStatus = null,
+	): SignRequestStatus {
+		if ($fileStatus === FileEntity::STATUS_DRAFT) {
+			return SignRequestStatus::DRAFT;
+		}
+
+		if ($fileStatus === FileEntity::STATUS_ABLE_TO_SIGN) {
+			return $this->determineStatusForAbleToSignFile($signingOrder);
+		}
+
+		if ($signerStatus !== null) {
+			return $this->handleExplicitSignerStatus($signerStatus, $signingOrder, $fileId, $currentStatus);
+		}
+
+		return $this->getDefaultStatusByFlow($signingOrder);
+	}
+
+	private function determineStatusForAbleToSignFile(int $signingOrder): SignRequestStatus {
+		if ($this->sequentialSigningService->isOrderedNumericFlow()) {
+			return $signingOrder === 1 ? SignRequestStatus::ABLE_TO_SIGN : SignRequestStatus::DRAFT;
+		}
+		return SignRequestStatus::ABLE_TO_SIGN;
+	}
+
+	private function handleExplicitSignerStatus(
+		int $signerStatus,
+		int $signingOrder,
+		int $fileId,
+		?SignRequestStatus $currentStatus,
+	): SignRequestStatus {
+		$desiredStatus = SignRequestStatus::from($signerStatus);
+
+		if ($currentStatus !== null && !$this->sequentialSigningService->isStatusUpgrade($currentStatus, $desiredStatus)) {
+			return $currentStatus;
+		}
+
+		return $this->sequentialSigningService->validateStatusByOrder($desiredStatus, $signingOrder, $fileId);
+	}
+
+	private function getDefaultStatusByFlow(int $signingOrder): SignRequestStatus {
+		if (!$this->sequentialSigningService->isOrderedNumericFlow()) {
+			return SignRequestStatus::ABLE_TO_SIGN;
+		}
+
+		return $signingOrder === 1 ? SignRequestStatus::ABLE_TO_SIGN : SignRequestStatus::DRAFT;
+	}
+}

tests/php/Unit/Service/FileStatusServiceTest.php

@@ -0,0 +1,91 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Libresign\Tests\Unit\Service;
+
+use OCA\Libresign\Db\File as FileEntity;
+use OCA\Libresign\Db\FileMapper;
+use OCA\Libresign\Service\FileStatusService;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+class FileStatusServiceTest extends TestCase {
+	private FileMapper $fileMapper;
+	private FileStatusService $service;
+
+	protected function setUp(): void {
+		parent::setUp();
+		$this->fileMapper = $this->createMock(FileMapper::class);
+		$this->service = new FileStatusService($this->fileMapper);
+	}
+
+	#[DataProvider('fileStatusUpgradeScenarios')]
+	public function testUpdateFileStatusIfUpgrade(int $currentStatus, int $newStatus, bool $shouldUpdate): void {
+		$file = new FileEntity();
+		$file->setStatus($currentStatus);
+
+		if ($shouldUpdate) {
+			$this->fileMapper->expects($this->once())
+				->method('update')
+				->with($file)
+				->willReturn($file);
+		} else {
+			$this->fileMapper->expects($this->never())->method('update');
+		}
+
+		$result = $this->service->updateFileStatusIfUpgrade($file, $newStatus);
+
+		$expectedStatus = $shouldUpdate ? $newStatus : $currentStatus;
+		$this->assertEquals($expectedStatus, $result->getStatus());
+	}
+
+	public static function fileStatusUpgradeScenarios(): array {
+		$draft = FileEntity::STATUS_DRAFT;
+		$able = FileEntity::STATUS_ABLE_TO_SIGN;
+		$partial = FileEntity::STATUS_PARTIAL_SIGNED;
+		$signed = FileEntity::STATUS_SIGNED;
+		$deleted = FileEntity::STATUS_DELETED;
+
+		return [
+			[$draft, $able, true],
+			[$draft, $partial, true],
+			[$draft, $signed, true],
+			[$draft, $deleted, true],
+			[$able, $partial, true],
+			[$able, $signed, true],
+			[$able, $deleted, true],
+			[$partial, $signed, true],
+			[$partial, $deleted, true],
+			[$signed, $deleted, true],
+			[$able, $draft, false],
+			[$partial, $draft, false],
+			[$partial, $able, false],
+			[$signed, $draft, false],
+			[$signed, $able, false],
+			[$signed, $partial, false],
+			[$deleted, $draft, false],
+		];
+	}
+
+	#[DataProvider('fileStatusNotificationScenarios')]
+	public function testCanNotifySigners(?int $fileStatus, bool $expected): void {
+		$result = $this->service->canNotifySigners($fileStatus);
+		$this->assertEquals($expected, $result);
+	}
+
+	public static function fileStatusNotificationScenarios(): array {
+		return [
+			[FileEntity::STATUS_DRAFT, false],
+			[FileEntity::STATUS_ABLE_TO_SIGN, true],
+			[FileEntity::STATUS_PARTIAL_SIGNED, false],
+			[FileEntity::STATUS_SIGNED, false],
+			[FileEntity::STATUS_DELETED, false],
+			[null, false],
+		];
+	}
+}