fix: correctly apply due dates for contract and invoice upload reminders

tests/Entity/DispositionTest.php

@@ -0,0 +1,96 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Entity;
+
+use App\Entity\Application;
+use App\Entity\Assignment;
+use App\Entity\Destination;
+use App\Entity\Disposition;
+use App\Entity\Teamer;
+use App\Entity\Upload;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * The two "document is due" flags on the teamer dashboard. Both rest on CarbonPeriodImmutable,
+ * whose isStarted() stays true once the period has begun - which is right for one of them and
+ * was a bug in the other, so the difference is pinned here.
+ */
+class DispositionTest extends TestCase
+{
+    private const CONTRACT_DEADLINE_DAYS = 5;
+    private const INVOICE_DEADLINE_DAYS = 14;
+
+    public function testTheContractIsNotDueBeforeTheUploadPeriodHasRunOut(): void
+    {
+        $disposition = $this->createDisposition(createdAt: 'today -2 days', assignmentEndsIn: '+30 days');
+
+        $this->assertFalse($disposition->isContractDue(self::CONTRACT_DEADLINE_DAYS));
+    }
+
+    public function testTheContractIsDueOnceTheUploadPeriodHasRunOut(): void
+    {
+        $disposition = $this->createDisposition(createdAt: 'today -6 days', assignmentEndsIn: '+30 days');
+
+        $this->assertTrue($disposition->isContractDue(self::CONTRACT_DEADLINE_DAYS));
+    }
+
+    /**
+     * The upload is blocked from the day before the assignment ends, so past that point the
+     * dashboard would be asking for something the workflow guard refuses.
+     */
+    public function testTheContractStopsBeingDueOnceTheUploadIsBlocked(): void
+    {
+        $disposition = $this->createDisposition(createdAt: 'today -60 days', assignmentEndsIn: '+1 day');
+
+        $this->assertFalse($disposition->isContractDue(self::CONTRACT_DEADLINE_DAYS));
+    }
+
+    public function testAnUploadedContractIsNeverDue(): void
+    {
+        $disposition = $this->createDisposition(createdAt: 'today -60 days', assignmentEndsIn: '+30 days');
+        $disposition->addDocument((new Upload())->setType(Upload::TYPE_CONTRACT));
+
+        $this->assertFalse($disposition->isContractDue(self::CONTRACT_DEADLINE_DAYS));
+    }
+
+    public function testTheInvoiceIsNotDueWhileTheAssignmentIsStillRunning(): void
+    {
+        $disposition = $this->createDisposition(createdAt: 'today -30 days', assignmentEndsIn: '+5 days');
+
+        $this->assertFalse($disposition->isInvoiceDue(self::INVOICE_DEADLINE_DAYS));
+    }
+
+    public function testTheInvoiceIsDueOnceTheAssignmentHasEnded(): void
+    {
+        $disposition = $this->createDisposition(createdAt: 'today -30 days', assignmentEndsIn: '-1 day');
+
+        $this->assertTrue($disposition->isInvoiceDue(self::INVOICE_DEADLINE_DAYS));
+    }
+
+    /**
+     * Nothing blocks a late invoice upload, so unlike the contract this must keep asking.
+     */
+    public function testTheInvoiceStaysDueAfterTheUploadPeriodHasRunOut(): void
+    {
+        $disposition = $this->createDisposition(createdAt: 'today -90 days', assignmentEndsIn: '-60 days');
+
+        $this->assertTrue($disposition->isInvoiceDue(self::INVOICE_DEADLINE_DAYS));
+    }
+
+    private function createDisposition(string $createdAt, string $assignmentEndsIn): Disposition
+    {
+        $destination = (new Destination())
+            ->setProduct('Skireise')
+            ->setDateFrom(new \DateTimeImmutable($assignmentEndsIn.' -7 days'))
+            ->setDateTo(new \DateTimeImmutable($assignmentEndsIn))
+        ;
+
+        $disposition = new Disposition(
+            new Application((new Assignment())->setDestination($destination), new Teamer())
+        );
+
+        return $disposition->setCreatedAt(new \DateTimeImmutable($createdAt));
+    }
+}

tests/EventListener/InvalidateApplicationsListenerTest.php

@@ -14,7 +14,7 @@ use App\Event\ApplicationDeletedEvent;
 use App\Event\DocumentConfirmedEvent;
 use App\EventListener\InvalidateApplicationsListener;
 use App\Repository\ApplicationRepository;
-use Carbon\CarbonPeriod;
+use Carbon\CarbonPeriodImmutable;
 use Doctrine\ORM\EntityManagerInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -83,7 +83,7 @@ class InvalidateApplicationsListenerTest extends TestCase
     public function testIgnoresTeamersAllowedToHaveOverlappingApplications(): void
     {
         $document = $this->createContract(
-            new CarbonPeriod('2025-01-10', '2025-01-20'),
+            new CarbonPeriodImmutable('2025-01-10', '2025-01-20'),
             $this->createTeamer(allowOverlapping: true)
         );
 
@@ -110,7 +110,7 @@ class InvalidateApplicationsListenerTest extends TestCase
     public function testDoesNotFlushWhenNothingOverlaps(): void
     {
         $teamer = $this->createTeamer();
-        $document = $this->createContract(new CarbonPeriod('2025-01-10', '2025-01-20'), $teamer);
+        $document = $this->createContract(new CarbonPeriodImmutable('2025-01-10', '2025-01-20'), $teamer);
 
         $this->applicationRepository
             ->method('findOverlappingForTeamer')
@@ -128,7 +128,7 @@ class InvalidateApplicationsListenerTest extends TestCase
     public function testQueriesOverlappingApplicationsForTheTeamerAndTheEffectivePeriod(): void
     {
         $teamer = $this->createTeamer();
-        $period = new CarbonPeriod('2025-01-10', '2025-01-20');
+        $period = new CarbonPeriodImmutable('2025-01-10', '2025-01-20');
         $document = $this->createContract($period, $teamer);
 
         $this->applicationRepository
@@ -144,7 +144,7 @@ class InvalidateApplicationsListenerTest extends TestCase
     public function testRemovesEveryOverlappingApplicationAndFlushesOnce(): void
     {
         $teamer = $this->createTeamer();
-        $document = $this->createContract(new CarbonPeriod('2025-01-10', '2025-01-20'), $teamer);
+        $document = $this->createContract(new CarbonPeriodImmutable('2025-01-10', '2025-01-20'), $teamer);
 
         $applications = [$this->createApplication(), $this->createApplication(), $this->createApplication()];
 
@@ -163,7 +163,7 @@ class InvalidateApplicationsListenerTest extends TestCase
     public function testAnnouncesEveryRemovalWithAnApplicationDeletedEvent(): void
     {
         $teamer = $this->createTeamer();
-        $document = $this->createContract(new CarbonPeriod('2025-01-10', '2025-01-20'), $teamer);
+        $document = $this->createContract(new CarbonPeriodImmutable('2025-01-10', '2025-01-20'), $teamer);
 
         $applications = [$this->createApplication(), $this->createApplication()];
 
@@ -191,7 +191,7 @@ class InvalidateApplicationsListenerTest extends TestCase
         return $teamer;
     }
 
-    private function createContract(?CarbonPeriod $period, Teamer $teamer): Upload&MockObject
+    private function createContract(?CarbonPeriodImmutable $period, Teamer $teamer): Upload&MockObject
     {
         $assignment = $this->createMock(Assignment::class);
         $assignment->method('getEffectivePeriod')->willReturn($period);

tests/Repository/ApplicationRepositoryTest.php

@@ -8,7 +8,7 @@ use App\Entity\Application;
 use App\Entity\Assignment;
 use App\Entity\Teamer;
 use App\Repository\ApplicationRepository;
-use Carbon\CarbonPeriod;
+use Carbon\CarbonPeriodImmutable;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Query;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
@@ -55,7 +55,7 @@ class ApplicationRepositoryTest extends KernelTestCase
     public function testOverlappingQueryBindsThePeriodBoundaries(): void
     {
         $teamer = new Teamer();
-        $period = new CarbonPeriod('2025-01-10', '2025-01-20');
+        $period = new CarbonPeriodImmutable('2025-01-10', '2025-01-20');
 
         $query = $this->createOverlappingQuery($teamer, $period);
 
@@ -66,7 +66,7 @@ class ApplicationRepositoryTest extends KernelTestCase
         $this->assertSame('2025-01-20', $query->getParameter('dateTo')->getValue()->format('Y-m-d'));
     }
 
-    private function createOverlappingQuery(?Teamer $teamer = null, ?CarbonPeriod $period = null): Query
+    private function createOverlappingQuery(?Teamer $teamer = null, ?CarbonPeriodImmutable $period = null): Query
     {
         /** @var EntityManagerInterface $entityManager */
         $entityManager = static::getContainer()->get(EntityManagerInterface::class);
@@ -76,7 +76,7 @@ class ApplicationRepositoryTest extends KernelTestCase
 
         return $repository->getOverlappingForTeamerQuery(
             $teamer ?? new Teamer(),
-            $period ?? new CarbonPeriod('2025-01-10', '2025-01-20')
+            $period ?? new CarbonPeriodImmutable('2025-01-10', '2025-01-20')
         );
     }
 }

tests/Repository/DispositionRepositoryTest.php

@@ -0,0 +1,158 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Repository;
+
+use App\Entity\Assignment;
+use App\Entity\Disposition;
+use App\Entity\Upload;
+use App\Repository\DispositionRepository;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Query;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+/**
+ * The reminder queries carry their rules in DQL, and both of them shipped broken for well over a
+ * year without anyone noticing - the contract one asked for a document that is NULL *and* has a
+ * creation date, the invoice one compared a DATE column against a timestamp. Neither fails loudly;
+ * they simply return nothing, so the cron reports "no reminders" forever. These tests compile the
+ * queries and pin the parts that made them silently empty.
+ */
+class DispositionRepositoryTest extends KernelTestCase
+{
+    /**
+     * The predecessor required `document IS NULL AND document.createdAt = :date` in one andX,
+     * which no row can satisfy. The date has to come off the disposition.
+     */
+    public function testContractReminderAnchorsOnTheDispositionAndNotOnTheMissingDocument(): void
+    {
+        $dql = $this->contractQuery()->getDQL();
+
+        $this->assertStringContainsString('document IS NULL', $dql);
+        $this->assertStringContainsString('disposition.createdAt >= :dayStart', $dql);
+        $this->assertStringContainsString('disposition.createdAt < :dayEnd', $dql);
+        $this->assertStringNotContainsString('document.createdAt', $dql);
+    }
+
+    /**
+     * disposition.createdAt is a timestamp and the cron runs at 01:00, so an equality would only
+     * ever match a disposition created at that exact second.
+     */
+    public function testContractReminderUsesAHalfOpenDayRange(): void
+    {
+        $query = $this->contractQuery(5);
+
+        $dayStart = $query->getParameter('dayStart')->getValue();
+        $dayEnd = $query->getParameter('dayEnd')->getValue();
+
+        $this->assertSame('00:00:00', $dayStart->format('H:i:s'));
+        $this->assertSame(
+            (new \DateTimeImmutable('today'))->modify('-5 days')->format('Y-m-d'),
+            $dayStart->format('Y-m-d')
+        );
+        $this->assertSame('1', $dayStart->diff($dayEnd)->format('%a'));
+        $this->assertDoesNotMatchRegularExpression('/disposition\.createdAt = :/', $query->getDQL());
+    }
+
+    /**
+     * An unfiltered join would let any other upload - a driver's licence, say - stand in for the
+     * contract and suppress the reminder.
+     */
+    public function testContractReminderOnlyJoinsTheContract(): void
+    {
+        $query = $this->contractQuery();
+
+        $this->assertStringContainsString('WITH document.type = :documentType', $query->getDQL());
+        $this->assertSame(Upload::TYPE_CONTRACT, $query->getParameter('documentType')->getValue());
+    }
+
+    public function testContractReminderSkipsDeadAssignmentsAndDeletedTeamers(): void
+    {
+        $query = $this->contractQuery();
+        $dql = $query->getDQL();
+
+        $this->assertStringContainsString('teamer.deletedAt IS NULL', $dql);
+        $this->assertStringContainsString('disposition.status = :dispositionStatus', $dql);
+        $this->assertStringContainsString('assignment.status NOT IN(:assignmentStatus)', $dql);
+
+        $this->assertSame(Disposition::STATUS_NEW, $query->getParameter('dispositionStatus')->getValue());
+        $this->assertSame(
+            [Assignment::STATUS_CALLED_OFF, Assignment::STATUS_DELETED],
+            $query->getParameter('assignmentStatus')->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.
+     */
+    public function testInvoiceReminderBindsADateAndNotATimestamp(): void
+    {
+        $value = $this
+            ->invoiceQuery(new \DateTimeImmutable('2026-03-15 01:00:05'))
+            ->getParameter('endDate')
+            ->getValue()
+        ;
+
+        $this->assertSame('2026-03-15', $value);
+    }
+
+    /**
+     * Without the IS NULL guard on the second branch, an assignment that moves the end date into
+     * the future still matches on its destination's date.
+     */
+    public function testInvoiceReminderGuardsTheDestinationFallback(): void
+    {
+        $dql = $this->invoiceQuery()->getDQL();
+
+        $this->assertStringContainsString(
+            'assignment.dateTo IS NOT NULL AND assignment.dateTo = :endDate',
+            $dql
+        );
+        $this->assertStringContainsString(
+            'assignment.dateTo IS NULL AND destination.dateTo = :endDate',
+            $dql
+        );
+    }
+
+    public function testInvoiceReminderOnlyJoinsTheInvoiceAndSkipsFinishedDispositions(): void
+    {
+        $query = $this->invoiceQuery();
+        $dql = $query->getDQL();
+
+        $this->assertStringContainsString('WITH document.type = :documentType', $dql);
+        $this->assertStringContainsString('document IS NULL', $dql);
+        $this->assertStringContainsString('teamer.deletedAt IS NULL', $dql);
+
+        $this->assertSame(Upload::TYPE_INVOICE, $query->getParameter('documentType')->getValue());
+        $this->assertSame(
+            [Disposition::STATUS_CALLED_OFF, Disposition::STATUS_COMPLETED],
+            $query->getParameter('dispositionStatus')->getValue()
+        );
+    }
+
+    private function contractQuery(int $deadlineDays = 5): Query
+    {
+        return $this->repository()->getContractReminderQuery($deadlineDays);
+    }
+
+    private function invoiceQuery(?\DateTimeImmutable $endDate = null): Query
+    {
+        return $this
+            ->repository()
+            ->getInvoiceReminderQuery($endDate ?? new \DateTimeImmutable('2026-03-15'))
+        ;
+    }
+
+    private function repository(): DispositionRepository
+    {
+        /** @var EntityManagerInterface $entityManager */
+        $entityManager = static::getContainer()->get(EntityManagerInterface::class);
+
+        /** @var DispositionRepository $repository */
+        $repository = $entityManager->getRepository(Disposition::class);
+
+        return $repository;
+    }
+}

tests/Service/Cron/UploadReminderServiceTest.php

@@ -0,0 +1,177 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Service\Cron;
+
+use App\Config\EmailTextKey;
+use App\Email\Mailer;
+use App\Entity\Application;
+use App\Entity\Assignment;
+use App\Entity\Destination;
+use App\Entity\Disposition;
+use App\Entity\Embeddable\Communication;
+use App\Entity\Teamer;
+use App\Repository\DispositionRepository;
+use App\Service\Cron\UploadReminderService;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\NullLogger;
+
+/**
+ * Pins the reminder rules themselves: which day each reminder targets, and which wording it uses.
+ *
+ * The queries behind them are covered in DispositionRepositoryTest; what is asserted here is the
+ * arithmetic the service hands over, because a reminder that asks for the wrong day is just as
+ * silent as the broken queries these replaced.
+ */
+class UploadReminderServiceTest extends TestCase
+{
+    private const CONTRACT_DEADLINE_DAYS = 5;
+    private const INVOICE_DEADLINE_DAYS = 14;
+
+    private DispositionRepository&MockObject $repository;
+    private Mailer&MockObject $mailer;
+    private UploadReminderService $service;
+
+    protected function setUp(): void
+    {
+        $this->repository = $this->createMock(DispositionRepository::class);
+        $this->mailer = $this->createMock(Mailer::class);
+
+        $this->service = new UploadReminderService(
+            $this->repository,
+            $this->mailer,
+            new NullLogger(),
+            self::CONTRACT_DEADLINE_DAYS,
+            self::INVOICE_DEADLINE_DAYS,
+        );
+    }
+
+    public function testTheContractReminderAsksForTheConfiguredPeriod(): void
+    {
+        $this->repository
+            ->expects($this->once())
+            ->method('findDispositionsForContractReminder')
+            ->with(self::CONTRACT_DEADLINE_DAYS)
+            ->willReturn([])
+        ;
+
+        $this->assertSame(
+            'No due contract upload reminders to be sent to teamers',
+            $this->service->sendContractUploadReminders()
+        );
+    }
+
+    public function testTheContractReminderMailsEachTeamerOnce(): void
+    {
+        $this->repository
+            ->method('findDispositionsForContractReminder')
+            ->willReturn([$this->createDisposition(), $this->createDisposition()])
+        ;
+
+        $this->mailer
+            ->expects($this->exactly(2))
+            ->method('createAndSendText')
+            ->with(EmailTextKey::REMINDER_CONTRACT_UPLOAD, $this->anything(), $this->anything())
+        ;
+
+        $this->assertSame(
+            'Sent 2 due contract upload reminders to teamers',
+            $this->service->sendContractUploadReminders()
+        );
+    }
+
+    /**
+     * The first reminder goes out the day after the assignment ended, the second on the last day
+     * of the upload period. Both are anchored on the end date, so each targets exactly one day and
+     * the cron can run twice without mailing anyone twice.
+     */
+    public function testTheTwoInvoiceRemindersTargetTheDayAfterTheEndAndTheDeadline(): void
+    {
+        $today = new \DateTimeImmutable('today');
+        $requested = [];
+
+        $this->repository
+            ->expects($this->exactly(2))
+            ->method('findDispositionsForInvoiceReminder')
+            ->willReturnCallback(function (\DateTimeImmutable $endDate) use (&$requested): array {
+                $requested[] = $endDate->format('Y-m-d');
+
+                return [];
+            })
+        ;
+
+        $this->service->sendInvoiceUploadReminders();
+
+        $this->assertSame([
+            $today->modify('-1 day')->format('Y-m-d'),
+            $today->modify('-'.self::INVOICE_DEADLINE_DAYS.' days')->format('Y-m-d'),
+        ], $requested);
+    }
+
+    /**
+     * The second mail must not repeat "within the next 14 days" - by then the period is over.
+     */
+    public function testTheSecondInvoiceReminderUsesTheFinalWording(): void
+    {
+        $this->repository
+            ->method('findDispositionsForInvoiceReminder')
+            ->willReturnOnConsecutiveCalls([$this->createDisposition()], [$this->createDisposition()])
+        ;
+
+        $keys = [];
+        $this->mailer
+            ->method('createAndSendText')
+            ->willReturnCallback(function (EmailTextKey $key) use (&$keys): void {
+                $keys[] = $key;
+            })
+        ;
+
+        $message = $this->service->sendInvoiceUploadReminders();
+
+        $this->assertSame([
+            EmailTextKey::REMINDER_INVOICE_UPLOAD,
+            EmailTextKey::REMINDER_INVOICE_UPLOAD_FINAL,
+        ], $keys);
+        $this->assertSame('Sent 2 due invoice upload reminders to teamers', $message);
+    }
+
+    public function testTheInvoiceReminderPassesTheDeadlineToTheWording(): void
+    {
+        $this->repository
+            ->method('findDispositionsForInvoiceReminder')
+            ->willReturnOnConsecutiveCalls([$this->createDisposition()], [])
+        ;
+
+        $this->mailer
+            ->expects($this->once())
+            ->method('createAndSendText')
+            ->with(
+                EmailTextKey::REMINDER_INVOICE_UPLOAD,
+                $this->callback(fn (array $placeholders): bool => self::INVOICE_DEADLINE_DAYS === $placeholders['invoiceUploadDeadlineDays']
+                    && 'Skireise' === substr((string) $placeholders['destination'], -8)),
+                ['to' => '[email protected]'],
+            )
+        ;
+
+        $this->service->sendInvoiceUploadReminders();
+    }
+
+    private function createDisposition(): Disposition
+    {
+        $destination = (new Destination())
+            ->setProduct('Skireise')
+            ->setDateFrom(new \DateTimeImmutable('2026-03-01'))
+            ->setDateTo(new \DateTimeImmutable('2026-03-14'))
+        ;
+
+        $assignment = (new Assignment())->setDestination($destination);
+
+        $teamer = (new Teamer())->setCommunication(
+            (new Communication())->setEmail('[email protected]')
+        );
+
+        return new Disposition(new Application($assignment, $teamer));
+    }
+}

tests/Service/Pdf/InvoiceRendererTest.php

@@ -17,7 +17,7 @@ use App\Service\Pdf\InvoiceApprover;
 use App\Service\Pdf\InvoiceRenderer;
 use App\Service\Pdf\Pdf;
 use App\Service\Upload\UploadHandler;
-use Carbon\CarbonPeriod;
+use Carbon\CarbonPeriodImmutable;
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
@@ -84,7 +84,7 @@ class InvoiceRendererTest extends WebTestCase
         ;
 
         $assignment = $this->createMock(Assignment::class);
-        $assignment->method('getEffectivePeriod')->willReturn(CarbonPeriod::create('2025-02-01', '2025-02-08'));
+        $assignment->method('getEffectivePeriod')->willReturn(CarbonPeriodImmutable::create('2025-02-01', '2025-02-08'));
         $assignment->method('getId')->willReturn(1234);
         $assignment->method('getDestination')->willReturn($destination);
         $assignment->method('getJobProfile')->willReturn($jobProfile);