From 489aafed3eacff2148f2c4209ccdf83c6c426b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Mon, 17 Aug 2026 11:55:02 +0200 Subject: [PATCH] fix: correctly apply due dates for contract and invoice upload reminders --- config/email_texts.yaml | 13 ++ config/services.yaml | 13 +- src/Config/EmailTextKey.php | 1 + src/Controller/Admin/IndexController.php | 4 +- src/Controller/Teamer/IndexController.php | 7 +- src/Entity/Assignment.php | 12 +- src/Entity/Disposition.php | 30 ++- .../DispositionWorkflowGuardSubscriber.php | 19 +- src/Model/TimelineFilterDto.php | 8 +- src/Repository/ApplicationRepository.php | 6 +- src/Repository/DispositionRepository.php | 133 ++++++++++++- src/Service/Assignment/TimelineService.php | 8 +- src/Service/Common/TimelineFilterHandler.php | 8 +- src/Service/Cron/UploadReminderService.php | 155 ++++++--------- src/Service/Teamer/AvailabilityProcessor.php | 4 +- src/Twig/AppRuntime.php | 10 +- tests/Entity/DispositionTest.php | 96 ++++++++++ .../InvalidateApplicationsListenerTest.php | 14 +- .../Repository/ApplicationRepositoryTest.php | 8 +- .../Repository/DispositionRepositoryTest.php | 158 ++++++++++++++++ .../Cron/UploadReminderServiceTest.php | 177 ++++++++++++++++++ tests/Service/Pdf/InvoiceRendererTest.php | 4 +- 22 files changed, 732 insertions(+), 156 deletions(-) create mode 100644 tests/Entity/DispositionTest.php create mode 100644 tests/Repository/DispositionRepositoryTest.php create mode 100644 tests/Service/Cron/UploadReminderServiceTest.php diff --git a/config/email_texts.yaml b/config/email_texts.yaml index 8c546ec..a2f6296 100644 --- a/config/email_texts.yaml +++ b/config/email_texts.yaml @@ -96,6 +96,19 @@ texts: body: |- vielen Dank für deinen Einsatz {destination}. Bitte lade deine Honorarnote innerhalb der nächsten {invoiceUploadDeadlineDays} Tage hoch, damit wir dein Honorar bearbeiten können. Solltest du deine Honorarnote zu spät hochladen, können wir sie eventuell nicht mehr annehmen. + # Goes out on the last day of the upload period, so it must not promise any time left. + reminder_invoice_upload_final: + label: 'Letzte Erinnerung: Honorarnote hochladen' + placeholders: ['destination', 'invoiceUploadDeadlineDays'] + subject: 'Letzte Erinnerung: Deine Honorarnote fehlt' + headline: 'Hallo aus Köln,' + body: |- + für deinen Einsatz {destination} liegt uns noch immer keine Honorarnote vor. + + Heute endet die Frist von {invoiceUploadDeadlineDays} Tagen nach deinem Einsatz. *Bitte lade deine Honorarnote noch heute hoch* - danach können wir sie eventuell nicht mehr annehmen. + + Bei Fragen melde dich gerne unter team@ep-reisen.de + reminder_disposition: label: 'Erinnerung: Einsatz steht bevor' placeholders: ['destination', 'jobProfile', 'product', 'dateFrom', 'dateTo', 'diffInDays', 'invoiceUploadDeadlineDays'] diff --git a/config/services.yaml b/config/services.yaml index cd4c912..ebf70e2 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -9,11 +9,14 @@ parameters: teamer_inactive_period: '-2 years' - # Deadlines quoted to teamers in the transactional mails. They are passed to the mail - # texts as placeholders rather than typed into the wording, so what an admin edits can - # never drift from the period the application actually works with. - contract_upload_deadline_days: 7 - # Also drives when the reminder goes out: three days before the deadline expires. + # The contract upload period, counted from the creation of the disposition. This is the + # single definition: the reminder mail, the "Honorarvertrag fällig" flag on the teamer + # dashboard, the admin list of overdue contracts and the wording of the mails all read it, + # so they cannot mark the same disposition due on three different days. + contract_upload_deadline_days: 5 + + # The invoice upload period, counted from the end of the assignment. The teamer is + # reminded the day after the assignment ends and again on the last day of this period. invoice_upload_deadline_days: 14 # Messenger transport the mails of a teamer mailing are queued on. An X-Bus-Transport diff --git a/src/Config/EmailTextKey.php b/src/Config/EmailTextKey.php index 0b7046e..515fcc9 100644 --- a/src/Config/EmailTextKey.php +++ b/src/Config/EmailTextKey.php @@ -17,5 +17,6 @@ enum EmailTextKey: string case DOCUMENT_REJECTED = 'document_rejected'; case REMINDER_CONTRACT_UPLOAD = 'reminder_contract_upload'; case REMINDER_INVOICE_UPLOAD = 'reminder_invoice_upload'; + case REMINDER_INVOICE_UPLOAD_FINAL = 'reminder_invoice_upload_final'; case REMINDER_DISPOSITION = 'reminder_disposition'; } diff --git a/src/Controller/Admin/IndexController.php b/src/Controller/Admin/IndexController.php index 1be426c..297e6b6 100644 --- a/src/Controller/Admin/IndexController.php +++ b/src/Controller/Admin/IndexController.php @@ -48,7 +48,9 @@ class IndexController extends AbstractController $newDriverLicensesCount = $uploadRepository->getCountByStatusAndType(Upload::STATUS_NEW, Upload::TYPE_DRIVER_LICENSE); $dispositionRepository = $this->entityManager->getRepository(Disposition::class); - $overdueContracts = $dispositionRepository->findOverdueContracts(); + $overdueContracts = $dispositionRepository->findOverdueContracts( + (int) $this->getParameter('contract_upload_deadline_days') + ); $newDispositions = $dispositionRepository->getNew(); $overdueFeedbacks = $dispositionRepository->findDispositionsWithOverdueFeedback(7); diff --git a/src/Controller/Teamer/IndexController.php b/src/Controller/Teamer/IndexController.php index 39950a0..75881c3 100644 --- a/src/Controller/Teamer/IndexController.php +++ b/src/Controller/Teamer/IndexController.php @@ -80,11 +80,16 @@ class IndexController extends AbstractController private function processDocuments(array $upcomingDispositions): array { + $contractUploadDeadlineDays = (int) $this->getParameter('contract_upload_deadline_days'); + $invoiceUploadDeadlineDays = (int) $this->getParameter('invoice_upload_deadline_days'); $dispositions = []; foreach ($upcomingDispositions as $disposition) { /** @var Disposition $disposition */ - if ($disposition->isContractDue() || $disposition->isContractRejected() || $disposition->isInvoiceDue()) { + if ($disposition->isContractDue($contractUploadDeadlineDays) + || $disposition->isContractRejected() + || $disposition->isInvoiceDue($invoiceUploadDeadlineDays) + ) { $dispositions[] = $disposition; } } diff --git a/src/Entity/Assignment.php b/src/Entity/Assignment.php index b74473d..8420aa8 100644 --- a/src/Entity/Assignment.php +++ b/src/Entity/Assignment.php @@ -6,7 +6,7 @@ use App\Entity\Traits\BlameableEntity; use App\Entity\Traits\SoftDeletableEntity; use App\Entity\Traits\TimestampableEntity; use App\Repository\AssignmentRepository; -use Carbon\CarbonPeriod; +use Carbon\CarbonPeriodImmutable; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\DBAL\Types\Types; @@ -270,13 +270,19 @@ class Assignment implements BlameableEntityInterface, TimestampableEntityInterfa return $this; } - public function getEffectivePeriod(): ?CarbonPeriod + /** + * Immutable on purpose: a plain CarbonPeriod hands out mutable Carbon instances, so a caller + * deriving one date from another - `$end = $period->getEndDate(); $end->addDays(14)` - moves + * the original as well and silently collapses its own period. CarbonPeriodImmutable yields + * CarbonImmutable from start, end and getEndDate(), which makes that mistake impossible. + */ + public function getEffectivePeriod(): ?CarbonPeriodImmutable { if (null !== $destination = $this->getDestination()) { $dateFrom = $this->getDateFrom() ?? $destination->getDateFrom(); $dateTo = $this->getDateTo() ?? $destination->getDateTo(); - return new CarbonPeriod($dateFrom, $dateTo); + return new CarbonPeriodImmutable($dateFrom, $dateTo); } return null; diff --git a/src/Entity/Disposition.php b/src/Entity/Disposition.php index 0d30622..7274af8 100644 --- a/src/Entity/Disposition.php +++ b/src/Entity/Disposition.php @@ -5,7 +5,7 @@ namespace App\Entity; use App\Entity\Traits\BlameableEntity; use App\Entity\Traits\TimestampableEntity; use App\Repository\DispositionRepository; -use Carbon\CarbonPeriod; +use Carbon\CarbonPeriodImmutable; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\DBAL\Types\Types; @@ -218,7 +218,11 @@ class Disposition implements BlameableEntityInterface, TimestampableEntityInterf return $this; } - public function isContractDue(): bool + /** + * The period is passed in rather than hardcoded so that this, the reminder mail and the + * admin overdue list all work from the same %contract_upload_deadline_days%. + */ + public function isContractDue(int $deadlineDays): bool { if (null !== $this->getDocumentByType(Upload::TYPE_CONTRACT)) { return false; @@ -226,12 +230,15 @@ class Disposition implements BlameableEntityInterface, TimestampableEntityInterf $assignment = $this->getAssignment(); $assignmentPeriod = $assignment->getEffectivePeriod(); - $dueDateFrom = $this->getCreatedAt()->modify('+7 days'); + $dueDateFrom = $this->getCreatedAt()->modify(sprintf('+%d days', $deadlineDays)); $dueDateTo = $assignmentPeriod->end->modify('-1 day'); - $period = CarbonPeriod::create($dueDateFrom, $dueDateTo); + $period = CarbonPeriodImmutable::create($dueDateFrom, $dueDateTo); - return $period->isStarted(); + // isStarted() alone stays true once the period has begun, so the dashboard kept + // asking for a contract on the last day of the assignment - by which point + // DispositionWorkflowGuardSubscriber already refuses the upload. + return $period->isStarted() && false === $period->isEnded(); } public function isContractRejected(): bool @@ -243,7 +250,14 @@ class Disposition implements BlameableEntityInterface, TimestampableEntityInterf return $contractDocument->isRejected(); } - public function isInvoiceDue(): bool + /** + * The period is passed in rather than hardcoded so that this and the two invoice reminder + * mails all work from the same %invoice_upload_deadline_days%. + * + * Unlike isContractDue() this deliberately stays true once the period has run out: nothing + * blocks a late invoice upload, so the teamer should keep being asked for it. + */ + public function isInvoiceDue(int $deadlineDays): bool { if (null !== $this->getDocumentByType(Upload::TYPE_INVOICE)) { return false; @@ -251,8 +265,10 @@ class Disposition implements BlameableEntityInterface, TimestampableEntityInterf $assignment = $this->getAssignment(); $assignmentPeriod = $assignment->getEffectivePeriod(); + $dueDateFrom = $assignmentPeriod->getEndDate(); + $dueDateTo = $dueDateFrom->addDays($deadlineDays); - $period = CarbonPeriod::create($assignmentPeriod->getEndDate(), $assignmentPeriod->getEndDate()->modify('+14 days')); + $period = CarbonPeriodImmutable::create($dueDateFrom, $dueDateTo); return $period->isStarted(); } diff --git a/src/EventListener/DispositionWorkflowGuardSubscriber.php b/src/EventListener/DispositionWorkflowGuardSubscriber.php index f63915b..2f9e90a 100644 --- a/src/EventListener/DispositionWorkflowGuardSubscriber.php +++ b/src/EventListener/DispositionWorkflowGuardSubscriber.php @@ -15,8 +15,10 @@ use Symfony\Contracts\Translation\TranslatorInterface; */ class DispositionWorkflowGuardSubscriber implements EventSubscriberInterface { - public function __construct(private readonly TranslatorInterface $translator) - { + public function __construct( + private readonly TranslatorInterface $translator, + private readonly int $contractUploadDeadlineDays, + ) { } /** @@ -59,8 +61,12 @@ class DispositionWorkflowGuardSubscriber implements EventSubscriberInterface /** * Guards the contract upload transition. - * Blocks upload if the deadline has passed (day before assignment starts). - * Contracts must be uploaded within 7 days of disposition creation until assignment start. + * + * The only thing blocked here is an upload from the day before the assignment *ends* - + * deliberately late, so a teamer who is behind can still hand the contract in rather than + * having to go through the office. Passing the upload deadline is a soft signal + * (dashboard flag, admin overdue list, reminder mail), never a block, which is why + * $dueDateFrom below only feeds the message and takes no part in the decision. * * @param GuardEvent $event The workflow guard event */ @@ -72,7 +78,10 @@ class DispositionWorkflowGuardSubscriber implements EventSubscriberInterface $assignmentPeriod = $assignment->getEffectivePeriod(); $today = new \DateTimeImmutable(); - $dueDateFrom = $disposition->getCreatedAt()->modify('+ 7days'); + $dueDateFrom = $disposition + ->getCreatedAt() + ->modify(sprintf('+%d days', $this->contractUploadDeadlineDays)) + ; $dueDateTo = $assignmentPeriod->end->modify('-1 day'); // Block upload if deadline has passed diff --git a/src/Model/TimelineFilterDto.php b/src/Model/TimelineFilterDto.php index 700f890..7394937 100644 --- a/src/Model/TimelineFilterDto.php +++ b/src/Model/TimelineFilterDto.php @@ -2,12 +2,12 @@ namespace App\Model; -use Carbon\CarbonPeriod; +use Carbon\CarbonPeriodImmutable; class TimelineFilterDto extends AssignmentFilterDto { protected bool $includePast = true; - protected ?CarbonPeriod $_period = null; + protected ?CarbonPeriodImmutable $_period = null; public function getDateFrom(): ?\DateTimeImmutable { @@ -27,12 +27,12 @@ class TimelineFilterDto extends AssignmentFilterDto return $this->dateTo; } - public function getPeriod(): ?CarbonPeriod + public function getPeriod(): ?CarbonPeriodImmutable { return $this->_period; } - public function setPeriod(?CarbonPeriod $_period): static + public function setPeriod(?CarbonPeriodImmutable $_period): static { $this->_period = $_period; diff --git a/src/Repository/ApplicationRepository.php b/src/Repository/ApplicationRepository.php index 9a4e330..78149e7 100644 --- a/src/Repository/ApplicationRepository.php +++ b/src/Repository/ApplicationRepository.php @@ -8,7 +8,7 @@ use App\Entity\Teamer; use App\Model\ApplicationFilterDto; use App\Model\AssignmentFilterDto; use App\Repository\Traits\QueryHelperTrait; -use Carbon\CarbonPeriod; +use Carbon\CarbonPeriodImmutable; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\ORM\Query; use Doctrine\Persistence\ManagerRegistry; @@ -172,7 +172,7 @@ class ApplicationRepository extends ServiceEntityRepository * * @return array */ - public function findOverlappingForTeamer(Teamer $teamer, CarbonPeriod $period): array + public function findOverlappingForTeamer(Teamer $teamer, CarbonPeriodImmutable $period): array { return $this ->getOverlappingForTeamerQuery($teamer, $period) @@ -180,7 +180,7 @@ class ApplicationRepository extends ServiceEntityRepository ; } - public function getOverlappingForTeamerQuery(Teamer $teamer, CarbonPeriod $period): Query + public function getOverlappingForTeamerQuery(Teamer $teamer, CarbonPeriodImmutable $period): Query { $qb = $this->createQueryBuilder('application'); diff --git a/src/Repository/DispositionRepository.php b/src/Repository/DispositionRepository.php index f04bdb5..16e485e 100644 --- a/src/Repository/DispositionRepository.php +++ b/src/Repository/DispositionRepository.php @@ -5,8 +5,12 @@ namespace App\Repository; use App\Entity\Assignment; use App\Entity\Disposition; use App\Entity\Teamer; +use App\Entity\Upload; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\ORM\Query; +use Doctrine\ORM\Query\Expr\Join; +use Doctrine\ORM\Query\Expr\Orx; +use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ManagerRegistry; /** @@ -481,9 +485,129 @@ class DispositionRepository extends ServiceEntityRepository }, $results); } - public function findOverdueContracts(): array + /** + * Dispositions to remind about a missing contract, on the day the upload period runs out. + * + * Anchored on the creation of the disposition, which is what the upload period counts + * from. The range is a half-open day rather than an equality: disposition.createdAt is a + * timestamp, and the cron runs at 01:00, so "=" would only ever match a disposition that + * happened to be created at that exact second. + * + * @param int $deadlineDays %contract_upload_deadline_days% + */ + public function findDispositionsForContractReminder(int $deadlineDays): array { - $dueDate = (new \DateTimeImmutable())->modify('-7 days'); + return $this->getContractReminderQuery($deadlineDays)->getResult(); + } + + public function getContractReminderQuery(int $deadlineDays): Query + { + $dayStart = (new \DateTimeImmutable('today'))->modify(sprintf('-%d days', $deadlineDays)); + + $qb = $this->createQueryBuilder('disposition'); + + return $qb + ->select('disposition', 'assignment', 'destination', 'teamer') + ->innerJoin('disposition.assignment', 'assignment') + ->innerJoin('assignment.destination', 'destination') + // restricted to the contract, so an unrelated upload does not suppress the reminder + ->leftJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') + ->innerJoin('disposition.teamer', 'teamer') + ->where($qb->expr()->andX( + $qb->expr()->isNull('document'), + // deleted teamers are excluded here rather than when sending, so that the + // count the caller reports stays truthful + $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'), + )) + ->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')) + ->getQuery() + ; + } + + /** + * Dispositions to remind about a missing invoice, for assignments ending on one given day. + * + * Both sends - the day after the assignment ends and again on the last day of the upload + * period - come through here with a different $endDate, which is what keeps each of them + * a one-shot without needing a record of what has already been sent. + * + * Anchored on the end date rather than Disposition::STATUS_ENDED on purpose: there is no + * column recording when a disposition ended, and DispositionStatusService runs after the + * reminders in CronCommand, so a status-based rule would always be one run late. + */ + public function findDispositionsForInvoiceReminder(\DateTimeImmutable $endDate): array + { + return $this->getInvoiceReminderQuery($endDate)->getResult(); + } + + public function getInvoiceReminderQuery(\DateTimeImmutable $endDate): Query + { + $qb = $this->createQueryBuilder('disposition'); + + return $qb + ->select('disposition', 'assignment', 'destination', 'teamer') + ->innerJoin('disposition.assignment', 'assignment') + ->innerJoin('assignment.destination', 'destination') + ->leftJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') + ->innerJoin('disposition.teamer', 'teamer') + ->where($qb->expr()->andX( + $qb->expr()->isNull('document'), + $qb->expr()->isNull('teamer.deletedAt'), + $qb->expr()->notIn('disposition.status', ':dispositionStatus'), + $qb->expr()->notIn('assignment.status', ':assignmentStatus'), + $this->effectiveDateToEquals($qb, ':endDate'), + )) + ->setParameter('documentType', Upload::TYPE_INVOICE) + ->setParameter('dispositionStatus', [Disposition::STATUS_CALLED_OFF, Disposition::STATUS_COMPLETED]) + ->setParameter('assignmentStatus', [Assignment::STATUS_CALLED_OFF, Assignment::STATUS_DELETED]) + // bound as a date string, not as a DateTimeImmutable: both columns are DATE, and + // Doctrine would otherwise bind 'Y-m-d H:i:s', which a DATE never equals + ->setParameter('endDate', $endDate->format('Y-m-d')) + ->getQuery() + ; + } + + /** + * Compares the assignment's end date against $parameter, falling back to the destination's + * when the assignment does not override it. + * + * The fallback is guarded on both sides - without the isNull() on the second branch an + * assignment that moves the end date into the future would still match on the + * destination's date. Kept in one place because the per-assignment date override is due to + * be removed (docs/remove-assignment-date-override.md), and this then collapses to a plain + * comparison on destination.dateTo. + */ + private function effectiveDateToEquals(QueryBuilder $qb, string $parameter): Orx + { + return $qb->expr()->orX( + $qb->expr()->andX( + $qb->expr()->isNotNull('assignment.dateTo'), + $qb->expr()->eq('assignment.dateTo', $parameter) + ), + $qb->expr()->andX( + $qb->expr()->isNull('assignment.dateTo'), + $qb->expr()->eq('destination.dateTo', $parameter) + ), + ); + } + + /** + * Dispositions whose contract upload period has run out, for the admin dashboard. + * + * @param int $deadlineDays %contract_upload_deadline_days%, so this list marks a + * disposition overdue on the same day the reminder mail goes out + */ + public function findOverdueContracts(int $deadlineDays): array + { + $dueDate = (new \DateTimeImmutable())->modify(sprintf('-%d days', $deadlineDays)); $qb = $this->createQueryBuilder('disposition'); @@ -492,7 +616,9 @@ class DispositionRepository extends ServiceEntityRepository ->innerJoin('disposition.assignment', 'assignment') ->innerJoin('assignment.jobProfile', 'job_profile') ->innerJoin('assignment.destination', 'destination') - ->leftJoin('disposition.documents', 'document') + // the join is restricted to the contract, so that an unrelated upload - a + // driver's licence, say - does not hide a disposition from this list + ->leftJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') ->innerJoin('disposition.teamer', 'teamer') ->innerJoin('teamer.user', 'user') ->where($qb->expr()->andX( @@ -500,6 +626,7 @@ class DispositionRepository extends ServiceEntityRepository $qb->expr()->eq('disposition.status', ':status'), $qb->expr()->isNull('document') )) + ->setParameter('documentType', Upload::TYPE_CONTRACT) ->setParameter('dueDate', $dueDate) ->setParameter('status', Disposition::STATUS_NEW) ->orderBy('disposition.createdAt', 'ASC') diff --git a/src/Service/Assignment/TimelineService.php b/src/Service/Assignment/TimelineService.php index 4868b1a..7910fa6 100644 --- a/src/Service/Assignment/TimelineService.php +++ b/src/Service/Assignment/TimelineService.php @@ -4,7 +4,7 @@ namespace App\Service\Assignment; use App\Entity\Assignment; use App\Model\TimelineItem; -use Carbon\CarbonPeriod; +use Carbon\CarbonPeriodImmutable; /** * Service for processing assignments into timeline visualization data. @@ -110,13 +110,13 @@ class TimelineService * Returns a period from 6 weeks ago to 6 months in the future, * providing context for both recent and upcoming assignments. * - * @return CarbonPeriod The active time window for timeline display + * @return CarbonPeriodImmutable The active time window for timeline display */ - public function getActiveWindow(): CarbonPeriod + public function getActiveWindow(): CarbonPeriodImmutable { $dateFrom = (new \DateTimeImmutable())->modify('-6 weeks'); $dateTo = (new \DateTimeImmutable())->modify('+6 months'); - return CarbonPeriod::create($dateFrom, $dateTo); + return CarbonPeriodImmutable::create($dateFrom, $dateTo); } } diff --git a/src/Service/Common/TimelineFilterHandler.php b/src/Service/Common/TimelineFilterHandler.php index 47fe61c..31c5f5b 100644 --- a/src/Service/Common/TimelineFilterHandler.php +++ b/src/Service/Common/TimelineFilterHandler.php @@ -4,16 +4,16 @@ namespace App\Service\Common; use App\Model\AbstractFilterDto; use App\Model\TimelineFilterDto; -use Carbon\CarbonPeriod; +use Carbon\CarbonPeriodImmutable; class TimelineFilterHandler extends AbstractFilterHandler { protected string $namespace = 'filter:timeline'; protected string $modelClass = TimelineFilterDto::class; - protected ?CarbonPeriod $period = null; + protected ?CarbonPeriodImmutable $period = null; - public function setPeriod(CarbonPeriod $period): void + public function setPeriod(CarbonPeriodImmutable $period): void { $this->period = $period; } @@ -44,7 +44,7 @@ class TimelineFilterHandler extends AbstractFilterHandler $filterDto->setHotels($data['hotels']); } - $filterDto->setPeriod(new CarbonPeriod($periodFrom, $periodTo)); + $filterDto->setPeriod(new CarbonPeriodImmutable($periodFrom, $periodTo)); return $filterDto; } diff --git a/src/Service/Cron/UploadReminderService.php b/src/Service/Cron/UploadReminderService.php index ed2dd4e..db91dc3 100644 --- a/src/Service/Cron/UploadReminderService.php +++ b/src/Service/Cron/UploadReminderService.php @@ -5,61 +5,35 @@ namespace App\Service\Cron; use App\Config\EmailTextKey; use App\Email\Mailer; use App\Entity\Disposition; -use App\Entity\Upload; use App\Repository\DispositionRepository; -use Doctrine\Common\Collections\ArrayCollection; -use Doctrine\ORM\Query\Expr\Join; -use Doctrine\ORM\Query\Parameter; use Psr\Log\LoggerInterface; +/** + * The nightly reminders about documents a teamer still owes us. + * + * Both rules pick out exactly one day, which is what makes them idempotent: the cron can run + * twice without mailing anyone twice, and nothing has to record what was already sent. + */ class UploadReminderService { - /** - * How long before the invoice deadline the reminder goes out. - */ - private const INVOICE_REMINDER_LEAD_DAYS = 3; - public function __construct( private readonly DispositionRepository $dispositionRepository, private readonly Mailer $mailer, private readonly LoggerInterface $logger, + private readonly int $contractUploadDeadlineDays, private readonly int $invoiceUploadDeadlineDays, ) { } + /** + * Reminds about a missing contract on the last day of the upload period, counted from the + * creation of the disposition. + */ public function sendContractUploadReminders(): string { - // Contracts have to be uploaded before assignments' begin date. Reminder is - // sent 7 and 14 days after disposition has been created. - $contractDueDateFirst = (new \DateTimeImmutable())->modify('-7 days'); - $contractDueDateSecond = (new \DateTimeImmutable())->modify('-14 days'); - - // Find dispositions of assignments with due contract upload - $qb = $this->dispositionRepository->createQueryBuilder('disposition'); - - $dispositions = $qb - ->select('disposition', 'assignment', 'destination') - ->innerJoin('disposition.assignment', 'assignment') - ->innerJoin('assignment.destination', 'destination') - ->leftJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') - ->innerJoin('disposition.teamer', 'teamer') - ->where($qb->expr()->andX( - $qb->expr()->isNull('document'), - // deleted teamers are excluded here rather than when sending, so that the - // count reported below stays truthful - $qb->expr()->isNull('teamer.deletedAt'), - $qb->expr()->orX( - $qb->expr()->eq('document.createdAt', ':contractDueDateFirst'), - $qb->expr()->eq('document.createdAt', ':contractDueDateSecond'), - ) - )) - ->setParameters(new ArrayCollection([ - new Parameter('documentType', Upload::TYPE_CONTRACT), - new Parameter('contractDueDateFirst', $contractDueDateFirst), - new Parameter('contractDueDateSecond', $contractDueDateSecond), - ])) - ->getQuery() - ->getResult() + $dispositions = $this + ->dispositionRepository + ->findDispositionsForContractReminder($this->contractUploadDeadlineDays) ; if (0 === $count = count($dispositions)) { @@ -70,12 +44,8 @@ class UploadReminderService } foreach ($dispositions as $disposition) { - $teamer = $disposition->getTeamer(); - - $this->mailer->createAndSendText(EmailTextKey::REMINDER_CONTRACT_UPLOAD, [ + $this->send(EmailTextKey::REMINDER_CONTRACT_UPLOAD, $disposition, [ 'destination' => (string) $disposition->getAssignment()->getDestination(), - ], [ - 'to' => $teamer->getCommunication()->getEmail(), ]); } @@ -85,65 +55,58 @@ class UploadReminderService return $message; } + /** + * Reminds about a missing invoice twice: the day after the assignment ended, and again on + * the last day of the upload period. The second mail has its own wording, because by then + * there is no time left to promise. + */ public function sendInvoiceUploadReminders(): string { - // Invoices have to be uploaded until invoiceUploadDeadlineDays after end of - // assignment. Reminder is sent three days before end of this period, so the - // offset follows the deadline the mail itself quotes. - $invoiceDueDate = (new \DateTimeImmutable()) - ->modify(sprintf('-%d days', $this->invoiceUploadDeadlineDays - self::INVOICE_REMINDER_LEAD_DAYS)) - ; + $today = new \DateTimeImmutable('today'); - // Find dispositions of assignments with due invoice upload - $qb = $this->dispositionRepository->createQueryBuilder('disposition'); + $count = $this->sendInvoiceUploadRemindersForEndDate( + $today->modify('-1 day'), + EmailTextKey::REMINDER_INVOICE_UPLOAD + ); - $dispositions = $qb - ->select('disposition', 'assignment', 'destination') - ->innerJoin('disposition.assignment', 'assignment') - ->innerJoin('assignment.destination', 'destination') - ->leftJoin('disposition.documents', 'document', Join::WITH, 'document.type = :documentType') - ->innerJoin('disposition.teamer', 'teamer') - ->where($qb->expr()->andX( - $qb->expr()->isNull('document'), - $qb->expr()->isNull('teamer.deletedAt'), - $qb->expr()->orX( - // Due date can be determined by assignment's date which potentially overrides destination date - $qb->expr()->andX( - $qb->expr()->isNotNull('assignment.dateTo'), - $qb->expr()->eq('assignment.dateTo', ':dateTo') - ), - $qb->expr()->eq('destination.dateTo', ':dateTo'), - ) - )) - ->setParameters(new ArrayCollection([ - new Parameter('documentType', Upload::TYPE_INVOICE), - new Parameter('dateTo', $invoiceDueDate), - ])) - ->getQuery() - ->getResult() - ; + $count += $this->sendInvoiceUploadRemindersForEndDate( + $today->modify(sprintf('-%d days', $this->invoiceUploadDeadlineDays)), + EmailTextKey::REMINDER_INVOICE_UPLOAD_FINAL + ); - if (0 === $count = count($dispositions)) { - $message = 'No due invoice upload reminders to be sent to teamers'; - $this->logger->info($message); + $message = 0 === $count + ? 'No due invoice upload reminders to be sent to teamers' + : 'Sent '.$count.' due invoice upload reminders to teamers'; - return $message; - } - - foreach ($dispositions as $disposition) { - $teamer = $disposition->getTeamer(); - - $this->mailer->createAndSendText(EmailTextKey::REMINDER_INVOICE_UPLOAD, [ - 'destination' => (string) $disposition->getAssignment()->getDestination(), - 'invoiceUploadDeadlineDays' => $this->invoiceUploadDeadlineDays, - ], [ - 'to' => $teamer->getCommunication()->getEmail(), - ]); - } - - $message = 'Sent '.$count.' due invoice upload reminders to teamers'; $this->logger->info($message); return $message; } + + private function sendInvoiceUploadRemindersForEndDate(\DateTimeImmutable $endDate, EmailTextKey $key): int + { + $dispositions = $this + ->dispositionRepository + ->findDispositionsForInvoiceReminder($endDate) + ; + + foreach ($dispositions as $disposition) { + $this->send($key, $disposition, [ + 'destination' => (string) $disposition->getAssignment()->getDestination(), + 'invoiceUploadDeadlineDays' => $this->invoiceUploadDeadlineDays, + ]); + } + + return count($dispositions); + } + + /** + * @param array $placeholders + */ + private function send(EmailTextKey $key, Disposition $disposition, array $placeholders): void + { + $this->mailer->createAndSendText($key, $placeholders, [ + 'to' => $disposition->getTeamer()->getCommunication()->getEmail(), + ]); + } } diff --git a/src/Service/Teamer/AvailabilityProcessor.php b/src/Service/Teamer/AvailabilityProcessor.php index 2d9de71..5e5a662 100644 --- a/src/Service/Teamer/AvailabilityProcessor.php +++ b/src/Service/Teamer/AvailabilityProcessor.php @@ -3,7 +3,7 @@ namespace App\Service\Teamer; use App\Entity\Availability; -use Carbon\Carbon; +use Carbon\CarbonImmutable; class AvailabilityProcessor { @@ -28,7 +28,7 @@ class AvailabilityProcessor /** @var Availability $record */ $dateFrom = $record->getDateFrom(); $year = $dateFrom->format('Y'); - $month = (new Carbon($dateFrom))->locale('de_DE')->monthName; + $month = (new CarbonImmutable($dateFrom))->locale('de_DE')->monthName; if (false === isset($availabilities[$year])) { $availabilities[$year] = []; diff --git a/src/Twig/AppRuntime.php b/src/Twig/AppRuntime.php index d32b428..ab03b05 100644 --- a/src/Twig/AppRuntime.php +++ b/src/Twig/AppRuntime.php @@ -8,7 +8,7 @@ use App\Entity\Teamer; use App\Entity\Upload; use App\Service\Teamer\PickupResolver; use App\Service\Upload\UploadHandler; -use Carbon\Carbon; +use Carbon\CarbonImmutable; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Contracts\Translation\TranslatorInterface; use Twig\Environment; @@ -222,13 +222,13 @@ class AppRuntime implements RuntimeExtensionInterface public function dateDiffForHumans(\DateTimeInterface $dateTime, ?\DateTimeInterface $other = null): string { - $oldLocale = Carbon::getLocale(); + $oldLocale = CarbonImmutable::getLocale(); $locale = $this->requestStack->getMainRequest()->getLocale(); - Carbon::setLocale($locale); + CarbonImmutable::setLocale($locale); - $result = Carbon::instance($dateTime)->diffForHumans($other); + $result = CarbonImmutable::instance($dateTime)->diffForHumans($other); - Carbon::setLocale($oldLocale); + CarbonImmutable::setLocale($oldLocale); return $result; } diff --git a/tests/Entity/DispositionTest.php b/tests/Entity/DispositionTest.php new file mode 100644 index 0000000..279de8c --- /dev/null +++ b/tests/Entity/DispositionTest.php @@ -0,0 +1,96 @@ +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)); + } +} diff --git a/tests/EventListener/InvalidateApplicationsListenerTest.php b/tests/EventListener/InvalidateApplicationsListenerTest.php index 59af75f..3ca2e72 100644 --- a/tests/EventListener/InvalidateApplicationsListenerTest.php +++ b/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); diff --git a/tests/Repository/ApplicationRepositoryTest.php b/tests/Repository/ApplicationRepositoryTest.php index f155089..812e910 100644 --- a/tests/Repository/ApplicationRepositoryTest.php +++ b/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') ); } } diff --git a/tests/Repository/DispositionRepositoryTest.php b/tests/Repository/DispositionRepositoryTest.php new file mode 100644 index 0000000..fe06926 --- /dev/null +++ b/tests/Repository/DispositionRepositoryTest.php @@ -0,0 +1,158 @@ +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; + } +} diff --git a/tests/Service/Cron/UploadReminderServiceTest.php b/tests/Service/Cron/UploadReminderServiceTest.php new file mode 100644 index 0000000..7163c82 --- /dev/null +++ b/tests/Service/Cron/UploadReminderServiceTest.php @@ -0,0 +1,177 @@ +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' => 'teamer@example.com'], + ) + ; + + $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('teamer@example.com') + ); + + return new Disposition(new Application($assignment, $teamer)); + } +} diff --git a/tests/Service/Pdf/InvoiceRendererTest.php b/tests/Service/Pdf/InvoiceRendererTest.php index 4608e5f..ea8b0a9 100644 --- a/tests/Service/Pdf/InvoiceRendererTest.php +++ b/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);