pal-e-docs

Review: Apple subscription model + migration (R2)

review-1636-2026-06-27-r2 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Re-review (round 2) of board item #1636. Prior review: review-1636-2026-06-27 (NEEDS_REFINEMENT).

Round 1 Resolution

  • [x] [SCOPE] Create architecture note arch-app-store -- RESOLVED. Note exists (ID 2224, slug arch-app-store, title "Architecture: App Store Billing"). Covers components, data model, dependency chain, key decisions.
  • [x] [BODY] Clarify that existing Stripe subscription_status is DB-level only -- RESOLVED. Context section now reads "DB-level enum: active/past_due/canceled/none", making clear there is no model-level enum declaration.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Child of #66, Sub-ticket A of 4
  • [x] Repo -- ldraney/westside-basketball
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag -- none (internal model change, acceptable)
  • [x] Acceptance Criteria -- 4 criteria
  • [x] Test Expectations -- 3 expectations + run command
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:WS-S35 label -- "As an admin, I want to set and manage subscription pricing on a player's detail page so that I control what each family pays for their program tier" -- verified in project-westside-basketball user-stories section under Admin (Marcus)
  • [x] story note verified -- found in project-westside-basketball user-stories section
  • [x] arch:app-store label -- "Architecture: App Store Billing" -- verified: arch-app-store note exists in pal-e-docs (ID 2224)
  • [x] arch note verified -- arch-app-store covers data model, components, dependency chain
  • [x] Forgejo issue -- ldraney/westside-basketball#79, open
  • [x] Parent issue -- #66 (Subscription UI), open, in next_up

File Targets

  • [x] app/models/player.rb -- verified: file exists (14 lines, associations only). No existing enum declarations in model. DB schema confirms subscription_status enum and Stripe columns exist at schema level.
  • [x] db/migrate/*_add_apple_subscription_to_players.rb -- new file to create. db/migrate/ directory does not exist yet (schema was loaded from DB dump). Agent must create directory. Not a blocker.
  • [x] test/models/player_test.rb -- verified: file exists (2 existing tests for associations and table name).

Repo Placement

OK. Issue filed on ldraney/westside-basketball, all file targets are within the same repo. Single-repo change.

Dependencies

  • Parent: #66 (board item #1611) in next_up, decomposed into 4 sub-tickets.
  • Downstream dependents: #80 (depends:79, WS-S36), #82 (depends:58,depends:79, WS-S35), #83 (depends:79, WS-S37) -- 3 items depend on this ticket.
  • No upstream blockers on #79 itself. The migration is self-contained.
  • #65 (webhook infra, board item #1610, next_up) provides data that populates these fields, but #79 does not depend on #65 to ship. Fields can exist before the webhook fills them.

Acceptance Criteria

4 AC, all verifiable by agent. Test commands are real. Criteria are clear and complete.

Blast Radius

Additive-only change. New apple_subscription_status is a separate PostgreSQL enum type. No collision with existing subscriptionstatus enum or Stripe columns. No blast radius concerns.

Decomposition Assessment

3 file targets in 1 repo. 4 acceptance criteria. Estimated agent work: 2-3 minutes. No decomposition needed.

NEW FINDING: Enum Value Mismatch

The issue body and arch-app-store note disagree on enum values:
Issue #79 (AC1) arch-app-store (Data Model)
active subscribed
expired expired
in_billing_retry billing_retry
in_grace_period grace_period
revoked revoked
none none
Two classes of discrepancy:
  • Naming: active vs subscribed -- semantically different. Apple Server Notifications v2 uses neither; the closest Apple status is ACTIVE. The issue and arch note must agree on one name.
  • Prefix: in_billing_retry/in_grace_period vs billing_retry/grace_period -- the in_ prefix adds clarity but diverges from the arch note. Must pick one convention.
This mismatch would cause the agent to implement values that conflict with the arch note, creating confusion for downstream tickets #80, #82, #83 that consume this enum.

Recommendation

  • [BODY] Align enum values in issue #79 AC1 with arch-app-store Data Model section (or vice versa). The two sources must agree. Recommend: pick one set, update both the issue body AND the arch note to match. Suggest using the arch note values (subscribed, grace_period, billing_retry) since they are shorter and closer to Rails enum conventions, but this is an owner decision.