diff --git a/.changes/unreleased/Fixed-20251015-110202.yaml b/.changes/unreleased/Fixed-20251015-110202.yaml new file mode 100644 index 000000000..06b9c65a7 --- /dev/null +++ b/.changes/unreleased/Fixed-20251015-110202.yaml @@ -0,0 +1,6 @@ +kind: Fixed +body: Fix the execution of daily cronjob notification, when the previous last execution storage was invalid +time: 2025-10-15T11:02:02.704326098+02:00 +custom: + Issue: "448" + SchemaChange: No schema change diff --git a/.junie/guidelines.md b/.junie/guidelines.md index a53bfe8cf..ff84fdffa 100644 --- a/.junie/guidelines.md +++ b/.junie/guidelines.md @@ -240,9 +240,6 @@ The tests are run from the project's root (not from the bundle's root). # Run all tests vendor/bin/phpunit -# Run tests for a specific bundle -vendor/bin/phpunit --testsuite NameBundle - # Run a specific test file vendor/bin/phpunit path/to/TestFile.php @@ -250,6 +247,9 @@ vendor/bin/phpunit path/to/TestFile.php vendor/bin/phpunit --filter methodName path/to/TestFile.php ``` +When writing tests, only test specific files. Do not run all tests or the full +test suite. + #### Test Structure Tests are organized by bundle and follow the same structure as the bundle itself: diff --git a/src/Bundle/ChillMainBundle/Notification/Email/DailyNotificationDigestCronjob.php b/src/Bundle/ChillMainBundle/Notification/Email/DailyNotificationDigestCronjob.php index 5ed6696f7..c80b9cc67 100644 --- a/src/Bundle/ChillMainBundle/Notification/Email/DailyNotificationDigestCronjob.php +++ b/src/Bundle/ChillMainBundle/Notification/Email/DailyNotificationDigestCronjob.php @@ -53,11 +53,16 @@ readonly class DailyNotificationDigestCronjob implements CronJobInterface public function run(array $lastExecutionData): ?array { $now = $this->clock->now(); + if (isset($lastExecutionData['last_execution'])) { $lastExecution = \DateTimeImmutable::createFromFormat( \DateTimeImmutable::ATOM, $lastExecutionData['last_execution'] ); + + if (false === $lastExecution) { + $lastExecution = $now->sub(new \DateInterval('P1D')); + } } else { $lastExecution = $now->sub(new \DateInterval('P1D')); } @@ -96,7 +101,7 @@ readonly class DailyNotificationDigestCronjob implements CronJobInterface ]); return [ - 'last_execution' => $now->format('Y-m-d-H:i:s.u e'), + 'last_execution' => $now->format(\DateTimeInterface::ATOM), ]; } } diff --git a/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobFunctionalTest.php b/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobFunctionalTest.php index 0caeebc36..a1f749399 100644 --- a/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobFunctionalTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobFunctionalTest.php @@ -37,10 +37,5 @@ class DailyNotificationDigestCronJobFunctionalTest extends KernelTestCase $actual = $this->dailyNotificationDigestCronjob->run([]); self::assertArrayHasKey('last_execution', $actual); - self::assertInstanceOf( - \DateTimeImmutable::class, - \DateTimeImmutable::createFromFormat('Y-m-d-H:i:s.u e', $actual['last_execution']), - 'test that the string can be converted to a date' - ); } } diff --git a/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobTest.php b/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobTest.php index 5894385c6..075929838 100644 --- a/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Notification/Email/DailyNotificationDigestCronJobTest.php @@ -12,16 +12,21 @@ declare(strict_types=1); namespace Chill\MainBundle\Tests\Notification\Email; use Chill\MainBundle\Notification\Email\DailyNotificationDigestCronjob; +use Chill\MainBundle\Notification\Email\NotificationEmailMessages\ScheduleDailyNotificationDigestMessage; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Result; +use Doctrine\DBAL\Statement; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Symfony\Component\Clock\ClockInterface; +use Symfony\Component\Clock\MockClock; +use Symfony\Component\Messenger\Envelope; use Symfony\Component\Messenger\MessageBusInterface; /** * @internal * - * @coversNothing + * @covers \DailyNotificationDigestCronjob */ class DailyNotificationDigestCronJobTest extends TestCase { @@ -30,6 +35,7 @@ class DailyNotificationDigestCronJobTest extends TestCase private MessageBusInterface $messageBus; private LoggerInterface $logger; private DailyNotificationDigestCronjob $cronjob; + private \DateTimeImmutable $firstNow; protected function setUp(): void { @@ -38,6 +44,8 @@ class DailyNotificationDigestCronJobTest extends TestCase $this->messageBus = $this->createMock(MessageBusInterface::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->firstNow = new \DateTimeImmutable('2024-01-02T07:15:00+00:00'); + $this->cronjob = new DailyNotificationDigestCronjob( $this->clock, $this->connection, @@ -78,4 +86,129 @@ class DailyNotificationDigestCronJobTest extends TestCase 'hour 23 - should not run' => [23, false], ]; } + + public function testRunFirstExecutionReturnsStateAndDispatches(): array + { + // Use MockClock for deterministic time + $firstNow = $this->firstNow; + $clock = new MockClock($firstNow); + + // Mock DBAL statement/result + $statement = $this->createMock(Statement::class); + $result = $this->createMock(Result::class); + + $this->connection->method('prepare')->willReturn($statement); + $statement->method('bindValue')->willReturnSelf(); + $statement->method('executeQuery')->willReturn($result); + + $rows = [ + ['user_id' => 10], + ['user_id' => 42], + ]; + $result->method('fetchAllAssociative')->willReturn($rows); + + $dispatched = []; + $this->messageBus->method('dispatch')->willReturnCallback(function ($message) use (&$dispatched) { + $dispatched[] = $message; + + return new Envelope($message); + }); + + $cron = new DailyNotificationDigestCronjob($clock, $this->connection, $this->messageBus, $this->logger); + $state = $cron->run([]); + + // Assert dispatch count and message contents + self::assertCount(2, $dispatched); + $expectedLast = $firstNow->sub(new \DateInterval('P1D')); + foreach ($dispatched as $i => $msg) { + self::assertInstanceOf(ScheduleDailyNotificationDigestMessage::class, $msg); + self::assertTrue(in_array($msg->getUserId(), [10, 42], true)); + self::assertEquals($firstNow, $msg->getCurrentDateTime(), 'compare the current date'); + self::assertEquals($expectedLast, $msg->getLastExecutionDateTime(), 'compare the last execution date'); + } + + // Assert returned state + self::assertIsArray($state); + self::assertArrayHasKey('last_execution', $state); + self::assertSame($firstNow->format(\DateTimeInterface::ATOM), $state['last_execution']); + + return $state; + } + + /** + * @depends testRunFirstExecutionReturnsStateAndDispatches + */ + public function testRunSecondExecutionUsesPreviousState(array $previousState): void + { + $firstNow = $this->firstNow; + $secondNow = $firstNow->add(new \DateInterval('P1D')); + $clock = new MockClock($secondNow); + + // Mock DBAL for a single user this time + $statement = $this->createMock(Statement::class); + $result = $this->createMock(Result::class); + + $this->connection->method('prepare')->willReturn($statement); + $statement->method('bindValue')->willReturnSelf(); + $statement->method('executeQuery')->willReturn($result); + + $rows = [ + ['user_id' => 7], + ]; + $result->method('fetchAllAssociative')->willReturn($rows); + + $captured = []; + $this->messageBus->method('dispatch')->willReturnCallback(function ($message) use (&$captured) { + $captured[] = $message; + + return new Envelope($message); + }); + + $cron = new DailyNotificationDigestCronjob($clock, $this->connection, $this->messageBus, $this->logger); + $cron->run($previousState); + + self::assertCount(1, $captured); + $msg = $captured[0]; + self::assertInstanceOf(ScheduleDailyNotificationDigestMessage::class, $msg); + self::assertEquals(7, $msg->getUserId()); + self::assertEquals($secondNow, $msg->getCurrentDateTime(), 'compare the current date'); + self::assertEquals($firstNow, $msg->getLastExecutionDateTime(), 'compare the last execution date'); + } + + public function testRunWithInvalidExecutionState(): void + { + $firstNow = new \DateTimeImmutable('2025-10-14T10:30:00 Europe/Brussels'); + $previousExpected = $firstNow->sub(new \DateInterval('P1D')); + $clock = new MockClock($firstNow); + + // Mock DBAL for a single user this time + $statement = $this->createMock(Statement::class); + $result = $this->createMock(Result::class); + + $this->connection->method('prepare')->willReturn($statement); + $statement->method('bindValue')->willReturnSelf(); + $statement->method('executeQuery')->willReturn($result); + + $rows = [ + ['user_id' => 7], + ]; + $result->method('fetchAllAssociative')->willReturn($rows); + + $captured = []; + $this->messageBus->method('dispatch')->willReturnCallback(function ($message) use (&$captured) { + $captured[] = $message; + + return new Envelope($message); + }); + + $cron = new DailyNotificationDigestCronjob($clock, $this->connection, $this->messageBus, $this->logger); + $cron->run(['last_execution' => 'invalid data']); + + self::assertCount(1, $captured); + $msg = $captured[0]; + self::assertInstanceOf(ScheduleDailyNotificationDigestMessage::class, $msg); + self::assertEquals(7, $msg->getUserId()); + self::assertEquals($firstNow, $msg->getCurrentDateTime(), 'compare the current date'); + self::assertEquals($previousExpected, $msg->getLastExecutionDateTime(), 'compare the last execution date'); + } }