feat(auth): automate inactive account notification enqueue#20581
Draft
dschom wants to merge 1 commit into
Draft
Conversation
Because: - Manually opening a webservices-infra PR per FXA-13709 for each new batch of inactive accounts is repetitive chore work. This commit: - Replaces --start-date/--end-date/--active-by-date/--results-limit on enqueue-inactive-account-deletions.ts with --batch-size (default 1000). - Each run picks the oldest N candidates (createdAt < two-years-ago, ORDER BY createdAt ASC, LIMIT batchSize); the floor advances implicitly as deleted accounts drop out. - Filters out UIDs that already have an inactiveAccountFirstWarning emailSent event in Firestore before enqueueing, making overlapping runs safe. Emits accounts.inactive.cron.skipped-already-notified. - Wires StatsD, AuthFirestore, and AccountEventsManager into the typedi Container for the script. Fixes FXA-13709
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the inactive-account deletion enqueue script to support scheduled batch-based processing and Firestore-based duplicate notification checks.
Changes:
- Replaces date-range/result-limit CLI options with
--batch-size. - Adds Firestore/account-events wiring and skips already-sent first-warning emails before enqueueing.
- Updates script integration tests for new defaults and batch-size validation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
packages/fxa-auth-server/scripts/delete-inactive-accounts/enqueue-inactive-account-deletions.ts |
Implements batch candidate selection, dependency wiring, and already-notified skip logic. |
packages/fxa-auth-server/test/scripts/delete-inactive-accounts/enqueue-inactive-account-deletions.in.spec.ts |
Updates dry-run assertions and adds non-positive batch-size validation coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+531
to
+537
| const priorEvents = await accountEventsManager.findEmailEvents( | ||
| uid, | ||
| 'emailSent', | ||
| 'inactiveAccountFirstWarning', | ||
| 0, | ||
| Date.now() | ||
| ); |
Comment on lines
+538
to
+541
| if (priorEvents && priorEvents.length > 0) { | ||
| statsd.increment('accounts.inactive.cron.skipped-already-notified'); | ||
| debugLog(`Skipping ${uid}: first-warning email already sent.`); | ||
| return; |
Comment on lines
+535
to
+536
| 0, | ||
| Date.now() |
Comment on lines
+136
to
+139
| '--batch-size [number]', | ||
| `The max number of candidate accounts to consider per run. Defaults to ${defaultBatchSize}.`, | ||
| (x) => parseInt(x), | ||
| defaultResultsLImit | ||
| defaultBatchSize |
Comment on lines
+341
to
+347
| const accountsQuery = accountWhereAndOrderByQueryBuilder( | ||
| startDateTimestamp, | ||
| endDateTimestamp, | ||
| activeByDateTimestamp | ||
| ) | ||
| .select('accounts.uid') | ||
| .limit(batchSize); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
webservices-infraPR per FXA-13709 for each new batch of inactive accounts is repetitive chore work.This pull request
--start-date,--end-date,--active-by-date,--results-limitfromenqueue-inactive-account-deletions.ts; adds--batch-size(default 1000).inactiveAccountFirstWarningemailSentevent in Firestore before enqueueing, so overlapping runs are safe. Emitsaccounts.inactive.cron.skipped-already-notified.StatsD,AuthFirestore, andAccountEventsManagerinto the typedi Container for the script.Issue that this pull request solves
Closes: FXA-13709
Checklist
How to review (Optional)
packages/fxa-auth-server/scripts/delete-inactive-accounts/enqueue-inactive-account-deletions.tscreatedAtfilter relies onORDER BY ASC + LIMITplus the Firestore dedup to converge correctly across runs.Other information (Optional)
Follow-up in
webservices-infra: add a Kubernetes CronJob manifest that invokes this script with--batch-size=<N> --dry-run=false --enqueue-emails=trueon a schedule.