pal-e-docs

Review: Add apology opening to first-payment email

review-1005-2026-04-13 Doc

review ready
Project

Verdict: READY

Board item #1005 — forgejo_admin/basketball-api#477. Small, tightly-scoped email template change following the #473 checkout 409 fix. Marcus-approved verbatim wording in the issue Constraints section.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — references #473
  • [x] Repo — forgejo_admin/basketball-api
  • [x] User Story — parent perspective with clear want/so-that
  • [x] Context — explains why re-send is needed
  • [x] File Targets — single function identified, exclusions listed
  • [x] Acceptance Criteria — 5 items, all verifiable
  • [x] Test Expectations — explicit pytest invocation and test name
  • [x] Constraints — verbatim apology text + style matching rules
  • [x] Checklist — standard
  • [x] Related — project + lineage issue

Traceability

  • [x] story:WS-S22 — "As a parent, I want to receive clear email communications with action links so that I never miss a deadline" — verified in project-westside-basketball user-stories section (stories-parent-list). Perfect fit: clarity of comms is exactly this story.
  • [x] arch:basketball-api label present on board item
  • [ ] arch note MISSING — arch-basketball-api does not exist in pal-e-docs (semantic search + list_notes confirm). Non-blocking for this ticket — pre-existing platform-wide gap flagged in prior reviews (review-763, review-836, review-840, etc.). [SCOPE] Create architecture note arch-basketball-api (standing recommendation, not a blocker for #1005).
  • [x] Forgejo issue — forgejo_admin/basketball-api#477, open
  • [x] scope:urgent label — justified (revenue-critical follow-up to deployed fix; 100% of April blast recipients affected)

File Targets

  • [x] src/basketball_api/services/email.py — verified. send_first_payment_email() exists at line 1021. body_html f-string starts at line 1064 (first paragraph at 1065–1067 is "Hi {parent_name},"). plain_body f-string starts at line 1108 (opens with "Hi {parent.name},\n\n"). Both locations where the apology paragraph should be inserted are unambiguous. _brand_wrapper() call at 1107 confirms the "inside the branded wrapper" guidance — the apology goes inside body_html, not around _brand_wrapper.
  • [x] Exclusions verified correct:
  • [x] tests/test_first_payment_email.py — verified. test_send_first_payment_email_180_fee exists at line 116 (good anchor for an updated assertion or a new sibling test). Test file is the right location per the issue's pytest tests/test_first_payment_email.py -v directive.

Repo Placement

OK. Single-repo change in basketball-api. Forgejo issue filed on the correct repo.

Dependencies

Upstream #473 (checkout 409 fix) is merged and deployed per issue context. No in-flight tickets block this. Downstream: Marcus will want to re-send the April blast after merge + QA; that send is gated by the 7-step blast protocol and is NOT part of this ticket's scope.

Acceptance Criteria

All 5 criteria are agent-verifiable:
  • HTML apology first — assertable via string position check
  • Plaintext apology first — assertable via string position check
  • Existing content unchanged — existing tests guard this
  • Existing tests pass — CI
  • New test verifies apology — explicit test name + location given
Marcus's exact wording is in the Constraints block, so the assertion target is unambiguous.

Blast Radius

Low. Only send_first_payment_email is touched. Other email builders in services/email.py (welcome, jersey, contract, tournament fee, etc.) use similar patterns but are explicitly out of scope — no other blast went out with the broken checkout link. Callers of this function (blast endpoint in routes/admin.py, plus tests in test_first_payment_blast.py) continue to work because function signature is unchanged.

Decomposition Assessment

5-minute rule check: 1 file target, 1 test file, 5 AC, single function, verbatim copy provided. Well under decomposition thresholds. No decomposition needed.

Recommendation

READY. No blocking action needed.
  • [SCOPE] Standing recommendation (not blocking this ticket): Create architecture note arch-basketball-api to close the traceability leg that has been flagged across multiple prior reviews.
Dispatch a dev agent with the Forgejo issue URL — the spec is complete and unambiguous.