[Team Deletions] deletion e-mails - #6624
Conversation
f7cc2ed to
5d3e926
Compare
|
@aerosol thanks, the copy looks good! |
| from(sch in TeamDeletionSchedule, | ||
| where: sch.status == :scheduled, | ||
| where: sch.first_notice_due_date <= ^today, | ||
| preload: [team: [:owners, :billing_members]] |
There was a problem hiding this comment.
That's a very minor improvement but at least team preload could be done from a join.
| from(sch in TeamDeletionSchedule, | ||
| where: sch.status == :first_notice_sent, | ||
| where: sch.deletion_date <= ^reminder_threshold, | ||
| preload: [team: [:owners, :billing_members]] |
There was a problem hiding this comment.
Ditto about preloading team from a join.
|
|
||
| pending_trial_schedules_by_team_id = | ||
| notifications | ||
| |> Enum.filter(&(&1.deadline == tomorrow)) |
There was a problem hiding this comment.
Even though we don't do any fancy stuff like mixing calendars, I'd opt for using Date.compare instead of struct comparison, just to be safe.
| pending_trial_schedules_by_team_id | ||
| |> Map.values() | ||
| |> Enum.each(fn schedule -> | ||
| TeamDeletionSchedules.mark_first_notice_sent(schedule, report_if_invalid?: true) |
There was a problem hiding this comment.
Why not run it directly after sending the notification in the original loop? 🤔
There was a problem hiding this comment.
Poor man's fault tolerance. Correct me if I'm wrong: first, we send out a bunch of notifications and store "send_accept_traffic_until_notifications" rows for each, in the same loop, to minimize the risk of sending dupe e-mails in case the job runs again.
Then in another loop, we'll mark deletions schedules as "first notice sent" so deletion schedules can advance. If any of those DB updates fails, the worker gets restarted - it won't send first notice e-mails again, but it will advance the deletion schedules that weren't touched due to previous crash.
If we had it in one loop, a crash would restart the worker but there's a chance it would then skip advancing deletion schedules leaving them in stale state forever.
There was a problem hiding this comment.
Nevermind, you were right, will fix that 🫡
3 participants
Changes
This PR introduces two new emails: deletion full notice (T-30d, with trial/subscription branching and capped site list) and deletion reminder (T-5d). Each subscription is re-cehcekd before sending to address the reactivation race.
AcceptTrafficUntil worker's "stats stop tomorrow" e-mail now includes the pending deletion date for steady-state (non-backlog) expired trials, instead of separate e-mail.
The worker is disabled for now.
@metmarkosaric if you could have a final look at the copy please