Skip to content

Make sendPolicyAnnouncementJob more robust - #1243

Merged
tarrow merged 6 commits into
mainfrom
T432211
Sep 2, 2026
Merged

Make sendPolicyAnnouncementJob more robust#1243
tarrow merged 6 commits into
mainfrom
T432211

Conversation

@rosalieper

@rosalieper rosalieper commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

This adds additional try-catch logic to make running this more robust.

It specifically adds a test for a real error we saw caused by attempting to email
a malformed email address.

It does not record to the DB successful emails but it does add log output that
will be persisted in GCE for longer than the mailgun logs.

Bug: T432211

Comment thread app/Jobs/SendPolicyAnnouncementJob.php
@tarrow

tarrow commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

I think it's important to note @outdooracorn was suggesting either add the try-catch or use Queueable on the notification.

What we learnt so far is that testing if adding either of these two things makes the situation more robust is harder.

If we use Notification::fake() all notifications, even to users with malformed email addresses, appear to work because it's the process of actually sending the email that fails.

We also learnt that by default notifications use MailMessage (a subclass of SimpleMessage) rather than a complete Mailable. This means that testing with Mail::fake() is also not possible

@tarrow

tarrow commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

There is also a wiki_notification_sent_records table in the database with the idea being that we record each type of Notification that we send out in the database so that if there is an error we have a record of what Notifications were sent to Mailgun. Given that we are already part way through sending these emails, I don't think it's worth investing in that for now.

We had a look at this and realised that this table is for notifications about Wikis to be sent to managers. It doesn't seem to be useful from the perspective of just Users entirely independent of Wikis. For example, it really needs a wiki_id which we don't have in this situation.

We did discuss if we ought to implement a new table to track these changes but figured that this would add yet more time when we want to prioritise getting these mails sent out and we also want to stop thinking about this topic in general.

@tarrow tarrow changed the title Temporarily modify sendPolicyAnnouncementJob Make sendPolicyAnnouncementJob more robust Sep 2, 2026
@tarrow
tarrow merged commit 0dbf83f into main Sep 2, 2026
4 checks passed
@tarrow
tarrow deleted the T432211 branch September 2, 2026 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants