diff --git a/src/Entity/Disposition.php b/src/Entity/Disposition.php index 7274af8..981aae6 100644 --- a/src/Entity/Disposition.php +++ b/src/Entity/Disposition.php @@ -233,6 +233,14 @@ class Disposition implements BlameableEntityInterface, TimestampableEntityInterf $dueDateFrom = $this->getCreatedAt()->modify(sprintf('+%d days', $deadlineDays)); $dueDateTo = $assignmentPeriod->end->modify('-1 day'); + // A short-notice assignment can end before the normal $deadlineDays window would even + // start. Left uncapped, dueDateFrom >= dueDateTo makes isStarted() and isEnded() equal + // at every instant below, so the flag would never light up; treat the contract as due + // right away instead. + if ($dueDateFrom >= $dueDateTo) { + $dueDateFrom = $this->getCreatedAt(); + } + $period = CarbonPeriodImmutable::create($dueDateFrom, $dueDateTo); // isStarted() alone stays true once the period has begun, so the dashboard kept diff --git a/src/Repository/DispositionRepository.php b/src/Repository/DispositionRepository.php index 16e485e..91fb7df 100644 --- a/src/Repository/DispositionRepository.php +++ b/src/Repository/DispositionRepository.php @@ -502,10 +502,27 @@ class DispositionRepository extends ServiceEntityRepository public function getContractReminderQuery(int $deadlineDays): Query { - $dayStart = (new \DateTimeImmutable('today'))->modify(sprintf('-%d days', $deadlineDays)); + $today = new \DateTimeImmutable('today'); + $dayStart = $today->modify(sprintf('-%d days', $deadlineDays)); + $tomorrow = $today->modify('+1 day'); $qb = $this->createQueryBuilder('disposition'); + // A short-notice assignment can end before the normal $deadlineDays window would even + // start (DispositionWorkflowGuardSubscriber::guardUploadContract blocks uploads from + // assignment.end - 1 day on). For those, remind on the block day instead of the normal + // day, which would otherwise fall on or after the block and describe an upload that is + // already refused. isContractDue() caps the same way. + $normalDay = $qb->expr()->andX( + $qb->expr()->gte('disposition.createdAt', ':dayStart'), + $qb->expr()->lt('disposition.createdAt', ':dayEnd'), + $this->effectiveDateToGreaterThan($qb, ':tomorrow'), + ); + $shortNoticeDay = $qb->expr()->andX( + $this->effectiveDateToEquals($qb, ':tomorrow'), + $qb->expr()->gte('disposition.createdAt', ':dayStart'), + ); + return $qb ->select('disposition', 'assignment', 'destination', 'teamer') ->innerJoin('disposition.assignment', 'assignment') @@ -520,14 +537,16 @@ class DispositionRepository extends ServiceEntityRepository $qb->expr()->isNull('teamer.deletedAt'), $qb->expr()->eq('disposition.status', ':dispositionStatus'), $qb->expr()->notIn('assignment.status', ':assignmentStatus'), - $qb->expr()->gte('disposition.createdAt', ':dayStart'), - $qb->expr()->lt('disposition.createdAt', ':dayEnd'), + $qb->expr()->orX($normalDay, $shortNoticeDay), )) ->setParameter('documentType', Upload::TYPE_CONTRACT) ->setParameter('dispositionStatus', Disposition::STATUS_NEW) ->setParameter('assignmentStatus', [Assignment::STATUS_CALLED_OFF, Assignment::STATUS_DELETED]) ->setParameter('dayStart', $dayStart) ->setParameter('dayEnd', $dayStart->modify('+1 day')) + // bound as date strings, not DateTimeImmutable: both columns are DATE, and + // Doctrine would otherwise bind 'Y-m-d H:i:s', which a DATE never equals + ->setParameter('tomorrow', $tomorrow->format('Y-m-d')) ->getQuery() ; } @@ -599,6 +618,23 @@ class DispositionRepository extends ServiceEntityRepository ); } + /** + * Same fallback as effectiveDateToEquals(), but for "later than $parameter". + */ + private function effectiveDateToGreaterThan(QueryBuilder $qb, string $parameter): Orx + { + return $qb->expr()->orX( + $qb->expr()->andX( + $qb->expr()->isNotNull('assignment.dateTo'), + $qb->expr()->gt('assignment.dateTo', $parameter) + ), + $qb->expr()->andX( + $qb->expr()->isNull('assignment.dateTo'), + $qb->expr()->gt('destination.dateTo', $parameter) + ), + ); + } + /** * Dispositions whose contract upload period has run out, for the admin dashboard. * @@ -607,7 +643,17 @@ class DispositionRepository extends ServiceEntityRepository */ public function findOverdueContracts(int $deadlineDays): array { - $dueDate = (new \DateTimeImmutable())->modify(sprintf('-%d days', $deadlineDays)); + return $this->getOverdueContractsQuery($deadlineDays)->getResult(); + } + + public function getOverdueContractsQuery(int $deadlineDays): Query + { + // anchored on midnight, like getContractReminderQuery()'s dayStart/dayEnd: a + // now()-relative anchor would disagree with the reminder query for part of each day. + // +1 day so a disposition is included from the same calendar day its reminder mail + // goes out, matching that query's dayStart (== reminderDay), not the day after it. + $reminderDay = (new \DateTimeImmutable('today'))->modify(sprintf('-%d days', $deadlineDays)); + $dueDate = $reminderDay->modify('+1 day'); $qb = $this->createQueryBuilder('disposition'); @@ -622,16 +668,20 @@ class DispositionRepository extends ServiceEntityRepository ->innerJoin('disposition.teamer', 'teamer') ->innerJoin('teamer.user', 'user') ->where($qb->expr()->andX( - $qb->expr()->lte('disposition.createdAt', ':dueDate'), + $qb->expr()->lt('disposition.createdAt', ':dueDate'), $qb->expr()->eq('disposition.status', ':status'), - $qb->expr()->isNull('document') + $qb->expr()->isNull('document'), + // kept in sync with getContractReminderQuery(), so the dashboard and the + // reminder mail agree on which dispositions are overdue + $qb->expr()->isNull('teamer.deletedAt'), + $qb->expr()->notIn('assignment.status', ':assignmentStatus'), )) ->setParameter('documentType', Upload::TYPE_CONTRACT) ->setParameter('dueDate', $dueDate) ->setParameter('status', Disposition::STATUS_NEW) + ->setParameter('assignmentStatus', [Assignment::STATUS_CALLED_OFF, Assignment::STATUS_DELETED]) ->orderBy('disposition.createdAt', 'ASC') ->getQuery() - ->getResult() ; } } diff --git a/tests/Entity/DispositionTest.php b/tests/Entity/DispositionTest.php index 279de8c..5ad05b4 100644 --- a/tests/Entity/DispositionTest.php +++ b/tests/Entity/DispositionTest.php @@ -47,6 +47,19 @@ class DispositionTest extends TestCase $this->assertFalse($disposition->isContractDue(self::CONTRACT_DEADLINE_DAYS)); } + /** + * A short-notice assignment can end before the normal $deadlineDays window would even + * start (assignment ends in 4 days, but the normal deadline is 5 days out). Left uncapped, + * dueDateFrom > dueDateTo made isStarted() and isEnded() equal at every instant, so this + * never returned true. + */ + public function testTheContractIsDueImmediatelyWhenTheAssignmentEndsBeforeTheNormalDeadline(): void + { + $disposition = $this->createDisposition(createdAt: 'today', assignmentEndsIn: '+4 days'); + + $this->assertTrue($disposition->isContractDue(self::CONTRACT_DEADLINE_DAYS)); + } + public function testAnUploadedContractIsNeverDue(): void { $disposition = $this->createDisposition(createdAt: 'today -60 days', assignmentEndsIn: '+30 days'); diff --git a/tests/Repository/DispositionRepositoryTest.php b/tests/Repository/DispositionRepositoryTest.php index fe06926..672c7a9 100644 --- a/tests/Repository/DispositionRepositoryTest.php +++ b/tests/Repository/DispositionRepositoryTest.php @@ -83,6 +83,36 @@ class DispositionRepositoryTest extends KernelTestCase ); } + /** + * A short-notice assignment can end before the normal $deadlineDays window would even + * start. Without the short-notice branch, that disposition's reminder day falls on or + * after guardUploadContract() already blocks the upload, and gets missed by the + * assignment-end-not-yet-reached guard on the normal branch. + */ + public function testContractReminderHasAShortNoticeBranchForEarlyAssignmentEnds(): void + { + $dql = $this->contractQuery()->getDQL(); + + $this->assertStringContainsString( + 'assignment.dateTo IS NOT NULL AND assignment.dateTo > :tomorrow', + $dql + ); + $this->assertStringContainsString( + 'assignment.dateTo IS NOT NULL AND assignment.dateTo = :tomorrow', + $dql + ); + } + + public function testContractReminderBindsTomorrowAsADateAndNotATimestamp(): void + { + $query = $this->contractQuery(5); + + $this->assertSame( + (new \DateTimeImmutable('today +1 day'))->format('Y-m-d'), + $query->getParameter('tomorrow')->getValue() + ); + } + /** * The bug that killed this query: destination.dateTo and assignment.dateTo are DATE columns, * and Doctrine binds a DateTimeImmutable as 'Y-m-d H:i:s', which a DATE never equals. @@ -132,11 +162,48 @@ class DispositionRepositoryTest extends KernelTestCase ); } + /** + * findOverdueContracts() lacked the assignment-status and teamer.deletedAt filters that + * getContractReminderQuery() has, so the admin list and the reminder mail disagreed on + * which dispositions counted, despite the docblock's claim that they stay in sync. + */ + public function testOverdueContractsSharesTheReminderQueryScope(): void + { + $dql = $this->overdueContractsQuery()->getDQL(); + + $this->assertStringContainsString('teamer.deletedAt IS NULL', $dql); + $this->assertStringContainsString('assignment.status NOT IN(:assignmentStatus)', $dql); + } + + /** + * findOverdueContracts() used to anchor on now(), while the reminder query anchors on + * midnight - the two disagreed on the boundary for part of each day. dueDate also has to + * land the day *after* the reminder day, so a disposition stays included from the same + * calendar day its reminder mail goes out rather than the day after. + */ + public function testOverdueContractsAnchorsOnTheSameDayTheReminderGoesOut(): void + { + $query = $this->overdueContractsQuery(5); + + $dueDate = $query->getParameter('dueDate')->getValue(); + + $this->assertSame('00:00:00', $dueDate->format('H:i:s')); + $this->assertSame( + (new \DateTimeImmutable('today'))->modify('-4 days')->format('Y-m-d'), + $dueDate->format('Y-m-d') + ); + } + private function contractQuery(int $deadlineDays = 5): Query { return $this->repository()->getContractReminderQuery($deadlineDays); } + private function overdueContractsQuery(int $deadlineDays = 5): Query + { + return $this->repository()->getOverdueContractsQuery($deadlineDays); + } + private function invoiceQuery(?\DateTimeImmutable $endDate = null): Query { return $this