pal-e-docs

Review: Admin approval UI on Crew tab for pending join requests

review-1907-2026-07-25 Review

review ready
Project

Verdict: READY

Template Completeness

  • [x] Type — Feature
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related
All 12 required feature template sections present.

Traceability

  • [x] story:saas label — SaaS subscription, business codes for crew onboarding, admin approval of join requests, Apple IAP billing
  • [x] story note verified — found in project-landscaping-assistant user-stories section (key: saas, children: #309, #310, #312)
  • [x] arch:rails-app label — shared Rails deployment architecture
  • [x] arch note verified — arch-rails-app note exists in pal-e-docs (active, architecture type)
  • [x] Forgejo issue — ldraney/landscaping-assistant#310, state: open

File Targets

  • [x] app/controllers/crew_controller.rb — verified: exists (11 lines), has require_role :admin, :super_admin at line 2, currently only index and show actions. Ticket correctly states existing guard will cover new approve/deny actions.
  • [x] app/views/crew/index.html.erb — verified: exists (27 lines), currently renders crew members list.
  • [x] app/views/crew/_pending_request.html.erb — NEW file (confirmed does not exist yet). Correctly identified as creation target.
  • [x] config/routes.rb — verified: exists (70 lines), has resources :crew, only: [:index, :show] at line 53. Ticket correctly identifies adding member routes under existing resource.
  • [x] app/models/user_business.rb — verified: exists (12 lines), has STATUSES = %w[pending approved denied] and ROLES constants but no scopes yet. Ticket correctly identifies adding pending and for_business scopes.
  • [x] spec/requests/crew_spec.rb — verified: exists (149 lines, 19 existing examples). Uses sign_in_as helper. Agent can follow established patterns for new approve/deny specs.
  • [x] app/views/layouts/application.html.erb — verified: exists, has crew nav tab link at line 69. Badge addition target is clearly identifiable.

Repo Placement

OK — all 7 file targets are in ldraney/landscaping-assistant. Forgejo issue filed on same repo. No cross-repo changes needed.

Dependencies

  • business_codes feature flag — NOT registered in lib/tasks/feature_flags.rake yet. Ticket correctly documents this as a dependency: "not yet created — needed before this or #309 can ship." The flag registration must happen in a separate task before this ticket can be deployed, but the code can be written and merged behind the flag.
  • Profile join form #309 (board item #1906, backlog) — parallel sibling. Creates UserBusiness records that this ticket consumes. Can be developed independently.
  • Keycloak registration field #312 (board item #1909, backlog) — parallel sibling. Another path to create UserBusiness records.
  • Business code migration — documented as dependency for join_code column on Business table. Required for the overall flow but not for the UI code in this ticket.
  • Multi-tenancy items — #1755 (needs_approval) and #1756 (todo) are in-flight multi-tenancy work. These may introduce a current_business helper. This ticket does not depend on them — the admin's business can be derived via UserBusiness.find_by(user_id: current_user[:sub], status: "approved") -> .business since current_user[:sub] stores the Keycloak UUID and matches UserBusiness.user_id.

Acceptance Criteria

12 acceptance criteria. All are testable by an agent:
  • Show/hide pending section — testable via request specs with feature flag toggle and pending records
  • Approve/deny actions — testable via PATCH requests checking UserBusiness status transitions
  • Role enforcement — existing spec pattern covers admin/non-admin access (19 existing examples demonstrate the approach)
  • Turbo Stream responses — established pattern exists in days_controller.rb and work_queue_items_controller.rb
  • Badge count on nav tab — testable via response body assertions on the layout
AC #5 (visible tabs update on next page load) and AC #6 (denied state on Profile page) are cross-ticket effects that happen automatically via existing role-based tab logic and the Profile ticket respectively. Not directly testable in isolation but the underlying mechanism is verified.

Blast Radius

  • Layout change — badge on crew nav tab in application.html.erb affects all pages. Low risk since it is additive (a badge count span) and gated by feature flag + pending count.
  • Turbo Stream pattern — established in codebase. Agent can follow existing format.turbo_stream patterns from days_controller.rb.
  • No blast radius to sibling services — UserBusiness is only referenced in model specs and this controller. No other controllers consume it yet.

Decomposition Assessment

7 file targets across 1 repo (within 3-file/2-repo threshold). 12 acceptance criteria (above 5-criterion threshold). However, all criteria serve one cohesive UI feature — a pending-requests section with approve/deny buttons plus a badge. Decomposing would create artificial sub-tickets with tight coupling and coordination overhead. Files are small (controller: 11 lines, model: 12 lines, view: 27 lines, routes: 70 lines). Estimated agent time: 5-8 minutes (borderline but acceptable given cohesion). No decomposition needed.

Recommendation

No action needed.
Observation (informational, not blocking): The ticket does not explicitly document how to derive the admin's business from the session. The path is: current_user[:sub] (Keycloak UUID) -> UserBusiness.find_by(user_id: sub, status: "approved") -> .business. The Context section's "CrewMember vs UserBusiness" explanation plus the session controller's sub: auth.uid storage provide enough context for an agent to derive this, so it is not a blocker.