diff --git a/app/Jobs/SendPolicyAnnouncementJob.php b/app/Jobs/SendPolicyAnnouncementJob.php index 75a835ab..ef60d70f 100644 --- a/app/Jobs/SendPolicyAnnouncementJob.php +++ b/app/Jobs/SendPolicyAnnouncementJob.php @@ -4,22 +4,30 @@ use App\Notifications\PolicyAnnouncementNotification; use App\User; -use Illuminate\Support\Facades\Notification; +use Illuminate\Support\Facades\Log; /** * WARNING: This job is NOT idempotent. DO NOT RUN IT MULTIPLE TIMES. * There is also no error handling or mechanism for recording which users have been sent an email. * - * This has created an issue on production where a user with an empty string as an email address + * This has created an issue on production where a user with a weird string as an email address * (due to a request to remove PII) caused Notification::send() to throw an error and the remaining * emails to not be sent. * * DO NOT REPEAT THIS PATTERN FOR OTHER JOBS */ class SendPolicyAnnouncementJob extends Job { - public function handle() { + public function handle(): void { $users = User::query()->whereNotNull('email')->get(); - Notification::send($users, new PolicyAnnouncementNotification()); + $users->each(function (User $user) { + try { + $user->notify(new PolicyAnnouncementNotification()); + + Log::info('PolicyAnnouncementNotification sent successfully', [$user->id, $user->email]); + } catch (\Exception $exception) { + Log::error($exception->getMessage(), [$user->id, $user->email]); + } + }); } } diff --git a/tests/Jobs/SendPolicyAnnouncementJobTest.php b/tests/Jobs/SendPolicyAnnouncementJobTest.php index dd80c7f0..dda67f89 100644 --- a/tests/Jobs/SendPolicyAnnouncementJobTest.php +++ b/tests/Jobs/SendPolicyAnnouncementJobTest.php @@ -7,6 +7,7 @@ use App\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Notification; +use Symfony\Component\Mime\Exception\RfcComplianceException; use Tests\TestCase; class SendPolicyAnnouncementJobTest extends TestCase { @@ -26,4 +27,24 @@ public function testThePolicyAnnouncementEmailToAllUsers() { Notification::assertSentTo($users, PolicyAnnouncementNotification::class); } + + public function testItNotifiedAllUsersEvenIfMailerThrowsRfcComplianceException() { + // This test specifically simualtes the situation we saw in T432211#12270805 + Notification::shouldReceive('send') + ->once() + ->andThrow(new RfcComplianceException()); + Notification::shouldReceive('send') + ->atLeast() + ->times(3); + + User::factory()->createMany([ + ['email' => 'asdfghjklertyuiopcvbnm'], + ['email' => 'user1@email.com'], + ['email' => 'user2@email.com'], + ['email' => ''], + ['email' => 'user5@email.com'], + ]); + $job = new SendPolicyAnnouncementJob(); + $job->handle(); + } }