Review: Apple subscription model + migration
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_statusesin 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.