Review: Admin user management and promotion

review-1794-2026-07-06 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Depends on #6, #8. Sprint 5.
  • [x] Repo — ldraney/intelligentstaffingsystems
  • [x] User Story — As an admin, I want to see all leads/clients and promote leads
  • [x] Context — Admin-only view, promote button, Keycloak admin API
  • [x] File Targets — 3 files listed (but incomplete, see below)
  • [x] Feature Flag — none
  • [x] Acceptance Criteria — 9 items
  • [x] Test Expectations — 3 items + run command
  • [x] Constraints — KeycloakAdminService, audit trail, docs-in-PR
  • [x] Checklist — present
  • [x] Related — project, blockers, user stories listed
All template sections present.

Traceability

  • [x] story:admin label — Epic 6 (US-6.1, US-6.2)
  • [x] story note verified — found in project-iss user-stories section (Key "admin", backing "Epic 6 (US-6.1–6.2)", role "Admin", metric "Pipeline list; lead→client promotion with audit")
  • [x] arch:rails label — Rails application component
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-rails for component rails
  • [x] Forgejo issue — ldraney/intelligentstaffingsystems#14, open

File Targets

  • [x] app/controllers/admin/users_controller.rb — verified: directory does not exist yet, correct for new file creation
  • [x] app/views/admin/users/ — verified: directory does not exist yet, correct for new views
  • [x] app/assets/stylesheets/admin.css — verified: file does not exist yet, correct for new stylesheet
  • [ ] app/services/keycloak_admin_service.rb — MISSING from File Targets. Service exists but only has get_user and update_user methods. Keycloak role assignment requires a different API endpoint (POST /admin/realms/{realm}/users/{id}/role-mappings/realm). A new method (e.g. assign_realm_role) must be added for promotion to work.
  • [ ] config/routes.rb — MISSING from File Targets. No admin namespace routes exist; must be added.
  • [ ] app/models/lead.rb — MISSING from File Targets. Model needs a promote! method and PaperTrail integration (if kept). Currently has role enum and promoted_at in schema but no promotion logic.
  • [ ] Gemfile — MISSING from File Targets. Constraints reference PaperTrail for audit trail but the gem is not installed.

Repo Placement

OK. Issue filed on ldraney/intelligentstaffingsystems, all file targets are in that repo. Single-repo scope.

Dependencies

  • #6 (Keycloak OIDC auth) — in validation column. Provides session/auth infrastructure. Required.
  • #8 (Lead model) — in validation column. Provides Lead model with role enum, promoted_at column. Required.
  • #13 (Profile view, edit, logout) — in validation column. Introduced KeycloakAdminService. Required for the admin API client. Issue body correctly identifies this dependency in Constraints.
  • All three blockers are in validation, not yet done. Ticket cannot move to in_progress until blockers reach done.

Acceptance Criteria

  • 9 AC items total. All are verifiable by an agent.
  • AC#3 (sortable/filterable by role) — adds frontend complexity. Testable via controller tests with params.
  • AC#5 (promotion updates Keycloak role AND Lead.role) — requires KeycloakAdminService extension (see File Targets issue above).
  • AC#6 (confirmation dialog) — requires JavaScript. Standard Rails pattern (data-turbo-confirm or similar).
  • AC#9 (docs-in-PR rule) — meta-requirement, standard for this project.
  • Test expectations reference rails test — real command, verified.

Blast Radius

  • KeycloakAdminService is also used by ProfileController (#13). Extending it with role assignment methods should not affect profile operations.
  • Lead.role is synced from Keycloak on every login (sessions_controller.rb line 38-46). After promotion, the user's next login will pick up the new role from Keycloak. The session role is a login-time snapshot (documented in docs/security.md line 270). No blast radius concern.
  • No other admin namespace controllers exist — this is the first. No naming conflicts.

Decomposition Assessment

  • File targets: 3 listed + 4 unlisted = 7 total, all in 1 repo — below the ">3 files across >2 repos" threshold
  • Acceptance criteria: 9 items — exceeds >5 threshold
  • Estimated agent work: moderate (controller + views + routes + service extension + model logic + JS confirmation + PaperTrail setup)
  • Assessment: borderline. The AC count exceeds the threshold, but all work is in a single domain (admin user management) within one controller namespace. If the File Targets and PaperTrail issues are resolved, a single agent pass is feasible. No decomposition required at this time, but revisit if scope grows.

Recommendations

  • [BODY] Add app/services/keycloak_admin_service.rb to File Targets — needs assign_realm_role(keycloak_sub, role_name) method for Keycloak role mapping API (POST /admin/realms/{realm}/users/{id}/role-mappings/realm). Current service only has get_user and update_user.
  • [BODY] Add config/routes.rb to File Targets — needs admin namespace routes (namespace :admin { resources :users, only: [:index, :show] { member { patch :promote } } } or similar).
  • [BODY] Add app/models/lead.rb to File Targets — needs promote! method and PaperTrail integration.
  • [BODY] Clarify PaperTrail dependency: either add Gemfile to File Targets for PaperTrail gem installation, or remove PaperTrail from Constraints and document that promoted_at timestamp alone serves as the audit trail. Note: docs/security.md line 189 also references PaperTrail, so the doc would need updating if PaperTrail is removed from scope.
  • [SCOPE] Create architecture note arch-rails for component rails.