Review: Spike: Rails email sending -- ActionMailer setup and docs
Verdict: NEEDS_REFINEMENT
Template Completeness
- [x] Type -- Spike
- [x] Lineage -- standalone, references westside-emails and basketball-api
- [x] Repo -- present (but wrong name, see below)
- [x] Question -- well-structured with 5 sub-questions
- [x] Deliverables -- 4 items, includes docs file and follow-up ticket updates
- [x] Time-box -- 2 hours / 1 session
- [x] Related -- references project, stories, and downstream issues
Traceability
- [ ] story label MISMATCH -- board item has
story:emailbutstory:emailis not a canonical story ID. Issue body referencesstory:WS-S7(admin branded email announcements) andstory:WS-S32(generic email blast system). Both exist on the project page under Admin (Marcus). [LABEL] Change board label fromstory:emailtostory:WS-S7(primary story this spike serves). - [x] story notes verified -- WS-S7 ("As an admin, I want to send branded email announcements so that parent comms are professional and consistent") and WS-S32 ("As an admin, I want a generic email blast system with pluggable audience queries") both found in project-westside-basketball user-stories section.
- [ ] arch note MISSING -- board item has
arch:westside-basketballbut noarch-westside-basketballnote exists in pal-e-docs. [SCOPE] Create architecture notearch-westside-basketballfor the westside-basketball component, or change label to a more specific arch component (e.g.,arch:emailwhich is used on other board items like #750, #751, #752). - [x] Forgejo issue -- ldraney/westside-basketball#47, open
File Targets
- [x]
app/mailers/application_mailer.rb-- verified: exists with placeholderfrom: "from@example.com"andlayout "mailer" - [x]
app/views/emails/-- verified: contains compose.html.erb, inbox.html.erb, index.html.erb (email preview pages, not mailer templates -- issue description is accurate) - [x] Gemfile
google-apis-gmail_v1-- verified: present at line 20 - [x]
config/environments/production.rbSMTP settings -- verified: all commented-out defaults,action_mailer.default_url_optionsset toexample.com - [x] Mailer layouts exist --
app/views/layouts/mailer.html.erbandmailer.text.erbpresent - [ ]
app/services/gmail_client.rbNOT MENTIONED -- this file exists and contains a working Gmail OAuth send implementation (GmailClient class with credentials/token file paths, send_email method). This is directly relevant to the spike's core question (ActionMailer vs Gmail API) and should be referenced in the issue. [BODY] Addapp/services/gmail_client.rbto the Question section under "Existing state" -- it's a working Gmail OAuth sender that the spike needs to evaluate.
Repo Placement
MISMATCH: Issue body says
ldraney/westside-ror in three places (Repo section, follow-up references to #48 and #49). The repo was renamed to ldraney/westside-basketball. Forgejo redirects the old URLs, but the issue body should be updated for clarity. [BODY] Replace all westside-ror references with westside-basketball.Dependencies
- Board item #1580 (Email: Queens EYBL tournament, Forgejo #48) has
depends:47label -- correctly documented as dependent - Board item #1581 (Email #2: TBD, Forgejo #49) has
depends:47andstatus:blocked-on-spec-- correctly documented as dependent and blocked on Marcus's input - Board item #1582 (Report: players behind on payment, Forgejo #50) is in backlog but has no dependency on this spike -- independent
- Existing closed work: PR #17 "Refactor email system to ActionMailer with Gmail API delivery" is merged. Issue #14 "Refactor email sending to use ActionMailer with Gmail API delivery adapter" is still open. The spike should reference these -- they may already contain decisions that inform this spike. [BODY] Add PR #17 (merged) and issue #14 (open) to the Related section -- previous ActionMailer refactor work.
Acceptance Criteria
Spike deliverables are clear and testable:
docs/email-sending.md-- verifiable by file existence and content reviewApplicationMailerupdated -- verifiable by reading the file- Production/dev email config -- verifiable by reading config files or documented secrets list
- Follow-up tickets #48/#49 updated -- verifiable by reading issue bodies
One concern: the deliverable "ApplicationMailer updated with correct default from: address" goes beyond spike output (a spike produces docs + tickets, not code changes). This could be a quick config change that's fine to bundle, but technically it's implementation, not investigation. Minor -- acceptable for a 2pt spike.
Blast Radius
Low blast radius. This is a docs-only spike. The existing
GmailClient service and related email infrastructure (EmailLog model, email_log table) are already in production. The spike's investigation may recommend changes but won't make them. No downstream services are affected by documentation.Note: The legacy
basketball-api (Python/FastAPI) has its own email system with MJML templates (board items #750-#753 show completed email infrastructure there). The spike should be aware this exists to avoid reinventing patterns.Decomposition Assessment
No decomposition needed. Single repo, 2 points, 1 doc file output, 2-hour time-box. Well within the 5-minute agent pass threshold for spike investigation.
Recommendations
[LABEL]Change board item story label fromstory:emailtostory:WS-S7(canonical story ID from project page)[SCOPE]Create architecture notearch-westside-basketballin pal-e-docs, OR change board label toarch:email(which has an existing usage pattern on this board)[BODY]Replacewestside-rorwithwestside-basketballin Repo section and Related section references[BODY]Addapp/services/gmail_client.rbto "Existing state" in the Question section -- it's a working Gmail OAuth sender directly relevant to the spike's core question[BODY]Add PR #17 (merged ActionMailer refactor) and issue #14 (open ActionMailer refactor issue) to the Related section