Review: Admin list page Apple badges

review-1637-2026-06-27 Review

review approved

Verdict: APPROVED

Round 3 re-review. Both round 1 and round 2 findings have been addressed. Scope is solid.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Child of #66
  • [x] Repo -- ldraney/westside-basketball
  • [x] User Story -- well-formed (admin viewing player list)
  • [x] Context -- explains existing Stripe badge pattern and dependency on #79
  • [x] File Targets -- 3 modify targets + 2 do-not-touch entries
  • [x] Feature Flag -- none (admin-only UI)
  • [x] Acceptance Criteria -- 2 items
  • [x] Test Expectations -- 2 unit tests + run command
  • [x] Constraints -- 4 constraints including enum alignment
  • [x] Checklist -- present
  • [x] Related -- arch-app-store, project page, parent, dependency

Traceability

  • [x] story:WS-S36 label -- "As an admin, I want to see Apple subscription lifecycle states per player"
  • [x] story note verified -- found in project-westside-basketball user-stories section (Admin/Marcus list)
  • [x] arch:app-store label -- App Store Billing component
  • [x] arch note verified -- arch-app-store note exists in pal-e-docs (ID 2224)
  • [x] Forgejo issue -- ldraney/westside-basketball#80, open

File Targets

  • [x] app/helpers/application_helper.rb -- verified: exists (22 lines), has subscription_badge_variant and subscription_badge_label helper patterns to parallel
  • [x] app/views/admin/players/index.html.erb -- verified: exists (112 lines), uses badge helpers at lines 77-78 (desktop) and 94-95 (mobile cards)
  • [x] app/assets/stylesheets/application.css -- verified: exists (1907 lines), has status-badge--paid (green), --unpaid (red), --partial (yellow), --active (red) but NO orange or gray badge variants. Round 2 correctly identified this gap.
  • [x] app/models/player.rb -- correctly marked do-not-touch (model changes in #79)
  • [x] app/views/admin/players/show.html.erb -- correctly marked do-not-touch (does not exist yet, detail page is sub-ticket C/#82)

Enum Alignment (critical cross-ticket check)

  • [x] Issue #80 enum values: subscribed, expired, billing_retry, grace_period, revoked, none
  • [x] Issue #79 (dependency) enum values: subscribed, expired, billing_retry, grace_period, revoked, none
  • [x] arch-app-store Data Model: subscribed, grace_period, billing_retry, expired, revoked, none
  • [x] All three sources are aligned

Repo Placement

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

Dependencies

  • [x] depends:79 (model+migration) -- correctly documented. #79 must merge first to provide the apple_subscription_status enum on the Player model. Board item #1636 is in backlog.
  • [x] parent:66 (Subscription UI parent) -- correctly documented. This is sub-ticket B of 4.
  • No circular dependencies. No undocumented dependencies.

Acceptance Criteria

2 criteria, both agent-verifiable:
  • AC1: Admin list page shows Apple badge -- verifiable via view inspection and integration test
  • AC2: Badge colors follow semantic convention -- verifiable via unit tests on helper methods
Test file test/helpers/application_helper_test.rb exists with 10 existing tests for Stripe badge helpers. Run command is valid.

Blast Radius

Low. No existing Apple subscription references in the codebase. Changes are additive -- new helpers alongside existing ones, new CSS classes alongside existing ones, new column in existing table. Existing Stripe badge behavior is untouched.

Decomposition Assessment

No decomposition needed. 3 file targets in 1 repo, 2 acceptance criteria, estimated agent work under 5 minutes. Well-scoped for a single pass.

Recommendation

No action needed. Scope is complete and ready for implementation after #79 merges.