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
69 changes: 69 additions & 0 deletions internal/jobs/expire_unexported_coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,75 @@ func TestRenderAnonExpiryEmail_MissingParamsRenderEmpty(t *testing.T) {
}
}

// TestRenderAnonExpiryEmail_EmptyHoursConsistency pins the
// subject-vs-body parity for an empty hours_remaining param. Prior to
// the EMAIL-BUGBASH-WORKER-2026-05-19 P3-1 fix, the subject defaulted
// to "1h" while the body rendered " hours" (empty value, plural form
// = true). The body and subject MUST agree on the same fallback so
// recipients never see " hours" with no number or a singular/plural
// mismatch. This is a registry-style guard: every rendered surface
// (subject, html body H2, html body "Time left" row, text body) must
// include a non-empty hour count.
func TestRenderAnonExpiryEmail_EmptyHoursConsistency(t *testing.T) {
subj, html, text := renderAnonExpiryEmail(map[string]string{
"resource_type": "postgres",
"reminder_index": "3",
"token_prefix": "tok-abcd",
"expires_at": "2026-05-22T00:00:00Z",
"upgrade_url": "https://x/upgrade",
"resource_url": "https://x/res",
// hours_remaining intentionally omitted.
})

// Subject MUST end with "1h" (the subject's existing fallback).
if !strings.HasSuffix(subj, "1h") {
t.Errorf("subject must end with '1h' on empty hours_remaining, got %q", subj)
}

// Every rendered surface must contain "1 hour" (singular, matching
// the subject's "1h") — never " hour" or " hours" (empty number).
// The H2, "Time left" row, and text body all use the same template.
surfaces := map[string]string{"html": html, "text": text}
for name, body := range surfaces {
if !strings.Contains(body, "1 hour") {
t.Errorf("%s body must include '1 hour' (singular) on empty hours_remaining, got: %s", name, body)
}
if strings.Contains(body, "1 hours") {
t.Errorf("%s body must not include '1 hours' (incorrect plural), got: %s", name, body)
}
// Never an empty-number rendering. Catches re-introducing the bug.
if strings.Contains(body, " hour ") && !strings.Contains(body, "1 hour ") {
t.Errorf("%s body has bare ' hour ' with no preceding number, got: %s", name, body)
}
if strings.Contains(body, " hours") && !strings.Contains(body, "1 hours") &&
!hasDigitBefore(body, " hours") {
t.Errorf("%s body has ' hours' with no preceding digit, got: %s", name, body)
}
}
}

// hasDigitBefore reports whether every occurrence of `needle` in `s`
// is preceded by an ASCII digit. Helper for the empty-number guard:
// "12 hours" passes (digit before), " hours" fails (space before).
func hasDigitBefore(s, needle string) bool {
i := 0
for {
idx := strings.Index(s[i:], needle)
if idx == -1 {
return true
}
pos := i + idx
if pos == 0 {
return false
}
c := s[pos-1]
if c < '0' || c > '9' {
return false
}
i = pos + len(needle)
}
}

// TestAnonExpirySubject_AllBranches walks every reminder_index prefix
// and the default fallback (any other value).
func TestAnonExpirySubject_AllBranches(t *testing.T) {
Expand Down
23 changes: 18 additions & 5 deletions internal/jobs/expiry_reminder_email.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,19 +170,32 @@ type anonExpiryView struct {
// template.Must). On the off-chance a future template change breaks
// parsing, the panic surfaces in CI rather than at runtime.
//
// Missing param keys render as empty strings. The caller (forwarder)
// is responsible for not invoking this for kinds other than
// anon.expiry_warning.
// Missing param keys render as empty strings, EXCEPT hours_remaining
// which defaults to "1" so the subject ("expires in 1h") and the body
// ("expires in 1 hour") stay consistent. Without this defaulting, an
// empty hours_remaining would render "expires in hours" (note the
// double space + plural "s") — a P3 bug from EMAIL-BUGBASH-WORKER
// 2026-05-19. The caller (forwarder) is responsible for not invoking
// this for kinds other than anon.expiry_warning.
func renderAnonExpiryEmail(params map[string]string) (subject, html, text string) {
// Default the hours_remaining BEFORE deciding plurality so the body
// matches the subject's "1h" fallback path (see anonExpirySubject).
// "1" is the conservative default for a final-reminder cadence — if
// the forwarder ever loses the field, the email still reads as a
// last-chance nudge rather than mysteriously empty.
hoursRemaining := params["hours_remaining"]
if hoursRemaining == "" {
hoursRemaining = "1"
}
view := anonExpiryView{
ResourceType: params["resource_type"],
HoursRemaining: params["hours_remaining"],
HoursRemaining: hoursRemaining,
ExpiresAt: params["expires_at"],
ReminderIndex: params["reminder_index"],
TokenPrefix: params["token_prefix"],
UpgradeURL: params["upgrade_url"],
ResourceURL: params["resource_url"],
Plural: params["hours_remaining"] != "1",
Plural: hoursRemaining != "1",
}

subject = anonExpirySubject(view.ReminderIndex, view.ResourceType, view.HoursRemaining)
Expand Down
Loading