From b81a6efc404081fd935bd13f3c2930ebd2f9a44f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 22:33:00 +0000 Subject: [PATCH 01/16] docs(r4): plan the mailers wave; plan the job queue as R8 R4 gets its task table: a mail contract with typed inputs, samples, text/html templates and per-sample goldens; the 14 mails converted in parallel; a /dev/mail preview behind a DEV_ROUTES flag; and the mail sender moved to one transaction per mail. The job queue becomes its own wave, R8, which takes over Q9. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UmD6gdC2ENrRcg4XZVZuxd --- docs/implementation-plan.md | 1 + docs/open-questions.md | 4 - docs/plan/r4.md | 180 +++++++++++++++++++++++++++++++++--- docs/plan/r8.md | 35 +++++++ 4 files changed, 205 insertions(+), 15 deletions(-) create mode 100644 docs/plan/r8.md diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index 230dffd5..85c492e7 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -11,6 +11,7 @@ support read directly. Rules for reading and verifying code cheaply are in `AGEN Where the modernization is going: - declared, golden-tested mailers (R4); +- one job queue for mail and feeds (R8); - structured logging through one [zap](https://github.com/uber-go/zap) logger, passed explicitly (R7); - no unmaintained or duplicate dependencies (R6); - shared plumbing in [gogo](https://github.com/can3p/gogo) (`docs/gogo-extraction.md`). diff --git a/docs/open-questions.md b/docs/open-questions.md index 96c3c3ff..83d7db50 100644 --- a/docs/open-questions.md +++ b/docs/open-questions.md @@ -8,10 +8,6 @@ When the owner answers, the answer goes where it applies (`docs/product.md` for `docs/architecture.md` for how it is built, the wave file for what a wave builds) and the question is deleted here. The reasoning goes into the history of the wave that acts on it. -- **Q9. The RSS poller downloads feeds and images inside the DB transaction** - that holds the feed row lock (`refreshFeeds` in `pkg/service/feeds/poller.go`). - With `GlobalImageDownloadTimeout` per item, a transaction can stay open for minutes. Restructure when - moving email and feed work to a general job queue? - **Q10. Audit and idempotency for mutations (narrowed 2026-09-24).** The layering is decided (`docs/architecture.md`): services take an actor, check authorization, and HTTP, API and CLI are transports over them. Still open: should every diff --git a/docs/plan/r4.md b/docs/plan/r4.md index 1f08e7cf..2f24288e 100644 --- a/docs/plan/r4.md +++ b/docs/plan/r4.md @@ -1,22 +1,180 @@ ## R4 — Mailers -Status: planned · Depends on: — · Branch: `refactor/r4-mailers` +Status: planned · Depends on: — · Branch: `claude/zealous-shannon-ieo14d` Every email becomes a declared definition: - a typed input; - a sample factory that touches no database, including the awkward variants (no subject, long subject, markup in user content, a linked URL); -- a template rendered through `html/template`; -- a golden file for the text and the HTML; -- a development preview page that lists every email rendered from its - samples. +- templates: the subject and the text through `text/template`, the HTML through `html/template`; +- a golden file per sample holding the subject, the text and the HTML; +- a development preview page that lists every email rendered from its samples. -Call sites stop building mail bodies with `fmt.Sprintf`. The mail goldens in -`pkg/mail` (`testdata/*.golden`) define "unchanged output", and escaping user -content becomes `html/template`'s job instead of each mail's. +Call sites stop building mail bodies with `fmt.Sprintf`, and escaping user content becomes `html/template`'s +job instead of each mail's. All 14 mails are in scope: the 9 in `pkg/mail` and the 5 admin notifications in +`pkg/admin` (new user, new waiting list member, signup confirmed, throwaway email signup, page failure), which +move into `pkg/mail`. -Consider replacing `dbsender`'s ad-hoc poller with a -general job queue, where the email is enqueued in the same transaction as the -state change. tommy is where a human checks what was actually delivered. E2E and browser tests assert on the mail tommy received; the only e2e code that knows about the queue is `App.WaitMailSent`/`QueuedMails` in `e2e/harness.go`, so a job queue replaces just those. +Decided by the owner: +- **Whitespace may change.** The goldens in `pkg/mail/testdata` define "unchanged output" up to whitespace: + a template may drop the stray tab indentation of today's bodies. `git diff -w` on an existing golden is + empty, or every remaining hunk is listed in the task's report with its cause (e.g. `html/template` + normalizing a URL in an `href`), and the coordinator accepts it. +- **The preview lives behind a flag**, `--dev-routes` / `DEV_ROUTES`, off by default and turned on only in + the local config (`docker-compose.yml`'s app service and `.env.example`). With it off the routes are not + mounted at all. +- **The job queue is not in R4.** Mail already enters `outgoing_emails` in the same transaction as the change + that causes it; a general queue mostly pays off for the feed poller and is its own wave, R8. R4 only fixes + the sender (M6). + +### Constraints for every task + +- `pkg/mail` exports one envelope type, `mail.Envelope` (M0 folds `Outgoing` into it). +- A mail's public constructor keeps its signature, so its service callers don't change. Only M0 (the + envelope type) and M4 (the admin mails' new package) edit callers. +- A sample is named after the golden it reproduces: `testdata/.golden`, so an existing golden is + rewritten in place and its diff reviewable. New samples add new goldens. +- The golden format is the current `mailsToGolden` one (EmailType, UniqueID, From, To, Subject, TEXT, HTML). +- A per-mail Go test whose cases are now samples is deleted; the `_NotToMyself` tests (no mail for yourself) + stay. The coordinator `git rm`s test files left empty and the admin goldens M4 replaces. +- `e2e/` is not edited, except the route-table row M5 adds. + +| Task | Tier | Owns | Done when | +|---|---|---|---| +| S0 | coordinator | `pkg/mail/*_test.go` split, `pkg/config/config.go`, `pkg/config/config_test.go`, `docker-compose.yml`, `.env.example` | the mail tests are split by area and the two settings exist; `make test-q PKG="./pkg/mail/... ./pkg/config/..."` passes | +| M0 | strong | see the section | the generic golden test renders LoginCode and ConfirmSignup from their samples with whitespace-only golden diffs, and `make check-q` passes | +| M1 | cheap | see the section | ConfirmWaitingList and Invitation render from templates; their goldens have whitespace-only diffs | +| M2 | mid | see the section | NewPost, PostPrompt and PostPromptAnswer render from templates; goldens whitespace-only | +| M3 | mid | see the section | PostCommentAuthor and PostCommentParticipants render from templates; goldens whitespace-only | +| M4 | mid | see the section | the 5 admin mails are declared in `pkg/mail` with goldens; `pkg/admin` keeps only the panic formatting | +| M5 | mid | see the section | `/dev/mail` lists every sample of every mail with the flag on, and 404s with it off | +| M6 | mid | see the section | each queued mail is sent in its own transaction and the retry schedule is a setting | + +Order: S0, then M0 alone (it defines the contract), then M1–M6 in parallel. M5 starts with only M0's mails +registered; the registry fills as M1–M4 land. + +### S0 Split the tests and add the settings + +The coordinator does this by script before M0. + +- Split `pkg/mail/mail_test.go` by verbatim moves: the helpers, `TestConfirmSignup` and `TestLoginCode` stay; + the `TestNewPost*`, `TestPostPrompt*` tests go to `posts_test.go`; the `TestPostComment*` tests to + `comments_test.go`. Prove each move by diffing line multisets. +- `config.Web.DevRoutes` (`Switch`, `--dev-routes`, `DEV_ROUTES`, default false, "Serve the /dev pages + (mail previews); local development only") and `config.Mail.RetryIntervals` (`[]time.Duration`, + `--email-retry-intervals`, `EMAIL_RETRY_INTERVALS`, comma-separated, default `10s,60s,30m`, "Waits between + attempts to send a mail; a mail fails after one attempt more than there are waits"), with rows in + `config_test.go`. +- `DEV_ROUTES: "true"` in `docker-compose.yml`'s app environment and `DEV_ROUTES=true` in `.env.example`. + +### M0 The mail contract + +Owns `pkg/mail/mail.go` (new), `pkg/mail/templates/` (new), `pkg/mail/confirm_signup.go`, +`pkg/mail/login_code.go`, `pkg/mail/new_post.go` (only to remove `Outgoing`), `pkg/mail/mail_test.go`, +`pkg/mail/golden_test.go` (new), `pkg/mail/testdata/confirm_signup.golden`, +`pkg/mail/testdata/login_code.golden`, `pkg/service/posts/posts.go`, and the `Outgoing` → `Envelope` +renames in `pkg/mail/*_test.go` and `pkg/service/posts/*.go`. + +Define in `pkg/mail/mail.go`: + +- `Envelope` (unique id, email type, `*sender.Mail`), replacing `Outgoing` everywhere; `posts.Service.queue` + and `queueE` take `*mail.Envelope`. +- A declaration of one mail: its email type (the queue's `email_type`), its template name, a typed input, + and a `Samples` function returning named inputs built from in-memory `model` structs, no database. The + shape is yours (a generic `Definition[In]` with a `Render(In) (*Envelope, error)` is the expected one); + the from address, the recipient and the unique id are part of the input or computed from it. +- Templates embedded with `embed.FS` from `pkg/mail/templates/.subject.txt`, `.txt` (both + `text/template`) and `.html` (`html/template`), parsed once at init; a missing or broken template + panics at init, not at send time. +- A registry, `mail.All()`, listing every declared mail with its rendered samples, in a stable order. The + preview page (M5) and the golden test use it; it is the only list of mails. +- `golden_test.go`: one test, `TestMails_Goldens`, that renders every sample of every registered mail into + `testdata/.golden` with the `mailsToGolden` format (move the helper here), plus a test that every + registered mail has at least one sample and every sample name is unique. + +Convert LoginCode and ConfirmSignup as the reference conversions: their constructors keep their signatures +and build the input, then render. Their samples are `login_code` and `confirm_signup`; delete +`TestLoginCode` and `TestConfirmSignup` once the goldens reproduce them. + +Write the conventions the other tasks follow (where a mail's declaration, samples and templates live, how a +`template.HTML` markdown body enters an input) as the package doc comment of `mail.go`; M1–M4 read it. + +### M1 Account mails + +Owns `pkg/mail/confirm_waiting_list.go`, `pkg/mail/invite.go`, their templates in `pkg/mail/templates/`, +`pkg/mail/send_internal_test.go`, `pkg/mail/testdata/send_actual_confirm_waiting_list.golden`, +`pkg/mail/testdata/send_actual_invitation.golden`. + +Follow the conventions in `pkg/mail/mail.go`'s doc comment. Samples: the existing two, keeping their names. +Delete `TestSendActualConfirmWaitingList` and `TestSendActualInvitation` once the goldens reproduce them; the +`ValidateFormat` tests stay. + +### M2 Post mails + +Owns `pkg/mail/new_post.go`, `pkg/mail/post_prompt.go`, `pkg/mail/post_prompt_answer.go`, their templates, +`pkg/mail/posts_test.go`, and the goldens `new_post*.golden`, `post_prompt.golden`, +`post_prompt_answer.golden` in `pkg/mail/testdata`. + +Samples reproduce every existing golden under its name (`new_post`, `new_post_no_subject`, +`new_post_with_url`, `new_post_special_chars`, `new_post_empty_body`, `post_prompt`, `post_prompt_answer`) +and add `new_post_long_subject`, `new_post_markup_in_body` (a raw ` and a link`)}, + } +} - if post.URL != nil { - urlText = fmt.Sprintf("\nLinked URL: %s", post.URL.URL) - htmlUrlSection = fmt.Sprintf(`

Linked URL: %s

`, html.EscapeString(post.URL.URL), html.EscapeString(post.URL.URL)) +// NewPost formats the notification about a new post for one connection of its +// author. It returns nil when there is nobody to notify. +func NewPost(site links.Site, from string, mediaReplacer types.Replacer[string], user *model.User, connection *model.User, post *model.Post) *Envelope { + // we're not sending email notifications to ourselves + if user.ID == connection.ID { + return nil } - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: connection.Email, - }, - }, - Subject: fmt.Sprintf("New post \"%s\" from %s", subject, user.Username), - Text: fmt.Sprintf(`Hi! - -@%s has published a new post "%s"%s - -Head to the post to leave a comment! %s`, user.Username, subject, urlText, link), - Html: fmt.Sprintf(` -

Hi!

- -

@%s has published a new post "%s"

- -
%s
%s - -

Head to the post to leave a comment! %s

`, - html.EscapeString(user.Username), - html.EscapeString(subject), - htmlbody, - htmlUrlSection, - html.EscapeString(link), - html.EscapeString(link)), + in := NewPostInput{ + From: from, + PostID: post.ID, + ConnectionID: connection.ID, + To: connection.Email, + Username: user.Username, + Subject: postops.PostSubject(post.Subject), + Link: site.Abs("post", post.ID), + // there reason to omit body in the text version is that we should redo the logic with cut, gallery etc + // and I have no desire to spend time on that + Body: newPostBody(site, post.ID, post.Body, mediaReplacer), + } + + if post.URL != nil { + in.LinkedURL = post.URL.URL } - return &Envelope{UniqueID: post.ID + connection.ID, Type: "post_notification", Mail: mail} + return newPostMail.MustRender(in) } diff --git a/pkg/mail/post_prompt.go b/pkg/mail/post_prompt.go index 55a7318e..17f1f677 100644 --- a/pkg/mail/post_prompt.go +++ b/pkg/mail/post_prompt.go @@ -1,48 +1,57 @@ package mail import ( - "fmt" - "html" - "net/mail" - - "github.com/can3p/gogo/sender" "github.com/can3p/pcom/pkg/links" "github.com/can3p/pcom/pkg/model" ) -// PostPrompt formats the notification about a prompt for its recipient. It -// returns nil when there is nobody to notify. -func PostPrompt(site links.Site, from string, asker *model.User, recipient *model.User, postPrompt *model.PostPrompt) *Outgoing { - // we're not sending email notifications to ourselves - if asker.ID == recipient.ID { - return nil - } - - link := site.Abs("write", "prompt", postPrompt.ID) +// PostPromptInput is what the post prompt mail shows: who asks, what they +// ask and where to answer. +type PostPromptInput struct { + From string + PromptID string + To string + Asker string + Message string + Link string +} - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: recipient.Email, - }, - }, - Subject: fmt.Sprintf("New prompt from %s", asker.Username), - Text: fmt.Sprintf(`Hi! +// Header addresses the mail to the recipient of the prompt, unique per +// prompt. +func (in PostPromptInput) Header() Header { + return Header{UniqueID: in.PromptID, From: FromPcom(in.From), To: To(in.To)} +} -@%s has asked you to write a post on "%s" +var postPromptMail = declare("post_prompt", "post_prompt", postPromptSamples) -Head to new post page to give an update! %s`, asker.Username, postPrompt.Message, link), - Html: fmt.Sprintf(` -

Hi!

+func postPromptSamples() []Sample[PostPromptInput] { + sample := func(message string) PostPromptInput { + return PostPromptInput{ + From: SampleFrom, PromptID: "prompt-1", To: "recipient@example.test", Asker: "alice", + Message: message, Link: SampleSite.Abs("write", "prompt", "prompt-1"), + } + } -

@%s has asked you to write a post on "%s"

+ return []Sample[PostPromptInput]{ + {Name: "post_prompt", Input: sample("What is your favorite hobby?")}, + {Name: "post_prompt_markup", Input: sample(`What is bold & "quoted" ?`)}, + } +} -

Head to new post page to give an update! %s

`, html.EscapeString(asker.Username), html.EscapeString(postPrompt.Message), html.EscapeString(link), html.EscapeString(link)), +// PostPrompt formats the notification about a prompt for its recipient. It +// returns nil when there is nobody to notify. +func PostPrompt(site links.Site, from string, asker *model.User, recipient *model.User, postPrompt *model.PostPrompt) *Envelope { + // we're not sending email notifications to ourselves + if asker.ID == recipient.ID { + return nil } - return &Outgoing{UniqueID: postPrompt.ID, Type: "post_prompt", Mail: mail} + return postPromptMail.MustRender(PostPromptInput{ + From: from, + PromptID: postPrompt.ID, + To: recipient.Email, + Asker: asker.Username, + Message: postPrompt.Message, + Link: site.Abs("write", "prompt", postPrompt.ID), + }) } diff --git a/pkg/mail/post_prompt_answer.go b/pkg/mail/post_prompt_answer.go index 854f03b6..58db9115 100644 --- a/pkg/mail/post_prompt_answer.go +++ b/pkg/mail/post_prompt_answer.go @@ -1,47 +1,51 @@ package mail import ( - "fmt" - "github.com/samber/lo" - "html" - "net/mail" - - "github.com/can3p/gogo/sender" "github.com/can3p/pcom/pkg/links" "github.com/can3p/pcom/pkg/model" "github.com/can3p/pcom/pkg/postops" + "github.com/samber/lo" ) -// PostPromptAnswer formats the notification for the asker of a prompt that -// the recipient answered with a post. -func PostPromptAnswer(site links.Site, from string, asker, recipient *model.User, post *model.Post, postPrompt *model.PostPrompt) *Outgoing { - link := site.Abs("post", lo.FromPtr(postPrompt.PostID)) - - subject := postops.PostSubject(post.Subject) - - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: asker.Email, - }, - }, - Subject: fmt.Sprintf("Response to your prompt from %s", recipient.Username), - Text: fmt.Sprintf(`Hi! - -@%s has responded on your prompt "%s" with the post "%s" +// PostPromptAnswerInput is what the post prompt answer mail shows: the +// prompt, who answered it and the post they answered with. +type PostPromptAnswerInput struct { + From string + PostID string + To string + Responder string + Message string + Subject string + Link string +} -Check out their post! %s`, recipient.Username, postPrompt.Message, subject, link), - Html: fmt.Sprintf(` -

Hi!

+// Header addresses the mail to the asker of the prompt, unique per +// answering post. +func (in PostPromptAnswerInput) Header() Header { + return Header{UniqueID: in.PostID, From: FromPcom(in.From), To: To(in.To)} +} -

@%s has responded on your prompt "%s" with the post "%s"

+var postPromptAnswerMail = declare("post_prompt_answer", "post_prompt_answer", postPromptAnswerSamples) -

Head to new post page to give an update! %s

`, html.EscapeString(recipient.Username), html.EscapeString(postPrompt.Message), html.EscapeString(subject), html.EscapeString(link), html.EscapeString(link)), +func postPromptAnswerSamples() []Sample[PostPromptAnswerInput] { + return []Sample[PostPromptAnswerInput]{ + {Name: "post_prompt_answer", Input: PostPromptAnswerInput{ + From: SampleFrom, PostID: "post-1", To: "asker@example.test", Responder: "bob", + Message: "What is your favorite hobby?", Subject: "My Answer", Link: SampleSite.Abs("post", "post-1"), + }}, } +} - return &Outgoing{UniqueID: post.ID, Type: "post_prompt_answer", Mail: mail} +// PostPromptAnswer formats the notification for the asker of a prompt that +// the recipient answered with a post. +func PostPromptAnswer(site links.Site, from string, asker, recipient *model.User, post *model.Post, postPrompt *model.PostPrompt) *Envelope { + return postPromptAnswerMail.MustRender(PostPromptAnswerInput{ + From: from, + PostID: post.ID, + To: asker.Email, + Responder: recipient.Username, + Message: postPrompt.Message, + Subject: postops.PostSubject(post.Subject), + Link: site.Abs("post", lo.FromPtr(postPrompt.PostID)), + }) } diff --git a/pkg/mail/posts_test.go b/pkg/mail/posts_test.go index 88dd4d64..355aecf4 100644 --- a/pkg/mail/posts_test.go +++ b/pkg/mail/posts_test.go @@ -8,41 +8,65 @@ import ( "github.com/can3p/pcom/pkg/mail" "github.com/can3p/pcom/pkg/model" "github.com/can3p/pcom/pkg/testutil/fakesender" - "github.com/can3p/pcom/pkg/testutil/golden" "github.com/stretchr/testify/require" ) -func TestNewPost(t *testing.T) { - t.Parallel() +// sampleEnvelope is the rendering of the sample called name. +func sampleEnvelope(t *testing.T, name string) *mail.Envelope { + t.Helper() - user := &model.User{ - ID: "user-1", - Email: "user@example.test", - Username: "alice", - } - connection := &model.User{ - ID: "user-2", - Email: "connection@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new("Test Post"), - Body: "This is a test post body", - UserID: user.ID, + for _, m := range mail.All() { + for _, s := range m.Samples { + if s.Name == name { + require.NoError(t, s.Err) + + return s.Envelope + } + } } - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } + require.Failf(t, "no such sample", "sample %s", name) - err := deliver(ctx, sender)(mail.NewPost(links.Site{}, testFrom, mediaReplacer, user, connection, post)) - require.NoError(t, err) + return nil +} - sent := sender.Sent() - require.Len(t, sent, 1) +func keepMedia(in string) (bool, string) { return false, in } - golden.Assert(t, "new_post", mailsToGolden(sent)) +func postUsers() (user, connection *model.User) { + return &model.User{ID: "user-1", Email: "user@example.test", Username: "alice"}, + &model.User{ID: "user-2", Email: "connection@example.test", Username: "bob"} +} + +func TestNewPost_MatchesSamples(t *testing.T) { + t.Parallel() + + user, connection := postUsers() + newPost := func(subject *string, body string) *model.Post { + return &model.Post{ID: "post-1", Subject: subject, Body: body, UserID: user.ID} + } + withURL := newPost(new("Check out this article"), "This is a test post body") + withURL.URLID = new("url-1") + setPostURL(withURL, &model.NormalizedURL{ID: "url-1", URL: "https://example.com/article"}) + + tests := []struct { + sample string + post *model.Post + }{ + {"new_post", newPost(new("Test Post"), "This is a test post body")}, + {"new_post_no_subject", newPost(nil, "This is a test post body")}, + {"new_post_with_url", withURL}, + {"new_post_special_chars", newPost(new(`Test bold and "quotes"`), "This is a test post body")}, + {"new_post_empty_body", newPost(new("Test Post"), "")}, + } + + for _, tt := range tests { + t.Run(tt.sample, func(t *testing.T) { + t.Parallel() + + got := mail.NewPost(links.Site{}, testFrom, keepMedia, user, connection, tt.post) + require.Equal(t, sampleEnvelope(t, tt.sample), got) + }) + } } func TestNewPost_NotToMyself(t *testing.T) { @@ -62,29 +86,20 @@ func TestNewPost_NotToMyself(t *testing.T) { sender := fakesender.New() ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } // Passing the same user as both author and connection should skip sending - err := deliver(ctx, sender)(mail.NewPost(links.Site{}, testFrom, mediaReplacer, user, user, post)) + err := deliver(ctx, sender)(mail.NewPost(links.Site{}, testFrom, keepMedia, user, user, post)) require.NoError(t, err) sent := sender.Sent() require.Empty(t, sent, "should not send email to self") } -func TestPostPrompt(t *testing.T) { +func TestPostPrompt_MatchesSample(t *testing.T) { t.Parallel() - asker := &model.User{ - ID: "user-1", - Email: "asker@example.test", - Username: "alice", - } - recipient := &model.User{ - ID: "user-2", - Email: "recipient@example.test", - Username: "bob", - } + asker := &model.User{ID: "user-1", Email: "asker@example.test", Username: "alice"} + recipient := &model.User{ID: "user-2", Email: "recipient@example.test", Username: "bob"} prompt := &model.PostPrompt{ ID: "prompt-1", AskerID: asker.ID, @@ -92,16 +107,8 @@ func TestPostPrompt(t *testing.T) { Message: "What is your favorite hobby?", } - sender := fakesender.New() - ctx := context.Background() - - err := deliver(ctx, sender)(mail.PostPrompt(links.Site{}, testFrom, asker, recipient, prompt)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "post_prompt", mailsToGolden(sent)) + got := mail.PostPrompt(links.Site{}, testFrom, asker, recipient, prompt) + require.Equal(t, sampleEnvelope(t, "post_prompt"), got) } func TestPostPrompt_NotToMyself(t *testing.T) { @@ -130,19 +137,11 @@ func TestPostPrompt_NotToMyself(t *testing.T) { require.Empty(t, sent, "should not send email to self") } -func TestPostPromptAnswer(t *testing.T) { +func TestPostPromptAnswer_MatchesSample(t *testing.T) { t.Parallel() - asker := &model.User{ - ID: "user-1", - Email: "asker@example.test", - Username: "alice", - } - responder := &model.User{ - ID: "user-2", - Email: "responder@example.test", - Username: "bob", - } + asker := &model.User{ID: "user-1", Email: "asker@example.test", Username: "alice"} + responder := &model.User{ID: "user-2", Email: "responder@example.test", Username: "bob"} post := &model.Post{ ID: "post-1", Subject: new("My Answer"), @@ -157,152 +156,6 @@ func TestPostPromptAnswer(t *testing.T) { PostID: new(post.ID), } - sender := fakesender.New() - ctx := context.Background() - - err := deliver(ctx, sender)(mail.PostPromptAnswer(links.Site{}, testFrom, asker, responder, post, prompt)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "post_prompt_answer", mailsToGolden(sent)) -} - -func TestNewPost_NoSubject(t *testing.T) { - t.Parallel() - - user := &model.User{ - ID: "user-1", - Email: "user@example.test", - Username: "alice", - } - connection := &model.User{ - ID: "user-2", - Email: "connection@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: nil, // No subject - Body: "This is a test post body", - UserID: user.ID, - } - - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } - - err := deliver(ctx, sender)(mail.NewPost(links.Site{}, testFrom, mediaReplacer, user, connection, post)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "new_post_no_subject", mailsToGolden(sent)) -} - -func TestNewPost_WithLinkedURL(t *testing.T) { - t.Parallel() - - user := &model.User{ - ID: "user-1", - Email: "user@example.test", - Username: "alice", - } - connection := &model.User{ - ID: "user-2", - Email: "connection@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new("Check out this article"), - Body: "This is a test post body", - UserID: user.ID, - URLID: new("url-1"), - } - // Post has a linked URL relation loaded - setPostURL(post, &model.NormalizedURL{ - ID: "url-1", - URL: "https://example.com/article", - }) - - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } - - err := deliver(ctx, sender)(mail.NewPost(links.Site{}, testFrom, mediaReplacer, user, connection, post)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "new_post_with_url", mailsToGolden(sent)) -} - -func TestNewPost_SubjectWithSpecialChars(t *testing.T) { - t.Parallel() - - user := &model.User{ - ID: "user-1", - Email: "user@example.test", - Username: "alice", - } - connection := &model.User{ - ID: "user-2", - Email: "connection@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new(`Test bold and "quotes"`), - Body: "This is a test post body", - UserID: user.ID, - } - - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } - - err := deliver(ctx, sender)(mail.NewPost(links.Site{}, testFrom, mediaReplacer, user, connection, post)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "new_post_special_chars", mailsToGolden(sent)) -} - -func TestNewPost_EmptyBody(t *testing.T) { - t.Parallel() - - user := &model.User{ - ID: "user-1", - Email: "user@example.test", - Username: "alice", - } - connection := &model.User{ - ID: "user-2", - Email: "connection@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new("Test Post"), - Body: "", // Empty body - UserID: user.ID, - } - - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } - - err := deliver(ctx, sender)(mail.NewPost(links.Site{}, testFrom, mediaReplacer, user, connection, post)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "new_post_empty_body", mailsToGolden(sent)) + got := mail.PostPromptAnswer(links.Site{}, testFrom, asker, responder, post, prompt) + require.Equal(t, sampleEnvelope(t, "post_prompt_answer"), got) } diff --git a/pkg/mail/templates/new_post.html b/pkg/mail/templates/new_post.html new file mode 100644 index 00000000..bc2352b4 --- /dev/null +++ b/pkg/mail/templates/new_post.html @@ -0,0 +1,8 @@ + +

Hi!

+ +

@{{.Username}} has published a new post "{{.Subject}}"

+ +
{{.Body}}
{{if .LinkedURL}}

Linked URL: {{.LinkedURL}}

{{end}} + +

Head to the post to leave a comment! {{.Link}}

diff --git a/pkg/mail/templates/new_post.subject.txt b/pkg/mail/templates/new_post.subject.txt new file mode 100644 index 00000000..5a5ec69c --- /dev/null +++ b/pkg/mail/templates/new_post.subject.txt @@ -0,0 +1 @@ +New post "{{.Subject}}" from {{.Username}} diff --git a/pkg/mail/templates/new_post.txt b/pkg/mail/templates/new_post.txt new file mode 100644 index 00000000..87a5e466 --- /dev/null +++ b/pkg/mail/templates/new_post.txt @@ -0,0 +1,6 @@ +Hi! + +@{{.Username}} has published a new post "{{.Subject}}"{{if .LinkedURL}} +Linked URL: {{.LinkedURL}}{{end}} + +Head to the post to leave a comment! {{.Link}} diff --git a/pkg/mail/templates/post_prompt.html b/pkg/mail/templates/post_prompt.html new file mode 100644 index 00000000..c10892f1 --- /dev/null +++ b/pkg/mail/templates/post_prompt.html @@ -0,0 +1,6 @@ + +

Hi!

+ +

@{{.Asker}} has asked you to write a post on "{{.Message}}"

+ +

Head to new post page to give an update! {{.Link}}

diff --git a/pkg/mail/templates/post_prompt.subject.txt b/pkg/mail/templates/post_prompt.subject.txt new file mode 100644 index 00000000..a93a49c8 --- /dev/null +++ b/pkg/mail/templates/post_prompt.subject.txt @@ -0,0 +1 @@ +New prompt from {{.Asker}} diff --git a/pkg/mail/templates/post_prompt.txt b/pkg/mail/templates/post_prompt.txt new file mode 100644 index 00000000..5b7b36e6 --- /dev/null +++ b/pkg/mail/templates/post_prompt.txt @@ -0,0 +1,5 @@ +Hi! + +@{{.Asker}} has asked you to write a post on "{{.Message}}" + +Head to new post page to give an update! {{.Link}} diff --git a/pkg/mail/templates/post_prompt_answer.html b/pkg/mail/templates/post_prompt_answer.html new file mode 100644 index 00000000..ab697629 --- /dev/null +++ b/pkg/mail/templates/post_prompt_answer.html @@ -0,0 +1,6 @@ + +

Hi!

+ +

@{{.Responder}} has responded on your prompt "{{.Message}}" with the post "{{.Subject}}"

+ +

Head to new post page to give an update! {{.Link}}

diff --git a/pkg/mail/templates/post_prompt_answer.subject.txt b/pkg/mail/templates/post_prompt_answer.subject.txt new file mode 100644 index 00000000..c6e4cb73 --- /dev/null +++ b/pkg/mail/templates/post_prompt_answer.subject.txt @@ -0,0 +1 @@ +Response to your prompt from {{.Responder}} diff --git a/pkg/mail/templates/post_prompt_answer.txt b/pkg/mail/templates/post_prompt_answer.txt new file mode 100644 index 00000000..3f54e001 --- /dev/null +++ b/pkg/mail/templates/post_prompt_answer.txt @@ -0,0 +1,5 @@ +Hi! + +@{{.Responder}} has responded on your prompt "{{.Message}}" with the post "{{.Subject}}" + +Check out their post! {{.Link}} diff --git a/pkg/mail/testdata/new_post.golden b/pkg/mail/testdata/new_post.golden index 2cb5f15b..cfb72b2e 100644 --- a/pkg/mail/testdata/new_post.golden +++ b/pkg/mail/testdata/new_post.golden @@ -13,11 +13,11 @@ Head to the post to leave a comment! /posts/post-1 --- HTML --- -

Hi!

+

Hi!

-

@alice has published a new post "Test Post"

+

@alice has published a new post "Test Post"

-

This is a test post body

+

This is a test post body

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! /posts/post-1

diff --git a/pkg/mail/testdata/new_post_empty_body.golden b/pkg/mail/testdata/new_post_empty_body.golden index 9fe2a5a8..a9bfce79 100644 --- a/pkg/mail/testdata/new_post_empty_body.golden +++ b/pkg/mail/testdata/new_post_empty_body.golden @@ -13,10 +13,10 @@ Head to the post to leave a comment! /posts/post-1 --- HTML --- -

Hi!

+

Hi!

-

@alice has published a new post "Test Post"

+

@alice has published a new post "Test Post"

-
+
-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! /posts/post-1

diff --git a/pkg/mail/testdata/new_post_long_subject.golden b/pkg/mail/testdata/new_post_long_subject.golden new file mode 100644 index 00000000..d182941e --- /dev/null +++ b/pkg/mail/testdata/new_post_long_subject.golden @@ -0,0 +1,23 @@ +EmailType: post_notification +UniqueID: post-1user-2 +From: Your pcom +To: connection@example.test +Subject: New post "A very long subject that goes on and on and on to see how the mail copes with a title that no one would reasonably write but someone surely will" from alice + +--- TEXT --- +Hi! + +@alice has published a new post "A very long subject that goes on and on and on to see how the mail copes with a title that no one would reasonably write but someone surely will" + +Head to the post to leave a comment! /posts/post-1 + +--- HTML --- + +

Hi!

+ +

@alice has published a new post "A very long subject that goes on and on and on to see how the mail copes with a title that no one would reasonably write but someone surely will"

+ +

This is a test post body

+
+ +

Head to the post to leave a comment! /posts/post-1

diff --git a/pkg/mail/testdata/new_post_markup_in_body.golden b/pkg/mail/testdata/new_post_markup_in_body.golden new file mode 100644 index 00000000..4d49f68c --- /dev/null +++ b/pkg/mail/testdata/new_post_markup_in_body.golden @@ -0,0 +1,23 @@ +EmailType: post_notification +UniqueID: post-1user-2 +From: Your pcom +To: connection@example.test +Subject: New post "Test Post" from alice + +--- TEXT --- +Hi! + +@alice has published a new post "Test Post" + +Head to the post to leave a comment! /posts/post-1 + +--- HTML --- + +

Hi!

+ +

@alice has published a new post "Test Post"

+ +

Hello alert(1) and a link

+
+ +

Head to the post to leave a comment! /posts/post-1

diff --git a/pkg/mail/testdata/new_post_no_subject.golden b/pkg/mail/testdata/new_post_no_subject.golden index 8e025ce8..50b3f50f 100644 --- a/pkg/mail/testdata/new_post_no_subject.golden +++ b/pkg/mail/testdata/new_post_no_subject.golden @@ -13,11 +13,11 @@ Head to the post to leave a comment! /posts/post-1 --- HTML --- -

Hi!

+

Hi!

-

@alice has published a new post "No Subject"

+

@alice has published a new post "No Subject"

-

This is a test post body

+

This is a test post body

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! /posts/post-1

diff --git a/pkg/mail/testdata/new_post_special_chars.golden b/pkg/mail/testdata/new_post_special_chars.golden index e3ffce66..d2cf5291 100644 --- a/pkg/mail/testdata/new_post_special_chars.golden +++ b/pkg/mail/testdata/new_post_special_chars.golden @@ -13,11 +13,11 @@ Head to the post to leave a comment! /posts/post-1 --- HTML --- -

Hi!

+

Hi!

-

@alice has published a new post "Test <b>bold</b> and "quotes""

+

@alice has published a new post "Test <b>bold</b> and "quotes""

-

This is a test post body

+

This is a test post body

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! /posts/post-1

diff --git a/pkg/mail/testdata/new_post_with_url.golden b/pkg/mail/testdata/new_post_with_url.golden index 8d573736..c1b577db 100644 --- a/pkg/mail/testdata/new_post_with_url.golden +++ b/pkg/mail/testdata/new_post_with_url.golden @@ -14,11 +14,11 @@ Head to the post to leave a comment! /posts/post-1 --- HTML --- -

Hi!

+

Hi!

-

@alice has published a new post "Check out this article"

+

@alice has published a new post "Check out this article"

-

This is a test post body

+

This is a test post body

Linked URL: https://example.com/article

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! /posts/post-1

diff --git a/pkg/mail/testdata/post_prompt.golden b/pkg/mail/testdata/post_prompt.golden index 99628b85..c94b8c84 100644 --- a/pkg/mail/testdata/post_prompt.golden +++ b/pkg/mail/testdata/post_prompt.golden @@ -13,8 +13,8 @@ Head to new post page to give an update! /write?prompt=prompt-1 --- HTML --- -

Hi!

+

Hi!

-

@alice has asked you to write a post on "What is your favorite hobby?"

+

@alice has asked you to write a post on "What is your favorite hobby?"

-

Head to new post page to give an update! /write?prompt=prompt-1

+

Head to new post page to give an update! /write?prompt=prompt-1

diff --git a/pkg/mail/testdata/post_prompt_answer.golden b/pkg/mail/testdata/post_prompt_answer.golden index 5f4b1d7b..e1cf95be 100644 --- a/pkg/mail/testdata/post_prompt_answer.golden +++ b/pkg/mail/testdata/post_prompt_answer.golden @@ -13,8 +13,8 @@ Check out their post! /posts/post-1 --- HTML --- -

Hi!

+

Hi!

-

@bob has responded on your prompt "What is your favorite hobby?" with the post "My Answer"

+

@bob has responded on your prompt "What is your favorite hobby?" with the post "My Answer"

-

Head to new post page to give an update! /posts/post-1

+

Head to new post page to give an update! /posts/post-1

diff --git a/pkg/mail/testdata/post_prompt_markup.golden b/pkg/mail/testdata/post_prompt_markup.golden new file mode 100644 index 00000000..e230b373 --- /dev/null +++ b/pkg/mail/testdata/post_prompt_markup.golden @@ -0,0 +1,20 @@ +EmailType: post_prompt +UniqueID: prompt-1 +From: Your pcom +To: recipient@example.test +Subject: New prompt from alice + +--- TEXT --- +Hi! + +@alice has asked you to write a post on "What is bold & "quoted" ?" + +Head to new post page to give an update! /write?prompt=prompt-1 + +--- HTML --- + +

Hi!

+ +

@alice has asked you to write a post on "What is <b>bold</b> & "quoted" <script>alert(1)</script>?"

+ +

Head to new post page to give an update! /write?prompt=prompt-1

From 9bf3141bf211a40298a9dde3677535823be94291 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 22:57:11 +0000 Subject: [PATCH 08/16] refactor(r4): M6 send each queued mail in its own transaction; retry schedule as a setting Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UmD6gdC2ENrRcg4XZVZuxd --- cmd/web/serve.go | 2 +- pkg/mail/sender/dbsender/dbsender.go | 75 +++++++++++++++------ pkg/mail/sender/dbsender/dbsender_test.go | 82 +++++++++++++++++++++-- pkg/repo/mail_queue.go | 36 ++++++++-- 4 files changed, 161 insertions(+), 34 deletions(-) diff --git a/cmd/web/serve.go b/cmd/web/serve.go index f2df8ae4..8c621886 100644 --- a/cmd/web/serve.go +++ b/cmd/web/serve.go @@ -44,7 +44,7 @@ func (c *serveCmd) Execute([]string) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - dbSender := dbsender.NewSender(repo.New(db), mailjet.NewSenderFromConfig(&cfg.Mail.Mailjet)) + dbSender := dbsender.NewSender(repo.New(db), mailjet.NewSenderFromConfig(&cfg.Mail.Mailjet), dbsender.WithRetryIntervals(cfg.Mail.RetryIntervals)) go dbSender.RunPoller(ctx, cfg.Mail.PollInterval) diff --git a/pkg/mail/sender/dbsender/dbsender.go b/pkg/mail/sender/dbsender/dbsender.go index 316edcbe..eb393d21 100644 --- a/pkg/mail/sender/dbsender/dbsender.go +++ b/pkg/mail/sender/dbsender/dbsender.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "log/slog" + "slices" "time" "github.com/can3p/gogo/sender" @@ -13,23 +14,43 @@ import ( "github.com/google/uuid" ) -const attemptsNumber = 3 - -var retryIntervals = []time.Duration{10 * time.Second, 60 * time.Second, 30 * time.Minute} +// DefaultRetryIntervals is the wait before each retry of a failed mail, equal +// to the default of config.Mail.RetryIntervals. A mail fails for good after +// one attempt more than there are intervals. +var DefaultRetryIntervals = []time.Duration{10 * time.Second, 60 * time.Second, 30 * time.Minute} type dbSender struct { - realSender sender.Sender - store *repo.Store + realSender sender.Sender + store *repo.Store + retryIntervals []time.Duration +} + +// Option configures the sender. +type Option func(*dbSender) + +// WithRetryIntervals sets the wait before each retry of a failed mail; a mail +// fails for good after len(intervals)+1 attempts. +func WithRetryIntervals(intervals []time.Duration) Option { + return func(m *dbSender) { + m.retryIntervals = slices.Clone(intervals) + } } // compile-time check that dbSender is the mail queue. var _ repo.MailQueue = (*dbSender)(nil) -func NewSender(db *repo.Store, realSender sender.Sender) *dbSender { - return &dbSender{ - realSender: realSender, - store: db, +func NewSender(db *repo.Store, realSender sender.Sender, opts ...Option) *dbSender { + m := &dbSender{ + realSender: realSender, + store: db, + retryIntervals: slices.Clone(DefaultRetryIntervals), } + + for _, opt := range opts { + opt(m) + } + + return m } // RunPoller sends the queued mail every interval until ctx is done. @@ -57,25 +78,35 @@ func (m *dbSender) sendEmails(ctx context.Context) (err error) { } }() - return m.store.Tx(ctx, func(tx *repo.Store) error { - pending, err := tx.GetPendingEmails(ctx) + ids, err := m.store.ListDueEmailIDs(ctx) + if err != nil { + return err + } + + for _, id := range ids { + if err := m.sendOne(ctx, id); err != nil { + slog.Warn("failed to send email", "email_id", id, "err", err) + } + } + + return nil +} +// sendOne sends one mail in its own transaction: it locks the row, sends the +// mail and records the outcome. A row another worker holds, or that is no +// longer new, is skipped. A failure here leaves every other mail alone. +func (m *dbSender) sendOne(ctx context.Context, id string) error { + return m.store.Tx(ctx, func(tx *repo.Store) error { + outgoing, err := tx.LockDueEmail(ctx, id) if err != nil { return err } - if len(pending) == 0 { + if outgoing == nil { return nil } - for _, outgoing := range pending { - if err := m.trySendEmail(ctx, tx, outgoing); err != nil { - slog.Warn("failed to send email", "email_id", outgoing.ID, "err", err) - continue - } - } - - return nil + return m.trySendEmail(ctx, tx, outgoing) }) } @@ -93,8 +124,8 @@ func (m *dbSender) trySendEmail(ctx context.Context, tx *repo.Store, outgoing *m outgoing.Status = model.OutgoingEmailStatusSent outgoing.SentAt = new(time.Now()) } else { - if outgoing.AttemptsNumber < attemptsNumber { - outgoing.TryAt = time.Now().Add(retryIntervals[outgoing.AttemptsNumber]) + if outgoing.AttemptsNumber < len(m.retryIntervals) { + outgoing.TryAt = time.Now().Add(m.retryIntervals[outgoing.AttemptsNumber]) outgoing.AttemptsNumber = outgoing.AttemptsNumber + 1 } else { outgoing.Status = model.OutgoingEmailStatusFailed diff --git a/pkg/mail/sender/dbsender/dbsender_test.go b/pkg/mail/sender/dbsender/dbsender_test.go index ffbb84d7..c3c768f4 100644 --- a/pkg/mail/sender/dbsender/dbsender_test.go +++ b/pkg/mail/sender/dbsender/dbsender_test.go @@ -99,21 +99,21 @@ func TestTrySendEmail(t *testing.T) { real.FailWith(errors.New("smtp down")) - // Attempts 1 through attemptsNumber fail and reschedule with the + // Attempts 1 through len(DefaultRetryIntervals) fail and reschedule with the // matching interval, keeping the email in the New status. - for i := range attemptsNumber { + for i := range len(DefaultRetryIntervals) { trySend(t, ctx, m, store, outgoing) require.Equal(t, i+1, outgoing.AttemptsNumber, "attempt %d", i+1) require.Equal(t, model.OutgoingEmailStatusNew, outgoing.Status, "attempt %d", i+1) - require.WithinDuration(t, time.Now().Add(retryIntervals[i]), outgoing.TryAt, 5*time.Second, "attempt %d", i+1) + require.WithinDuration(t, time.Now().Add(DefaultRetryIntervals[i]), outgoing.TryAt, 5*time.Second, "attempt %d", i+1) } - // One more failure past attemptsNumber gives up for good. + // One more failure past the last interval gives up for good. trySend(t, ctx, m, store, outgoing) require.Equal(t, model.OutgoingEmailStatusFailed, outgoing.Status) - require.Equal(t, attemptsNumber, outgoing.AttemptsNumber) + require.Equal(t, len(DefaultRetryIntervals), outgoing.AttemptsNumber) require.Empty(t, real.Sent(), "the real sender never succeeded, so nothing should be recorded as sent") }) @@ -189,7 +189,7 @@ func TestSendEmails(t *testing.T) { // Exhausted its attempts: must not be retried either. failedEmail := newOutgoingEmail(t, ctx, store) setup.FailWith(errors.New("smtp down")) - for range attemptsNumber + 1 { + for range len(DefaultRetryIntervals) + 1 { trySend(t, ctx, setupSender, store, failedEmail) } require.Equal(t, model.OutgoingEmailStatusFailed, failedEmail.Status) @@ -371,3 +371,73 @@ func TestRunPoller_LongIntervalSendsNothingEarly(t *testing.T) { require.Never(t, func() bool { return len(real.Sent()) > 0 }, 500*time.Millisecond, 50*time.Millisecond) } + +func TestSendEmails_OneFailingUpdateLeavesOthersSent(t *testing.T) { + t.Parallel() + + db := testdb.New(t).DB + store := repo.New(db) + ctx := context.Background() + + real := fakesender.New() + m := NewSender(store, real.Delivery()) + + first := newOutgoingEmail(t, ctx, store) + broken := newOutgoingEmail(t, ctx, store) + last := newOutgoingEmail(t, ctx, store) + + // The update that records the outcome of one mail fails; the others are + // processed in their own transactions and stay sent. + _, err := db.ExecContext(ctx, ` + CREATE FUNCTION fail_outgoing_update() RETURNS trigger AS $$ + BEGIN + IF NEW.id = '`+broken.ID+`' THEN + RAISE EXCEPTION 'update refused'; + END IF; + RETURN NEW; + END + $$ LANGUAGE plpgsql; + CREATE TRIGGER fail_outgoing_update BEFORE UPDATE ON outgoing_emails + FOR EACH ROW EXECUTE FUNCTION fail_outgoing_update();`) + require.NoError(t, err) + + require.NoError(t, m.sendEmails(ctx)) + + status := map[string]model.OutgoingEmailStatus{} + for _, r := range listOutgoing(t, ctx, store) { + status[r.ID] = r.Status + } + + require.Equal(t, model.OutgoingEmailStatusSent, status[first.ID]) + require.Equal(t, model.OutgoingEmailStatusSent, status[last.ID]) + require.Equal(t, model.OutgoingEmailStatusNew, status[broken.ID], "the mail whose update failed stays queued") +} + +func TestWithRetryIntervals(t *testing.T) { + t.Parallel() + + db := testdb.New(t).DB + store := repo.New(db) + ctx := context.Background() + + intervals := []time.Duration{time.Hour, 2 * time.Hour} + real := fakesender.New() + real.FailWith(errors.New("smtp down")) + m := NewSender(store, real.Delivery(), WithRetryIntervals(intervals)) + + outgoing := newOutgoingEmail(t, ctx, store) + + for i, interval := range intervals { + trySend(t, ctx, m, store, outgoing) + + require.Equal(t, i+1, outgoing.AttemptsNumber) + require.Equal(t, model.OutgoingEmailStatusNew, outgoing.Status) + require.WithinDuration(t, time.Now().Add(interval), outgoing.TryAt, 5*time.Second) + } + + // The failure after the last interval gives up. + trySend(t, ctx, m, store, outgoing) + + require.Equal(t, model.OutgoingEmailStatusFailed, outgoing.Status) + require.Equal(t, len(intervals), outgoing.AttemptsNumber) +} diff --git a/pkg/repo/mail_queue.go b/pkg/repo/mail_queue.go index e024d834..7bf6f129 100644 --- a/pkg/repo/mail_queue.go +++ b/pkg/repo/mail_queue.go @@ -2,6 +2,8 @@ package repo import ( "context" + "database/sql" + "errors" "time" "github.com/can3p/gogo/sender" @@ -21,20 +23,44 @@ func (s *Store) SendMail(ctx context.Context, snd MailQueue, uniqueID, emailType return snd.Send(ctx, s.exec, uniqueID, emailType, mail) } -// GetPendingEmails returns emails ready to be sent, locked for update. -func (s *Store) GetPendingEmails(ctx context.Context) ([]*model.OutgoingEmail, error) { - var emails []*model.OutgoingEmail +// ListDueEmailIDs returns the ids of the emails ready to be sent, oldest first. +// It takes no lock: LockDueEmail claims each one. +func (s *Store) ListDueEmailIDs(ctx context.Context) ([]string, error) { + var ids []string - err := s.query().NewSelect().Model(&emails). + err := s.query().NewSelect().Model((*model.OutgoingEmail)(nil)). + Column("id"). + Where("status = ?", model.OutgoingEmailStatusNew). + Where("try_at < ?", time.Now()). + Order("id"). + Scan(ctx, &ids) + if err != nil { + return nil, err + } + + return ids, nil +} + +// LockDueEmail locks one email for update inside the store's transaction. It +// returns nil when another worker holds the row or it is no longer new and due. +func (s *Store) LockDueEmail(ctx context.Context, id string) (*model.OutgoingEmail, error) { + email := new(model.OutgoingEmail) + + err := s.query().NewSelect().Model(email). + Where("id = ?", id). Where("status = ?", model.OutgoingEmailStatusNew). Where("try_at < ?", time.Now()). For("UPDATE SKIP LOCKED"). Scan(ctx) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { return nil, err } - return emails, nil + return email, nil } // UpdateOutgoingEmail updates an outgoing email record. From 6529d2aa02773f32ec8f595a551ff1d9d5f052b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 22:57:18 +0000 Subject: [PATCH 09/16] refactor(r4): M3 comment mails from templates Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UmD6gdC2ENrRcg4XZVZuxd --- pkg/mail/comments_test.go | 269 +++++------------- pkg/mail/post_comment_author.go | 182 ++++++++---- pkg/mail/post_comment_participants.go | 84 ++---- pkg/mail/templates/post_comment_author.html | 8 + .../templates/post_comment_author.subject.txt | 1 + pkg/mail/templates/post_comment_author.txt | 8 + .../templates/post_comment_participants.html | 8 + .../post_comment_participants.subject.txt | 1 + .../templates/post_comment_participants.txt | 8 + .../post_comment_author_markup.golden | 39 +++ ..._comment_author_markup_long_subject.golden | 25 ++ .../post_comment_participants_markup.golden | 39 +++ ...nt_participants_markup_long_subject.golden | 25 ++ 13 files changed, 388 insertions(+), 309 deletions(-) create mode 100644 pkg/mail/templates/post_comment_author.html create mode 100644 pkg/mail/templates/post_comment_author.subject.txt create mode 100644 pkg/mail/templates/post_comment_author.txt create mode 100644 pkg/mail/templates/post_comment_participants.html create mode 100644 pkg/mail/templates/post_comment_participants.subject.txt create mode 100644 pkg/mail/templates/post_comment_participants.txt create mode 100644 pkg/mail/testdata/post_comment_author_markup.golden create mode 100644 pkg/mail/testdata/post_comment_author_markup_long_subject.golden create mode 100644 pkg/mail/testdata/post_comment_participants_markup.golden create mode 100644 pkg/mail/testdata/post_comment_participants_markup_long_subject.golden diff --git a/pkg/mail/comments_test.go b/pkg/mail/comments_test.go index fc09a617..d437d72c 100644 --- a/pkg/mail/comments_test.go +++ b/pkg/mail/comments_test.go @@ -9,47 +9,89 @@ import ( "github.com/can3p/pcom/pkg/mail" "github.com/can3p/pcom/pkg/model" "github.com/can3p/pcom/pkg/testutil/fakesender" - "github.com/can3p/pcom/pkg/testutil/golden" "github.com/stretchr/testify/require" ) -func TestPostCommentAuthor(t *testing.T) { - t.Parallel() +// sampleEnvelope is the rendering of the sample name. +func sampleEnvelope(t *testing.T, name string) *mail.Envelope { + t.Helper() - commenter := &model.User{ - ID: "user-1", - Email: "commenter@example.test", - Username: "alice", - } - author := &model.User{ - ID: "user-2", - Email: "author@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new("Original Post"), - Body: "Post body", - UserID: author.ID, - } - comment := &model.PostComment{ - ID: "comment-1", - PostID: post.ID, - UserID: commenter.ID, - Body: "Nice post!", + for _, m := range mail.All() { + for _, s := range m.Samples { + if s.Name == name { + require.NoError(t, s.Err) + + return s.Envelope + } + } } - sender := fakesender.New() - ctx := context.Background() + require.Failf(t, "no such sample", "sample %s", name) + + return nil +} + +type formatComment func(commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (*mail.Envelope, error) + +func formatAuthor(commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (*mail.Envelope, error) { mediaReplacer := func(in string) (bool, string) { return false, in } - err := deliverE(ctx, sender)(mail.PostCommentAuthor(links.Site{}, testFrom, mediaReplacer, commenter, author, post, comment, false)) - require.NoError(t, err) + return mail.PostCommentAuthor(links.Site{}, testFrom, mediaReplacer, commenter, recipient, post, comment, edited) +} - sent := sender.Sent() - require.Len(t, sent, 1) +func formatParticipants(commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (*mail.Envelope, error) { + mediaReplacer := func(in string) (bool, string) { return false, in } + + return mail.PostCommentParticipants(links.Site{}, testFrom, mediaReplacer, commenter, recipient, post, comment, edited) +} + +// The comment mails are built from models; each case builds the models of +// the sample it names and expects the sample's rendering, so a wrong mapping +// from models to the input fails. +func TestPostCommentMails_MatchSamples(t *testing.T) { + t.Parallel() + + commenter := &model.User{ID: "user-1", Email: "commenter@example.test", Username: "alice"} + newPost := func() *model.Post { + return &model.Post{ID: "post-1", Subject: new("Original Post"), Body: "Post body", UserID: "user-3"} + } + withURL := newPost() + withURL.URLID = new("url-1") + setPostURL(withURL, &model.NormalizedURL{ID: "url-1", URL: "https://example.com/article"}) + + for _, tc := range []struct { + name string + recipient string + post *model.Post + body string + edited bool + format formatComment + }{ + {"post_comment_author", "author@example.test", newPost(), "Nice post!", false, formatAuthor}, + {"post_comment_author_edited", "recipient@example.test", newPost(), "Nice post, edited!", true, formatAuthor}, + {"post_comment_author_with_url", "author@example.test", withURL, "Nice post!", false, formatAuthor}, + {"post_comment_participants", "participant@example.test", newPost(), "Great comment!", false, formatParticipants}, + {"post_comment_participants_edited", "recipient@example.test", newPost(), "Nice post, edited!", true, formatParticipants}, + {"post_comment_participants_with_url", "participant@example.test", withURL, "Great comment!", false, formatParticipants}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + comment := &model.PostComment{ID: "comment-1", PostID: tc.post.ID, UserID: commenter.ID, Body: tc.body} + if tc.edited { + comment.EditedAt = new(time.Date(2026, 9, 30, 12, 0, 0, 0, time.UTC)) + } - golden.Assert(t, "post_comment_author", mailsToGolden(sent)) + recipient := &model.User{ID: "user-2", Email: tc.recipient, Username: "bob"} + got, err := tc.format(commenter, recipient, tc.post, comment, tc.edited) + require.NoError(t, err) + require.Equal(t, sampleEnvelope(t, tc.name), got) + + if tc.edited { + require.Contains(t, got.UniqueID, "2026-09-30T12:00:00Z") + } + }) + } } func TestPostCommentAuthor_NotToMyself(t *testing.T) { @@ -85,45 +127,6 @@ func TestPostCommentAuthor_NotToMyself(t *testing.T) { require.Empty(t, sent, "should not send email to self") } -func TestPostCommentParticipants(t *testing.T) { - t.Parallel() - - commenter := &model.User{ - ID: "user-1", - Email: "commenter@example.test", - Username: "alice", - } - participant := &model.User{ - ID: "user-2", - Email: "participant@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new("Original Post"), - Body: "Post body", - UserID: "user-3", - } - comment := &model.PostComment{ - ID: "comment-1", - PostID: post.ID, - UserID: commenter.ID, - Body: "Great comment!", - } - - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } - - err := deliverE(ctx, sender)(mail.PostCommentParticipants(links.Site{}, testFrom, mediaReplacer, commenter, participant, post, comment, false)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "post_comment_participants", mailsToGolden(sent)) -} - func TestPostCommentParticipants_NotToMyself(t *testing.T) { t.Parallel() @@ -156,129 +159,3 @@ func TestPostCommentParticipants_NotToMyself(t *testing.T) { sent := sender.Sent() require.Empty(t, sent, "should not send email to self") } - -func TestPostCommentAuthor_WithURL(t *testing.T) { - t.Parallel() - - commenter := &model.User{ - ID: "user-1", - Email: "commenter@example.test", - Username: "alice", - } - author := &model.User{ - ID: "user-2", - Email: "author@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new("Original Post"), - Body: "Post body", - UserID: author.ID, - URLID: new("url-1"), - } - setPostURL(post, &model.NormalizedURL{ - ID: "url-1", - URL: "https://example.com/article", - }) - comment := &model.PostComment{ - ID: "comment-1", - PostID: post.ID, - UserID: commenter.ID, - Body: "Nice post!", - } - - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } - - err := deliverE(ctx, sender)(mail.PostCommentAuthor(links.Site{}, testFrom, mediaReplacer, commenter, author, post, comment, false)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "post_comment_author_with_url", mailsToGolden(sent)) -} - -func TestPostCommentParticipants_WithURL(t *testing.T) { - t.Parallel() - - commenter := &model.User{ - ID: "user-1", - Email: "commenter@example.test", - Username: "alice", - } - participant := &model.User{ - ID: "user-2", - Email: "participant@example.test", - Username: "bob", - } - post := &model.Post{ - ID: "post-1", - Subject: new("Original Post"), - Body: "Post body", - UserID: "user-3", - URLID: new("url-1"), - } - setPostURL(post, &model.NormalizedURL{ - ID: "url-1", - URL: "https://example.com/article", - }) - comment := &model.PostComment{ - ID: "comment-1", - PostID: post.ID, - UserID: commenter.ID, - Body: "Great comment!", - } - - sender := fakesender.New() - ctx := context.Background() - mediaReplacer := func(in string) (bool, string) { return false, in } - - err := deliverE(ctx, sender)(mail.PostCommentParticipants(links.Site{}, testFrom, mediaReplacer, commenter, participant, post, comment, false)) - require.NoError(t, err) - - sent := sender.Sent() - require.Len(t, sent, 1) - - golden.Assert(t, "post_comment_participants_with_url", mailsToGolden(sent)) -} - -func TestPostCommentEdited(t *testing.T) { - t.Parallel() - - commenter := &model.User{ID: "user-1", Email: "commenter@example.test", Username: "alice"} - recipient := &model.User{ID: "user-2", Email: "recipient@example.test", Username: "bob"} - post := &model.Post{ID: "post-1", Subject: new("Original Post"), Body: "Post body", UserID: recipient.ID} - comment := &model.PostComment{ - ID: "comment-1", - PostID: post.ID, - UserID: commenter.ID, - Body: "Nice post, edited!", - EditedAt: new(time.Date(2026, 9, 30, 12, 0, 0, 0, time.UTC)), - } - mediaReplacer := func(in string) (bool, string) { return false, in } - - for name, format := range map[string]func() (*mail.Envelope, error){ - "post_comment_author_edited": func() (*mail.Envelope, error) { - return mail.PostCommentAuthor(links.Site{}, testFrom, mediaReplacer, commenter, recipient, post, comment, true) - }, - "post_comment_participants_edited": func() (*mail.Envelope, error) { - return mail.PostCommentParticipants(links.Site{}, testFrom, mediaReplacer, commenter, recipient, post, comment, true) - }, - } { - t.Run(name, func(t *testing.T) { - t.Parallel() - - sender := fakesender.New() - require.NoError(t, deliverE(context.Background(), sender)(format())) - - sent := sender.Sent() - require.Len(t, sent, 1) - require.Contains(t, sent[0].UniqueID, "2026-09-30T12:00:00Z") - - golden.Assert(t, name, mailsToGolden(sent)) - }) - } -} diff --git a/pkg/mail/post_comment_author.go b/pkg/mail/post_comment_author.go index 20e61398..92ab0376 100644 --- a/pkg/mail/post_comment_author.go +++ b/pkg/mail/post_comment_author.go @@ -1,84 +1,168 @@ package mail import ( - "fmt" - "github.com/samber/lo" - "html" - "net/mail" + "html/template" "strings" "time" - "github.com/can3p/gogo/sender" "github.com/can3p/pcom/pkg/links" "github.com/can3p/pcom/pkg/markdown" "github.com/can3p/pcom/pkg/model" "github.com/can3p/pcom/pkg/postops" "github.com/can3p/pcom/pkg/types" "github.com/pkg/errors" + "github.com/samber/lo" ) -// PostCommentAuthor formats the notification for the author of a post about -// a new comment, or an edited one when edited is set. It returns nil when -// there is nobody to notify. -func PostCommentAuthor(site links.Site, from string, mediaReplacer types.Replacer[string], user *model.User, author *model.User, post *model.Post, comment *model.PostComment, edited bool) (*Outgoing, error) { - // we're not sending email notifications to ourselves - if user.ID == author.ID { - return nil, nil +// postComment is what the two comment mails show: one comment, the post it +// belongs to and who it is for. The mails differ in their wording only. +type postComment struct { + From string + To string + UniqueUserID string + CommentID string + Commenter string + Subject string + // URL is the url the post links to, empty when it links to none. + URL string + // Link is the absolute link to the comment. + Link string + // Body is the comment as markdown with its image urls replaced. + Body string + // BodyHTML is the comment rendered for mail. + BodyHTML template.HTML + Edited bool + EditedAt time.Time +} + +// uniqueID is the comment and the recipient; every edit is a new mail, the +// queue drops a repeated unique id. +func (in postComment) uniqueID() string { + id := in.CommentID + in.UniqueUserID + if in.Edited { + id += in.EditedAt.UTC().Format(time.RFC3339Nano) } - link := site.Abs("comment", post.ID, comment.ID) + return id +} + +func (in postComment) header() Header { + return Header{UniqueID: in.uniqueID(), From: FromPcom(in.From), To: To(in.To)} +} + +// Quote is the comment as a text quote. +func (in postComment) Quote() string { + return "> " + strings.Join(strings.Split(in.Body, "\n"), "\n> ") +} + +// newPostComment maps the models of a comment mail to its input. +func newPostComment(site links.Site, from string, mediaReplacer types.Replacer[string], commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (postComment, error) { body, err := markdown.ReplaceImageUrls(comment.Body, mediaReplacer) if err != nil { // ReplaceImageUrls only fails if goldmark cannot render, which no input triggers, so no test covers this. - return nil, errors.Wrap(err, "failed to render the comment") + return postComment{}, errors.Wrap(err, "failed to render the comment") } - htmlBody := markdown.ToEnrichedTemplate(comment.Body, types.ViewEmail, mediaReplacer, site.Abs) - - subject := postops.PostSubject(post.Subject) - // Get linked URL if available - var urlText string - var htmlUrlSection string + in := postComment{ + From: from, + To: recipient.Email, + UniqueUserID: recipient.ID, + CommentID: comment.ID, + Commenter: commenter.Username, + Subject: postops.PostSubject(post.Subject), + Link: site.Abs("comment", post.ID, comment.ID), + Body: body, + BodyHTML: markdown.ToEnrichedTemplate(comment.Body, types.ViewEmail, mediaReplacer, site.Abs), + Edited: edited, + EditedAt: lo.FromPtr(comment.EditedAt), + } if post.URL != nil { - urlText = fmt.Sprintf("\nLinked URL: %s", post.URL.URL) - htmlUrlSection = fmt.Sprintf(`

Linked URL: %s

`, html.EscapeString(post.URL.URL), html.EscapeString(post.URL.URL)) + in.URL = post.URL.URL } - verb, subjectLine := "has left a comment in", "New comment in your post" - uniqueID := comment.ID + user.ID - if edited { - verb, subjectLine = "has edited a comment in", "Edited comment in your post" - // every edit is a new mail, the queue drops a repeated unique id - uniqueID += lo.FromPtr(comment.EditedAt).UTC().Format(time.RFC3339Nano) + return in, nil +} + +type postCommentSample struct { + name string + in postComment +} + +// postCommentSamples are the samples both comment mails share, built from +// in-memory models. name is the sample name of the plain one, the others add +// a suffix; to is the recipient's address and body the comment of the plain and +// the linked-url ones. +func postCommentSamples(name, to, body string) []postCommentSample { + identity := func(in string) (bool, string) { return false, in } + commenter := &model.User{ID: "user-1", Email: "commenter@example.test", Username: "alice"} + recipient := &model.User{ID: "user-2", Email: to, Username: "bob"} + post := func(subject string) *model.Post { + return &model.Post{ID: "post-1", Subject: &subject, Body: "Post body", UserID: "user-3"} + } + comment := func(body string) *model.PostComment { + return &model.PostComment{ID: "comment-1", PostID: "post-1", UserID: commenter.ID, Body: body} + } + editedRecipient := &model.User{ID: "user-2", Email: "recipient@example.test", Username: "bob"} + build := func(p *model.Post, c *model.PostComment, edited bool) postComment { + to := recipient + if edited { + to = editedRecipient + } + + in, err := newPostComment(SampleSite, SampleFrom, identity, commenter, to, p, c, edited) + if err != nil { + panic(err) + } + + return in } - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: author.Email, - }, - }, - Subject: fmt.Sprintf("%s \"%s\"", subjectLine, subject), - Text: fmt.Sprintf(`Hi! + withURL := post("Original Post") + withURL.URL = &model.NormalizedURL{ID: "url-1", URL: "https://example.com/article"} + edited := comment("Nice post, edited!") + edited.EditedAt = new(time.Date(2026, 9, 30, 12, 0, 0, 0, time.UTC)) -@%s %s your post "%s".%s + return []postCommentSample{ + {name, build(post("Original Post"), comment(body), false)}, + {name + "_edited", build(post("Original Post"), edited, true)}, + {name + "_with_url", build(withURL, comment(body), false)}, + {name + "_markup", build(post("Original Post"), comment("Nice **post**, see [the docs](https://example.com/a?b=1&c=2).\n\n- one\n- \n\n> quoted & \"done\""), false)}, + {name + "_markup_long_subject", build(post("A very long & \"quoted\" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit"), comment("Nice **post**!"), false)}, + } +} -%s +// PostCommentAuthorInput is what the mail to the author of a post about a +// comment shows. +type PostCommentAuthorInput struct{ postComment } -Checkout the comment in the post: %s`, user.Username, verb, subject, urlText, "> "+strings.Join(strings.Split(body, "\n"), "\n> "), link), - Html: fmt.Sprintf(` -

Hi!

+// Header addresses the mail to the author of the post. +func (in PostCommentAuthorInput) Header() Header { return in.header() } -

@%s %s your post "%s".

%s +var postCommentAuthorMail = declare("comment_notification", "post_comment_author", postCommentAuthorSamples) -
%s
+func postCommentAuthorSamples() []Sample[PostCommentAuthorInput] { + return lo.Map(postCommentSamples("post_comment_author", "author@example.test", "Nice post!"), func(s postCommentSample, _ int) Sample[PostCommentAuthorInput] { + s.in.UniqueUserID = "user-1" // the commenter, as in PostCommentAuthor + return Sample[PostCommentAuthorInput]{Name: s.name, Input: PostCommentAuthorInput{s.in}} + }) +} -

Checkout the comment in the post.

`, html.EscapeString(user.Username), verb, html.EscapeString(subject), htmlUrlSection, htmlBody, html.EscapeString(link)), +// PostCommentAuthor formats the notification for the author of a post about +// a new comment, or an edited one when edited is set. It returns nil when +// there is nobody to notify. +func PostCommentAuthor(site links.Site, from string, mediaReplacer types.Replacer[string], user *model.User, author *model.User, post *model.Post, comment *model.PostComment, edited bool) (*Envelope, error) { + // we're not sending email notifications to ourselves + if user.ID == author.ID { + return nil, nil } - return &Outgoing{UniqueID: uniqueID, Type: "comment_notification", Mail: mail}, nil + in, err := newPostComment(site, from, mediaReplacer, user, author, post, comment, edited) + if err != nil { + return nil, err + } + + // the unique id of this mail has always been made of the commenter, not of the recipient + in.UniqueUserID = user.ID + + return postCommentAuthorMail.Render(PostCommentAuthorInput{in}) } diff --git a/pkg/mail/post_comment_participants.go b/pkg/mail/post_comment_participants.go index 4b8f4a76..13a48490 100644 --- a/pkg/mail/post_comment_participants.go +++ b/pkg/mail/post_comment_participants.go @@ -1,84 +1,40 @@ package mail import ( - "fmt" - "github.com/samber/lo" - "html" - "net/mail" - "strings" - "time" - - "github.com/can3p/gogo/sender" "github.com/can3p/pcom/pkg/links" - "github.com/can3p/pcom/pkg/markdown" "github.com/can3p/pcom/pkg/model" - "github.com/can3p/pcom/pkg/postops" "github.com/can3p/pcom/pkg/types" - "github.com/pkg/errors" + "github.com/samber/lo" ) +// PostCommentParticipantsInput is what the mail to a user who commented on a +// post earlier shows about a new comment. +type PostCommentParticipantsInput struct{ postComment } + +// Header addresses the mail to the participant. +func (in PostCommentParticipantsInput) Header() Header { return in.header() } + +var postCommentParticipantsMail = declare("comment_notification", "post_comment_participants", postCommentParticipantsSamples) + +func postCommentParticipantsSamples() []Sample[PostCommentParticipantsInput] { + return lo.Map(postCommentSamples("post_comment_participants", "participant@example.test", "Great comment!"), func(s postCommentSample, _ int) Sample[PostCommentParticipantsInput] { + return Sample[PostCommentParticipantsInput]{Name: s.name, Input: PostCommentParticipantsInput{s.in}} + }) +} + // PostCommentParticipants formats the notification about a new comment for a // user who commented on the post earlier. It returns nil when there is nobody // to notify. With edited set it is about an edited comment instead. -func PostCommentParticipants(site links.Site, from string, mediaReplacer types.Replacer[string], commentAuthor *model.User, participant *model.User, post *model.Post, comment *model.PostComment, edited bool) (*Outgoing, error) { +func PostCommentParticipants(site links.Site, from string, mediaReplacer types.Replacer[string], commentAuthor *model.User, participant *model.User, post *model.Post, comment *model.PostComment, edited bool) (*Envelope, error) { // we're not sending email notifications to ourselves if commentAuthor.ID == participant.ID { return nil, nil } - link := site.Abs("comment", post.ID, comment.ID) - body, err := markdown.ReplaceImageUrls(comment.Body, mediaReplacer) + in, err := newPostComment(site, from, mediaReplacer, commentAuthor, participant, post, comment, edited) if err != nil { - // ReplaceImageUrls only fails if goldmark cannot render, which no input triggers, so no test covers this. - return nil, errors.Wrap(err, "failed to render the comment") - } - htmlBody := markdown.ToEnrichedTemplate(comment.Body, types.ViewEmail, mediaReplacer, site.Abs) - - subject := postops.PostSubject(post.Subject) - - // Get linked URL if available - var urlText string - var htmlUrlSection string - if post.URL != nil { - urlText = fmt.Sprintf("\nLinked URL: %s", post.URL.URL) - htmlUrlSection = fmt.Sprintf(`

Linked URL: %s

`, html.EscapeString(post.URL.URL), html.EscapeString(post.URL.URL)) - } - - verb, subjectLine := "has left a comment in", "New comment in the post" - uniqueID := comment.ID + participant.ID - if edited { - verb, subjectLine = "has edited a comment in", "Edited comment in the post" - // every edit is a new mail, the queue drops a repeated unique id - uniqueID += lo.FromPtr(comment.EditedAt).UTC().Format(time.RFC3339Nano) - } - - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: participant.Email, - }, - }, - Subject: fmt.Sprintf("%s \"%s\"", subjectLine, subject), - Text: fmt.Sprintf(`Hi! - -@%s %s the post "%s" where you've also left a comment.%s - -%s - -Checkout the comment in the post: %s`, commentAuthor.Username, verb, subject, urlText, "> "+strings.Join(strings.Split(body, "\n"), "\n> "), link), - Html: fmt.Sprintf(` -

Hi!

- -

@%s %s the post "%s" where you've also left a comment.

%s - -
%s
- -

Checkout the comment in the post.

`, html.EscapeString(commentAuthor.Username), verb, html.EscapeString(subject), htmlUrlSection, htmlBody, html.EscapeString(link)), + return nil, err } - return &Outgoing{UniqueID: uniqueID, Type: "comment_notification", Mail: mail}, nil + return postCommentParticipantsMail.Render(PostCommentParticipantsInput{in}) } diff --git a/pkg/mail/templates/post_comment_author.html b/pkg/mail/templates/post_comment_author.html new file mode 100644 index 00000000..eedbaca6 --- /dev/null +++ b/pkg/mail/templates/post_comment_author.html @@ -0,0 +1,8 @@ + +

Hi!

+ +

@{{.Commenter}} {{if .Edited}}has edited a comment in{{else}}has left a comment in{{end}} your post "{{.Subject}}".

{{with .URL}}

Linked URL: {{.}}

{{end}} + +
{{.BodyHTML}}
+ +

Checkout the comment in the post.

diff --git a/pkg/mail/templates/post_comment_author.subject.txt b/pkg/mail/templates/post_comment_author.subject.txt new file mode 100644 index 00000000..0e0f1417 --- /dev/null +++ b/pkg/mail/templates/post_comment_author.subject.txt @@ -0,0 +1 @@ +{{if .Edited}}Edited comment in your post{{else}}New comment in your post{{end}} "{{.Subject}}" diff --git a/pkg/mail/templates/post_comment_author.txt b/pkg/mail/templates/post_comment_author.txt new file mode 100644 index 00000000..6785ee6f --- /dev/null +++ b/pkg/mail/templates/post_comment_author.txt @@ -0,0 +1,8 @@ +Hi! + +@{{.Commenter}} {{if .Edited}}has edited a comment in{{else}}has left a comment in{{end}} your post "{{.Subject}}".{{with .URL}} +Linked URL: {{.}}{{end}} + +{{.Quote}} + +Checkout the comment in the post: {{.Link}} diff --git a/pkg/mail/templates/post_comment_participants.html b/pkg/mail/templates/post_comment_participants.html new file mode 100644 index 00000000..f6e3e1f3 --- /dev/null +++ b/pkg/mail/templates/post_comment_participants.html @@ -0,0 +1,8 @@ + +

Hi!

+ +

@{{.Commenter}} {{if .Edited}}has edited a comment in{{else}}has left a comment in{{end}} the post "{{.Subject}}" where you've also left a comment.

{{with .URL}}

Linked URL: {{.}}

{{end}} + +
{{.BodyHTML}}
+ +

Checkout the comment in the post.

diff --git a/pkg/mail/templates/post_comment_participants.subject.txt b/pkg/mail/templates/post_comment_participants.subject.txt new file mode 100644 index 00000000..f4f59723 --- /dev/null +++ b/pkg/mail/templates/post_comment_participants.subject.txt @@ -0,0 +1 @@ +{{if .Edited}}Edited comment in the post{{else}}New comment in the post{{end}} "{{.Subject}}" diff --git a/pkg/mail/templates/post_comment_participants.txt b/pkg/mail/templates/post_comment_participants.txt new file mode 100644 index 00000000..289c3bf2 --- /dev/null +++ b/pkg/mail/templates/post_comment_participants.txt @@ -0,0 +1,8 @@ +Hi! + +@{{.Commenter}} {{if .Edited}}has edited a comment in{{else}}has left a comment in{{end}} the post "{{.Subject}}" where you've also left a comment.{{with .URL}} +Linked URL: {{.}}{{end}} + +{{.Quote}} + +Checkout the comment in the post: {{.Link}} diff --git a/pkg/mail/testdata/post_comment_author_markup.golden b/pkg/mail/testdata/post_comment_author_markup.golden new file mode 100644 index 00000000..b77413a7 --- /dev/null +++ b/pkg/mail/testdata/post_comment_author_markup.golden @@ -0,0 +1,39 @@ +EmailType: comment_notification +UniqueID: comment-1user-1 +From: Your pcom +To: author@example.test +Subject: New comment in your post "Original Post" + +--- TEXT --- +Hi! + +@alice has left a comment in your post "Original Post". + +> Nice **post**, see [the docs](https://example.com/a?b=1&c=2). +> +> - one +> - +> +> > quoted & "done" + +Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 + +--- HTML --- + +

Hi!

+ +

@alice has left a comment in your post "Original Post".

+ +

Nice post, see the docs.

+
    +
  • one
    • +
  • + +
    • +
+
+

quoted & "done"

+
+
+ +

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_author_markup_long_subject.golden b/pkg/mail/testdata/post_comment_author_markup_long_subject.golden new file mode 100644 index 00000000..5442db63 --- /dev/null +++ b/pkg/mail/testdata/post_comment_author_markup_long_subject.golden @@ -0,0 +1,25 @@ +EmailType: comment_notification +UniqueID: comment-1user-1 +From: Your pcom +To: author@example.test +Subject: New comment in your post "A very long & "quoted" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit" + +--- TEXT --- +Hi! + +@alice has left a comment in your post "A very long & "quoted" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit". + +> Nice **post**! + +Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 + +--- HTML --- + +

Hi!

+ +

@alice has left a comment in your post "A <b>very</b> long & "quoted" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit".

+ +

Nice post!

+
+ +

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_participants_markup.golden b/pkg/mail/testdata/post_comment_participants_markup.golden new file mode 100644 index 00000000..7b53852e --- /dev/null +++ b/pkg/mail/testdata/post_comment_participants_markup.golden @@ -0,0 +1,39 @@ +EmailType: comment_notification +UniqueID: comment-1user-2 +From: Your pcom +To: participant@example.test +Subject: New comment in the post "Original Post" + +--- TEXT --- +Hi! + +@alice has left a comment in the post "Original Post" where you've also left a comment. + +> Nice **post**, see [the docs](https://example.com/a?b=1&c=2). +> +> - one +> - +> +> > quoted & "done" + +Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 + +--- HTML --- + +

Hi!

+ +

@alice has left a comment in the post "Original Post" where you've also left a comment.

+ +

Nice post, see the docs.

+
    +
  • one
    • +
  • + +
    • +
+
+

quoted & "done"

+
+
+ +

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_participants_markup_long_subject.golden b/pkg/mail/testdata/post_comment_participants_markup_long_subject.golden new file mode 100644 index 00000000..960dd03d --- /dev/null +++ b/pkg/mail/testdata/post_comment_participants_markup_long_subject.golden @@ -0,0 +1,25 @@ +EmailType: comment_notification +UniqueID: comment-1user-2 +From: Your pcom +To: participant@example.test +Subject: New comment in the post "A very long & "quoted" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit" + +--- TEXT --- +Hi! + +@alice has left a comment in the post "A very long & "quoted" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit" where you've also left a comment. + +> Nice **post**! + +Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 + +--- HTML --- + +

Hi!

+ +

@alice has left a comment in the post "A <b>very</b> long & "quoted" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit" where you've also left a comment.

+ +

Nice post!

+
+ +

Checkout the comment in the post.

From 258810446061b65fc32cef5ba2dba13357103544 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 22:57:18 +0000 Subject: [PATCH 10/16] refactor(r4): M4 declare the admin mails in pkg/mail Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UmD6gdC2ENrRcg4XZVZuxd --- pkg/admin/notifications.go | 155 -------------- pkg/admin/notifications_test.go | 192 ------------------ pkg/admin/panics.go | 53 +---- pkg/mail/admin_new_user.go | 42 ++++ pkg/mail/admin_new_waiting_list_member.go | 51 +++++ pkg/mail/admin_page_failure.go | 60 ++++++ pkg/mail/admin_signup_confirmed.go | 35 ++++ pkg/mail/admin_test.go | 172 ++++++++++++++++ pkg/mail/admin_throwaway_email.go | 36 ++++ pkg/mail/templates/admin_new_user.html | 9 + pkg/mail/templates/admin_new_user.subject.txt | 1 + pkg/mail/templates/admin_new_user.txt | 7 + .../admin_new_waiting_list_member.html | 8 + .../admin_new_waiting_list_member.subject.txt | 1 + .../admin_new_waiting_list_member.txt | 6 + pkg/mail/templates/admin_page_failure.html | 8 + .../templates/admin_page_failure.subject.txt | 1 + pkg/mail/templates/admin_page_failure.txt | 8 + .../templates/admin_signup_confirmed.html | 8 + .../admin_signup_confirmed.subject.txt | 1 + pkg/mail/templates/admin_signup_confirmed.txt | 6 + pkg/mail/templates/admin_throwaway_email.html | 3 + .../admin_throwaway_email.subject.txt | 1 + pkg/mail/templates/admin_throwaway_email.txt | 3 + pkg/mail/testdata/admin_new_user.golden | 25 +++ .../admin_new_waiting_list_member.golden | 23 +++ ...dmin_new_waiting_list_member_reason.golden | 23 +++ pkg/mail/testdata/admin_page_failure.golden | 40 ++++ .../admin_page_failure_anonymous.golden | 40 ++++ .../testdata/admin_signup_confirmed.golden | 23 +++ .../testdata/admin_throwaway_email.golden | 15 ++ pkg/service/accounts/codes.go | 3 +- pkg/service/accounts/invites.go | 3 +- pkg/service/accounts/signup.go | 7 +- pkg/web/app/router.go | 3 +- 35 files changed, 665 insertions(+), 407 deletions(-) create mode 100644 pkg/mail/admin_new_user.go create mode 100644 pkg/mail/admin_new_waiting_list_member.go create mode 100644 pkg/mail/admin_page_failure.go create mode 100644 pkg/mail/admin_signup_confirmed.go create mode 100644 pkg/mail/admin_test.go create mode 100644 pkg/mail/admin_throwaway_email.go create mode 100644 pkg/mail/templates/admin_new_user.html create mode 100644 pkg/mail/templates/admin_new_user.subject.txt create mode 100644 pkg/mail/templates/admin_new_user.txt create mode 100644 pkg/mail/templates/admin_new_waiting_list_member.html create mode 100644 pkg/mail/templates/admin_new_waiting_list_member.subject.txt create mode 100644 pkg/mail/templates/admin_new_waiting_list_member.txt create mode 100644 pkg/mail/templates/admin_page_failure.html create mode 100644 pkg/mail/templates/admin_page_failure.subject.txt create mode 100644 pkg/mail/templates/admin_page_failure.txt create mode 100644 pkg/mail/templates/admin_signup_confirmed.html create mode 100644 pkg/mail/templates/admin_signup_confirmed.subject.txt create mode 100644 pkg/mail/templates/admin_signup_confirmed.txt create mode 100644 pkg/mail/templates/admin_throwaway_email.html create mode 100644 pkg/mail/templates/admin_throwaway_email.subject.txt create mode 100644 pkg/mail/templates/admin_throwaway_email.txt create mode 100644 pkg/mail/testdata/admin_new_user.golden create mode 100644 pkg/mail/testdata/admin_new_waiting_list_member.golden create mode 100644 pkg/mail/testdata/admin_new_waiting_list_member_reason.golden create mode 100644 pkg/mail/testdata/admin_page_failure.golden create mode 100644 pkg/mail/testdata/admin_page_failure_anonymous.golden create mode 100644 pkg/mail/testdata/admin_signup_confirmed.golden create mode 100644 pkg/mail/testdata/admin_throwaway_email.golden diff --git a/pkg/admin/notifications.go b/pkg/admin/notifications.go index f83e90ad..d78da5da 100644 --- a/pkg/admin/notifications.go +++ b/pkg/admin/notifications.go @@ -1,156 +1 @@ package admin - -import ( - "fmt" - "html" - "net/mail" - - "github.com/can3p/gogo/sender" - "github.com/can3p/pcom/pkg/links" - pcommail "github.com/can3p/pcom/pkg/mail" - "github.com/can3p/pcom/pkg/model" - "github.com/google/uuid" - "github.com/samber/lo" -) - -// NewUser tells the admin about a new account. -func NewUser(site links.Site, from, adminAddress string, user *model.User) *pcommail.Envelope { - blogURL := site.Abs("user", user.Username) - - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: adminAddress, - }, - }, - Subject: "New User on pcom", - Text: fmt.Sprintf(` - Hi! - - New user alert: - - * ID: %s - * Blog: %s - * Email: %s`, user.ID, blogURL, user.Email), - Html: fmt.Sprintf(` -

Hi!

- -

New user alert:

- -
    -
  • ID: %s
    • -
  • Blog: %s
    • -
  • Email: %s
    • -
`, html.EscapeString(user.ID), html.EscapeString(blogURL), html.EscapeString(blogURL), html.EscapeString(user.Email)), - } - - return &pcommail.Envelope{UniqueID: user.ID, Type: "admin_new_user", Mail: mail} -} - -// NewWaitingListMember tells the admin about a new waiting list entry. -func NewWaitingListMember(site links.Site, from, adminAddress string, waitingList *model.UserSignupRequest) *pcommail.Envelope { - r := lo.FromPtr(waitingList.Reason) - - if r == "" { - r = "Not specified" - } - - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: adminAddress, - }, - }, - Subject: "New waiting list member on pcom", - Text: fmt.Sprintf(` - Hi! - - New waiting list member alert: - - * Email address: %s - * Reason: %s - `, waitingList.Email, r), - Html: fmt.Sprintf(` -

Hi!

- -

New waiting list member alert:

- -
    -
  • Email address: %s
    • -
  • Reason: %s
    • -
`, - html.EscapeString(waitingList.Email), html.EscapeString(r)), - } - - return &pcommail.Envelope{UniqueID: waitingList.ID, Type: "new_waiting_list_member", Mail: mail} -} - -// SignupConfirmed tells the admin that an account confirmed its email. -func SignupConfirmed(site links.Site, from, adminAddress string, user *model.User) *pcommail.Envelope { - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: adminAddress, - }, - }, - Subject: "New User confirmed email on pcom", - Text: fmt.Sprintf(` - Hi! - - New conrirmed user alert: - - * ID: %s - * Email: %s`, user.ID, user.Email), - Html: fmt.Sprintf(` -

Hi!

- -

New conrirmed user alert:

- -
    -
  • ID: %s
    • -
  • Email: %s
    • -
`, html.EscapeString(user.ID), html.EscapeString(user.Email)), - } - - return &pcommail.Envelope{UniqueID: user.ID, Type: "signup_confirmed", Mail: mail} -} - -// ThrowAwayEmailSignupAttempt tells the admin that somebody tried to sign up -// with a throwaway email domain. -func ThrowAwayEmailSignupAttempt(from, adminAddress string, email string) *pcommail.Envelope { - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: adminAddress, - }, - }, - Subject: "An attempt to use a throwaway email domain on pcom", - Text: fmt.Sprintf(` - Hi! - - A user has just tried to use a throwaway email: %s`, email), - Html: fmt.Sprintf(` -

Hi!

- -

A user has just tried to use a throwaway email: %s

- `, html.EscapeString(email)), - } - - return &pcommail.Envelope{UniqueID: uuid.NewString(), Type: "throw_away_email_signup", Mail: mail} -} diff --git a/pkg/admin/notifications_test.go b/pkg/admin/notifications_test.go index ec3655da..79dcfc9f 100644 --- a/pkg/admin/notifications_test.go +++ b/pkg/admin/notifications_test.go @@ -1,193 +1 @@ package admin_test - -import ( - "context" - "regexp" - "testing" - - "github.com/can3p/gogo/sender" - "github.com/can3p/pcom/pkg/admin" - "github.com/can3p/pcom/pkg/links" - pcommail "github.com/can3p/pcom/pkg/mail" - "github.com/can3p/pcom/pkg/model" - "github.com/can3p/pcom/pkg/testutil/fakesender" - "github.com/can3p/pcom/pkg/testutil/golden" - "github.com/stretchr/testify/require" -) - -const ( - testFrom = "noreply@pcom.test" - testAdmin = "admin@pcom.test" -) - -// send queues the way a service would: through the sender, with no database. -func send(ctx context.Context, s *fakesender.Sender, m *pcommail.Envelope) error { - return s.Send(ctx, nil, m.UniqueID, m.Type, m.Mail) -} - -// normalizeUUID replaces UUIDs with a placeholder for golden test consistency -func normalizeUUID(content string) string { - // Match UUID pattern: 8-4-4-4-12 hex digits - uuidPattern := regexp.MustCompile(`[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`) - return uuidPattern.ReplaceAllString(content, "[UUID]") -} - -func TestNotifyNewUser(t *testing.T) { - t.Parallel() - - ctx := context.Background() - sender := fakesender.New() - - user := &model.User{ - ID: "0190a3b4-0000-7000-8000-000000000001", - Email: "alice@example.test", - Username: "alice", - } - - require.NoError(t, send(ctx, sender, admin.NewUser(links.Site{}, testFrom, testAdmin, user))) - - sent := sender.Sent() - require.Len(t, sent, 1) - - mail := sent[0].Mail - requireAddresses(t, mail) - require.Equal(t, "admin_new_user", sent[0].EmailType) - require.Equal(t, "New User on pcom", mail.Subject) - - // Golden test for text content - golden.Assert(t, "notify_new_user_text", []byte(normalizeUUID(mail.Text))) - golden.Assert(t, "notify_new_user_html", []byte(normalizeUUID(mail.Html))) -} - -func TestNotifyNewWaitingListMember(t *testing.T) { - t.Parallel() - - ctx := context.Background() - sender := fakesender.New() - - signup := &model.UserSignupRequest{ - ID: "0190a3b4-0000-7000-8000-000000000002", - Email: "signup@example.test", - } - - require.NoError(t, send(ctx, sender, admin.NewWaitingListMember(links.Site{}, testFrom, testAdmin, signup))) - - sent := sender.Sent() - require.Len(t, sent, 1) - - mail := sent[0].Mail - requireAddresses(t, mail) - require.Equal(t, "new_waiting_list_member", sent[0].EmailType) - require.Equal(t, "New waiting list member on pcom", mail.Subject) - - // Golden test for text content - golden.Assert(t, "notify_new_waiting_list_member_text", []byte(normalizeUUID(mail.Text))) - golden.Assert(t, "notify_new_waiting_list_member_html", []byte(normalizeUUID(mail.Html))) -} - -func TestNotifySignupConfirmed(t *testing.T) { - t.Parallel() - - ctx := context.Background() - sender := fakesender.New() - - user := &model.User{ - ID: "0190a3b4-0000-7000-8000-000000000001", - Email: "alice@example.test", - Username: "alice", - } - - require.NoError(t, send(ctx, sender, admin.SignupConfirmed(links.Site{}, testFrom, testAdmin, user))) - - sent := sender.Sent() - require.Len(t, sent, 1) - - mail := sent[0].Mail - requireAddresses(t, mail) - require.Equal(t, "signup_confirmed", sent[0].EmailType) - require.Equal(t, "New User confirmed email on pcom", mail.Subject) - - // Golden test for text content - golden.Assert(t, "notify_signup_confirmed_text", []byte(normalizeUUID(mail.Text))) - golden.Assert(t, "notify_signup_confirmed_html", []byte(normalizeUUID(mail.Html))) -} - -func TestNotifyThrowAwayEmailSignupAttempt(t *testing.T) { - t.Parallel() - - ctx := context.Background() - sender := fakesender.New() - - email := "test@throwaway.example.com" - require.NoError(t, send(ctx, sender, admin.ThrowAwayEmailSignupAttempt(testFrom, testAdmin, email))) - - sent := sender.Sent() - require.Len(t, sent, 1) - - mail := sent[0].Mail - requireAddresses(t, mail) - require.Equal(t, "throw_away_email_signup", sent[0].EmailType) - require.Equal(t, "An attempt to use a throwaway email domain on pcom", mail.Subject) - - // Golden test for text content - golden.Assert(t, "notify_throwaway_email_text", []byte(normalizeUUID(mail.Text))) - golden.Assert(t, "notify_throwaway_email_html", []byte(normalizeUUID(mail.Html))) -} - -// notifiers lists every admin notification with all user-controlled fields -// set to hostile markup (a tag plus quotes). -func notifiers() map[string]func(s *fakesender.Sender) error { - const hostile = `">x` - - ctx := context.Background() - user := &model.User{ID: "user-1", Email: hostile, Username: hostile} - signup := &model.UserSignupRequest{ID: "signup-1", Email: hostile} - signup.Reason = new(hostile) - - return map[string]func(s *fakesender.Sender) error{ - "NewUser": func(s *fakesender.Sender) error { - return send(ctx, s, admin.NewUser(links.Site{}, testFrom, testAdmin, user)) - }, - "NewWaitingListMember": func(s *fakesender.Sender) error { - return send(ctx, s, admin.NewWaitingListMember(links.Site{}, testFrom, testAdmin, signup)) - }, - "SignupConfirmed": func(s *fakesender.Sender) error { - return send(ctx, s, admin.SignupConfirmed(links.Site{}, testFrom, testAdmin, user)) - }, - "ThrowAwayEmail": func(s *fakesender.Sender) error { - return send(ctx, s, admin.ThrowAwayEmailSignupAttempt(testFrom, testAdmin, hostile)) - }, - } -} - -func TestNotifications_EscapeUserContentInHTML(t *testing.T) { - t.Parallel() - - for name, notify := range notifiers() { - t.Run(name, func(t *testing.T) { - t.Parallel() - - s := fakesender.New() - require.NoError(t, notify(s)) - - sent := s.Sent() - require.Len(t, sent, 1) - - requireAddresses(t, sent[0].Mail) - html := sent[0].Mail.Html - require.NotContains(t, html, "x") - require.NotContains(t, html, `"><`) - require.Contains(t, html, "<b>x</b>") - require.Contains(t, html, """) - }) - } -} - -// Every notification goes from the sender address to the admin address. -func requireAddresses(t *testing.T, m *sender.Mail) { - t.Helper() - - require.Equal(t, testFrom, m.From.Address) - require.Len(t, m.To, 1) - require.Equal(t, testAdmin, m.To[0].Address) -} diff --git a/pkg/admin/panics.go b/pkg/admin/panics.go index 84f904a0..dc9fdd9a 100644 --- a/pkg/admin/panics.go +++ b/pkg/admin/panics.go @@ -4,20 +4,14 @@ import ( "bytes" "errors" "fmt" - "html" "net" "net/http/httputil" - "net/mail" "os" "runtime" "strings" "time" - "github.com/can3p/gogo/sender" - pcommail "github.com/can3p/pcom/pkg/mail" - "github.com/can3p/pcom/pkg/model" "github.com/gin-gonic/gin" - "github.com/google/uuid" ) var ( @@ -27,51 +21,8 @@ var ( slash = []byte("/") ) -// PageFailure tells the admin that a page panicked. -func PageFailure(from, adminAddress string, c *gin.Context, err any, user *model.User) *pcommail.Envelope { - decodedStack := strings.Split(ClonedCustomRecovery(c, err), "\r\n") - - userInfo := "Anonymous" - - if user != nil { - userInfo = fmt.Sprintf("email (%s), id (%s)", user.Email, user.ID) - } - - mail := &sender.Mail{ - From: mail.Address{ - Address: from, - Name: "Your pcom", - }, - To: []mail.Address{ - { - Address: adminAddress, - }, - }, - Subject: "Panic on the page", - Text: fmt.Sprintf(` - Hi! - - Panic on the page - - * User: %s - * Request data: - - %s - `, userInfo, strings.Join(decodedStack, "\r\n")), - Html: fmt.Sprintf(` -

Hi!

- -

Panic on the page:

- -
    -
  • user: %s
    • -
  • Request data: 
    %s
    • -
`, html.EscapeString(userInfo), html.EscapeString(strings.Join(decodedStack, "\r\n"))), - } - - return &pcommail.Envelope{UniqueID: uuid.NewString(), Type: "panic_notification", Mail: mail} -} - +// ClonedCustomRecovery formats a recovered panic for the admin: the request +// without its secrets, the panic value and the stack. func ClonedCustomRecovery(c *gin.Context, err any) string { // Check for a broken connection, as it is not really a // condition that warrants a panic stack trace. diff --git a/pkg/mail/admin_new_user.go b/pkg/mail/admin_new_user.go new file mode 100644 index 00000000..6976c094 --- /dev/null +++ b/pkg/mail/admin_new_user.go @@ -0,0 +1,42 @@ +package mail + +import ( + "github.com/can3p/pcom/pkg/links" + "github.com/can3p/pcom/pkg/model" +) + +// sampleAdmin is the admin address the samples of the admin mails go to. +const sampleAdmin = "admin@pcom.test" + +// AdminNewUserInput is what the new account alert to the admin shows. +type AdminNewUserInput struct { + From string + AdminAddress string + ID string + BlogURL string + Email string +} + +// Header addresses the mail to the admin, unique per account. +func (in AdminNewUserInput) Header() Header { + return Header{UniqueID: in.ID, From: FromPcom(in.From), To: To(in.AdminAddress)} +} + +var adminNewUserMail = declare("admin_new_user", "admin_new_user", adminNewUserSamples) + +func adminNewUserSamples() []Sample[AdminNewUserInput] { + return []Sample[AdminNewUserInput]{ + {Name: "admin_new_user", Input: AdminNewUserInput{ + From: SampleFrom, AdminAddress: sampleAdmin, + ID: "0190a3b4-0000-7000-8000-000000000001", BlogURL: SampleSite.Abs("user", "alice"), Email: "alice@example.test", + }}, + } +} + +// AdminNewUser tells the admin about a new account. +func AdminNewUser(site links.Site, from, adminAddress string, user *model.User) *Envelope { + return adminNewUserMail.MustRender(AdminNewUserInput{ + From: from, AdminAddress: adminAddress, + ID: user.ID, BlogURL: site.Abs("user", user.Username), Email: user.Email, + }) +} diff --git a/pkg/mail/admin_new_waiting_list_member.go b/pkg/mail/admin_new_waiting_list_member.go new file mode 100644 index 00000000..cb829f08 --- /dev/null +++ b/pkg/mail/admin_new_waiting_list_member.go @@ -0,0 +1,51 @@ +package mail + +import ( + "github.com/can3p/pcom/pkg/model" + "github.com/samber/lo" +) + +const noReasonGiven = "Not specified" + +// AdminNewWaitingListMemberInput is what the waiting list alert to the admin +// shows. +type AdminNewWaitingListMemberInput struct { + From string + AdminAddress string + ID string + Email string + Reason string +} + +// Header addresses the mail to the admin, unique per waiting list entry. +func (in AdminNewWaitingListMemberInput) Header() Header { + return Header{UniqueID: in.ID, From: FromPcom(in.From), To: To(in.AdminAddress)} +} + +var adminNewWaitingListMemberMail = declare("new_waiting_list_member", "admin_new_waiting_list_member", adminNewWaitingListMemberSamples) + +func adminNewWaitingListMemberSamples() []Sample[AdminNewWaitingListMemberInput] { + return []Sample[AdminNewWaitingListMemberInput]{ + {Name: "admin_new_waiting_list_member", Input: AdminNewWaitingListMemberInput{ + From: SampleFrom, AdminAddress: sampleAdmin, + ID: "0190a3b4-0000-7000-8000-000000000002", Email: "signup@example.test", Reason: noReasonGiven, + }}, + {Name: "admin_new_waiting_list_member_reason", Input: AdminNewWaitingListMemberInput{ + From: SampleFrom, AdminAddress: sampleAdmin, + ID: "0190a3b4-0000-7000-8000-000000000002", Email: "signup@example.test", Reason: "I read your blog", + }}, + } +} + +// AdminNewWaitingListMember tells the admin about a new waiting list entry. +func AdminNewWaitingListMember(from, adminAddress string, waitingList *model.UserSignupRequest) *Envelope { + reason := lo.FromPtr(waitingList.Reason) + if reason == "" { + reason = noReasonGiven + } + + return adminNewWaitingListMemberMail.MustRender(AdminNewWaitingListMemberInput{ + From: from, AdminAddress: adminAddress, + ID: waitingList.ID, Email: waitingList.Email, Reason: reason, + }) +} diff --git a/pkg/mail/admin_page_failure.go b/pkg/mail/admin_page_failure.go new file mode 100644 index 00000000..faeb778b --- /dev/null +++ b/pkg/mail/admin_page_failure.go @@ -0,0 +1,60 @@ +package mail + +import ( + "fmt" + + "github.com/can3p/pcom/pkg/model" + "github.com/google/uuid" +) + +const anonymousUser = "Anonymous" + +// AdminPageFailureInput is what the panic alert to the admin shows: who hit +// the page and the report of the request, the panic and its stack. +type AdminPageFailureInput struct { + From string + AdminAddress string + UniqueID string + User string + Report string +} + +// Header addresses the mail to the admin; every failure has its own id. +func (in AdminPageFailureInput) Header() Header { + return Header{UniqueID: in.UniqueID, From: FromPcom(in.From), To: To(in.AdminAddress)} +} + +var adminPageFailureMail = declare("panic_notification", "admin_page_failure", adminPageFailureSamples) + +func adminPageFailureSamples() []Sample[AdminPageFailureInput] { + const report = "[Recovery] 2026/01/02 - 03:04:05 panic recovered:\r\n" + + "GET /posts/1 HTTP/1.1\r\nHost: pcom.test\r\nCookie: \r\n\r\n" + + "something broke\r\n" + + "/app/pkg/web/handler.go:42 (0x1234)\r\n\tHandle: panic(err)\r\n" + + return []Sample[AdminPageFailureInput]{ + {Name: "admin_page_failure", Input: AdminPageFailureInput{ + From: SampleFrom, AdminAddress: sampleAdmin, + UniqueID: "0190a3b4-0000-7000-8000-000000000004", + User: "email (alice@example.test), id (0190a3b4-0000-7000-8000-000000000001)", Report: report, + }}, + {Name: "admin_page_failure_anonymous", Input: AdminPageFailureInput{ + From: SampleFrom, AdminAddress: sampleAdmin, + UniqueID: "0190a3b4-0000-7000-8000-000000000004", User: anonymousUser, Report: report, + }}, + } +} + +// AdminPageFailure tells the admin that a page panicked. report is the +// request, the panic value and the stack as the caller formatted them; user +// is nil for a visitor who is not logged in. +func AdminPageFailure(from, adminAddress, report string, user *model.User) *Envelope { + who := anonymousUser + if user != nil { + who = fmt.Sprintf("email (%s), id (%s)", user.Email, user.ID) + } + + return adminPageFailureMail.MustRender(AdminPageFailureInput{ + From: from, AdminAddress: adminAddress, UniqueID: uuid.NewString(), User: who, Report: report, + }) +} diff --git a/pkg/mail/admin_signup_confirmed.go b/pkg/mail/admin_signup_confirmed.go new file mode 100644 index 00000000..161d3dc9 --- /dev/null +++ b/pkg/mail/admin_signup_confirmed.go @@ -0,0 +1,35 @@ +package mail + +import "github.com/can3p/pcom/pkg/model" + +// AdminSignupConfirmedInput is what the confirmed account alert to the admin +// shows. +type AdminSignupConfirmedInput struct { + From string + AdminAddress string + ID string + Email string +} + +// Header addresses the mail to the admin, unique per account. +func (in AdminSignupConfirmedInput) Header() Header { + return Header{UniqueID: in.ID, From: FromPcom(in.From), To: To(in.AdminAddress)} +} + +var adminSignupConfirmedMail = declare("signup_confirmed", "admin_signup_confirmed", adminSignupConfirmedSamples) + +func adminSignupConfirmedSamples() []Sample[AdminSignupConfirmedInput] { + return []Sample[AdminSignupConfirmedInput]{ + {Name: "admin_signup_confirmed", Input: AdminSignupConfirmedInput{ + From: SampleFrom, AdminAddress: sampleAdmin, + ID: "0190a3b4-0000-7000-8000-000000000001", Email: "alice@example.test", + }}, + } +} + +// AdminSignupConfirmed tells the admin that an account confirmed its email. +func AdminSignupConfirmed(from, adminAddress string, user *model.User) *Envelope { + return adminSignupConfirmedMail.MustRender(AdminSignupConfirmedInput{ + From: from, AdminAddress: adminAddress, ID: user.ID, Email: user.Email, + }) +} diff --git a/pkg/mail/admin_test.go b/pkg/mail/admin_test.go new file mode 100644 index 00000000..a58c8afb --- /dev/null +++ b/pkg/mail/admin_test.go @@ -0,0 +1,172 @@ +package mail_test + +import ( + "context" + "testing" + + "github.com/can3p/pcom/pkg/links" + "github.com/can3p/pcom/pkg/mail" + "github.com/can3p/pcom/pkg/model" + "github.com/can3p/pcom/pkg/testutil/fakesender" + "github.com/stretchr/testify/require" +) + +const testAdmin = "admin@pcom.test" + +// sampleEnvelope is the rendering of the sample called name. +func sampleEnvelope(t *testing.T, name string) *mail.Envelope { + t.Helper() + + for _, m := range mail.All() { + for _, s := range m.Samples { + if s.Name == name { + require.NoError(t, s.Err) + return s.Envelope + } + } + } + + require.FailNow(t, "no sample", name) + + return nil +} + +// requireSample checks that got is the rendering of the sample called name. +// Mails whose id is generated per send take the sample's id. +func requireSample(t *testing.T, name string, got *mail.Envelope, generatedID bool) { + t.Helper() + + want := sampleEnvelope(t, name) + if generatedID { + require.NotEmpty(t, got.UniqueID) + got.UniqueID = want.UniqueID + } + + require.Equal(t, want, got) + require.Equal(t, testAdmin, got.Mail.To[0].Address) + require.Equal(t, testFrom, got.Mail.From.Address) +} + +func TestAdminNewUser(t *testing.T) { + t.Parallel() + + user := &model.User{ID: "0190a3b4-0000-7000-8000-000000000001", Email: "alice@example.test", Username: "alice"} + + got := mail.AdminNewUser(links.Site{}, testFrom, testAdmin, user) + require.Equal(t, "admin_new_user", got.Type) + require.Equal(t, "New User on pcom", got.Mail.Subject) + requireSample(t, "admin_new_user", got, false) +} + +func TestAdminNewWaitingListMember(t *testing.T) { + t.Parallel() + + signup := &model.UserSignupRequest{ID: "0190a3b4-0000-7000-8000-000000000002", Email: "signup@example.test"} + + got := mail.AdminNewWaitingListMember(testFrom, testAdmin, signup) + require.Equal(t, "new_waiting_list_member", got.Type) + require.Equal(t, "New waiting list member on pcom", got.Mail.Subject) + requireSample(t, "admin_new_waiting_list_member", got, false) + + signup.Reason = new("I read your blog") + requireSample(t, "admin_new_waiting_list_member_reason", mail.AdminNewWaitingListMember(testFrom, testAdmin, signup), false) +} + +func TestAdminSignupConfirmed(t *testing.T) { + t.Parallel() + + user := &model.User{ID: "0190a3b4-0000-7000-8000-000000000001", Email: "alice@example.test", Username: "alice"} + + got := mail.AdminSignupConfirmed(testFrom, testAdmin, user) + require.Equal(t, "signup_confirmed", got.Type) + require.Equal(t, "New User confirmed email on pcom", got.Mail.Subject) + requireSample(t, "admin_signup_confirmed", got, false) +} + +func TestAdminThrowAwayEmailSignupAttempt(t *testing.T) { + t.Parallel() + + got := mail.AdminThrowAwayEmailSignupAttempt(testFrom, testAdmin, "test@throwaway.example.com") + require.Equal(t, "throw_away_email_signup", got.Type) + require.Equal(t, "An attempt to use a throwaway email domain on pcom", got.Mail.Subject) + requireSample(t, "admin_throwaway_email", got, true) + + other := mail.AdminThrowAwayEmailSignupAttempt(testFrom, testAdmin, "test@throwaway.example.com") + require.NotEqual(t, got.UniqueID, other.UniqueID) +} + +func TestAdminPageFailure(t *testing.T) { + t.Parallel() + + user := &model.User{ID: "0190a3b4-0000-7000-8000-000000000001", Email: "alice@example.test"} + + // the sample report is what the router would pass in + in := adminPageFailureReport(t) + + got := mail.AdminPageFailure(testFrom, testAdmin, in, user) + require.Equal(t, "panic_notification", got.Type) + require.Equal(t, "Panic on the page", got.Mail.Subject) + requireSample(t, "admin_page_failure", got, true) + + requireSample(t, "admin_page_failure_anonymous", mail.AdminPageFailure(testFrom, testAdmin, in, nil), true) +} + +// adminPageFailureReport is the report the page failure samples carry. +func adminPageFailureReport(t *testing.T) string { + t.Helper() + + return "[Recovery] 2026/01/02 - 03:04:05 panic recovered:\r\n" + + "GET /posts/1 HTTP/1.1\r\nHost: pcom.test\r\nCookie: \r\n\r\n" + + "something broke\r\n" + + "/app/pkg/web/handler.go:42 (0x1234)\r\n\tHandle: panic(err)\r\n" +} + +// adminMailers lists every admin notification with all user-controlled +// fields set to hostile markup. +func adminMailers() map[string]func() *mail.Envelope { + user := &model.User{ID: "user-1", Email: hostile, Username: hostile} + signup := &model.UserSignupRequest{ID: "signup-1", Email: hostile, Reason: new(hostile)} + + return map[string]func() *mail.Envelope{ + "AdminNewUser": func() *mail.Envelope { + return mail.AdminNewUser(links.Site{}, testFrom, testAdmin, user) + }, + "AdminNewWaitingListMember": func() *mail.Envelope { + return mail.AdminNewWaitingListMember(testFrom, testAdmin, signup) + }, + "AdminSignupConfirmed": func() *mail.Envelope { + return mail.AdminSignupConfirmed(testFrom, testAdmin, user) + }, + "AdminThrowAwayEmail": func() *mail.Envelope { + return mail.AdminThrowAwayEmailSignupAttempt(testFrom, testAdmin, hostile) + }, + "AdminPageFailure": func() *mail.Envelope { + return mail.AdminPageFailure(testFrom, testAdmin, hostile, user) + }, + } +} + +func TestAdminMails_EscapeUserContentInHTML(t *testing.T) { + t.Parallel() + + for name, build := range adminMailers() { + t.Run(name, func(t *testing.T) { + t.Parallel() + + s := fakesender.New() + require.NoError(t, send(context.Background(), s, build())) + + sent := s.Sent() + require.Len(t, sent, 1) + require.Equal(t, testFrom, sent[0].Mail.From.Address) + require.Len(t, sent[0].Mail.To, 1) + require.Equal(t, testAdmin, sent[0].Mail.To[0].Address) + + html := sent[0].Mail.Html + require.NotContains(t, html, rawTag) + require.NotContains(t, html, `"><`) + require.Contains(t, html, escapedTag) + require.Contains(t, html, escapedQuo) + }) + } +} diff --git a/pkg/mail/admin_throwaway_email.go b/pkg/mail/admin_throwaway_email.go new file mode 100644 index 00000000..38483c27 --- /dev/null +++ b/pkg/mail/admin_throwaway_email.go @@ -0,0 +1,36 @@ +package mail + +import "github.com/google/uuid" + +// AdminThrowAwayEmailInput is what the throwaway email alert to the admin +// shows. +type AdminThrowAwayEmailInput struct { + From string + AdminAddress string + UniqueID string + Email string +} + +// Header addresses the mail to the admin; every attempt has its own id. +func (in AdminThrowAwayEmailInput) Header() Header { + return Header{UniqueID: in.UniqueID, From: FromPcom(in.From), To: To(in.AdminAddress)} +} + +var adminThrowAwayEmailMail = declare("throw_away_email_signup", "admin_throwaway_email", adminThrowAwayEmailSamples) + +func adminThrowAwayEmailSamples() []Sample[AdminThrowAwayEmailInput] { + return []Sample[AdminThrowAwayEmailInput]{ + {Name: "admin_throwaway_email", Input: AdminThrowAwayEmailInput{ + From: SampleFrom, AdminAddress: sampleAdmin, + UniqueID: "0190a3b4-0000-7000-8000-000000000003", Email: "test@throwaway.example.com", + }}, + } +} + +// AdminThrowAwayEmailSignupAttempt tells the admin that somebody tried to +// sign up with a throwaway email domain. +func AdminThrowAwayEmailSignupAttempt(from, adminAddress string, email string) *Envelope { + return adminThrowAwayEmailMail.MustRender(AdminThrowAwayEmailInput{ + From: from, AdminAddress: adminAddress, UniqueID: uuid.NewString(), Email: email, + }) +} diff --git a/pkg/mail/templates/admin_new_user.html b/pkg/mail/templates/admin_new_user.html new file mode 100644 index 00000000..5549150d --- /dev/null +++ b/pkg/mail/templates/admin_new_user.html @@ -0,0 +1,9 @@ +

Hi!

+ +

New user alert:

+ + diff --git a/pkg/mail/templates/admin_new_user.subject.txt b/pkg/mail/templates/admin_new_user.subject.txt new file mode 100644 index 00000000..ad6a8614 --- /dev/null +++ b/pkg/mail/templates/admin_new_user.subject.txt @@ -0,0 +1 @@ +New User on pcom diff --git a/pkg/mail/templates/admin_new_user.txt b/pkg/mail/templates/admin_new_user.txt new file mode 100644 index 00000000..175c52ec --- /dev/null +++ b/pkg/mail/templates/admin_new_user.txt @@ -0,0 +1,7 @@ +Hi! + +New user alert: + +* ID: {{.ID}} +* Blog: {{.BlogURL}} +* Email: {{.Email}} diff --git a/pkg/mail/templates/admin_new_waiting_list_member.html b/pkg/mail/templates/admin_new_waiting_list_member.html new file mode 100644 index 00000000..5a2e5438 --- /dev/null +++ b/pkg/mail/templates/admin_new_waiting_list_member.html @@ -0,0 +1,8 @@ +

Hi!

+ +

New waiting list member alert:

+ +
    +
  • Email address: {{.Email}}
    • +
  • Reason: {{.Reason}}
    • +
diff --git a/pkg/mail/templates/admin_new_waiting_list_member.subject.txt b/pkg/mail/templates/admin_new_waiting_list_member.subject.txt new file mode 100644 index 00000000..840e2d70 --- /dev/null +++ b/pkg/mail/templates/admin_new_waiting_list_member.subject.txt @@ -0,0 +1 @@ +New waiting list member on pcom diff --git a/pkg/mail/templates/admin_new_waiting_list_member.txt b/pkg/mail/templates/admin_new_waiting_list_member.txt new file mode 100644 index 00000000..ab656b38 --- /dev/null +++ b/pkg/mail/templates/admin_new_waiting_list_member.txt @@ -0,0 +1,6 @@ +Hi! + +New waiting list member alert: + +* Email address: {{.Email}} +* Reason: {{.Reason}} diff --git a/pkg/mail/templates/admin_page_failure.html b/pkg/mail/templates/admin_page_failure.html new file mode 100644 index 00000000..dfa9c0ad --- /dev/null +++ b/pkg/mail/templates/admin_page_failure.html @@ -0,0 +1,8 @@ +

Hi!

+ +

Panic on the page:

+ +
    +
  • user: {{.User}}
    • +
  • Request data: 
    {{.Report}}
    • +
diff --git a/pkg/mail/templates/admin_page_failure.subject.txt b/pkg/mail/templates/admin_page_failure.subject.txt new file mode 100644 index 00000000..93f5ef62 --- /dev/null +++ b/pkg/mail/templates/admin_page_failure.subject.txt @@ -0,0 +1 @@ +Panic on the page diff --git a/pkg/mail/templates/admin_page_failure.txt b/pkg/mail/templates/admin_page_failure.txt new file mode 100644 index 00000000..83ab3850 --- /dev/null +++ b/pkg/mail/templates/admin_page_failure.txt @@ -0,0 +1,8 @@ +Hi! + +Panic on the page + +* User: {{.User}} +* Request data: + +{{.Report}} diff --git a/pkg/mail/templates/admin_signup_confirmed.html b/pkg/mail/templates/admin_signup_confirmed.html new file mode 100644 index 00000000..e1d83e4f --- /dev/null +++ b/pkg/mail/templates/admin_signup_confirmed.html @@ -0,0 +1,8 @@ +

Hi!

+ +

New conrirmed user alert:

+ +
    +
  • ID: {{.ID}}
    • +
  • Email: {{.Email}}
    • +
diff --git a/pkg/mail/templates/admin_signup_confirmed.subject.txt b/pkg/mail/templates/admin_signup_confirmed.subject.txt new file mode 100644 index 00000000..60201f21 --- /dev/null +++ b/pkg/mail/templates/admin_signup_confirmed.subject.txt @@ -0,0 +1 @@ +New User confirmed email on pcom diff --git a/pkg/mail/templates/admin_signup_confirmed.txt b/pkg/mail/templates/admin_signup_confirmed.txt new file mode 100644 index 00000000..1f623f0e --- /dev/null +++ b/pkg/mail/templates/admin_signup_confirmed.txt @@ -0,0 +1,6 @@ +Hi! + +New conrirmed user alert: + +* ID: {{.ID}} +* Email: {{.Email}} diff --git a/pkg/mail/templates/admin_throwaway_email.html b/pkg/mail/templates/admin_throwaway_email.html new file mode 100644 index 00000000..0c3b952c --- /dev/null +++ b/pkg/mail/templates/admin_throwaway_email.html @@ -0,0 +1,3 @@ +

Hi!

+ +

A user has just tried to use a throwaway email: {{.Email}}

diff --git a/pkg/mail/templates/admin_throwaway_email.subject.txt b/pkg/mail/templates/admin_throwaway_email.subject.txt new file mode 100644 index 00000000..60f7cdd6 --- /dev/null +++ b/pkg/mail/templates/admin_throwaway_email.subject.txt @@ -0,0 +1 @@ +An attempt to use a throwaway email domain on pcom diff --git a/pkg/mail/templates/admin_throwaway_email.txt b/pkg/mail/templates/admin_throwaway_email.txt new file mode 100644 index 00000000..0007288e --- /dev/null +++ b/pkg/mail/templates/admin_throwaway_email.txt @@ -0,0 +1,3 @@ +Hi! + +A user has just tried to use a throwaway email: {{.Email}} diff --git a/pkg/mail/testdata/admin_new_user.golden b/pkg/mail/testdata/admin_new_user.golden new file mode 100644 index 00000000..de355e9f --- /dev/null +++ b/pkg/mail/testdata/admin_new_user.golden @@ -0,0 +1,25 @@ +EmailType: admin_new_user +UniqueID: 0190a3b4-0000-7000-8000-000000000001 +From: Your pcom +To: admin@pcom.test +Subject: New User on pcom + +--- TEXT --- +Hi! + +New user alert: + +* ID: 0190a3b4-0000-7000-8000-000000000001 +* Blog: /users/alice +* Email: alice@example.test + +--- HTML --- +

Hi!

+ +

New user alert:

+ +
    +
  • ID: 0190a3b4-0000-7000-8000-000000000001
    • +
  • Blog: /users/alice
    • +
  • Email: alice@example.test
    • +
diff --git a/pkg/mail/testdata/admin_new_waiting_list_member.golden b/pkg/mail/testdata/admin_new_waiting_list_member.golden new file mode 100644 index 00000000..b9c5d887 --- /dev/null +++ b/pkg/mail/testdata/admin_new_waiting_list_member.golden @@ -0,0 +1,23 @@ +EmailType: new_waiting_list_member +UniqueID: 0190a3b4-0000-7000-8000-000000000002 +From: Your pcom +To: admin@pcom.test +Subject: New waiting list member on pcom + +--- TEXT --- +Hi! + +New waiting list member alert: + +* Email address: signup@example.test +* Reason: Not specified + +--- HTML --- +

Hi!

+ +

New waiting list member alert:

+ +
    +
  • Email address: signup@example.test
    • +
  • Reason: Not specified
    • +
diff --git a/pkg/mail/testdata/admin_new_waiting_list_member_reason.golden b/pkg/mail/testdata/admin_new_waiting_list_member_reason.golden new file mode 100644 index 00000000..f659bd92 --- /dev/null +++ b/pkg/mail/testdata/admin_new_waiting_list_member_reason.golden @@ -0,0 +1,23 @@ +EmailType: new_waiting_list_member +UniqueID: 0190a3b4-0000-7000-8000-000000000002 +From: Your pcom +To: admin@pcom.test +Subject: New waiting list member on pcom + +--- TEXT --- +Hi! + +New waiting list member alert: + +* Email address: signup@example.test +* Reason: I read your blog + +--- HTML --- +

Hi!

+ +

New waiting list member alert:

+ +
    +
  • Email address: signup@example.test
    • +
  • Reason: I read your blog
    • +
diff --git a/pkg/mail/testdata/admin_page_failure.golden b/pkg/mail/testdata/admin_page_failure.golden new file mode 100644 index 00000000..0e03b3fe --- /dev/null +++ b/pkg/mail/testdata/admin_page_failure.golden @@ -0,0 +1,40 @@ +EmailType: panic_notification +UniqueID: 0190a3b4-0000-7000-8000-000000000004 +From: Your pcom +To: admin@pcom.test +Subject: Panic on the page + +--- TEXT --- +Hi! + +Panic on the page + +* User: email (alice@example.test), id (0190a3b4-0000-7000-8000-000000000001) +* Request data: + +[Recovery] 2026/01/02 - 03:04:05 panic recovered: +GET /posts/1 HTTP/1.1 +Host: pcom.test +Cookie: + +something broke +/app/pkg/web/handler.go:42 (0x1234) + Handle: panic(err) + +--- HTML --- +

Hi!

+ +

Panic on the page:

+ +
    +
  • user: email (alice@example.test), id (0190a3b4-0000-7000-8000-000000000001)
    • +
  • Request data: 
    [Recovery] 2026/01/02 - 03:04:05 panic recovered:
+GET /posts/1 HTTP/1.1
+Host: pcom.test
+Cookie: <hidden>
+
+something broke
+/app/pkg/web/handler.go:42 (0x1234)
+	Handle: panic(err)
+
    • +
diff --git a/pkg/mail/testdata/admin_page_failure_anonymous.golden b/pkg/mail/testdata/admin_page_failure_anonymous.golden new file mode 100644 index 00000000..f1f64ef0 --- /dev/null +++ b/pkg/mail/testdata/admin_page_failure_anonymous.golden @@ -0,0 +1,40 @@ +EmailType: panic_notification +UniqueID: 0190a3b4-0000-7000-8000-000000000004 +From: Your pcom +To: admin@pcom.test +Subject: Panic on the page + +--- TEXT --- +Hi! + +Panic on the page + +* User: Anonymous +* Request data: + +[Recovery] 2026/01/02 - 03:04:05 panic recovered: +GET /posts/1 HTTP/1.1 +Host: pcom.test +Cookie: + +something broke +/app/pkg/web/handler.go:42 (0x1234) + Handle: panic(err) + +--- HTML --- +

Hi!

+ +

Panic on the page:

+ +
    +
  • user: Anonymous
    • +
  • Request data: 
    [Recovery] 2026/01/02 - 03:04:05 panic recovered:
+GET /posts/1 HTTP/1.1
+Host: pcom.test
+Cookie: <hidden>
+
+something broke
+/app/pkg/web/handler.go:42 (0x1234)
+	Handle: panic(err)
+
    • +
diff --git a/pkg/mail/testdata/admin_signup_confirmed.golden b/pkg/mail/testdata/admin_signup_confirmed.golden new file mode 100644 index 00000000..f71a137d --- /dev/null +++ b/pkg/mail/testdata/admin_signup_confirmed.golden @@ -0,0 +1,23 @@ +EmailType: signup_confirmed +UniqueID: 0190a3b4-0000-7000-8000-000000000001 +From: Your pcom +To: admin@pcom.test +Subject: New User confirmed email on pcom + +--- TEXT --- +Hi! + +New conrirmed user alert: + +* ID: 0190a3b4-0000-7000-8000-000000000001 +* Email: alice@example.test + +--- HTML --- +

Hi!

+ +

New conrirmed user alert:

+ +
    +
  • ID: 0190a3b4-0000-7000-8000-000000000001
    • +
  • Email: alice@example.test
    • +
diff --git a/pkg/mail/testdata/admin_throwaway_email.golden b/pkg/mail/testdata/admin_throwaway_email.golden new file mode 100644 index 00000000..02e89bb7 --- /dev/null +++ b/pkg/mail/testdata/admin_throwaway_email.golden @@ -0,0 +1,15 @@ +EmailType: throw_away_email_signup +UniqueID: 0190a3b4-0000-7000-8000-000000000003 +From: Your pcom +To: admin@pcom.test +Subject: An attempt to use a throwaway email domain on pcom + +--- TEXT --- +Hi! + +A user has just tried to use a throwaway email: test@throwaway.example.com + +--- HTML --- +

Hi!

+ +

A user has just tried to use a throwaway email: test@throwaway.example.com

diff --git a/pkg/service/accounts/codes.go b/pkg/service/accounts/codes.go index 4fc68e0e..bf36c041 100644 --- a/pkg/service/accounts/codes.go +++ b/pkg/service/accounts/codes.go @@ -13,7 +13,6 @@ import ( "math/big" "time" - "github.com/can3p/pcom/pkg/admin" "github.com/can3p/pcom/pkg/mail" "github.com/can3p/pcom/pkg/model" "github.com/can3p/pcom/pkg/pgsession" @@ -310,7 +309,7 @@ func (s *Service) FinishLogin(ctx context.Context, attemptID, code string) (*mod } if confirmed { - s.notifyAdmin(ctx, "signup confirmed", admin.SignupConfirmed(s.ident.Site, s.ident.From, s.ident.AdminAddress, user)) + s.notifyAdmin(ctx, "signup confirmed", mail.AdminSignupConfirmed(s.ident.From, s.ident.AdminAddress, user)) } return user, returnURL, nil diff --git a/pkg/service/accounts/invites.go b/pkg/service/accounts/invites.go index 7dda11f6..2ba4c4a6 100644 --- a/pkg/service/accounts/invites.go +++ b/pkg/service/accounts/invites.go @@ -5,7 +5,6 @@ import ( "fmt" "time" - "github.com/can3p/pcom/pkg/admin" "github.com/can3p/pcom/pkg/mail" "github.com/can3p/pcom/pkg/model" "github.com/can3p/pcom/pkg/pgsession" @@ -119,7 +118,7 @@ func (s *Service) AcceptInvite(ctx context.Context, invite *model.UserInvitation return err } - if err := s.send(ctx, tx, admin.NewUser(s.ident.Site, s.ident.From, s.ident.AdminAddress, u)); err != nil { + if err := s.send(ctx, tx, mail.AdminNewUser(s.ident.Site, s.ident.From, s.ident.AdminAddress, u)); err != nil { return err } diff --git a/pkg/service/accounts/signup.go b/pkg/service/accounts/signup.go index 9bb4fd82..8df1f95a 100644 --- a/pkg/service/accounts/signup.go +++ b/pkg/service/accounts/signup.go @@ -7,7 +7,6 @@ import ( "time" disposable "github.com/can3p/anti-disposable-email" - "github.com/can3p/pcom/pkg/admin" "github.com/can3p/pcom/pkg/forms/validation" "github.com/can3p/pcom/pkg/mail" "github.com/can3p/pcom/pkg/model" @@ -41,7 +40,7 @@ func (s *Service) CheckSignupEmail(ctx context.Context, address string) error { if parsedEmail.Disposable { go func() { - if err := s.send(ctx, s.store, admin.ThrowAwayEmailSignupAttempt(s.ident.From, s.ident.AdminAddress, address)); err != nil { + if err := s.send(ctx, s.store, mail.AdminThrowAwayEmailSignupAttempt(s.ident.From, s.ident.AdminAddress, address)); err != nil { log.Printf("failed to queue the throwaway email notification: %v", err) } }() @@ -138,7 +137,7 @@ func (s *Service) Register(ctx context.Context, email, username, attribution str return err } - if err := s.send(ctx, tx, admin.NewUser(s.ident.Site, s.ident.From, s.ident.AdminAddress, u)); err != nil { + if err := s.send(ctx, tx, mail.AdminNewUser(s.ident.Site, s.ident.From, s.ident.AdminAddress, u)); err != nil { return err } @@ -181,7 +180,7 @@ func (s *Service) JoinWaitingList(ctx context.Context, email, reason, attributio return fatal(err) } - return s.send(ctx, tx, admin.NewWaitingListMember(s.ident.Site, s.ident.From, s.ident.AdminAddress, request)) + return s.send(ctx, tx, mail.AdminNewWaitingListMember(s.ident.From, s.ident.AdminAddress, request)) }) } diff --git a/pkg/web/app/router.go b/pkg/web/app/router.go index c9cebb00..08825ca8 100644 --- a/pkg/web/app/router.go +++ b/pkg/web/app/router.go @@ -9,6 +9,7 @@ import ( "github.com/can3p/gogo/util/ginhelpers/csrf" "github.com/can3p/pcom/pkg/admin" "github.com/can3p/pcom/pkg/auth" + "github.com/can3p/pcom/pkg/mail" "github.com/can3p/pcom/pkg/pgsession" "github.com/can3p/pcom/pkg/service/registry" "github.com/can3p/pcom/pkg/util/ginhelpers/csp" @@ -50,7 +51,7 @@ func New(d *Deps) *gin.Engine { // session, and its MustGet would panic inside the recovery user := auth.GetAPIUserData(c).DBUser - if nerr := d.Services.Accounts.SendAdminMail(c, admin.PageFailure(d.Config.SenderAddress, d.Config.AdminAddress, c, err, user)); nerr != nil { + if nerr := d.Services.Accounts.SendAdminMail(c, mail.AdminPageFailure(d.Config.SenderAddress, d.Config.AdminAddress, admin.ClonedCustomRecovery(c, err), user)); nerr != nil { log.Printf("failed to queue the page failure notification: %v", nerr) } })) From 8c309cbd283b5a47e4be866e2458b6c2eff621b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 23:00:05 +0000 Subject: [PATCH 11/16] refactor(r4): merge M1-M6: one sampleEnvelope helper, drop the Outgoing alias and the emptied admin files Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UmD6gdC2ENrRcg4XZVZuxd --- pkg/admin/notifications.go | 1 - pkg/admin/notifications_test.go | 1 - .../testdata/notify_new_user_html.golden | 10 ---------- .../testdata/notify_new_user_text.golden | 8 -------- ...notify_new_waiting_list_member_html.golden | 9 --------- ...notify_new_waiting_list_member_text.golden | 8 -------- .../notify_signup_confirmed_html.golden | 9 --------- .../notify_signup_confirmed_text.golden | 7 ------- .../notify_throwaway_email_html.golden | 5 ----- .../notify_throwaway_email_text.golden | 4 ---- pkg/mail/admin_test.go | 18 ------------------ pkg/mail/comments_test.go | 19 ------------------- pkg/mail/golden_test.go | 19 +++++++++++++++++++ pkg/mail/mail.go | 6 ------ pkg/mail/posts_test.go | 19 ------------------- 15 files changed, 19 insertions(+), 124 deletions(-) delete mode 100644 pkg/admin/notifications.go delete mode 100644 pkg/admin/notifications_test.go delete mode 100644 pkg/admin/testdata/notify_new_user_html.golden delete mode 100644 pkg/admin/testdata/notify_new_user_text.golden delete mode 100644 pkg/admin/testdata/notify_new_waiting_list_member_html.golden delete mode 100644 pkg/admin/testdata/notify_new_waiting_list_member_text.golden delete mode 100644 pkg/admin/testdata/notify_signup_confirmed_html.golden delete mode 100644 pkg/admin/testdata/notify_signup_confirmed_text.golden delete mode 100644 pkg/admin/testdata/notify_throwaway_email_html.golden delete mode 100644 pkg/admin/testdata/notify_throwaway_email_text.golden diff --git a/pkg/admin/notifications.go b/pkg/admin/notifications.go deleted file mode 100644 index d78da5da..00000000 --- a/pkg/admin/notifications.go +++ /dev/null @@ -1 +0,0 @@ -package admin diff --git a/pkg/admin/notifications_test.go b/pkg/admin/notifications_test.go deleted file mode 100644 index 79dcfc9f..00000000 --- a/pkg/admin/notifications_test.go +++ /dev/null @@ -1 +0,0 @@ -package admin_test diff --git a/pkg/admin/testdata/notify_new_user_html.golden b/pkg/admin/testdata/notify_new_user_html.golden deleted file mode 100644 index f81ce103..00000000 --- a/pkg/admin/testdata/notify_new_user_html.golden +++ /dev/null @@ -1,10 +0,0 @@ - -

Hi!

- -

New user alert:

- -
    -
  • ID: [UUID]
    • -
  • Blog: /users/alice
    • -
  • Email: alice@example.test
    • -
\ No newline at end of file diff --git a/pkg/admin/testdata/notify_new_user_text.golden b/pkg/admin/testdata/notify_new_user_text.golden deleted file mode 100644 index 8511ca2d..00000000 --- a/pkg/admin/testdata/notify_new_user_text.golden +++ /dev/null @@ -1,8 +0,0 @@ - - Hi! - - New user alert: - - * ID: [UUID] - * Blog: /users/alice - * Email: alice@example.test \ No newline at end of file diff --git a/pkg/admin/testdata/notify_new_waiting_list_member_html.golden b/pkg/admin/testdata/notify_new_waiting_list_member_html.golden deleted file mode 100644 index 58f2b227..00000000 --- a/pkg/admin/testdata/notify_new_waiting_list_member_html.golden +++ /dev/null @@ -1,9 +0,0 @@ - -

Hi!

- -

New waiting list member alert:

- -
    -
  • Email address: signup@example.test
    • -
  • Reason: Not specified
    • -
\ No newline at end of file diff --git a/pkg/admin/testdata/notify_new_waiting_list_member_text.golden b/pkg/admin/testdata/notify_new_waiting_list_member_text.golden deleted file mode 100644 index af6ef4d0..00000000 --- a/pkg/admin/testdata/notify_new_waiting_list_member_text.golden +++ /dev/null @@ -1,8 +0,0 @@ - - Hi! - - New waiting list member alert: - - * Email address: signup@example.test - * Reason: Not specified - \ No newline at end of file diff --git a/pkg/admin/testdata/notify_signup_confirmed_html.golden b/pkg/admin/testdata/notify_signup_confirmed_html.golden deleted file mode 100644 index e6861420..00000000 --- a/pkg/admin/testdata/notify_signup_confirmed_html.golden +++ /dev/null @@ -1,9 +0,0 @@ - -

Hi!

- -

New conrirmed user alert:

- -
    -
  • ID: [UUID]
    • -
  • Email: alice@example.test
    • -
\ No newline at end of file diff --git a/pkg/admin/testdata/notify_signup_confirmed_text.golden b/pkg/admin/testdata/notify_signup_confirmed_text.golden deleted file mode 100644 index 0ffb8c0a..00000000 --- a/pkg/admin/testdata/notify_signup_confirmed_text.golden +++ /dev/null @@ -1,7 +0,0 @@ - - Hi! - - New conrirmed user alert: - - * ID: [UUID] - * Email: alice@example.test \ No newline at end of file diff --git a/pkg/admin/testdata/notify_throwaway_email_html.golden b/pkg/admin/testdata/notify_throwaway_email_html.golden deleted file mode 100644 index 483c3e05..00000000 --- a/pkg/admin/testdata/notify_throwaway_email_html.golden +++ /dev/null @@ -1,5 +0,0 @@ - -

Hi!

- -

A user has just tried to use a throwaway email: test@throwaway.example.com

- \ No newline at end of file diff --git a/pkg/admin/testdata/notify_throwaway_email_text.golden b/pkg/admin/testdata/notify_throwaway_email_text.golden deleted file mode 100644 index 629e1fb2..00000000 --- a/pkg/admin/testdata/notify_throwaway_email_text.golden +++ /dev/null @@ -1,4 +0,0 @@ - - Hi! - - A user has just tried to use a throwaway email: test@throwaway.example.com \ No newline at end of file diff --git a/pkg/mail/admin_test.go b/pkg/mail/admin_test.go index a58c8afb..b0def589 100644 --- a/pkg/mail/admin_test.go +++ b/pkg/mail/admin_test.go @@ -13,24 +13,6 @@ import ( const testAdmin = "admin@pcom.test" -// sampleEnvelope is the rendering of the sample called name. -func sampleEnvelope(t *testing.T, name string) *mail.Envelope { - t.Helper() - - for _, m := range mail.All() { - for _, s := range m.Samples { - if s.Name == name { - require.NoError(t, s.Err) - return s.Envelope - } - } - } - - require.FailNow(t, "no sample", name) - - return nil -} - // requireSample checks that got is the rendering of the sample called name. // Mails whose id is generated per send take the sample's id. func requireSample(t *testing.T, name string, got *mail.Envelope, generatedID bool) { diff --git a/pkg/mail/comments_test.go b/pkg/mail/comments_test.go index d437d72c..9bb67541 100644 --- a/pkg/mail/comments_test.go +++ b/pkg/mail/comments_test.go @@ -12,25 +12,6 @@ import ( "github.com/stretchr/testify/require" ) -// sampleEnvelope is the rendering of the sample name. -func sampleEnvelope(t *testing.T, name string) *mail.Envelope { - t.Helper() - - for _, m := range mail.All() { - for _, s := range m.Samples { - if s.Name == name { - require.NoError(t, s.Err) - - return s.Envelope - } - } - } - - require.Failf(t, "no such sample", "sample %s", name) - - return nil -} - type formatComment func(commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (*mail.Envelope, error) func formatAuthor(commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (*mail.Envelope, error) { diff --git a/pkg/mail/golden_test.go b/pkg/mail/golden_test.go index 0e66095e..e47e8dfb 100644 --- a/pkg/mail/golden_test.go +++ b/pkg/mail/golden_test.go @@ -66,3 +66,22 @@ func TestMails_Registry(t *testing.T) { } } } + +// sampleEnvelope is the rendering of the sample called name. +func sampleEnvelope(t *testing.T, name string) *mail.Envelope { + t.Helper() + + for _, m := range mail.All() { + for _, s := range m.Samples { + if s.Name == name { + require.NoError(t, s.Err) + + return s.Envelope + } + } + } + + require.Failf(t, "no such sample", "sample %s", name) + + return nil +} diff --git a/pkg/mail/mail.go b/pkg/mail/mail.go index e8644794..6e174a9a 100644 --- a/pkg/mail/mail.go +++ b/pkg/mail/mail.go @@ -76,12 +76,6 @@ type Envelope struct { Mail *sender.Mail } -// Outgoing is the former name of Envelope, kept while the mails not yet -// declared with declare still return it. -// -// Deprecated: use Envelope. -type Outgoing = Envelope - // Header is the addressing of one mail: the unique id the queue drops repeats // by, and who it is from and to. type Header struct { diff --git a/pkg/mail/posts_test.go b/pkg/mail/posts_test.go index 355aecf4..e0c81e7d 100644 --- a/pkg/mail/posts_test.go +++ b/pkg/mail/posts_test.go @@ -11,25 +11,6 @@ import ( "github.com/stretchr/testify/require" ) -// sampleEnvelope is the rendering of the sample called name. -func sampleEnvelope(t *testing.T, name string) *mail.Envelope { - t.Helper() - - for _, m := range mail.All() { - for _, s := range m.Samples { - if s.Name == name { - require.NoError(t, s.Err) - - return s.Envelope - } - } - } - - require.Failf(t, "no such sample", "sample %s", name) - - return nil -} - func keepMedia(in string) (bool, string) { return false, in } func postUsers() (user, connection *model.User) { From a54ee2675e47abf35a3db8e042889bcc4ed2280d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 23:08:32 +0000 Subject: [PATCH 12/16] refactor(r4): tighten the mail tests after the assertion audit Comment samples are literal inputs rather than built by the constructors' own mapper; samples use a site with a root, so a constructor that ignores its site fails; the post-prompt-answer sample tells the post id from the prompt's post id; the send-error test that exercised no mail code moves to the posts service as a rollback test; redundant admin asserts are dropped. Golden changes: links gain https://pcom.test, and post_prompt_answer's unique id is post-2. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UmD6gdC2ENrRcg4XZVZuxd --- pkg/mail/admin_page_failure.go | 11 ++- pkg/mail/admin_test.go | 26 +------ pkg/mail/comments_test.go | 4 +- pkg/mail/escape_test.go | 18 ----- pkg/mail/mail.go | 2 +- pkg/mail/post_comment_author.go | 74 +++++++++---------- pkg/mail/post_prompt_answer.go | 2 +- pkg/mail/posts_test.go | 10 +-- pkg/mail/send_internal_test.go | 5 +- pkg/mail/testdata/admin_new_user.golden | 4 +- pkg/mail/testdata/new_post.golden | 4 +- pkg/mail/testdata/new_post_empty_body.golden | 4 +- .../testdata/new_post_long_subject.golden | 4 +- .../testdata/new_post_markup_in_body.golden | 4 +- pkg/mail/testdata/new_post_no_subject.golden | 4 +- .../testdata/new_post_special_chars.golden | 4 +- pkg/mail/testdata/new_post_with_url.golden | 4 +- pkg/mail/testdata/post_comment_author.golden | 4 +- .../post_comment_author_edited.golden | 4 +- .../post_comment_author_markup.golden | 4 +- ..._comment_author_markup_long_subject.golden | 4 +- .../post_comment_author_with_url.golden | 4 +- .../testdata/post_comment_participants.golden | 4 +- .../post_comment_participants_edited.golden | 4 +- .../post_comment_participants_markup.golden | 4 +- ...nt_participants_markup_long_subject.golden | 4 +- .../post_comment_participants_with_url.golden | 4 +- pkg/mail/testdata/post_prompt.golden | 4 +- pkg/mail/testdata/post_prompt_answer.golden | 6 +- pkg/mail/testdata/post_prompt_markup.golden | 4 +- .../send_actual_confirm_waiting_list.golden | 4 +- .../testdata/send_actual_invitation.golden | 4 +- pkg/service/posts/comments_edit_db_test.go | 25 +++++++ 33 files changed, 129 insertions(+), 142 deletions(-) diff --git a/pkg/mail/admin_page_failure.go b/pkg/mail/admin_page_failure.go index faeb778b..309f1daa 100644 --- a/pkg/mail/admin_page_failure.go +++ b/pkg/mail/admin_page_failure.go @@ -9,6 +9,12 @@ import ( const anonymousUser = "Anonymous" +// SampleAdminPageFailureReport is the report the page failure samples carry. +const SampleAdminPageFailureReport = "[Recovery] 2026/01/02 - 03:04:05 panic recovered:\r\n" + + "GET /posts/1 HTTP/1.1\r\nHost: pcom.test\r\nCookie: \r\n\r\n" + + "something broke\r\n" + + "/app/pkg/web/handler.go:42 (0x1234)\r\n\tHandle: panic(err)\r\n" + // AdminPageFailureInput is what the panic alert to the admin shows: who hit // the page and the report of the request, the panic and its stack. type AdminPageFailureInput struct { @@ -27,10 +33,7 @@ func (in AdminPageFailureInput) Header() Header { var adminPageFailureMail = declare("panic_notification", "admin_page_failure", adminPageFailureSamples) func adminPageFailureSamples() []Sample[AdminPageFailureInput] { - const report = "[Recovery] 2026/01/02 - 03:04:05 panic recovered:\r\n" + - "GET /posts/1 HTTP/1.1\r\nHost: pcom.test\r\nCookie: \r\n\r\n" + - "something broke\r\n" + - "/app/pkg/web/handler.go:42 (0x1234)\r\n\tHandle: panic(err)\r\n" + const report = SampleAdminPageFailureReport return []Sample[AdminPageFailureInput]{ {Name: "admin_page_failure", Input: AdminPageFailureInput{ diff --git a/pkg/mail/admin_test.go b/pkg/mail/admin_test.go index b0def589..7f66515e 100644 --- a/pkg/mail/admin_test.go +++ b/pkg/mail/admin_test.go @@ -25,8 +25,6 @@ func requireSample(t *testing.T, name string, got *mail.Envelope, generatedID bo } require.Equal(t, want, got) - require.Equal(t, testAdmin, got.Mail.To[0].Address) - require.Equal(t, testFrom, got.Mail.From.Address) } func TestAdminNewUser(t *testing.T) { @@ -34,9 +32,7 @@ func TestAdminNewUser(t *testing.T) { user := &model.User{ID: "0190a3b4-0000-7000-8000-000000000001", Email: "alice@example.test", Username: "alice"} - got := mail.AdminNewUser(links.Site{}, testFrom, testAdmin, user) - require.Equal(t, "admin_new_user", got.Type) - require.Equal(t, "New User on pcom", got.Mail.Subject) + got := mail.AdminNewUser(mail.SampleSite, testFrom, testAdmin, user) requireSample(t, "admin_new_user", got, false) } @@ -46,8 +42,6 @@ func TestAdminNewWaitingListMember(t *testing.T) { signup := &model.UserSignupRequest{ID: "0190a3b4-0000-7000-8000-000000000002", Email: "signup@example.test"} got := mail.AdminNewWaitingListMember(testFrom, testAdmin, signup) - require.Equal(t, "new_waiting_list_member", got.Type) - require.Equal(t, "New waiting list member on pcom", got.Mail.Subject) requireSample(t, "admin_new_waiting_list_member", got, false) signup.Reason = new("I read your blog") @@ -60,8 +54,6 @@ func TestAdminSignupConfirmed(t *testing.T) { user := &model.User{ID: "0190a3b4-0000-7000-8000-000000000001", Email: "alice@example.test", Username: "alice"} got := mail.AdminSignupConfirmed(testFrom, testAdmin, user) - require.Equal(t, "signup_confirmed", got.Type) - require.Equal(t, "New User confirmed email on pcom", got.Mail.Subject) requireSample(t, "admin_signup_confirmed", got, false) } @@ -69,8 +61,6 @@ func TestAdminThrowAwayEmailSignupAttempt(t *testing.T) { t.Parallel() got := mail.AdminThrowAwayEmailSignupAttempt(testFrom, testAdmin, "test@throwaway.example.com") - require.Equal(t, "throw_away_email_signup", got.Type) - require.Equal(t, "An attempt to use a throwaway email domain on pcom", got.Mail.Subject) requireSample(t, "admin_throwaway_email", got, true) other := mail.AdminThrowAwayEmailSignupAttempt(testFrom, testAdmin, "test@throwaway.example.com") @@ -83,26 +73,14 @@ func TestAdminPageFailure(t *testing.T) { user := &model.User{ID: "0190a3b4-0000-7000-8000-000000000001", Email: "alice@example.test"} // the sample report is what the router would pass in - in := adminPageFailureReport(t) + in := mail.SampleAdminPageFailureReport got := mail.AdminPageFailure(testFrom, testAdmin, in, user) - require.Equal(t, "panic_notification", got.Type) - require.Equal(t, "Panic on the page", got.Mail.Subject) requireSample(t, "admin_page_failure", got, true) requireSample(t, "admin_page_failure_anonymous", mail.AdminPageFailure(testFrom, testAdmin, in, nil), true) } -// adminPageFailureReport is the report the page failure samples carry. -func adminPageFailureReport(t *testing.T) string { - t.Helper() - - return "[Recovery] 2026/01/02 - 03:04:05 panic recovered:\r\n" + - "GET /posts/1 HTTP/1.1\r\nHost: pcom.test\r\nCookie: \r\n\r\n" + - "something broke\r\n" + - "/app/pkg/web/handler.go:42 (0x1234)\r\n\tHandle: panic(err)\r\n" -} - // adminMailers lists every admin notification with all user-controlled // fields set to hostile markup. func adminMailers() map[string]func() *mail.Envelope { diff --git a/pkg/mail/comments_test.go b/pkg/mail/comments_test.go index 9bb67541..e30c2e65 100644 --- a/pkg/mail/comments_test.go +++ b/pkg/mail/comments_test.go @@ -17,13 +17,13 @@ type formatComment func(commenter, recipient *model.User, post *model.Post, comm func formatAuthor(commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (*mail.Envelope, error) { mediaReplacer := func(in string) (bool, string) { return false, in } - return mail.PostCommentAuthor(links.Site{}, testFrom, mediaReplacer, commenter, recipient, post, comment, edited) + return mail.PostCommentAuthor(mail.SampleSite, testFrom, mediaReplacer, commenter, recipient, post, comment, edited) } func formatParticipants(commenter, recipient *model.User, post *model.Post, comment *model.PostComment, edited bool) (*mail.Envelope, error) { mediaReplacer := func(in string) (bool, string) { return false, in } - return mail.PostCommentParticipants(links.Site{}, testFrom, mediaReplacer, commenter, recipient, post, comment, edited) + return mail.PostCommentParticipants(mail.SampleSite, testFrom, mediaReplacer, commenter, recipient, post, comment, edited) } // The comment mails are built from models; each case builds the models of diff --git a/pkg/mail/escape_test.go b/pkg/mail/escape_test.go index 5e7ace43..412b5fe3 100644 --- a/pkg/mail/escape_test.go +++ b/pkg/mail/escape_test.go @@ -2,7 +2,6 @@ package mail_test import ( "context" - "errors" "testing" "github.com/can3p/pcom/pkg/links" @@ -76,20 +75,3 @@ func TestMailers_EscapeUserContentInHTML(t *testing.T) { }) } } - -func TestMailers_ReturnSendErrors(t *testing.T) { - t.Parallel() - - sendErr := errors.New("database is gone") - - for name, send := range mailers() { - t.Run(name, func(t *testing.T) { - t.Parallel() - - s := fakesender.New() - s.FailWith(sendErr) - - require.ErrorIs(t, send(s), sendErr) - }) - } -} diff --git a/pkg/mail/mail.go b/pkg/mail/mail.go index 6e174a9a..54cbde85 100644 --- a/pkg/mail/mail.go +++ b/pkg/mail/mail.go @@ -100,7 +100,7 @@ type Sample[In Input] struct { const SampleFrom = "noreply@pcom.test" // SampleSite is the site samples format their links for. -var SampleSite = links.Site{} +var SampleSite = links.Site{Root: "https://pcom.test"} // FromPcom is the from address of every pcom mail, at address from. func FromPcom(from string) mail.Address { diff --git a/pkg/mail/post_comment_author.go b/pkg/mail/post_comment_author.go index 92ab0376..9473151f 100644 --- a/pkg/mail/post_comment_author.go +++ b/pkg/mail/post_comment_author.go @@ -88,46 +88,46 @@ type postCommentSample struct { in postComment } -// postCommentSamples are the samples both comment mails share, built from -// in-memory models. name is the sample name of the plain one, the others add -// a suffix; to is the recipient's address and body the comment of the plain and -// the linked-url ones. +// postCommentSamples are the samples both comment mails share, written as +// literal inputs so that no mapper from models stands on both sides of the +// tests. name is the sample name of the plain one, the others add a suffix; +// to is the recipient's address and body the comment of the plain and the +// linked-url ones. func postCommentSamples(name, to, body string) []postCommentSample { - identity := func(in string) (bool, string) { return false, in } - commenter := &model.User{ID: "user-1", Email: "commenter@example.test", Username: "alice"} - recipient := &model.User{ID: "user-2", Email: to, Username: "bob"} - post := func(subject string) *model.Post { - return &model.Post{ID: "post-1", Subject: &subject, Body: "Post body", UserID: "user-3"} - } - comment := func(body string) *model.PostComment { - return &model.PostComment{ID: "comment-1", PostID: "post-1", UserID: commenter.ID, Body: body} - } - editedRecipient := &model.User{ID: "user-2", Email: "recipient@example.test", Username: "bob"} - build := func(p *model.Post, c *model.PostComment, edited bool) postComment { - to := recipient - if edited { - to = editedRecipient - } - - in, err := newPostComment(SampleSite, SampleFrom, identity, commenter, to, p, c, edited) - if err != nil { - panic(err) - } - - return in - } - - withURL := post("Original Post") - withURL.URL = &model.NormalizedURL{ID: "url-1", URL: "https://example.com/article"} - edited := comment("Nice post, edited!") - edited.EditedAt = new(time.Date(2026, 9, 30, 12, 0, 0, 0, time.UTC)) + const ( + commentLink = "https://pcom.test/posts/post-1#commentpost-1comment-1" + subject = "Original Post" + ) return []postCommentSample{ - {name, build(post("Original Post"), comment(body), false)}, - {name + "_edited", build(post("Original Post"), edited, true)}, - {name + "_with_url", build(withURL, comment(body), false)}, - {name + "_markup", build(post("Original Post"), comment("Nice **post**, see [the docs](https://example.com/a?b=1&c=2).\n\n- one\n- \n\n> quoted & \"done\""), false)}, - {name + "_markup_long_subject", build(post("A very long & \"quoted\" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit"), comment("Nice **post**!"), false)}, + {name, postComment{ + From: SampleFrom, To: to, UniqueUserID: "user-2", CommentID: "comment-1", Commenter: "alice", + Subject: subject, URL: "", Link: commentLink, Body: body, BodyHTML: template.HTML("

" + body + "

\n"), + Edited: false, EditedAt: time.Time{}, + }}, + {name + "_edited", postComment{ + From: SampleFrom, To: "recipient@example.test", UniqueUserID: "user-2", CommentID: "comment-1", Commenter: "alice", + Subject: subject, URL: "", Link: commentLink, Body: "Nice post, edited!", BodyHTML: "

Nice post, edited!

\n", + Edited: true, EditedAt: time.Date(2026, 9, 30, 12, 0, 0, 0, time.UTC), + }}, + {name + "_with_url", postComment{ + From: SampleFrom, To: to, UniqueUserID: "user-2", CommentID: "comment-1", Commenter: "alice", + Subject: subject, URL: "https://example.com/article", Link: commentLink, Body: body, BodyHTML: template.HTML("

" + body + "

\n"), + Edited: false, EditedAt: time.Time{}, + }}, + {name + "_markup", postComment{ + From: SampleFrom, To: to, UniqueUserID: "user-2", CommentID: "comment-1", Commenter: "alice", + Subject: subject, URL: "", Link: commentLink, + Body: "Nice **post**, see [the docs](https://example.com/a?b=1&c=2).\n\n- one\n- \n\n> quoted & \"done\"", + BodyHTML: "

Nice post, see the docs.

\n
    \n
  • one
    • \n
  • \n\n
    • \n
\n
\n

quoted & "done"

\n
\n", + Edited: false, EditedAt: time.Time{}, + }}, + {name + "_markup_long_subject", postComment{ + From: SampleFrom, To: to, UniqueUserID: "user-2", CommentID: "comment-1", Commenter: "alice", + Subject: "A very long & \"quoted\" subject that goes on and on to see how a mail client wraps a subject line of this length, past any sensible limit", + URL: "", Link: commentLink, Body: "Nice **post**!", BodyHTML: "

Nice post!

\n", + Edited: false, EditedAt: time.Time{}, + }}, } } diff --git a/pkg/mail/post_prompt_answer.go b/pkg/mail/post_prompt_answer.go index 58db9115..f1683cc2 100644 --- a/pkg/mail/post_prompt_answer.go +++ b/pkg/mail/post_prompt_answer.go @@ -30,7 +30,7 @@ var postPromptAnswerMail = declare("post_prompt_answer", "post_prompt_answer", p func postPromptAnswerSamples() []Sample[PostPromptAnswerInput] { return []Sample[PostPromptAnswerInput]{ {Name: "post_prompt_answer", Input: PostPromptAnswerInput{ - From: SampleFrom, PostID: "post-1", To: "asker@example.test", Responder: "bob", + From: SampleFrom, PostID: "post-2", To: "asker@example.test", Responder: "bob", Message: "What is your favorite hobby?", Subject: "My Answer", Link: SampleSite.Abs("post", "post-1"), }}, } diff --git a/pkg/mail/posts_test.go b/pkg/mail/posts_test.go index e0c81e7d..e7e5c3dd 100644 --- a/pkg/mail/posts_test.go +++ b/pkg/mail/posts_test.go @@ -44,7 +44,7 @@ func TestNewPost_MatchesSamples(t *testing.T) { t.Run(tt.sample, func(t *testing.T) { t.Parallel() - got := mail.NewPost(links.Site{}, testFrom, keepMedia, user, connection, tt.post) + got := mail.NewPost(mail.SampleSite, testFrom, keepMedia, user, connection, tt.post) require.Equal(t, sampleEnvelope(t, tt.sample), got) }) } @@ -88,7 +88,7 @@ func TestPostPrompt_MatchesSample(t *testing.T) { Message: "What is your favorite hobby?", } - got := mail.PostPrompt(links.Site{}, testFrom, asker, recipient, prompt) + got := mail.PostPrompt(mail.SampleSite, testFrom, asker, recipient, prompt) require.Equal(t, sampleEnvelope(t, "post_prompt"), got) } @@ -124,7 +124,7 @@ func TestPostPromptAnswer_MatchesSample(t *testing.T) { asker := &model.User{ID: "user-1", Email: "asker@example.test", Username: "alice"} responder := &model.User{ID: "user-2", Email: "responder@example.test", Username: "bob"} post := &model.Post{ - ID: "post-1", + ID: "post-2", Subject: new("My Answer"), Body: "Here is my answer to the prompt", UserID: responder.ID, @@ -134,9 +134,9 @@ func TestPostPromptAnswer_MatchesSample(t *testing.T) { AskerID: asker.ID, RecipientID: responder.ID, Message: "What is your favorite hobby?", - PostID: new(post.ID), + PostID: new("post-1"), } - got := mail.PostPromptAnswer(links.Site{}, testFrom, asker, responder, post, prompt) + got := mail.PostPromptAnswer(mail.SampleSite, testFrom, asker, responder, post, prompt) require.Equal(t, sampleEnvelope(t, "post_prompt_answer"), got) } diff --git a/pkg/mail/send_internal_test.go b/pkg/mail/send_internal_test.go index dec59475..4a87be0e 100644 --- a/pkg/mail/send_internal_test.go +++ b/pkg/mail/send_internal_test.go @@ -3,7 +3,6 @@ package mail import ( "testing" - "github.com/can3p/pcom/pkg/links" "github.com/can3p/pcom/pkg/model" "github.com/stretchr/testify/require" ) @@ -14,7 +13,7 @@ func TestConfirmWaitingList_MatchesSample(t *testing.T) { waitingList := &model.UserSignupRequest{ID: "request-1", Email: "newuser@example.test"} want := confirmWaitingListMail.MustRender(confirmWaitingListSamples()[0].Input) - require.Equal(t, want, ConfirmWaitingList(links.Site{}, SampleFrom, waitingList)) + require.Equal(t, want, ConfirmWaitingList(SampleSite, SampleFrom, waitingList)) } func TestInvitation_MatchesSample(t *testing.T) { @@ -23,7 +22,7 @@ func TestInvitation_MatchesSample(t *testing.T) { invite := &model.UserInvitation{ID: "invite-1"} want := inviteMail.MustRender(inviteSamples()[0].Input) - require.Equal(t, want, Invitation(links.Site{}, SampleFrom, invite, "newuser@example.test")) + require.Equal(t, want, Invitation(SampleSite, SampleFrom, invite, "newuser@example.test")) } func TestValidateFormat_InvalidFormat(t *testing.T) { diff --git a/pkg/mail/testdata/admin_new_user.golden b/pkg/mail/testdata/admin_new_user.golden index de355e9f..680324ca 100644 --- a/pkg/mail/testdata/admin_new_user.golden +++ b/pkg/mail/testdata/admin_new_user.golden @@ -10,7 +10,7 @@ Hi! New user alert: * ID: 0190a3b4-0000-7000-8000-000000000001 -* Blog: /users/alice +* Blog: https://pcom.test/users/alice * Email: alice@example.test --- HTML --- @@ -20,6 +20,6 @@ New user alert: diff --git a/pkg/mail/testdata/new_post.golden b/pkg/mail/testdata/new_post.golden index cfb72b2e..affbc04a 100644 --- a/pkg/mail/testdata/new_post.golden +++ b/pkg/mail/testdata/new_post.golden @@ -9,7 +9,7 @@ Hi! @alice has published a new post "Test Post" -Head to the post to leave a comment! /posts/post-1 +Head to the post to leave a comment! https://pcom.test/posts/post-1 --- HTML --- @@ -20,4 +20,4 @@ Head to the post to leave a comment! /posts/post-1

This is a test post body

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/new_post_empty_body.golden b/pkg/mail/testdata/new_post_empty_body.golden index a9bfce79..5ee4e56a 100644 --- a/pkg/mail/testdata/new_post_empty_body.golden +++ b/pkg/mail/testdata/new_post_empty_body.golden @@ -9,7 +9,7 @@ Hi! @alice has published a new post "Test Post" -Head to the post to leave a comment! /posts/post-1 +Head to the post to leave a comment! https://pcom.test/posts/post-1 --- HTML --- @@ -19,4 +19,4 @@ Head to the post to leave a comment! /posts/post-1
-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/new_post_long_subject.golden b/pkg/mail/testdata/new_post_long_subject.golden index d182941e..70a4259f 100644 --- a/pkg/mail/testdata/new_post_long_subject.golden +++ b/pkg/mail/testdata/new_post_long_subject.golden @@ -9,7 +9,7 @@ Hi! @alice has published a new post "A very long subject that goes on and on and on to see how the mail copes with a title that no one would reasonably write but someone surely will" -Head to the post to leave a comment! /posts/post-1 +Head to the post to leave a comment! https://pcom.test/posts/post-1 --- HTML --- @@ -20,4 +20,4 @@ Head to the post to leave a comment! /posts/post-1

This is a test post body

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/new_post_markup_in_body.golden b/pkg/mail/testdata/new_post_markup_in_body.golden index 4d49f68c..819e484a 100644 --- a/pkg/mail/testdata/new_post_markup_in_body.golden +++ b/pkg/mail/testdata/new_post_markup_in_body.golden @@ -9,7 +9,7 @@ Hi! @alice has published a new post "Test Post" -Head to the post to leave a comment! /posts/post-1 +Head to the post to leave a comment! https://pcom.test/posts/post-1 --- HTML --- @@ -20,4 +20,4 @@ Head to the post to leave a comment! /posts/post-1

Hello alert(1) and a link

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/new_post_no_subject.golden b/pkg/mail/testdata/new_post_no_subject.golden index 50b3f50f..06b5c569 100644 --- a/pkg/mail/testdata/new_post_no_subject.golden +++ b/pkg/mail/testdata/new_post_no_subject.golden @@ -9,7 +9,7 @@ Hi! @alice has published a new post "No Subject" -Head to the post to leave a comment! /posts/post-1 +Head to the post to leave a comment! https://pcom.test/posts/post-1 --- HTML --- @@ -20,4 +20,4 @@ Head to the post to leave a comment! /posts/post-1

This is a test post body

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/new_post_special_chars.golden b/pkg/mail/testdata/new_post_special_chars.golden index d2cf5291..372577e2 100644 --- a/pkg/mail/testdata/new_post_special_chars.golden +++ b/pkg/mail/testdata/new_post_special_chars.golden @@ -9,7 +9,7 @@ Hi! @alice has published a new post "Test bold and "quotes"" -Head to the post to leave a comment! /posts/post-1 +Head to the post to leave a comment! https://pcom.test/posts/post-1 --- HTML --- @@ -20,4 +20,4 @@ Head to the post to leave a comment! /posts/post-1

This is a test post body

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/new_post_with_url.golden b/pkg/mail/testdata/new_post_with_url.golden index c1b577db..3a694afe 100644 --- a/pkg/mail/testdata/new_post_with_url.golden +++ b/pkg/mail/testdata/new_post_with_url.golden @@ -10,7 +10,7 @@ Hi! @alice has published a new post "Check out this article" Linked URL: https://example.com/article -Head to the post to leave a comment! /posts/post-1 +Head to the post to leave a comment! https://pcom.test/posts/post-1 --- HTML --- @@ -21,4 +21,4 @@ Head to the post to leave a comment! /posts/post-1

This is a test post body

Linked URL: https://example.com/article

-

Head to the post to leave a comment! /posts/post-1

+

Head to the post to leave a comment! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/post_comment_author.golden b/pkg/mail/testdata/post_comment_author.golden index d3667add..ae6ab0ba 100644 --- a/pkg/mail/testdata/post_comment_author.golden +++ b/pkg/mail/testdata/post_comment_author.golden @@ -11,7 +11,7 @@ Hi! > Nice post! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -22,4 +22,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Nice post!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_author_edited.golden b/pkg/mail/testdata/post_comment_author_edited.golden index cc6575ef..dce7c880 100644 --- a/pkg/mail/testdata/post_comment_author_edited.golden +++ b/pkg/mail/testdata/post_comment_author_edited.golden @@ -11,7 +11,7 @@ Hi! > Nice post, edited! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -22,4 +22,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Nice post, edited!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_author_markup.golden b/pkg/mail/testdata/post_comment_author_markup.golden index b77413a7..285a522a 100644 --- a/pkg/mail/testdata/post_comment_author_markup.golden +++ b/pkg/mail/testdata/post_comment_author_markup.golden @@ -16,7 +16,7 @@ Hi! > > > quoted & "done" -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -36,4 +36,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1
-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_author_markup_long_subject.golden b/pkg/mail/testdata/post_comment_author_markup_long_subject.golden index 5442db63..d8aa9852 100644 --- a/pkg/mail/testdata/post_comment_author_markup_long_subject.golden +++ b/pkg/mail/testdata/post_comment_author_markup_long_subject.golden @@ -11,7 +11,7 @@ Hi! > Nice **post**! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -22,4 +22,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Nice post!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_author_with_url.golden b/pkg/mail/testdata/post_comment_author_with_url.golden index 0d6bdb97..e2bf411a 100644 --- a/pkg/mail/testdata/post_comment_author_with_url.golden +++ b/pkg/mail/testdata/post_comment_author_with_url.golden @@ -12,7 +12,7 @@ Linked URL: https://example.com/article > Nice post! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -23,4 +23,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Nice post!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_participants.golden b/pkg/mail/testdata/post_comment_participants.golden index 19501998..67365b3d 100644 --- a/pkg/mail/testdata/post_comment_participants.golden +++ b/pkg/mail/testdata/post_comment_participants.golden @@ -11,7 +11,7 @@ Hi! > Great comment! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -22,4 +22,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Great comment!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_participants_edited.golden b/pkg/mail/testdata/post_comment_participants_edited.golden index 6c02925e..fc2d9084 100644 --- a/pkg/mail/testdata/post_comment_participants_edited.golden +++ b/pkg/mail/testdata/post_comment_participants_edited.golden @@ -11,7 +11,7 @@ Hi! > Nice post, edited! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -22,4 +22,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Nice post, edited!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_participants_markup.golden b/pkg/mail/testdata/post_comment_participants_markup.golden index 7b53852e..7055544e 100644 --- a/pkg/mail/testdata/post_comment_participants_markup.golden +++ b/pkg/mail/testdata/post_comment_participants_markup.golden @@ -16,7 +16,7 @@ Hi! > > > quoted & "done" -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -36,4 +36,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1
-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_participants_markup_long_subject.golden b/pkg/mail/testdata/post_comment_participants_markup_long_subject.golden index 960dd03d..908142c3 100644 --- a/pkg/mail/testdata/post_comment_participants_markup_long_subject.golden +++ b/pkg/mail/testdata/post_comment_participants_markup_long_subject.golden @@ -11,7 +11,7 @@ Hi! > Nice **post**! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -22,4 +22,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Nice post!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_comment_participants_with_url.golden b/pkg/mail/testdata/post_comment_participants_with_url.golden index cd6c9603..d373daca 100644 --- a/pkg/mail/testdata/post_comment_participants_with_url.golden +++ b/pkg/mail/testdata/post_comment_participants_with_url.golden @@ -12,7 +12,7 @@ Linked URL: https://example.com/article > Great comment! -Checkout the comment in the post: /posts/post-1#commentpost-1comment-1 +Checkout the comment in the post: https://pcom.test/posts/post-1#commentpost-1comment-1 --- HTML --- @@ -23,4 +23,4 @@ Checkout the comment in the post: /posts/post-1#commentpost-1comment-1

Great comment!

-

Checkout the comment in the post.

+

Checkout the comment in the post.

diff --git a/pkg/mail/testdata/post_prompt.golden b/pkg/mail/testdata/post_prompt.golden index c94b8c84..d58607b0 100644 --- a/pkg/mail/testdata/post_prompt.golden +++ b/pkg/mail/testdata/post_prompt.golden @@ -9,7 +9,7 @@ Hi! @alice has asked you to write a post on "What is your favorite hobby?" -Head to new post page to give an update! /write?prompt=prompt-1 +Head to new post page to give an update! https://pcom.test/write?prompt=prompt-1 --- HTML --- @@ -17,4 +17,4 @@ Head to new post page to give an update! /write?prompt=prompt-1

@alice has asked you to write a post on "What is your favorite hobby?"

-

Head to new post page to give an update! /write?prompt=prompt-1

+

Head to new post page to give an update! https://pcom.test/write?prompt=prompt-1

diff --git a/pkg/mail/testdata/post_prompt_answer.golden b/pkg/mail/testdata/post_prompt_answer.golden index e1cf95be..fb53f54d 100644 --- a/pkg/mail/testdata/post_prompt_answer.golden +++ b/pkg/mail/testdata/post_prompt_answer.golden @@ -1,5 +1,5 @@ EmailType: post_prompt_answer -UniqueID: post-1 +UniqueID: post-2 From: Your pcom To: asker@example.test Subject: Response to your prompt from bob @@ -9,7 +9,7 @@ Hi! @bob has responded on your prompt "What is your favorite hobby?" with the post "My Answer" -Check out their post! /posts/post-1 +Check out their post! https://pcom.test/posts/post-1 --- HTML --- @@ -17,4 +17,4 @@ Check out their post! /posts/post-1

@bob has responded on your prompt "What is your favorite hobby?" with the post "My Answer"

-

Head to new post page to give an update! /posts/post-1

+

Head to new post page to give an update! https://pcom.test/posts/post-1

diff --git a/pkg/mail/testdata/post_prompt_markup.golden b/pkg/mail/testdata/post_prompt_markup.golden index e230b373..c94ba66b 100644 --- a/pkg/mail/testdata/post_prompt_markup.golden +++ b/pkg/mail/testdata/post_prompt_markup.golden @@ -9,7 +9,7 @@ Hi! @alice has asked you to write a post on "What is bold & "quoted" ?" -Head to new post page to give an update! /write?prompt=prompt-1 +Head to new post page to give an update! https://pcom.test/write?prompt=prompt-1 --- HTML --- @@ -17,4 +17,4 @@ Head to new post page to give an update! /write?prompt=prompt-1

@alice has asked you to write a post on "What is <b>bold</b> & "quoted" <script>alert(1)</script>?"

-

Head to new post page to give an update! /write?prompt=prompt-1

+

Head to new post page to give an update! https://pcom.test/write?prompt=prompt-1

diff --git a/pkg/mail/testdata/send_actual_confirm_waiting_list.golden b/pkg/mail/testdata/send_actual_confirm_waiting_list.golden index 0a2bd9e1..f5feac4c 100644 --- a/pkg/mail/testdata/send_actual_confirm_waiting_list.golden +++ b/pkg/mail/testdata/send_actual_confirm_waiting_list.golden @@ -10,7 +10,7 @@ Hi! Thank you for your interest pcom! Please follow the link to confirm your email address -/confirm_waiting_list/request-1 +https://pcom.test/confirm_waiting_list/request-1 --- HTML --- @@ -18,4 +18,4 @@ Thank you for your interest pcom! Please follow the link to confirm your email a

Thank you for your interest pcom! Please follow the link to confirm your email address

-/confirm_waiting_list/request-1 +https://pcom.test/confirm_waiting_list/request-1 diff --git a/pkg/mail/testdata/send_actual_invitation.golden b/pkg/mail/testdata/send_actual_invitation.golden index 46f6c5b7..4ab73fe6 100644 --- a/pkg/mail/testdata/send_actual_invitation.golden +++ b/pkg/mail/testdata/send_actual_invitation.golden @@ -10,7 +10,7 @@ Hi! Welcome to pcom! Please follow the link to choose a username. You will log in with a code we mail you, so there is no password to set. -/invite/invite-1 +https://pcom.test/invite/invite-1 --- HTML --- @@ -18,4 +18,4 @@ Welcome to pcom! Please follow the link to choose a username. You will log in wi

Welcome to pcom! Please follow the link to choose a username. You will log in with a code we mail you, so there is no password to set.

-/invite/invite-1 +https://pcom.test/invite/invite-1 diff --git a/pkg/service/posts/comments_edit_db_test.go b/pkg/service/posts/comments_edit_db_test.go index c1d25a0c..7c3dbb82 100644 --- a/pkg/service/posts/comments_edit_db_test.go +++ b/pkg/service/posts/comments_edit_db_test.go @@ -156,3 +156,28 @@ func TestEditComment_LostConnection(t *testing.T) { require.Nil(t, got.EditedAt) require.Empty(t, out.Sent()) } + +// A mail that cannot be queued fails the change it reports, and the +// transaction rolls the change back. +func TestSaveAndComment_SenderFailureRollsBack(t *testing.T) { + t.Parallel() + + db := testdb.New(t).DB + ctx := context.Background() + w := newWorld(t, ctx, db) + out := fakesender.New() + s := posts.New(repo.New(db), out, nil) + sendErr := errors.New("queue is gone") + out.FailWith(sendErr) + + _, err := s.Save(ctx, w.author, posts.SaveInput{ + Subject: "published", Body: "a body for the post", Visibility: model.PostVisibilityPublic, Action: posts.ActionPublish, + }) + require.ErrorIs(t, err, sendErr) + require.Empty(t, testutil.Must(factory.ListPosts(ctx, db, w.author.ID))(t)) + + post := testutil.Must(factory.Post(ctx, db, w.author.ID, factory.Published()))(t) + require.ErrorIs(t, s.AddComment(ctx, w.direct, posts.CommentInput{PostID: post.ID, Body: "a fine comment"}), sendErr) + require.Empty(t, testutil.Must(factory.ListComments(ctx, db, post.ID))(t)) + require.Zero(t, commentsNumber(t, ctx, db, post.ID)) +} From ac8368c77ce6a24e85a6328481708166a2b8e9b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 23:30:47 +0000 Subject: [PATCH 13/16] docs(r4): close the wave in the plan Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UmD6gdC2ENrRcg4XZVZuxd --- docs/implementation-plan.md | 1 - docs/plan/r4.md | 180 ------------------------------------ docs/plan/r7.md | 2 +- docs/plan/r8.md | 5 +- 4 files changed, 4 insertions(+), 184 deletions(-) delete mode 100644 docs/plan/r4.md diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index 85c492e7..e745dc7e 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -10,7 +10,6 @@ support read directly. Rules for reading and verifying code cheaply are in `AGEN Where the modernization is going: -- declared, golden-tested mailers (R4); - one job queue for mail and feeds (R8); - structured logging through one [zap](https://github.com/uber-go/zap) logger, passed explicitly (R7); - no unmaintained or duplicate dependencies (R6); diff --git a/docs/plan/r4.md b/docs/plan/r4.md deleted file mode 100644 index e970ba32..00000000 --- a/docs/plan/r4.md +++ /dev/null @@ -1,180 +0,0 @@ -## R4 — Mailers - -Status: running · Depends on: — · Branch: `claude/zealous-shannon-ieo14d` - -Every email becomes a declared definition: - -- a typed input; -- a sample factory that touches no database, including the awkward variants - (no subject, long subject, markup in user content, a linked URL); -- templates: the subject and the text through `text/template`, the HTML through `html/template`; -- a golden file per sample holding the subject, the text and the HTML; -- a development preview page that lists every email rendered from its samples. - -Call sites stop building mail bodies with `fmt.Sprintf`, and escaping user content becomes `html/template`'s -job instead of each mail's. All 14 mails are in scope: the 9 in `pkg/mail` and the 5 admin notifications in -`pkg/admin` (new user, new waiting list member, signup confirmed, throwaway email signup, page failure), which -move into `pkg/mail`. - -Decided by the owner: - -- **Whitespace may change.** The goldens in `pkg/mail/testdata` define "unchanged output" up to whitespace: - a template may drop the stray tab indentation of today's bodies. `git diff -w` on an existing golden is - empty, or every remaining hunk is listed in the task's report with its cause (e.g. `html/template` - normalizing a URL in an `href`), and the coordinator accepts it. -- **The preview lives behind a flag**, `--dev-routes` / `DEV_ROUTES`, off by default and turned on only in - the local config (`docker-compose.yml`'s app service and `.env.example`). With it off the routes are not - mounted at all. -- **The job queue is not in R4.** Mail already enters `outgoing_emails` in the same transaction as the change - that causes it; a general queue mostly pays off for the feed poller and is its own wave, R8. R4 only fixes - the sender (M6). - -### Constraints for every task - -- `pkg/mail` exports one envelope type, `mail.Envelope` (M0 folds `Outgoing` into it). -- A mail's public constructor keeps its signature, so its service callers don't change. Only M0 (the - envelope type) and M4 (the admin mails' new package) edit callers. -- A sample is named after the golden it reproduces: `testdata/.golden`, so an existing golden is - rewritten in place and its diff reviewable. New samples add new goldens. -- The golden format is the current `mailsToGolden` one (EmailType, UniqueID, From, To, Subject, TEXT, HTML). -- A per-mail Go test whose cases are now samples is deleted; the `_NotToMyself` tests (no mail for yourself) - stay. The coordinator `git rm`s test files left empty and the admin goldens M4 replaces. -- `e2e/` is not edited, except the route-table row M5 adds. - -| Task | Tier | Owns | Done when | -|---|---|---|---| -| S0 | coordinator | `pkg/mail/*_test.go` split, `pkg/config/config.go`, `pkg/config/config_test.go`, `docker-compose.yml`, `.env.example` | the mail tests are split by area and the two settings exist; `make test-q PKG="./pkg/mail/... ./pkg/config/..."` passes | -| M0 | strong | see the section | the generic golden test renders LoginCode and ConfirmSignup from their samples with whitespace-only golden diffs, and `make check-q` passes | -| M1 | cheap | see the section | ConfirmWaitingList and Invitation render from templates; their goldens have whitespace-only diffs | -| M2 | mid | see the section | NewPost, PostPrompt and PostPromptAnswer render from templates; goldens whitespace-only | -| M3 | mid | see the section | PostCommentAuthor and PostCommentParticipants render from templates; goldens whitespace-only | -| M4 | mid | see the section | the 5 admin mails are declared in `pkg/mail` with goldens; `pkg/admin` keeps only the panic formatting | -| M5 | mid | see the section | `/dev/mail` lists every sample of every mail with the flag on, and 404s with it off | -| M6 | mid | see the section | each queued mail is sent in its own transaction and the retry schedule is a setting | - -Order: S0, then M0 alone (it defines the contract), then M1–M6 in parallel. M5 starts with only M0's mails -registered; the registry fills as M1–M4 land. - -### S0 Split the tests and add the settings - -The coordinator does this by script before M0. - -- Split `pkg/mail/mail_test.go` by verbatim moves: the helpers, `TestConfirmSignup` and `TestLoginCode` stay; - the `TestNewPost*`, `TestPostPrompt*` tests go to `posts_test.go`; the `TestPostComment*` tests to - `comments_test.go`. Prove each move by diffing line multisets. -- `config.Web.DevRoutes` (`Switch`, `--dev-routes`, `DEV_ROUTES`, default false, "Serve the /dev pages - (mail previews); local development only") and `config.Mail.RetryIntervals` (`[]time.Duration`, - `--email-retry-intervals`, `EMAIL_RETRY_INTERVALS`, comma-separated, default `10s,60s,30m`, "Waits between - attempts to send a mail; a mail fails after one attempt more than there are waits"), with rows in - `config_test.go`. -- `DEV_ROUTES: "true"` in `docker-compose.yml`'s app environment and `DEV_ROUTES=true` in `.env.example`. - -### M0 The mail contract - -Owns `pkg/mail/mail.go` (new), `pkg/mail/templates/` (new), `pkg/mail/confirm_signup.go`, -`pkg/mail/login_code.go`, `pkg/mail/new_post.go` (only to remove `Outgoing`), `pkg/mail/mail_test.go`, -`pkg/mail/golden_test.go` (new), `pkg/mail/testdata/confirm_signup.golden`, -`pkg/mail/testdata/login_code.golden`, `pkg/service/posts/posts.go`, and the `Outgoing` → `Envelope` -renames in `pkg/mail/*_test.go` and `pkg/service/posts/*.go`. - -Define in `pkg/mail/mail.go`: - -- `Envelope` (unique id, email type, `*sender.Mail`), replacing `Outgoing` everywhere; `posts.Service.queue` - and `queueE` take `*mail.Envelope`. -- A declaration of one mail: its email type (the queue's `email_type`), its template name, a typed input, - and a `Samples` function returning named inputs built from in-memory `model` structs, no database. The - shape is yours (a generic `Definition[In]` with a `Render(In) (*Envelope, error)` is the expected one); - the from address, the recipient and the unique id are part of the input or computed from it. -- Templates embedded with `embed.FS` from `pkg/mail/templates/.subject.txt`, `.txt` (both - `text/template`) and `.html` (`html/template`), parsed once at init; a missing or broken template - panics at init, not at send time. -- A registry, `mail.All()`, listing every declared mail with its rendered samples, in a stable order. The - preview page (M5) and the golden test use it; it is the only list of mails. -- `golden_test.go`: one test, `TestMails_Goldens`, that renders every sample of every registered mail into - `testdata/.golden` with the `mailsToGolden` format (move the helper here), plus a test that every - registered mail has at least one sample and every sample name is unique. - -Convert LoginCode and ConfirmSignup as the reference conversions: their constructors keep their signatures -and build the input, then render. Their samples are `login_code` and `confirm_signup`; delete -`TestLoginCode` and `TestConfirmSignup` once the goldens reproduce them. - -Write the conventions the other tasks follow (where a mail's declaration, samples and templates live, how a -`template.HTML` markdown body enters an input) as the package doc comment of `mail.go`; M1–M4 read it. - -### M1 Account mails - -Owns `pkg/mail/confirm_waiting_list.go`, `pkg/mail/invite.go`, their templates in `pkg/mail/templates/`, -`pkg/mail/send_internal_test.go`, `pkg/mail/testdata/send_actual_confirm_waiting_list.golden`, -`pkg/mail/testdata/send_actual_invitation.golden`. - -Follow the conventions in `pkg/mail/mail.go`'s doc comment. Samples: the existing two, keeping their names. -Delete `TestSendActualConfirmWaitingList` and `TestSendActualInvitation` once the goldens reproduce them; the -`ValidateFormat` tests stay. - -### M2 Post mails - -Owns `pkg/mail/new_post.go`, `pkg/mail/post_prompt.go`, `pkg/mail/post_prompt_answer.go`, their templates, -`pkg/mail/posts_test.go`, and the goldens `new_post*.golden`, `post_prompt.golden`, -`post_prompt_answer.golden` in `pkg/mail/testdata`. - -Samples reproduce every existing golden under its name (`new_post`, `new_post_no_subject`, -`new_post_with_url`, `new_post_special_chars`, `new_post_empty_body`, `post_prompt`, `post_prompt_answer`) -and add `new_post_long_subject`, `new_post_markup_in_body` (a raw `