diff --git a/src/modules/Invoice/Repository/InvoiceRepository.php b/src/modules/Invoice/Repository/InvoiceRepository.php index 1f5ee2127..f542b2544 100644 --- a/src/modules/Invoice/Repository/InvoiceRepository.php +++ b/src/modules/Invoice/Repository/InvoiceRepository.php @@ -246,6 +246,25 @@ class InvoiceRepository extends EntityRepository ->getResult(); } + /** + * Unpaid invoices whose due date is more than the given number of days + * in the past. Used by the cron cleanup that expires stale unpaid + * invoices. + * + * @return Invoice[] + */ + public function findUnpaidOlderThan(int $days): array + { + return $this->createQueryBuilder('i') + ->andWhere('i.status = :status') + ->andWhere('i.dueAt IS NOT NULL') + ->andWhere('DATE_DIFF(CURRENT_TIMESTAMP(), i.dueAt) > :days') + ->setParameter('status', Invoice::STATUS_UNPAID) + ->setParameter('days', $days) + ->getQuery() + ->getResult(); + } + /** * Latest invoice that has a non-null nr — fallback for invoice * number generation. diff --git a/src/modules/Invoice/Service.php b/src/modules/Invoice/Service.php index a202ff32d..04b4f4289 100644 --- a/src/modules/Invoice/Service.php +++ b/src/modules/Invoice/Service.php @@ -619,10 +619,16 @@ class Service implements InjectionAwareInterface $systemService = $di['mod_service']('System'); $remove_after_days = $systemService->getParamValue('remove_after_days'); if (isset($remove_after_days) && $remove_after_days) { - // removing old invoices + // removing old unpaid invoices, through rmInvoice() so related + // orders, invoice items, and reserved resources stay consistent $days = (int) $remove_after_days; - $sql = 'DELETE FROM invoice WHERE status = :status AND DATEDIFF(NOW(), due_at) > :days'; - $di['em']->getConnection()->executeStatement($sql, ['days' => $days, 'status' => Invoice::STATUS_UNPAID]); + $service = $di['mod_service']('invoice'); + $invoices = $service->getInvoiceRepository()->findUnpaidOlderThan($days); + foreach ($invoices as $invoiceModel) { + $id = $invoiceModel->getId(); + $service->rmInvoice($invoiceModel); + $di['logger']->info('Removed expired unpaid invoice #{id}', ['id' => $id]); + } } } diff --git a/src/modules/Invoice/tests/Unit/Repository/InvoiceRepositoryTest.php b/src/modules/Invoice/tests/Unit/Repository/InvoiceRepositoryTest.php index 378d6750d..079846bd2 100644 --- a/src/modules/Invoice/tests/Unit/Repository/InvoiceRepositoryTest.php +++ b/src/modules/Invoice/tests/Unit/Repository/InvoiceRepositoryTest.php @@ -242,3 +242,42 @@ test('getInvoiceTotals omits invoices without items', function (): void { expect($totals)->toBe([]); }); + +test('findUnpaidOlderThan returns only unpaid invoices whose due date is far enough in the past', function (): void { + $config = ORMSetup::createAttributeMetadataConfig([Path::join(__DIR__, '..', '..', '..', 'Entity')], true); + $config->setProxyDir(sys_get_temp_dir()); + $config->setProxyNamespace('FOSSBilling\\Tests\\DoctrineProxies'); + $entityManager = new EntityManager(DriverManager::getConnection(['driver' => 'pdo_sqlite', 'memory' => true]), $config); + + $metadata = array_map( + $entityManager->getClassMetadata(...), + [Invoice::class, InvoiceItem::class], + ); + (new Doctrine\ORM\Tools\SchemaTool($entityManager))->createSchema($metadata); + + $farOverdue = new Invoice(); + $farOverdue->setStatus(Invoice::STATUS_UNPAID); + $farOverdue->setDueAt(new DateTime('-10 days')); + $entityManager->persist($farOverdue); + + $recentlyOverdue = new Invoice(); + $recentlyOverdue->setStatus(Invoice::STATUS_UNPAID); + $recentlyOverdue->setDueAt(new DateTime('-2 days')); + $entityManager->persist($recentlyOverdue); + + $noDueDate = new Invoice(); + $noDueDate->setStatus(Invoice::STATUS_UNPAID); + $entityManager->persist($noDueDate); + + $paidButOverdue = new Invoice(); + $paidButOverdue->setStatus(Invoice::STATUS_PAID); + $paidButOverdue->setDueAt(new DateTime('-10 days')); + $entityManager->persist($paidButOverdue); + + $entityManager->flush(); + + $result = $entityManager->getRepository(Invoice::class)->findUnpaidOlderThan(5); + + expect($result)->toHaveCount(1) + ->and($result[0]->getId())->toBe($farOverdue->getId()); +}); diff --git a/src/modules/Invoice/tests/Unit/ServiceTest.php b/src/modules/Invoice/tests/Unit/ServiceTest.php index 99c211efb..aa9b652cc 100644 --- a/src/modules/Invoice/tests/Unit/ServiceTest.php +++ b/src/modules/Invoice/tests/Unit/ServiceTest.php @@ -451,7 +451,6 @@ test('handles after admin invoice reminder sent event', function (): void { }); test('handles after admin cron run event', function (): void { - $service = new Service(); $eventMock = Mockery::mock('\Box_Event'); $remove_after_days = 64; @@ -461,20 +460,31 @@ test('handles after admin cron run event', function (): void { ->atLeast()->once() ->andReturn($remove_after_days); - $connection = Mockery::mock(Doctrine\DBAL\Connection::class); - $connection->shouldReceive('executeStatement') - ->atLeast()->once(); + $invoiceModel = createEntity(Invoice::class, ['id' => 1]); + + $invoiceServiceMock = Mockery::mock(Service::class)->makePartial()->shouldAllowMockingProtectedMethods(); + $invoiceServiceMock->shouldReceive('rmInvoice') + ->once() + ->with($invoiceModel) + ->andReturn(true); $di = container(); - $di['mod_service'] = $di->protect(fn (): Mockery\MockInterface => $systemServiceMock); - $di['em']->shouldReceive('getConnection')->andReturn($connection); + $di['em']->getRepository(Invoice::class)->shouldReceive('findUnpaidOlderThan') + ->with(64) + ->once() + ->andReturn([$invoiceModel]); + $di['mod_service'] = $di->protect(fn (string $name = ''): object => match (strtolower($name)) { + 'system' => $systemServiceMock, + 'invoice' => $invoiceServiceMock, + default => Mockery::mock()->shouldIgnoreMissing(), + }); - $service->setDi($di); + $invoiceServiceMock->setDi($di); $eventMock->shouldReceive('getDi') ->atLeast()->once() ->andReturn($di); - $service->onAfterAdminCronRun($eventMock); + Service::onAfterAdminCronRun($eventMock); }); test('uses the client billing email for invoice notifications', function (): void { diff --git a/tests/Helpers/Container.php b/tests/Helpers/Container.php index 45ad4aade..90db03e33 100644 --- a/tests/Helpers/Container.php +++ b/tests/Helpers/Container.php @@ -273,6 +273,7 @@ function container(): Container $invoiceRepository->shouldReceive('findPaid')->byDefault()->andReturn([]); $invoiceRepository->shouldReceive('findByClientId')->byDefault()->andReturn([]); $invoiceRepository->shouldReceive('findUnpaidApprovedNotRemindedBefore')->byDefault()->andReturn([]); + $invoiceRepository->shouldReceive('findUnpaidOlderThan')->byDefault()->andReturn([]); $invoiceRepository->shouldReceive('findPaidByRelId')->byDefault()->andReturn([]); $invoiceItemRepository = \Mockery::mock(\Box\Mod\Invoice\Repository\InvoiceItemRepository::class)->shouldIgnoreMissing();