pal-e-docs

Review: Email: Queens EYBL Live Period tournament -- registration + travel details

review-1580-2026-06-23 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- depends on #47
  • [x] Repo -- ldraney/westside-basketball
  • [x] User Story -- As an admin (Marcus), send tournament details email
  • [x] Context -- EYBL Live Period, Las Vegas, July 9-13
  • [ ] File Targets -- MISSING. No file paths listed. Issue should specify which mailer class to create, which view template, and which controller action or rake task triggers the send.
  • [ ] Feature Flag -- MISSING. Section required by template-issue-feature. This is a new user-visible email send capability. Recommend: "none" with justification (one-shot admin-triggered action, not a persistent feature toggle), or flag it if email sending should be gated.
  • [x] Acceptance Criteria -- 4 items present
  • [ ] Test Expectations -- MISSING. No test section. Should specify ActionMailer test helpers, email delivery assertion, email_log record creation.
  • [x] Constraints -- present (dependency on #47, 72-hour deadline, audience query)
  • [ ] Checklist -- MISSING (PR opened, tests pass, no unrelated changes)
  • [x] Related -- present (links to #47, #49, story:WS-S7, sop-email-send)

Traceability

  • [ ] story:email label -- MISMATCH. Board item label is story:email, but issue body references story:WS-S7 (As an admin, I want to send branded email announcements so that parent comms are professional and consistent). WS-S7 is verified on project-westside-basketball user-stories section. [LABEL] Change board item label from story:email to story:WS-S7.
  • [ ] arch:westside-basketball label -- arch note MISSING. mcp__pal-e-docs__search_notes(query="arch-westside-basketball") returned no results. [SCOPE] Create architecture note arch-westside-basketball for the westside-basketball component.
  • [x] Forgejo issue -- #48, open, valid URL (redirects from old westside-ror name)

File Targets

No file targets listed in the issue. Based on repo inspection, the following files are relevant:
  • app/mailers/application_mailer.rb -- exists, still has default from@example.com (will be updated by #47)
  • app/services/gmail_client.rb -- exists, legacy Gmail API client
  • app/models/email_log.rb -- exists, for send logging
  • app/models/tournament.rb -- exists, tournament model
  • app/controllers/emails_controller.rb -- exists, email management
  • app/views/emails/ -- exists, email preview pages
Issue should specify: new mailer class (e.g. app/mailers/tournament_mailer.rb), new mailer view (e.g. app/views/tournament_mailer/queens_eybl.html.erb), and how the send is triggered (admin action, rake task, or controller endpoint).

Repo Placement

OK. Issue is filed on ldraney/westside-basketball, and the work belongs in that repo. Board item forgejo_issue_url still shows old name (westside-ror) but Forgejo redirects correctly.

Dependencies

  • depends:47 (Spike: Rails email sending -- ActionMailer setup and docs) -- board item #1579, currently in backlog. This is a hard dependency: #47 establishes the email sending mechanism (ActionMailer config, SMTP vs Gmail API decision, template strategy). #48 cannot be implemented until #47 is complete and merged.
  • Sibling: #49 (Email #2, TBD) -- board item #1581, also depends on #47. No circular dependency.
  • Dependency is correctly documented in both the issue body (Lineage, Constraints) and the board item labels (depends:47).

Acceptance Criteria

  • [x] "Email is sent to all Queens family email addresses" -- testable, but needs clarification: what defines "Queens family"? The Constraints section says "all parents with a daughter on a Queens team" which helps. Agent needs a query/scope to identify these records.
  • [x] "Email body matches the content above" -- testable via ActionMailer preview or test assertion against the template. The full email body is provided inline, which is good.
  • [x] "Email is sent from the configured Westside sending address" -- testable, depends on #47 establishing the address.
  • [x] "Send is logged (who received, timestamp)" -- testable, email_log model already exists.
AC are reasonable and verifiable. Could benefit from one more: "Audience query returns only Queens team parents (not Kings)".

Blast Radius

Low. This is a new mailer class + view + trigger. It does not modify existing email infrastructure (that is #47's job). No sibling services affected. The email_log model already exists. The main risk is sending to wrong recipients (Kings families instead of Queens), which AC should cover.

Decomposition Assessment

4 AC, 1 repo, ~3 file targets (mailer, view, trigger). Estimated agent work: under 5 minutes once #47 is complete. No decomposition needed.

Recommendations

  • [BODY] Add File Targets section: specify the new mailer class, mailer view template, and trigger mechanism (controller action or rake task).
  • [BODY] Add Feature Flag section: "none" with justification (admin-triggered one-shot action), or specify a flag if email sending should be gated.
  • [BODY] Add Test Expectations section: ActionMailer test helpers, email delivery count assertion, email_log record creation, audience query correctness.
  • [BODY] Add Checklist section (PR opened, tests pass, no unrelated changes).
  • [BODY] Add AC: "Audience query returns only Queens team parents (not Kings or other teams)".
  • [LABEL] Change board item label from story:email to story:WS-S7.
  • [SCOPE] Create architecture note arch-westside-basketball for the westside-basketball component.