From 3a8623fbe8607f3a66c147a67c1425202f947281 Mon Sep 17 00:00:00 2001 From: Perside Rosalie Date: Tue, 1 Sep 2026 13:58:01 +0200 Subject: [PATCH 1/6] Temporarily modify sendPolicyAnnouncementJob Make it possible to sent a list of excluded emails via the constructor Bug: T432211 --- app/Jobs/SendPolicyAnnouncementJob.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Jobs/SendPolicyAnnouncementJob.php b/app/Jobs/SendPolicyAnnouncementJob.php index 75a835ab..1cbe6b0f 100644 --- a/app/Jobs/SendPolicyAnnouncementJob.php +++ b/app/Jobs/SendPolicyAnnouncementJob.php @@ -17,8 +17,10 @@ * DO NOT REPEAT THIS PATTERN FOR OTHER JOBS */ class SendPolicyAnnouncementJob extends Job { + public function __construct(private readonly array $excludedEmails = []) {} + public function handle() { - $users = User::query()->whereNotNull('email')->get(); + $users = User::query()->whereNotNull('email')->whereNotIn('email', $this->excludedEmails)->get(); Notification::send($users, new PolicyAnnouncementNotification()); } From df2d5d4feaa36b21b67f08daedf166c880515f0c Mon Sep 17 00:00:00 2001 From: Perside Rosalie Date: Wed, 2 Sep 2026 12:19:54 +0200 Subject: [PATCH 2/6] Send the notification in a try catch and make the notification queueable --- app/Jobs/SendPolicyAnnouncementJob.php | 15 +++++-- .../PolicyAnnouncementNotification.php | 6 ++- tests/Jobs/SendPolicyAnnouncementJobTest.php | 42 +++++++++++++++++++ 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/app/Jobs/SendPolicyAnnouncementJob.php b/app/Jobs/SendPolicyAnnouncementJob.php index 1cbe6b0f..02363e93 100644 --- a/app/Jobs/SendPolicyAnnouncementJob.php +++ b/app/Jobs/SendPolicyAnnouncementJob.php @@ -4,13 +4,13 @@ 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. * @@ -19,9 +19,16 @@ class SendPolicyAnnouncementJob extends Job { public function __construct(private readonly array $excludedEmails = []) {} - public function handle() { + public function handle(): void { $users = User::query()->whereNotNull('email')->whereNotIn('email', $this->excludedEmails)->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/app/Notifications/PolicyAnnouncementNotification.php b/app/Notifications/PolicyAnnouncementNotification.php index fd6f3ea5..5e6b2b09 100644 --- a/app/Notifications/PolicyAnnouncementNotification.php +++ b/app/Notifications/PolicyAnnouncementNotification.php @@ -2,12 +2,16 @@ namespace App\Notifications; +use Illuminate\Bus\Queueable; +use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Notifications\Notification; use Illuminate\Support\Facades\Lang; use Illuminate\Support\HtmlString; -class PolicyAnnouncementNotification extends Notification { +class PolicyAnnouncementNotification extends Notification implements ShouldQueue { + use Queueable; + /** * Get the notification's delivery channels. * diff --git a/tests/Jobs/SendPolicyAnnouncementJobTest.php b/tests/Jobs/SendPolicyAnnouncementJobTest.php index dd80c7f0..0260678c 100644 --- a/tests/Jobs/SendPolicyAnnouncementJobTest.php +++ b/tests/Jobs/SendPolicyAnnouncementJobTest.php @@ -6,6 +6,7 @@ use App\Notifications\PolicyAnnouncementNotification; use App\User; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Mail; use Illuminate\Support\Facades\Notification; use Tests\TestCase; @@ -26,4 +27,45 @@ public function testThePolicyAnnouncementEmailToAllUsers() { Notification::assertSentTo($users, PolicyAnnouncementNotification::class); } + + public function testItExcludeUsersInTheExcludedEmailsList() { + Notification::fake(); + $excludedUser = User::factory()->create(['email' => 'excluded.email@email.com']); + $includedUser = User::factory()->create(['email' => 'included.email@email.com']); + + $job = new SendPolicyAnnouncementJob([$excludedUser->email]); + $job->handle(); + Notification::assertNotSentTo($excludedUser, PolicyAnnouncementNotification::class); + Notification::assertSentTo($includedUser, PolicyAnnouncementNotification::class); + } + + public function testItNotifiedAllUsersEvenIfSomeEmailsAreInvalid() { + Notification::fake(); + User::factory()->createMany([ + ['email' => 'user1@email.com'], + ['email' => 'user2@email.com'], + ['email' => ''], + ['email' => 'asdfghjklertyuiopcvbnm'], + ['email' => 'user5@email.com'], + ]); + $job = new SendPolicyAnnouncementJob(); + $job->handle(); + Notification::assertCount(5); + } + + public function testItQueuesMailsForAllValidEmails() { + $this->markTestSkipped('Mocking the mailer failing to send seems to be difficult'); + Mail::fake(); + User::factory()->createMany([ + ['email' => 'user1@email.com'], + ['email' => 'user2@email.com'], + ['email' => ''], + ['email' => 'asdfghjklertyuiopcvbnm'], + ['email' => 'user5@email.com'], + ]); + $job = new SendPolicyAnnouncementJob(); + $job->handle(); + // Seems like sending notifications doesn't actually result in queueing mails. + Mail::assertQueuedCount(3); + } } From ce855f8411a0d8b58e0862d021c7aaf0452a0c32 Mon Sep 17 00:00:00 2001 From: Thomas Arrow Date: Wed, 2 Sep 2026 13:18:26 +0100 Subject: [PATCH 3/6] only use the try-catch and not the queueable --- app/Notifications/PolicyAnnouncementNotification.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/Notifications/PolicyAnnouncementNotification.php b/app/Notifications/PolicyAnnouncementNotification.php index 5e6b2b09..fd6f3ea5 100644 --- a/app/Notifications/PolicyAnnouncementNotification.php +++ b/app/Notifications/PolicyAnnouncementNotification.php @@ -2,16 +2,12 @@ namespace App\Notifications; -use Illuminate\Bus\Queueable; -use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Notifications\Notification; use Illuminate\Support\Facades\Lang; use Illuminate\Support\HtmlString; -class PolicyAnnouncementNotification extends Notification implements ShouldQueue { - use Queueable; - +class PolicyAnnouncementNotification extends Notification { /** * Get the notification's delivery channels. * From fa0cd4868d3c2a2966f28e967ccc310a837036c6 Mon Sep 17 00:00:00 2001 From: Thomas Arrow Date: Wed, 2 Sep 2026 13:31:27 +0100 Subject: [PATCH 4/6] remove email exclusion logic --- app/Jobs/SendPolicyAnnouncementJob.php | 5 ++--- tests/Jobs/SendPolicyAnnouncementJobTest.php | 11 ----------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/app/Jobs/SendPolicyAnnouncementJob.php b/app/Jobs/SendPolicyAnnouncementJob.php index 02363e93..ef60d70f 100644 --- a/app/Jobs/SendPolicyAnnouncementJob.php +++ b/app/Jobs/SendPolicyAnnouncementJob.php @@ -17,14 +17,13 @@ * DO NOT REPEAT THIS PATTERN FOR OTHER JOBS */ class SendPolicyAnnouncementJob extends Job { - public function __construct(private readonly array $excludedEmails = []) {} - public function handle(): void { - $users = User::query()->whereNotNull('email')->whereNotIn('email', $this->excludedEmails)->get(); + $users = User::query()->whereNotNull('email')->get(); $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 0260678c..90962a87 100644 --- a/tests/Jobs/SendPolicyAnnouncementJobTest.php +++ b/tests/Jobs/SendPolicyAnnouncementJobTest.php @@ -28,17 +28,6 @@ public function testThePolicyAnnouncementEmailToAllUsers() { Notification::assertSentTo($users, PolicyAnnouncementNotification::class); } - public function testItExcludeUsersInTheExcludedEmailsList() { - Notification::fake(); - $excludedUser = User::factory()->create(['email' => 'excluded.email@email.com']); - $includedUser = User::factory()->create(['email' => 'included.email@email.com']); - - $job = new SendPolicyAnnouncementJob([$excludedUser->email]); - $job->handle(); - Notification::assertNotSentTo($excludedUser, PolicyAnnouncementNotification::class); - Notification::assertSentTo($includedUser, PolicyAnnouncementNotification::class); - } - public function testItNotifiedAllUsersEvenIfSomeEmailsAreInvalid() { Notification::fake(); User::factory()->createMany([ From d324596ff60bb61bbb78903a07a645be60eab374 Mon Sep 17 00:00:00 2001 From: Thomas Arrow Date: Wed, 2 Sep 2026 13:31:46 +0100 Subject: [PATCH 5/6] remove attempt to test using Mail::fake --- tests/Jobs/SendPolicyAnnouncementJobTest.php | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/tests/Jobs/SendPolicyAnnouncementJobTest.php b/tests/Jobs/SendPolicyAnnouncementJobTest.php index 90962a87..6bb0f4ee 100644 --- a/tests/Jobs/SendPolicyAnnouncementJobTest.php +++ b/tests/Jobs/SendPolicyAnnouncementJobTest.php @@ -6,7 +6,6 @@ use App\Notifications\PolicyAnnouncementNotification; use App\User; use Illuminate\Foundation\Testing\RefreshDatabase; -use Illuminate\Support\Facades\Mail; use Illuminate\Support\Facades\Notification; use Tests\TestCase; @@ -41,20 +40,4 @@ public function testItNotifiedAllUsersEvenIfSomeEmailsAreInvalid() { $job->handle(); Notification::assertCount(5); } - - public function testItQueuesMailsForAllValidEmails() { - $this->markTestSkipped('Mocking the mailer failing to send seems to be difficult'); - Mail::fake(); - User::factory()->createMany([ - ['email' => 'user1@email.com'], - ['email' => 'user2@email.com'], - ['email' => ''], - ['email' => 'asdfghjklertyuiopcvbnm'], - ['email' => 'user5@email.com'], - ]); - $job = new SendPolicyAnnouncementJob(); - $job->handle(); - // Seems like sending notifications doesn't actually result in queueing mails. - Mail::assertQueuedCount(3); - } } From 2d8ade784d931b29212b708a7fc34c56a5959a1f Mon Sep 17 00:00:00 2001 From: Thomas Arrow Date: Wed, 2 Sep 2026 13:53:14 +0100 Subject: [PATCH 6/6] add test for RfcCompliance exception --- tests/Jobs/SendPolicyAnnouncementJobTest.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/Jobs/SendPolicyAnnouncementJobTest.php b/tests/Jobs/SendPolicyAnnouncementJobTest.php index 6bb0f4ee..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 { @@ -27,17 +28,23 @@ public function testThePolicyAnnouncementEmailToAllUsers() { Notification::assertSentTo($users, PolicyAnnouncementNotification::class); } - public function testItNotifiedAllUsersEvenIfSomeEmailsAreInvalid() { - Notification::fake(); + 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' => 'asdfghjklertyuiopcvbnm'], ['email' => 'user5@email.com'], ]); $job = new SendPolicyAnnouncementJob(); $job->handle(); - Notification::assertCount(5); } }