From 65df36dd1ac3c24749d7b0d0d30d2a31403983fc Mon Sep 17 00:00:00 2001 From: Matthieu Date: Fri, 22 May 2026 16:48:49 +0200 Subject: [PATCH] =?UTF-8?q?fix(absences)=20:=20garde-fou=20solde=20n=C3=A9?= =?UTF-8?q?gatif=20=C3=A0=20l'approbation=20+=20coh=C3=A9rence=20fixture?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AbsenceBalanceService::availableForRequest() : jours disponibles (acquis N-1 + en cours N − pris) pour la période de la demande, null si type non suivi. - Blocage de l'approbation si countedDays > disponible, dans les deux chemins (REST AbsenceReviewProcessor + MCP ReviewAbsenceRequestTool), comme le motif décès. Les CP en cours d'acquisition restent posables, mais pas au-delà du droit total (plus de solde négatif silencieux à l'approbation). - Fixture : demande pending CP d'alice replacée dans sa période de référence 2025-2026 (26→29/05/2026, 4 j ouvrés) et solde pending aligné (5 → 4) ; plus de "en attente" orphelin non lié à une demande. - Test fonctionnel testApproveBeyondAvailableBalanceIsBlocked + employé de test doté d'un droit pour que les approbations existantes passent le garde-fou. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/DataFixtures/AppFixtures.php | 12 ++++-- .../Tool/Absence/ReviewAbsenceRequestTool.php | 10 +++++ src/Service/AbsenceBalanceService.php | 23 ++++++++++++ src/State/AbsenceReviewProcessor.php | 11 ++++++ .../Mcp/AbsenceRequestLifecycleTest.php | 37 +++++++++++++++++++ 5 files changed, 89 insertions(+), 4 deletions(-) diff --git a/src/DataFixtures/AppFixtures.php b/src/DataFixtures/AppFixtures.php index c7ebabf..ef9f2cb 100644 --- a/src/DataFixtures/AppFixtures.php +++ b/src/DataFixtures/AppFixtures.php @@ -686,8 +686,9 @@ class AppFixtures extends Fixture $cpPeriod = '2025-2026'; $balanceData = [ // [user, acquired (N-1), acquiring (N, en cours), taken, pending] + // Alice's `pending` matches her pending CP request below (same 2025-2026 period). [$admin, 10.0, 22.5, 5.0, 0.0], - [$userAlice, 8.0, 18.0, 2.0, 5.0], + [$userAlice, 8.0, 18.0, 2.0, 4.0], [$userBob, 0.0, 14.0, 0.0, 0.0], ]; @@ -717,12 +718,15 @@ class AppFixtures extends Fixture $approvedCp->setReviewedBy($admin); $manager->persist($approvedCp); + // Pending CP within Alice's current reference period (2025-2026), so the + // reserved days line up with her balance's `pending` bucket above. + // Tue 26 → Fri 29 May 2026 = 4 working days (Pentecost Mon 25/05 excluded). $pendingCp = new AbsenceRequest(); $pendingCp->setUser($userAlice); $pendingCp->setType(AbsenceType::PaidLeave); - $pendingCp->setStartDate(new DateTimeImmutable('2026-06-15')); - $pendingCp->setEndDate(new DateTimeImmutable('2026-06-19')); - $pendingCp->setCountedDays(5.0); + $pendingCp->setStartDate(new DateTimeImmutable('2026-05-26')); + $pendingCp->setEndDate(new DateTimeImmutable('2026-05-29')); + $pendingCp->setCountedDays(4.0); $pendingCp->setStatus(AbsenceStatus::Pending); $pendingCp->setCreatedAt(new DateTimeImmutable('-2 days')); $manager->persist($pendingCp); diff --git a/src/Mcp/Tool/Absence/ReviewAbsenceRequestTool.php b/src/Mcp/Tool/Absence/ReviewAbsenceRequestTool.php index 3391781..1556d8a 100644 --- a/src/Mcp/Tool/Absence/ReviewAbsenceRequestTool.php +++ b/src/Mcp/Tool/Absence/ReviewAbsenceRequestTool.php @@ -52,6 +52,16 @@ class ReviewAbsenceRequestTool assert($admin instanceof User); if ('approve' === $decision) { + // Never let an approval push the balance below zero (CP only). + $available = $this->balanceService->availableForRequest($request); + if (null !== $available && $request->getCountedDays() > $available + 1e-9) { + throw new InvalidArgumentException(sprintf( + 'Approving this request would put the balance below zero: %g day(s) requested but only %g available.', + $request->getCountedDays(), + $available, + )); + } + $request->setStatus(AbsenceStatus::Approved); $request->setRejectionReason(null); $this->balanceService->applyApproval($request); diff --git a/src/Service/AbsenceBalanceService.php b/src/Service/AbsenceBalanceService.php index 399cb64..471bd11 100644 --- a/src/Service/AbsenceBalanceService.php +++ b/src/Service/AbsenceBalanceService.php @@ -70,6 +70,29 @@ final readonly class AbsenceBalanceService $balance->setPending($balance->getPending() + $request->getCountedDays()); } + /** + * Days still available to take in the request's balance period + * (acquired N-1 + acquiring N − already taken), or null when the type is + * not balance-tracked (per-event leaves such as bereavement or marriage). + * + * Days currently reserved in PENDING are intentionally not subtracted: the + * request being reviewed already sits in that pending bucket, and approval + * only moves it to TAKEN. + */ + public function availableForRequest(AbsenceRequest $request): ?float + { + if (!$this->shouldTrack($request)) { + return null; + } + + /** @var User $user */ + $user = $request->getUser(); + $period = $this->periodFor($user, $request->getType(), $request->getStartDate()); + $balance = $this->balanceRepository->findOneForPeriod($user, $request->getType(), $period); + + return $balance?->getAvailable() ?? 0.0; + } + /** Move reserved days from PENDING to TAKEN on approval. */ public function applyApproval(AbsenceRequest $request): void { diff --git a/src/State/AbsenceReviewProcessor.php b/src/State/AbsenceReviewProcessor.php index 9e8a881..0eecb1e 100644 --- a/src/State/AbsenceReviewProcessor.php +++ b/src/State/AbsenceReviewProcessor.php @@ -58,6 +58,17 @@ final readonly class AbsenceReviewProcessor implements ProcessorInterface assert($admin instanceof User); if ($isApprove) { + // Never let an approval push the balance below zero (CP only): the + // days being accrued (N) are posable, but not beyond the entitlement. + $available = $this->balanceService->availableForRequest($data); + if (null !== $available && $data->getCountedDays() > $available + 1e-9) { + throw new UnprocessableEntityHttpException(sprintf( + 'Approving this request would put the balance below zero: %g day(s) requested but only %g available.', + $data->getCountedDays(), + $available, + )); + } + $data->setStatus(AbsenceStatus::Approved); $data->setRejectionReason(null); $this->balanceService->applyApproval($data); diff --git a/tests/Functional/Mcp/AbsenceRequestLifecycleTest.php b/tests/Functional/Mcp/AbsenceRequestLifecycleTest.php index a2b37f5..55fa495 100644 --- a/tests/Functional/Mcp/AbsenceRequestLifecycleTest.php +++ b/tests/Functional/Mcp/AbsenceRequestLifecycleTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Tests\Functional\Mcp; +use App\Entity\AbsenceBalance; use App\Entity\AbsencePolicy; use App\Entity\User; use App\Enum\AbsenceType; @@ -62,6 +63,15 @@ class AbsenceRequestLifecycleTest extends KernelTestCase $policy->setActive(true); } + // Entitlement so CP requests can be approved without breaching the + // no-negative-balance guard (period of the June 2026 test requests). + $balance = new AbsenceBalance(); + $balance->setUser($this->employee); + $balance->setType(AbsenceType::PaidLeave); + $balance->setPeriod('2026-2027'); + $balance->setAcquired(25.0); + $this->em->persist($balance); + $this->em->flush(); } @@ -110,6 +120,33 @@ class AbsenceRequestLifecycleTest extends KernelTestCase self::assertSame(5.0, $balance->getTaken()); } + public function testApproveBeyondAvailableBalanceIsBlocked(): void + { + $created = json_decode( + ($this->createTool($this->admin))($this->employee->getId(), 'cp', '2026-06-01', '2026-06-05'), + true, + ); + + // Shrink the entitlement below the 5 requested days. + $balance = self::getContainer()->get(AbsenceBalanceRepository::class) + ->findOneForPeriod($this->employee, AbsenceType::PaidLeave, '2026-2027') + ; + $balance->setAcquired(2.0); + $balance->setAcquiring(0.0); + $this->em->flush(); + + try { + ($this->reviewTool($this->admin))($created['id'], 'approve'); + self::fail('Expected approval to be blocked when it would breach the balance.'); + } catch (InvalidArgumentException $e) { + self::assertStringContainsString('below zero', $e->getMessage()); + } + + // Approval bailed out before mutating: nothing moved to taken, days stay reserved. + self::assertSame(0.0, $balance->getTaken()); + self::assertSame(5.0, $balance->getPending()); + } + public function testAdminCancelApprovedReleasesTaken(): void { $created = json_decode(