pal-e-docs

Review: Subscription management UI

review-1611-2026-06-27 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — references sibling App Store ticket and #58
  • [x] Repo — ldraney/westside-basketball
  • [x] User Story — admin wants subscription status on player detail page
  • [x] Context — describes StoreKit renewal states
  • [x] File Targets — 5 modify/create, 1 do-not-touch
  • [x] Feature Flag — none (admin-only, acceptable)
  • [x] Acceptance Criteria — 5 criteria
  • [x] Test Expectations — 3 test items with run command
  • [x] Constraints — 3 constraints listed
  • [x] Checklist — present
  • [x] Related — project-westside-basketball

Traceability

  • [x] story:WS-S9 label — "As an admin, I want to track payment status per player so that I know who owes what" — verified in project-westside-basketball user-stories section
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-rails-views for component rails-views. No matching note found in pal-e-docs.
  • [x] Forgejo issue — ldraney/westside-basketball#66, state: open

File Targets

  • [ ] app/models/subscription.rb — ISSUE: New file, but conflicts with existing Stripe-based subscription tracking on Player model. Player already has subscription_status enum (active/past_due/canceled/none) and stripe_subscription_id. Creating a separate Subscription model with Apple StoreKit states introduces two incompatible subscription systems with no documented coexistence or migration strategy.
  • [ ] db/migrate/*_create_subscriptions.rb — ISSUE: New table, but the existing subscriptionstatus Postgres enum on players (active/past_due/canceled/none) conflicts with the proposed Apple states (subscribed/grace_period/billing_retry/expired/revoked). No migration plan for existing data.
  • [ ] app/views/admin/players/show.html.erb — ISSUE: File does not exist. Issue #58 (player detail page) is marked done on board (item 1609) but the show view, controller action, and route are NOT present in main. Only index.html.erb exists in app/views/admin/players/. Controller has no def show. Routes: resources :players, only: [:index].
  • [ ] app/views/admin/players/_subscription_badge.html.erb — ISSUE: Badge partial proposed, but application_helper.rb already defines subscription_badge_variant and subscription_badge_label helpers used by the existing index view. The new badge would need to coexist with or replace the existing badge system.
  • [x] app/models/player.rb — verified: exists with 10 associations (belongs_to :parent, :tenant; has_many :player_teams, :teams, :contract_audit_logs, :email_logs, :orders, :payment_links, :registrations, :jersey_public_orders). No existing has_one :subscription.

Repo Placement

OK — issue is filed on ldraney/westside-basketball, which is the correct repo for the Rails app models and admin views.

Dependencies

  • depends:65 NOT MET — Issue #65 ("Configure App Store Connect subscription product and billing grace period") is in backlog (board item 1610) and open on Forgejo. This ticket provides the App Store Server Notifications v2 webhook endpoint that #66 depends on for AC #4 ("When webhook updates subscription state, badge reflects new state on next page load").
  • depends:58 (implicit) — Issue #58 ("Admin player detail page") is marked done on board but show.html.erb does not exist in main. The code may not be merged or deployed. This ticket targets show.html.erb which doesn't exist.
  • Existing Stripe system undocumented — The dependency on the existing Stripe subscription system (webhooks_controller.rb handles customer.subscription.updated/deleted, Player has stripe_subscription_id and subscription_status) is not documented. The issue does not explain whether Apple StoreKit replaces Stripe or runs alongside it.

Acceptance Criteria

  • [x] AC1: "Subscription model persists StoreKit renewal states per player" — testable but architecturally conflicted with existing Stripe model
  • [ ] AC2: "Player detail page shows color-coded subscription badge" — NOT testable: show.html.erb doesn't exist yet
  • [ ] AC3: "Player list page shows subscription status indicator per row" — ALREADY EXISTS: index.html.erb already renders subscription_badge_variant and subscription_badge_label via existing Stripe-based subscription_status. This AC is redundant unless it means Apple-specific badges, which is unclear.
  • [ ] AC4: "When webhook updates subscription state, badge reflects new state" — NOT testable: depends on #65 (App Store webhook) which is in backlog
  • [x] AC5: "Players without subscriptions show 'No subscription' gracefully" — testable

Blast Radius

  • Existing subscription UI will break or conflict — admin/players/index.html.erb, admin/delinquency_report, admin/roster_report, and application_helper.rb all reference Player#subscription_status (Stripe-based). A new Subscription model with different states will create confusion about which status is authoritative.
  • webhooks_controller.rb — existing Stripe webhook handler updates Player#subscription_status directly. The proposed app_store_controller.rb (from #65) would update the new Subscription model. Two webhook handlers updating two different subscription tracking systems.
  • stripe_client.rb — has update_subscription_price and create_subscription_payment_link methods. These won't know about Apple subscriptions.

Decomposition Assessment

5 file targets across 1 repo, 5 acceptance criteria. Fits within the 5-minute rule on surface count, BUT the architectural conflict with the existing Stripe subscription system makes this ticket fundamentally under-scoped. The ticket cannot be implemented as-written without first deciding the Stripe-vs-Apple coexistence strategy. No decomposition needed — scope clarification needed first.

Recommendation

  • [SCOPE] Resolve Stripe vs Apple StoreKit coexistence strategy before implementation. The codebase has a fully functional Stripe subscription system (Player#subscription_status, webhooks_controller.rb, stripe_client.rb, admin views). The issue proposes an Apple StoreKit system without acknowledging the existing one. Decision needed: replace Stripe with Apple? Run both? Migrate?
  • [BODY] Fix file target: app/views/admin/players/show.html.erb does not exist. Depends on #58 being merged first, or this ticket must include creating the show view.
  • [BODY] Remove or clarify AC #3 ("Player list page shows subscription status indicator per row") — this already exists for Stripe-based status. Clarify whether this means adding Apple-specific status alongside.
  • [BODY] Add context about existing Stripe subscription system: Player already has subscription_status enum (active/past_due/canceled/none) and stripe_subscription_id. Explain coexistence or migration plan.
  • [BODY] Update AC #4 to not depend on #65 webhook, or explicitly mark it as blocked until #65 ships.
  • [SCOPE] Create architecture note arch-rails-views for component rails-views.