Review: Apple subscription model + migration

review-1636-2026-06-27 Review

review ready

Verdict: APPROVED

Round 3 re-review. Round 2 found enum value mismatch (active/in_billing_retry/in_grace_period vs arch note's subscribed/billing_retry/grace_period). Issue body has been updated. All values now align with arch-app-store.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Child of #66
  • [x] Repo -- ldraney/westside-basketball
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag -- none (internal model change, correct)
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [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"
  • [x] story note verified -- WS-S35 found in project-westside-basketball user-stories section (Admin list, block 45874)
  • [x] arch:app-store label -- App Store Billing architecture
  • [x] arch note verified -- arch-app-store exists in pal-e-docs (note ID 2224, slug "arch-app-store")
  • [x] Forgejo issue -- ldraney/westside-basketball#79, state: open

File Targets

  • [x] app/models/player.rb -- verified: exists (14 lines), currently has no enum declarations. Migration will add apple_subscription_status enum, model will need enum declaration added.
  • [x] db/migrate/*_add_apple_subscription_to_players.rb -- new file. db/migrate/ directory does not exist yet (schema was loaded from SQL dump, not migrations). Agent will create the directory. Acceptable.
  • [x] test/models/player_test.rb -- verified: exists (19 lines), has association tests. New enum/field tests will be added.

Enum Value Alignment (Round 2 Fix Confirmed)

  • [x] Issue AC enum values: subscribed, expired, billing_retry, grace_period, revoked, none
  • [x] Issue File Targets enum values: subscribed/expired/billing_retry/grace_period/revoked/none
  • [x] arch-app-store enum values: subscribed, grace_period, billing_retry, expired, revoked, none
  • [x] All three match (order differs, which is inconsequential for PostgreSQL enum creation)

Repo Placement

OK. Issue filed on ldraney/westside-basketball, all file targets are in that repo. Single-repo change.

Dependencies

  • Parent: #66 (Subscription UI -- 3-page arch). This ticket is sub-ticket A of 4.
  • Upstream: #65 (App Store webhook) provides data that populates these fields, but is not a blocker for the model+migration work itself.
  • Downstream: Board items 1637, 1638, 1639 all carry depends:79 -- this ticket unblocks the admin list badges (#80), admin detail UI (#82), and player-facing status page (#83).
  • No blocking dependencies -- this ticket can proceed independently.

Acceptance Criteria

4 AC, all testable by an agent:
  • [x] Enum values verifiable via Player.apple_subscription_statuses in Rails console or unit test
  • [x] Column existence verifiable via schema inspection or model test
  • [x] Additive check verifiable by confirming existing subscription_status enum and Stripe columns unchanged
  • [x] Reversibility verifiable via rails db:migrate:down

Blast Radius

Low. The existing subscription_status enum (subscriptionstatus type: active/past_due/canceled/none) is used in 6+ files (delinquency_report_controller, roster_report_controller, webhooks_controller, admin/players/index view, application_helper). The ticket explicitly states NOT to touch these. The new apple_subscription_status is a separate PostgreSQL enum type with different values. No overlap or conflict. The expires_at column name could potentially conflict with OAuth token handling in gmail_client.rb, but that uses a local variable, not a Player column -- no conflict.

Decomposition Assessment

3 file targets, 1 repo, 4 AC. Well within the 5-minute rule. No decomposition needed.

Recommendation

No action needed.