fix: correctly calculate due dates for contract and invoice uploads

2026-08-17 12:49:36 +02:00
parent 0e1df6da7c
commit 31149fdd13
4 changed files with 145 additions and 7 deletions
@@ -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
@@ -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()
        ;
    }
 }
@@ -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');
@@ -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