From 3dde9f79f4d05165083d0ee0c85a053e367429f3 Mon Sep 17 00:00:00 2001 From: codegirl-007 Date: Thu, 27 Aug 2026 08:57:16 -0700 Subject: [PATCH 1/2] Generalize post reply notifications. Notify direct parent authors throughout threaded conversations while skipping self-replies, edits, and recipients without email. --- internal/mail/mail.go | 86 +++++++++++++---------- internal/mail/mail_test.go | 38 +++++----- internal/mail/recording.go | 8 +-- internal/web/posts.go | 58 ++++++++++++++- internal/web/posts_test.go | 136 ++++++++++++++++++++++++++++++++++++ internal/web/server.go | 1 + internal/web/server_test.go | 11 ++- 7 files changed, 279 insertions(+), 59 deletions(-) diff --git a/internal/mail/mail.go b/internal/mail/mail.go index 86084d6..6e0d789 100644 --- a/internal/mail/mail.go +++ b/internal/mail/mail.go @@ -5,6 +5,7 @@ import ( _ "embed" "fmt" "html" + "net/url" "os" "strings" @@ -14,24 +15,26 @@ import ( //go:embed mark.png var markPNG []byte -// QuestionAnswered is the payload for notifying a question author of a reply. -type QuestionAnswered struct { - ToEmail string - ToName string - QuestionID string - QuestionTitle string - AnswerBody string +// PostReply is the payload for notifying a post author of a direct reply. +type PostReply struct { + ToEmail string + ToName string + RootID string + RootTitle string + ReplyID string + ReplyBody string + ReplyAuthorName string } -// Notifier sends transactional email about answered questions. +// Notifier sends transactional email about post replies. type Notifier interface { - NotifyQuestionAnswered(ctx context.Context, msg QuestionAnswered) error + NotifyPostReply(ctx context.Context, msg PostReply) error } // Nop is a no-op Notifier used when Resend is not configured. type Nop struct{} -func (Nop) NotifyQuestionAnswered(context.Context, QuestionAnswered) error { return nil } +func (Nop) NotifyPostReply(context.Context, PostReply) error { return nil } // Resend sends via the Resend HTTP API. type Resend struct { @@ -62,7 +65,7 @@ func FromEnv() (Notifier, error) { }, nil } -func (r *Resend) NotifyQuestionAnswered(ctx context.Context, msg QuestionAnswered) error { +func (r *Resend) NotifyPostReply(ctx context.Context, msg PostReply) error { if r == nil || r.client == nil { return nil } @@ -70,59 +73,72 @@ func (r *Resend) NotifyQuestionAnswered(ctx context.Context, msg QuestionAnswere if to == "" { return nil } - text, htmlBody := questionAnsweredContent(r.baseURL, msg) + text, htmlBody := postReplyContent(r.baseURL, msg) params := &resend.SendEmailRequest{ From: r.from, To: []string{to}, - Subject: "Your question was answered", + Subject: "New reply to your post", Text: text, Html: htmlBody, Attachments: []*resend.Attachment{{ Content: markPNG, Filename: "ask-a-plumber-first.png", ContentType: "image/png", - ContentId: "answer-notification-mark", + ContentId: "reply-notification-mark", }}, } opts := &resend.SendEmailOptions{ - IdempotencyKey: "answer-notify:" + msg.QuestionID, + IdempotencyKey: "post-reply:" + msg.ReplyID, } _, err := r.client.Emails.SendWithOptions(ctx, params, opts) return err } -func questionAnsweredContent(baseURL string, msg QuestionAnswered) (string, string) { - link := strings.TrimRight(baseURL, "/") + "/questions/" + msg.QuestionID - title := strings.TrimSpace(msg.QuestionTitle) - if title == "" { - title = "your question" +func postReplyContent(baseURL string, msg PostReply) (string, string) { + link := strings.TrimRight(baseURL, "/") + + "/questions/" + url.PathEscape(msg.RootID) + + "#post-" + url.PathEscape(msg.ReplyID) + title := replyRootTitle(msg.RootTitle) + author := strings.TrimSpace(msg.ReplyAuthorName) + if author == "" { + author = "Someone" } text := fmt.Sprintf( - "Hi%s,\n\nYour question %q has an answer from a plumber:\n\n%s\n\nView it here:\n%s\n", + "Hi%s,\n\n%s replied in %q:\n\n%s\n\nView the reply:\n%s\n", greetingName(msg.ToName), + author, title, - msg.AnswerBody, + msg.ReplyBody, link, ) htmlBody := strings.NewReplacer( - "{{PREHEADER}}", html.EscapeString("A plumber answered "+title+"."), + "{{PREHEADER}}", html.EscapeString(author+" replied in "+title+"."), "{{GREETING}}", html.EscapeString(greetingName(msg.ToName)), "{{TITLE}}", html.EscapeString(title), - "{{ANSWER}}", html.EscapeString(msg.AnswerBody), + "{{AUTHOR}}", html.EscapeString(author), + "{{REPLY}}", html.EscapeString(msg.ReplyBody), "{{LINK}}", html.EscapeString(link), - "{{MARK}}", "cid:answer-notification-mark", - ).Replace(questionAnsweredHTML) + "{{MARK}}", "cid:reply-notification-mark", + ).Replace(postReplyHTML) return text, htmlBody } -const questionAnsweredHTML = ` +func replyRootTitle(title string) string { + title = strings.TrimSpace(title) + if title == "" { + return "your conversation" + } + return title +} + +const postReplyHTML = ` - Your question was answered + New reply to your conversation
{{PREHEADER}}
@@ -147,22 +163,22 @@ const questionAnsweredHTML = ` -
Shop response
-

Your question has an answer.

-

Hi{{GREETING}}, a plumber replied to:

+
New reply
+

The conversation has a new reply.

+

Hi{{GREETING}}, {{AUTHOR}} replied in:

“{{TITLE}}”

-
The answer
-
{{ANSWER}}
+
The reply
+
{{REPLY}}
- View the answer → + View the reply →
@@ -170,7 +186,7 @@ const questionAnsweredHTML = ` - You received this because you asked a question on Ask a Plumber First. + You received this because someone replied to your post on Ask a Plumber First. diff --git a/internal/mail/mail_test.go b/internal/mail/mail_test.go index 624505c..3b86d32 100644 --- a/internal/mail/mail_test.go +++ b/internal/mail/mail_test.go @@ -14,24 +14,27 @@ func TestEmbeddedMarkIsPNG(t *testing.T) { } } -func TestQuestionAnsweredContent(t *testing.T) { +func TestPostReplyContent(t *testing.T) { t.Parallel() - text, htmlBody := questionAnsweredContent("https://plumber.example/", QuestionAnswered{ - ToName: ``, - QuestionID: "question-123", - QuestionTitle: `Leaky sink`, - AnswerBody: "Replace the cartridge.\nThen test the handle. ", + text, htmlBody := postReplyContent("https://plumber.example/", PostReply{ + ToName: ``, + RootID: "question-123", + RootTitle: `Leaky sink`, + ReplyID: "reply-456", + ReplyBody: "Replace the cartridge.\nThen test the handle. ", + ReplyAuthorName: ``, }) for _, want := range []string{ "Ask a Plumber First", - "Shop response", - "cid:answer-notification-mark", - "https://plumber.example/questions/question-123", + "New reply", + "cid:reply-notification-mark", + "https://plumber.example/questions/question-123#post-reply-456", "white-space:pre-wrap", "<Sam & Pat>", "<b>Leaky sink</b>", + "<Jo & Co>", "<script>alert('x')</script>", } { if !strings.Contains(htmlBody, want) { @@ -41,6 +44,7 @@ func TestQuestionAnsweredContent(t *testing.T) { for _, unsafe := range []string{ "", "Leaky sink", + "", "", } { if strings.Contains(htmlBody, unsafe) { @@ -52,9 +56,9 @@ func TestQuestionAnsweredContent(t *testing.T) { } for _, want := range []string{ `Hi ,`, - `Your question "Leaky sink"`, + ` replied in "Leaky sink"`, "Replace the cartridge.\nThen test the handle.", - "https://plumber.example/questions/question-123", + "https://plumber.example/questions/question-123#post-reply-456", } { if !strings.Contains(text, want) { t.Errorf("text missing %q", want) @@ -62,15 +66,15 @@ func TestQuestionAnsweredContent(t *testing.T) { } } -func TestQuestionAnsweredContentUsesFallbackTitle(t *testing.T) { +func TestPostReplyContentUsesFallbacks(t *testing.T) { t.Parallel() - text, htmlBody := questionAnsweredContent("https://plumber.example", QuestionAnswered{}) - if !strings.Contains(text, `"your question"`) { + text, htmlBody := postReplyContent("https://plumber.example", PostReply{}) + if !strings.Contains(text, `Someone replied in "your conversation"`) { t.Errorf("text missing fallback title") } - if !strings.Contains(htmlBody, "a plumber replied to:

") || - !strings.Contains(htmlBody, "“your question”") { - t.Errorf("HTML missing fallback title") + if !strings.Contains(htmlBody, "Someone replied in:

") || + !strings.Contains(htmlBody, "“your conversation”") { + t.Errorf("HTML missing fallbacks") } } diff --git a/internal/mail/recording.go b/internal/mail/recording.go index 188eac8..f9da819 100644 --- a/internal/mail/recording.go +++ b/internal/mail/recording.go @@ -8,10 +8,10 @@ import ( // Recording is a test Notifier that records calls. type Recording struct { mu sync.Mutex - Msgs []QuestionAnswered + Msgs []PostReply } -func (r *Recording) NotifyQuestionAnswered(_ context.Context, msg QuestionAnswered) error { +func (r *Recording) NotifyPostReply(_ context.Context, msg PostReply) error { r.mu.Lock() defer r.mu.Unlock() r.Msgs = append(r.Msgs, msg) @@ -25,10 +25,10 @@ func (r *Recording) Len() int { } // Snapshot returns a copy of recorded messages. -func (r *Recording) Snapshot() []QuestionAnswered { +func (r *Recording) Snapshot() []PostReply { r.mu.Lock() defer r.mu.Unlock() - out := make([]QuestionAnswered, len(r.Msgs)) + out := make([]PostReply, len(r.Msgs)) copy(out, r.Msgs) return out } diff --git a/internal/web/posts.go b/internal/web/posts.go index cecefa3..b448853 100644 --- a/internal/web/posts.go +++ b/internal/web/posts.go @@ -5,12 +5,15 @@ import ( "database/sql" "errors" "fmt" + "log" "net/http" "net/url" "strings" + "time" "github.com/go-chi/chi/v5" + "plumber/internal/mail" "plumber/internal/store" ) @@ -37,7 +40,7 @@ func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { AuthorID: user.ID, Body: truncateRunes(body, 12000), } - var root *store.Post + var parent, root *store.Post if parentID == "" { post.Title = truncateRunes(strings.TrimSpace(r.PostFormValue("title")), 120) post.City = truncateRunes(strings.TrimSpace(r.PostFormValue("city")), 80) @@ -46,7 +49,7 @@ func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { return } } else { - parent, threadRoot, err := s.postAndRoot(r.Context(), parentID) + loadedParent, threadRoot, err := s.postAndRoot(r.Context(), parentID) if err != nil { if errors.Is(err, sql.ErrNoRows) { http.NotFound(w, r) @@ -63,6 +66,7 @@ func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { http.Error(w, "forbidden", http.StatusForbidden) return } + parent = loadedParent post.ParentID = &parent.ID root = threadRoot } @@ -78,6 +82,9 @@ func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { if root == nil { root = post } + if parent != nil { + s.notifyPostReply(parent, root, post, user) + } http.Redirect( w, r, @@ -86,6 +93,53 @@ func (s *Server) handleCreatePost(w http.ResponseWriter, r *http.Request) { ) } +// notifyPostReply asynchronously emails the direct parent post's author. +func (s *Server) notifyPostReply( + parent *store.Post, + root *store.Post, + reply *store.Post, + replyAuthor *store.User, +) { + if parent == nil || + root == nil || + reply == nil || + replyAuthor == nil || + s.cfg.Mail == nil || + parent.AuthorID == replyAuthor.ID { + return + } + if _, disabled := s.cfg.Mail.(mail.Nop); disabled { + return + } + msg := mail.PostReply{ + RootID: root.ID, + RootTitle: root.Title, + ReplyID: reply.ID, + ReplyBody: reply.Body, + ReplyAuthorName: replyAuthor.Name, + } + recipientID := parent.AuthorID + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + recipient, err := s.store.UserByID(ctx, recipientID) + if err != nil { + log.Printf("notify reply %s: load recipient: %v", msg.ReplyID, err) + return + } + if recipient == nil || strings.TrimSpace(recipient.Email) == "" { + return + } + msg.ToEmail = recipient.Email + msg.ToName = recipient.Name + if err := s.cfg.Mail.NotifyPostReply(ctx, msg); err != nil { + log.Printf("notify reply %s: %v", msg.ReplyID, err) + return + } + log.Printf("notify reply %s: accepted", msg.ReplyID) + }() +} + // handleEditPost updates only a post's body after verifying that the current // homeowner owns it or that an admin is editing an admin-authored post. func (s *Server) handleEditPost(w http.ResponseWriter, r *http.Request) { diff --git a/internal/web/posts_test.go b/internal/web/posts_test.go index 5587d07..0ea4829 100644 --- a/internal/web/posts_test.go +++ b/internal/web/posts_test.go @@ -7,7 +7,9 @@ import ( "net/url" "strings" "testing" + "time" + "plumber/internal/mail" "plumber/internal/pacific" "plumber/internal/store" ) @@ -258,6 +260,127 @@ func TestEditPostRoutePermissions(t *testing.T) { } } +func TestPostReplyNotifications(t *testing.T) { + t.Parallel() + + recording := &mail.Recording{} + srv, mem := newTestServer(t, Config{Mail: recording}) + handler := srv.Handler() + homeowner := seedUser(t, mem, uniq("homeowner"), "hunter22", store.RoleUser) + admin := seedUser(t, mem, uniq("admin"), "hunter22", store.RoleAdmin) + homeownerCookies := loginUser(t, handler, homeowner.Username, "hunter22") + adminCookies := loginUser(t, handler, admin.Username, "hunter22") + homeownerCSRF := csrfForCookies(t, handler, homeownerCookies) + adminCSRF := csrfForCookies(t, handler, adminCookies) + + rec := postForm(handler, "/posts", url.Values{ + "_csrf": {homeownerCSRF}, + "title": {"Leaky sink"}, + "body": {"Water under the cabinet."}, + }, homeownerCookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("root create status = %d: %s", rec.Code, rec.Body.String()) + } + if recording.Len() != 0 { + t.Fatalf("root create sent %d notifications", recording.Len()) + } + roots, err := mem.ListRootPosts(context.Background(), pacific.Today(), homeowner.ID) + if err != nil || len(roots) != 1 { + t.Fatalf("created roots = %+v, %v", roots, err) + } + root := roots[0] + + rec = postForm(handler, "/posts", url.Values{ + "_csrf": {adminCSRF}, + "parent_id": {root.ID}, + "body": {"Replace the cartridge."}, + }, adminCookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("admin reply status = %d: %s", rec.Code, rec.Body.String()) + } + thread, err := mem.GetPostThread(context.Background(), root.ID) + if err != nil || len(thread.Replies) != 1 { + t.Fatalf("admin reply thread = %+v, %v", thread, err) + } + adminReply := thread.Replies[0] + msgs := waitForMail(t, recording, 1) + if msg := msgs[0]; msg.ToEmail != homeowner.Email || + msg.RootID != root.ID || + msg.RootTitle != root.Title || + msg.ReplyID != adminReply.ID || + msg.ReplyBody != adminReply.Body || + msg.ReplyAuthorName != admin.Name { + t.Fatalf("admin reply notification = %+v", msg) + } + + rec = postForm(handler, "/posts", url.Values{ + "_csrf": {homeownerCSRF}, + "parent_id": {adminReply.ID}, + "body": {"That fixed the drip."}, + }, homeownerCookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("homeowner reply status = %d: %s", rec.Code, rec.Body.String()) + } + thread, err = mem.GetPostThread(context.Background(), root.ID) + if err != nil || len(thread.Replies[0].Replies) != 1 { + t.Fatalf("homeowner nested reply thread = %+v, %v", thread, err) + } + homeownerReply := thread.Replies[0].Replies[0] + msgs = waitForMail(t, recording, 2) + if msg := msgs[1]; msg.ToEmail != admin.Email || + msg.RootID != root.ID || + msg.ReplyID != homeownerReply.ID || + msg.ReplyAuthorName != homeowner.Name { + t.Fatalf("homeowner reply notification = %+v", msg) + } + + rec = postForm(handler, "/posts", url.Values{ + "_csrf": {homeownerCSRF}, + "parent_id": {root.ID}, + "body": {"A note to myself."}, + }, homeownerCookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("self reply status = %d: %s", rec.Code, rec.Body.String()) + } + rec = postForm(handler, "/posts/"+adminReply.ID+"/edit", url.Values{ + "_csrf": {adminCSRF}, + "body": {"Replace the ceramic cartridge."}, + }, adminCookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("edit status = %d: %s", rec.Code, rec.Body.String()) + } + + noEmail := &store.User{ + Username: uniq("no-email"), + PasswordHash: homeowner.PasswordHash, + Role: store.RoleUser, + } + if err := mem.CreateUser(context.Background(), noEmail); err != nil { + t.Fatal(err) + } + noEmailRoot := &store.Post{ + AuthorID: noEmail.ID, + Title: "Quiet thread", + Body: "No email configured.", + PostDate: pacific.Today(), + } + if err := mem.CreatePost(context.Background(), noEmailRoot); err != nil { + t.Fatal(err) + } + rec = postForm(handler, "/posts", url.Values{ + "_csrf": {adminCSRF}, + "parent_id": {noEmailRoot.ID}, + "body": {"This should not send."}, + }, adminCookies) + if rec.Code != http.StatusSeeOther { + t.Fatalf("no-email reply status = %d: %s", rec.Code, rec.Body.String()) + } + time.Sleep(50 * time.Millisecond) + if recording.Len() != 2 { + t.Fatalf("self, edit, or no-email action sent a notification: %+v", recording.Snapshot()) + } +} + func TestQuestionPageRendersNestedPostControls(t *testing.T) { t.Parallel() @@ -345,6 +468,19 @@ func TestQuestionPageRendersNestedPostControls(t *testing.T) { } } +func waitForMail(t *testing.T, recording *mail.Recording, want int) []mail.PostReply { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if recording.Len() >= want { + return recording.Snapshot() + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("recorded %d notifications, want %d", recording.Len(), want) + return nil +} + func csrfForCookies(t *testing.T, handler http.Handler, cookies []*http.Cookie) string { t.Helper() req := httptest.NewRequest(http.MethodGet, "/submit", nil) diff --git a/internal/web/server.go b/internal/web/server.go index 60c6274..30a7612 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -510,6 +510,7 @@ func (s *Server) handleAnswer(w http.ResponseWriter, r *http.Request) { http.Error(w, "could not save answer", http.StatusInternalServerError) return } + s.notifyPostReply(root, root, reply, u) location := "/questions/" + url.PathEscape(id) + "#post-" + url.PathEscape(reply.ID) if isHTMX(r) { w.Header().Set("HX-Redirect", location) diff --git a/internal/web/server_test.go b/internal/web/server_test.go index ee74077..c9d65d2 100644 --- a/internal/web/server_test.go +++ b/internal/web/server_test.go @@ -19,6 +19,7 @@ import ( "plumber" "plumber/internal/blob" + "plumber/internal/mail" "plumber/internal/pacific" "plumber/internal/store" ) @@ -483,7 +484,8 @@ func TestProfileAdminAnsweredListAndAvatarUpload(t *testing.T) { } func TestMutationsVoteAnswerHideAndCSRF(t *testing.T) { - srv, mem := newTestServer(t, Config{}) + recording := &mail.Recording{} + srv, mem := newTestServer(t, Config{Mail: recording}) h := srv.Handler() adminName := uniq("admin") userName := uniq("user") @@ -611,6 +613,13 @@ func TestMutationsVoteAnswerHideAndCSRF(t *testing.T) { if got := rec.Header().Get("HX-Redirect"); got != "/questions/"+q.ID+"#post-"+adminReply.ID { t.Fatalf("admin answer redirect = %q", got) } + msgs := waitForMail(t, recording, 1) + if msg := msgs[0]; msg.ToEmail != user.Email || + msg.RootID != q.ID || + msg.ReplyID != adminReply.ID || + msg.ReplyBody != adminReply.Body { + t.Fatalf("compatibility reply notification = %+v", msg) + } rec = httptest.NewRecorder() req = httptest.NewRequest(http.MethodGet, "/questions/"+q.ID, nil) -- 2.43.0 From 37c328c4981f046fb1c4c7ea150d39317161db04 Mon Sep 17 00:00:00 2001 From: codegirl-007 Date: Thu, 27 Aug 2026 09:01:03 -0700 Subject: [PATCH 2/2] Use canonical production domain. Reference https://www.askaplumberfirst.com for production email links and configuration examples. --- .env.example | 2 +- internal/mail/mail_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index 61c32b2..0a4207b 100644 --- a/.env.example +++ b/.env.example @@ -18,7 +18,7 @@ SECURE_COOKIE=0 # RESEND_API_KEY=re_xxxxxxxxx # RESEND_FROM=Ask a Plumber # Public site origin used in email links (required when Resend is enabled): -# APP_BASE_URL=https://askaplumber.example +# APP_BASE_URL=https://www.askaplumberfirst.com # DigitalOcean Spaces (profile avatars). Leave unset to disable uploads. # SPACES_KEY= # SPACES_SECRET= diff --git a/internal/mail/mail_test.go b/internal/mail/mail_test.go index 3b86d32..b481637 100644 --- a/internal/mail/mail_test.go +++ b/internal/mail/mail_test.go @@ -17,7 +17,7 @@ func TestEmbeddedMarkIsPNG(t *testing.T) { func TestPostReplyContent(t *testing.T) { t.Parallel() - text, htmlBody := postReplyContent("https://plumber.example/", PostReply{ + text, htmlBody := postReplyContent("https://www.askaplumberfirst.com/", PostReply{ ToName: ``, RootID: "question-123", RootTitle: `Leaky sink`, @@ -30,7 +30,7 @@ func TestPostReplyContent(t *testing.T) { "Ask a Plumber First", "New reply", "cid:reply-notification-mark", - "https://plumber.example/questions/question-123#post-reply-456", + "https://www.askaplumberfirst.com/questions/question-123#post-reply-456", "white-space:pre-wrap", "<Sam & Pat>", "<b>Leaky sink</b>", @@ -58,7 +58,7 @@ func TestPostReplyContent(t *testing.T) { `Hi ,`, ` replied in "Leaky sink"`, "Replace the cartridge.\nThen test the handle.", - "https://plumber.example/questions/question-123#post-reply-456", + "https://www.askaplumberfirst.com/questions/question-123#post-reply-456", } { if !strings.Contains(text, want) { t.Errorf("text missing %q", want) @@ -69,7 +69,7 @@ func TestPostReplyContent(t *testing.T) { func TestPostReplyContentUsesFallbacks(t *testing.T) { t.Parallel() - text, htmlBody := postReplyContent("https://plumber.example", PostReply{}) + text, htmlBody := postReplyContent("https://www.askaplumberfirst.com", PostReply{}) if !strings.Contains(text, `Someone replied in "your conversation"`) { t.Errorf("text missing fallback title") } -- 2.43.0