Review: Spike: Rails email sending -- ActionMailer setup and docs

review-1579-2026-06-23 Review

review needs-refinement

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:email but story:email is not a canonical story ID. Issue body references story:WS-S7 (admin branded email announcements) and story:WS-S32 (generic email blast system). Both exist on the project page under Admin (Marcus). [LABEL] Change board label from story:email to story: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-basketball but no arch-westside-basketball note exists in pal-e-docs. [SCOPE] Create architecture note arch-westside-basketball for the westside-basketball component, or change label to a more specific arch component (e.g., arch:email which 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 placeholder from: "from@example.com" and layout "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.rb SMTP settings -- verified: all commented-out defaults, action_mailer.default_url_options set to example.com
  • [x] Mailer layouts exist -- app/views/layouts/mailer.html.erb and mailer.text.erb present
  • [ ] app/services/gmail_client.rb NOT 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] Add app/services/gmail_client.rb to 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:47 label -- correctly documented as dependent
  • Board item #1581 (Email #2: TBD, Forgejo #49) has depends:47 and status: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 review
  • ApplicationMailer updated -- 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 from story:email to story:WS-S7 (canonical story ID from project page)
  • [SCOPE] Create architecture note arch-westside-basketball in pal-e-docs, OR change board label to arch:email (which has an existing usage pattern on this board)
  • [BODY] Replace westside-ror with westside-basketball in Repo section and Related section references
  • [BODY] Add app/services/gmail_client.rb to "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