From 45e5578f9e730927997e30cd1d469cef2f4f0deb Mon Sep 17 00:00:00 2001 From: louis Date: Thu, 9 Apr 2026 14:16:13 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Add=20maintenance=20task=20to=20rem?= =?UTF-8?q?ove=20unredeemed=20vouchers=20of=20deleted=20users?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a scheduled safety-net task that removes unredeemed vouchers belonging to soft-deleted users. While DeleteHandler already cleans up vouchers synchronously during user deletion, this task catches any that may have been missed due to race conditions or manual database changes. The task runs daily at 04:30 via the maintenance scheduler and can also be triggered manually from the admin maintenance page. Co-Authored-By: OpenCode --- assets/icons/heroicons/sparkles.svg | 1 + default_translations/de/messages.de.yml | 4 + default_translations/en/messages.en.yml | 4 + features/admin_maintenance.feature | 8 ++ .../Admin/MaintenanceController.php | 2 + src/Message/RemoveUnredeemedVouchers.php | 12 +++ .../RemoveUnredeemedVouchersHandler.php | 41 +++++++ src/Schedule/MaintenanceSchedule.php | 2 + templates/Admin/Maintenance/show.html.twig | 9 ++ .../RemoveUnredeemedVouchersHandlerTest.php | 102 ++++++++++++++++++ tests/Schedule/MaintenanceScheduleTest.php | 4 +- 11 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 assets/icons/heroicons/sparkles.svg create mode 100644 src/Message/RemoveUnredeemedVouchers.php create mode 100644 src/MessageHandler/RemoveUnredeemedVouchersHandler.php create mode 100644 tests/MessageHandler/RemoveUnredeemedVouchersHandlerTest.php diff --git a/assets/icons/heroicons/sparkles.svg b/assets/icons/heroicons/sparkles.svg new file mode 100644 index 000000000..cd7aed863 --- /dev/null +++ b/assets/icons/heroicons/sparkles.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/default_translations/de/messages.de.yml b/default_translations/de/messages.de.yml index dee884904..6ac2c1687 100644 --- a/default_translations/de/messages.de.yml +++ b/default_translations/de/messages.de.yml @@ -554,6 +554,10 @@ admin: title: Inaktive Nutzer:innen entfernen description: Löscht Nutzer:innen, die seit mehr als 2 Jahren inaktiv sind. Admin- und permanente Konten werden ausgenommen. button: Inaktive entfernen + removeUnredeemedVouchers: + title: Nicht eingelöste Einladungscodes entfernen + description: Entfernt nicht eingelöste Einladungscodes von gelöschten Nutzer:innen. + button: Einladungscodes entfernen unknown_task: Unbekannte Wartungsaufgabe. dispatched: Wartungsaufgabe erfolgreich gestartet. table: diff --git a/default_translations/en/messages.en.yml b/default_translations/en/messages.en.yml index 6d1ce8865..3116b1ab1 100644 --- a/default_translations/en/messages.en.yml +++ b/default_translations/en/messages.en.yml @@ -453,6 +453,10 @@ admin: title: Remove Inactive Users description: Delete users who have been inactive for more than 2 years. Admin and permanent users are excluded. button: Remove inactive users + removeUnredeemedVouchers: + title: Remove Unredeemed Invite Codes + description: Remove unredeemed invite codes from deleted users. + button: Remove unredeemed invite codes unknown_task: Unknown maintenance task. dispatched: Maintenance task dispatched successfully. title: Settings diff --git a/features/admin_maintenance.feature b/features/admin_maintenance.feature index df5eff512..8abdbb4c3 100644 --- a/features/admin_maintenance.feature +++ b/features/admin_maintenance.feature @@ -26,6 +26,7 @@ Feature: Admin (Maintenance) And I should see "Prune Webhook Deliveries" And I should see "Remove Inactive Users" And I should see "Unlink Redeemed Invite Codes" + And I should see "Remove Unredeemed Invite Codes" @maintenance Scenario: Admin can dispatch prune user notifications task @@ -54,3 +55,10 @@ Feature: Admin (Maintenance) When I am on "/admin/maintenance" And I press "Unlink invite codes" Then I should see "Maintenance task dispatched successfully" + + @maintenance + Scenario: Admin can dispatch remove unredeemed vouchers task + Given I am authenticated as "louis@example.org" + When I am on "/admin/maintenance" + And I press "Remove unredeemed invite codes" + Then I should see "Maintenance task dispatched successfully" diff --git a/src/Controller/Admin/MaintenanceController.php b/src/Controller/Admin/MaintenanceController.php index 98b45066d..badda3474 100644 --- a/src/Controller/Admin/MaintenanceController.php +++ b/src/Controller/Admin/MaintenanceController.php @@ -8,6 +8,7 @@ use App\Message\PruneUserNotifications; use App\Message\PruneWebhookDeliveries; use App\Message\RemoveInactiveUsers; +use App\Message\RemoveUnredeemedVouchers; use App\Message\UnlinkRedeemedVouchers; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -24,6 +25,7 @@ final class MaintenanceController extends AbstractController 'prune_user_notifications' => PruneUserNotifications::class, 'prune_webhook_deliveries' => PruneWebhookDeliveries::class, 'remove_inactive_users' => RemoveInactiveUsers::class, + 'remove_unredeemed_vouchers' => RemoveUnredeemedVouchers::class, 'unlink_redeemed_vouchers' => UnlinkRedeemedVouchers::class, ]; diff --git a/src/Message/RemoveUnredeemedVouchers.php b/src/Message/RemoveUnredeemedVouchers.php new file mode 100644 index 000000000..567a7aea1 --- /dev/null +++ b/src/Message/RemoveUnredeemedVouchers.php @@ -0,0 +1,12 @@ +entityManager->getRepository(Voucher::class) + ->createQueryBuilder('v') + ->join('v.user', 'u') + ->where('v.redeemedTime IS NULL') + ->andWhere('u.deleted = true') + ->getQuery() + ->getResult(); + + foreach ($vouchers as $voucher) { + $this->entityManager->remove($voucher); + } + + $this->entityManager->flush(); + + $this->logger->info('Removed unredeemed vouchers of deleted users', ['count' => count($vouchers)]); + } +} diff --git a/src/Schedule/MaintenanceSchedule.php b/src/Schedule/MaintenanceSchedule.php index 57ace03a9..be3429159 100644 --- a/src/Schedule/MaintenanceSchedule.php +++ b/src/Schedule/MaintenanceSchedule.php @@ -7,6 +7,7 @@ use App\Message\PruneUserNotifications; use App\Message\PruneWebhookDeliveries; use App\Message\RemoveInactiveUsers; +use App\Message\RemoveUnredeemedVouchers; use App\Message\SendWeeklyReport; use App\Message\UnlinkRedeemedVouchers; use DateTimeImmutable; @@ -33,6 +34,7 @@ public function getSchedule(): Schedule ->add(RecurringMessage::every('1 day', new PruneWebhookDeliveries(), new DateTimeImmutable('03:00'))) ->add(RecurringMessage::every('1 day', new PruneUserNotifications(), new DateTimeImmutable('03:30'))) ->add(RecurringMessage::every('1 day', new UnlinkRedeemedVouchers(), new DateTimeImmutable('04:00'))) + ->add(RecurringMessage::every('1 week', new RemoveUnredeemedVouchers(), new DateTimeImmutable('05:00'))) ->add(RecurringMessage::every('1 week', new RemoveInactiveUsers(), new DateTimeImmutable('06:00'))) ->add(RecurringMessage::every('1 week', new SendWeeklyReport(), new DateTimeImmutable('07:00'))); } diff --git a/templates/Admin/Maintenance/show.html.twig b/templates/Admin/Maintenance/show.html.twig index efeaf4f5a..be508cbb5 100644 --- a/templates/Admin/Maintenance/show.html.twig +++ b/templates/Admin/Maintenance/show.html.twig @@ -49,6 +49,15 @@ task: 'unlink_redeemed_vouchers', csrf_token_id: 'maintenance_unlink_redeemed_vouchers', } %} + + {% include 'Admin/_maintenance_card.html.twig' with { + icon: 'heroicons:trash', + title_key: 'admin.maintenance.removeUnredeemedVouchers.title', + description_key: 'admin.maintenance.removeUnredeemedVouchers.description', + button_key: 'admin.maintenance.removeUnredeemedVouchers.button', + task: 'remove_unredeemed_vouchers', + csrf_token_id: 'maintenance_remove_unredeemed_vouchers', + } %} diff --git a/tests/MessageHandler/RemoveUnredeemedVouchersHandlerTest.php b/tests/MessageHandler/RemoveUnredeemedVouchersHandlerTest.php new file mode 100644 index 000000000..9d9024397 --- /dev/null +++ b/tests/MessageHandler/RemoveUnredeemedVouchersHandlerTest.php @@ -0,0 +1,102 @@ +setDeleted(true); + + $voucher1 = new Voucher('A'); + $voucher1->setUser($deletedUser); + + $voucher2 = new Voucher('B'); + $voucher2->setUser($deletedUser); + + $expectedResultSet = [$voucher1, $voucher2]; + + $qb = $this->getMockBuilder(\Doctrine\ORM\QueryBuilder::class) + ->disableOriginalConstructor() + ->onlyMethods(['join', 'where', 'andWhere', 'getQuery']) + ->getMock(); + + $qb->expects($this->any())->method('join')->willReturnSelf(); + $qb->expects($this->any())->method('where')->willReturnSelf(); + $qb->expects($this->any())->method('andWhere')->willReturnSelf(); + + $query = $this->getMockBuilder(\Doctrine\ORM\Query::class) + ->disableOriginalConstructor() + ->onlyMethods(['getResult']) + ->getMock(); + $query->expects($this->once())->method('getResult')->willReturn($expectedResultSet); + + $qb->expects($this->once())->method('getQuery')->willReturn($query); + + $repo = $this->getMockBuilder(VoucherRepository::class) + ->disableOriginalConstructor() + ->onlyMethods(['createQueryBuilder']) + ->getMock(); + $repo->expects($this->once())->method('createQueryBuilder')->willReturn($qb); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->any())->method('getRepository')->willReturn($repo); + $em->expects($this->exactly(2))->method('remove')->with($this->isInstanceOf(Voucher::class)); + $em->expects($this->once())->method('flush'); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('info')->with('Removed unredeemed vouchers of deleted users', ['count' => 2]); + + $handler = new RemoveUnredeemedVouchersHandler($em, $logger); + $handler(new RemoveUnredeemedVouchers()); + } + + public function testHandlesEmptyResultGracefully(): void + { + $qb = $this->getMockBuilder(\Doctrine\ORM\QueryBuilder::class) + ->disableOriginalConstructor() + ->onlyMethods(['join', 'where', 'andWhere', 'getQuery']) + ->getMock(); + + $qb->expects($this->any())->method('join')->willReturnSelf(); + $qb->expects($this->any())->method('where')->willReturnSelf(); + $qb->expects($this->any())->method('andWhere')->willReturnSelf(); + + $query = $this->getMockBuilder(\Doctrine\ORM\Query::class) + ->disableOriginalConstructor() + ->onlyMethods(['getResult']) + ->getMock(); + $query->expects($this->once())->method('getResult')->willReturn([]); + + $qb->expects($this->once())->method('getQuery')->willReturn($query); + + $repo = $this->getMockBuilder(VoucherRepository::class) + ->disableOriginalConstructor() + ->onlyMethods(['createQueryBuilder']) + ->getMock(); + $repo->expects($this->once())->method('createQueryBuilder')->willReturn($qb); + + $em = $this->createMock(EntityManagerInterface::class); + $em->expects($this->any())->method('getRepository')->willReturn($repo); + $em->expects($this->never())->method('remove'); + $em->expects($this->once())->method('flush'); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once())->method('info')->with('Removed unredeemed vouchers of deleted users', ['count' => 0]); + + $handler = new RemoveUnredeemedVouchersHandler($em, $logger); + $handler(new RemoveUnredeemedVouchers()); + } +} diff --git a/tests/Schedule/MaintenanceScheduleTest.php b/tests/Schedule/MaintenanceScheduleTest.php index 1b78540d4..a2bc74a4b 100644 --- a/tests/Schedule/MaintenanceScheduleTest.php +++ b/tests/Schedule/MaintenanceScheduleTest.php @@ -7,6 +7,7 @@ use App\Message\PruneUserNotifications; use App\Message\PruneWebhookDeliveries; use App\Message\RemoveInactiveUsers; +use App\Message\RemoveUnredeemedVouchers; use App\Message\SendWeeklyReport; use App\Message\UnlinkRedeemedVouchers; use App\Schedule\MaintenanceSchedule; @@ -27,10 +28,11 @@ public function testScheduleContainsAllExpectedMessages(): void $recurringMessages, ); - self::assertCount(5, $recurringMessages); + self::assertCount(6, $recurringMessages); self::assertContains(PruneWebhookDeliveries::class, $messageTypes); self::assertContains(PruneUserNotifications::class, $messageTypes); self::assertContains(UnlinkRedeemedVouchers::class, $messageTypes); + self::assertContains(RemoveUnredeemedVouchers::class, $messageTypes); self::assertContains(RemoveInactiveUsers::class, $messageTypes); self::assertContains(SendWeeklyReport::class, $messageTypes); }