Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions app/Jobs/SendPolicyAnnouncementJob.php
Comment thread
tarrow marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
});
}
}
21 changes: 21 additions & 0 deletions tests/Jobs/SendPolicyAnnouncementJobTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
}
}
Loading