feat(notifications): grouped email digest, new music as a daily summary (#5346)
release / govulncheck (push) Successful in 21s
release / web (push) Successful in 1m19s
release / go (push) Successful in 1m39s
release / integration (push) Successful in 5m27s
release / android (push) Successful in 5m47s
release / Build signed APK (releases and dev) (push) Successful in 5m34s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 26s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 21s
release / web (push) Successful in 1m19s
release / go (push) Successful in 1m39s
release / integration (push) Successful in 5m27s
release / android (push) Successful in 5m47s
release / Build signed APK (releases and dev) (push) Successful in 5m34s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 26s
release / Verify release artifacts (tag releases only) (push) Skipped
Nothing is emailed per event. New music (request_completed) goes out at most once a day, at the summary hour in each user's own timezone, grouped by artist. Everything else is batched: one email a window after the first un-emailed item, holding whatever accumulated. - Migration 0074: notification_email_settings (summary hour, batch window, admin-configurable) and user_notification_email_state (batch start, last sent, failures and retry_after per user and group). Existing rows are stamped emailed so the upgrade sends no backlog. - The Notifier stamps emailed_at at write time when the recipient's email channel is off, so turning email on later doesn't send old items. - Read rows are never selected. A row is stamped only after the mailer accepts, in one transaction with the state, against the read's clock, so a coalesced row updated mid-send stays pending. - A failed send backs off 5m doubling to 6h; SMTP not configured just waits. - Links come from the public address; without one the email has none. - The mailer now RFC 2047-encodes subjects and strips line breaks from them. - Admin → Integrations gains a Notification emails card. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,223 @@
|
||||
package notifications_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/mailer"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/notifications"
|
||||
)
|
||||
|
||||
// emailUser is a user with an address, so the digest can reach them.
|
||||
func emailUser(t *testing.T, pool *pgxpool.Pool, name string, admin bool) pgtype.UUID {
|
||||
t.Helper()
|
||||
id := mkUser(t, dbq.New(pool), name, admin)
|
||||
_, err := pool.Exec(context.Background(), "UPDATE users SET email = $2 WHERE id = $1", id, name+"@example.com")
|
||||
require.NoError(t, err)
|
||||
return id
|
||||
}
|
||||
|
||||
func digestWith(pool *pgxpool.Pool) (*notifications.Digest, *mailer.FakeSender) {
|
||||
fake := &mailer.FakeSender{}
|
||||
return notifications.NewDigest(pool, fake, nil), fake
|
||||
}
|
||||
|
||||
func TestDigest_BatchGoesOutOnceAWindowAfterTheFirstItem(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
alice := emailUser(t, pool, "alice", false)
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
start := time.Now()
|
||||
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestApproved, notifications.ToUser(alice),
|
||||
notifications.Payload{Name: "Moe Shop – WWW"}.Map()))
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestRejected, notifications.ToUser(alice),
|
||||
notifications.Payload{Name: "Tycho – Awake", Reason: "already have it"}.Map()))
|
||||
|
||||
d.Tick(ctx, start.Add(30*time.Minute))
|
||||
require.Empty(t, fake.Sent, "inside the window nothing goes")
|
||||
|
||||
d.Tick(ctx, start.Add(61*time.Minute))
|
||||
require.Len(t, fake.Sent, 1, "both items, one email")
|
||||
require.Equal(t, "alice@example.com", fake.Sent[0].To)
|
||||
require.Equal(t, "Minstrel: 2 updates", fake.Sent[0].Subject)
|
||||
require.Contains(t, fake.Sent[0].TextBody, "Moe Shop – WWW is on its way.")
|
||||
require.Contains(t, fake.Sent[0].TextBody, "Tycho – Awake was declined: already have it")
|
||||
|
||||
d.Tick(ctx, start.Add(3*time.Hour))
|
||||
require.Len(t, fake.Sent, 1, "never sent twice")
|
||||
}
|
||||
|
||||
func TestDigest_ReadBeforeTheEmailIsNotEmailed(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
alice := emailUser(t, pool, "alice", false)
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
start := time.Now()
|
||||
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestApproved, notifications.ToUser(alice), nil))
|
||||
_, err := q.MarkAllNotificationsRead(ctx, dbq.MarkAllNotificationsReadParams{UserID: alice})
|
||||
require.NoError(t, err)
|
||||
|
||||
d.Tick(ctx, start.Add(2*time.Hour))
|
||||
require.Empty(t, fake.Sent, "everything was read: nothing to send")
|
||||
}
|
||||
|
||||
func TestDigest_EmailOffAtNotifyTimeIsNeverSent(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
alice := emailUser(t, pool, "alice", false)
|
||||
off := false
|
||||
_, err := notifications.SaveSettings(ctx, q, alice, false, []notifications.SettingChange{
|
||||
{Kind: notifications.KindRequestApproved, Email: &off},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
start := time.Now()
|
||||
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestApproved, notifications.ToUser(alice), nil))
|
||||
on := true
|
||||
_, err = notifications.SaveSettings(ctx, q, alice, false, []notifications.SettingChange{
|
||||
{Kind: notifications.KindRequestApproved, Email: &on},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
d.Tick(ctx, start.Add(2*time.Hour))
|
||||
require.Empty(t, fake.Sent, "turning email on later does not send what came before")
|
||||
}
|
||||
|
||||
func TestDigest_NoAddressNoEmail(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
bob := mkUser(t, dbq.New(pool), "bob", false)
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestApproved, notifications.ToUser(bob), nil))
|
||||
d.Tick(ctx, time.Now().Add(2*time.Hour))
|
||||
require.Empty(t, fake.Sent)
|
||||
}
|
||||
|
||||
func TestDigest_CoalescedItemAppearsOnceWithItsLatestCount(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
root := emailUser(t, pool, "root", true)
|
||||
on := true
|
||||
_, err := notifications.SaveSettings(ctx, q, root, true, []notifications.SettingChange{
|
||||
{Kind: notifications.KindTracksMissing, Email: &on},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
start := time.Now()
|
||||
|
||||
for _, c := range []int64{3, 2} {
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindTracksMissing,
|
||||
notifications.ToAdmins(pgtype.UUID{}), notifications.Payload{Count: c}.Map()))
|
||||
}
|
||||
d.Tick(ctx, start.Add(2*time.Hour))
|
||||
require.Len(t, fake.Sent, 1)
|
||||
require.Equal(t, "Minstrel: 5 tracks went missing", fake.Sent[0].Subject)
|
||||
}
|
||||
|
||||
func TestDigest_MailerFailureRetriesWithBackoffAndSendsOnce(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
alice := emailUser(t, pool, "alice", false)
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
start := time.Now()
|
||||
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestApproved, notifications.ToUser(alice), nil))
|
||||
|
||||
fake.FailNext = errors.New("smtp: 421 try later")
|
||||
d.Tick(ctx, start.Add(61*time.Minute))
|
||||
require.Empty(t, fake.Sent)
|
||||
|
||||
d.Tick(ctx, start.Add(63*time.Minute))
|
||||
require.Empty(t, fake.Sent, "waits out the first retry gap")
|
||||
|
||||
d.Tick(ctx, start.Add(67*time.Minute))
|
||||
require.Len(t, fake.Sent, 1, "then goes")
|
||||
|
||||
d.Tick(ctx, start.Add(80*time.Minute))
|
||||
require.Len(t, fake.Sent, 1, "and is not sent again")
|
||||
}
|
||||
|
||||
func TestDigest_NewMusicWaitsForTheSummaryHour(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
alice := emailUser(t, pool, "alice", false) // timezone defaults to UTC
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
|
||||
now := time.Now().UTC()
|
||||
slot := time.Date(now.Year(), now.Month(), now.Day(), 9, 0, 0, 0, time.UTC)
|
||||
if !slot.After(now) {
|
||||
slot = slot.AddDate(0, 0, 1)
|
||||
}
|
||||
|
||||
for _, p := range []notifications.Payload{
|
||||
{Name: "Moe Shop – WWW", Artist: "Moe Shop", Title: "WWW"},
|
||||
{Name: "Moe Shop – Pure", Artist: "Moe Shop", Title: "Pure"},
|
||||
} {
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestCompleted, notifications.ToUser(alice), p.Map()))
|
||||
}
|
||||
|
||||
d.Tick(ctx, slot.Add(-time.Minute))
|
||||
require.Empty(t, fake.Sent, "new music never goes out in an hourly batch")
|
||||
|
||||
d.Tick(ctx, slot)
|
||||
require.Len(t, fake.Sent, 1)
|
||||
require.Equal(t, "Minstrel: new music in your library", fake.Sent[0].Subject)
|
||||
require.Contains(t, fake.Sent[0].TextBody, "Moe Shop\n - WWW")
|
||||
require.Contains(t, fake.Sent[0].TextBody, " - Pure")
|
||||
|
||||
d.Tick(ctx, slot.Add(5*time.Hour))
|
||||
require.Len(t, fake.Sent, 1, "one summary a day")
|
||||
}
|
||||
|
||||
func TestEmailSettings_DefaultsMatchTheMigrationAndRoundTrip(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
|
||||
got, err := notifications.LoadEmailSettings(ctx, q)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, notifications.DefaultEmailSettings, got)
|
||||
|
||||
saved, err := notifications.SaveEmailSettings(ctx, q, notifications.EmailSettings{SummaryHour: 7, BatchWindowMinutes: 30})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, notifications.EmailSettings{SummaryHour: 7, BatchWindowMinutes: 30}, saved)
|
||||
got, err = notifications.LoadEmailSettings(ctx, q)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, saved, got)
|
||||
}
|
||||
|
||||
func TestDigest_UsesTheConfiguredBatchWindow(t *testing.T) {
|
||||
pool := testPool(t)
|
||||
ctx := context.Background()
|
||||
alice := emailUser(t, pool, "alice", false)
|
||||
_, err := notifications.SaveEmailSettings(ctx, dbq.New(pool), notifications.EmailSettings{SummaryHour: 9, BatchWindowMinutes: 15})
|
||||
require.NoError(t, err)
|
||||
n := notifications.New(pool, nil, nil)
|
||||
d, fake := digestWith(pool)
|
||||
start := time.Now()
|
||||
|
||||
require.NoError(t, n.Notify(ctx, notifications.KindRequestApproved, notifications.ToUser(alice), nil))
|
||||
d.Tick(ctx, start.Add(16*time.Minute))
|
||||
require.Len(t, fake.Sent, 1)
|
||||
}
|
||||
Reference in New Issue
Block a user