Review: CRM tab (admin): business pipeline with search and promotion

review-1826-2026-07-25 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag
  • [x] Acceptance Criteria (13 items)
  • [x] Test Expectations (10 items)
  • [x] Constraints
  • [x] Checklist
  • [x] Related
  • [x] Dependencies (bonus section, not required)
All 12 required sections present. Template is complete.

Traceability

  • [x] story:crm label -- maps to Epic 7: CRM (Admin) in docs/user-stories.md (US-7.1, US-7.2, US-7.3)
  • [ ] story note MISSING -- [SCOPE] project-iss user-stories section has "admin" key (Epic 6) but no "crm" key for Epic 7. Add "crm" story entry to project-iss user-stories section.
  • [x] arch:rails label -- arch-rails note exists in pal-e-docs (verified)
  • [x] arch note verified -- arch-rails note active with ISS Rails Architecture content
  • [x] Forgejo issue -- ldraney/intelligentstaffingsystems#56, state: open

File Targets

  • [x] app/controllers/crm_controller.rb -- verified: stub exists with require_role :admin and empty index method. Ticket correctly references existing stub.
  • [x] app/views/crm/index.html.erb -- verified: placeholder "coming soon" template exists (35 lines).
  • [x] app/views/crm/_business_card.html.erb -- to be created. OK.
  • [x] app/views/crm/show.html.erb -- to be created. OK.
  • [x] app/assets/stylesheets/crm.css -- to be created. OK.
  • [x] app/services/keycloak_admin_service.rb -- verified: exists with get_user/update_user methods. Note: make_request only supports :get and :put; needs :post for realm-role-mapping endpoint. Not mentioned in ticket.
  • [x] config/routes.rb -- verified: CRM stub route at line 42 (get "/crm" only). Needs expansion for show and promote actions.
  • [x] Gemfile -- verified: exists, no paper_trail currently.
  • [ ] app/models/lead.rb -- MISSING from file targets. AC requires search filtering (ILIKE) and test expectations list "Model test: Lead.search scope (name, email, business_name)". Model needs a search scope added.
  • [ ] test/controllers/crm_controller_test.rb -- MISSING from file targets. File exists with 7 stub tests for the placeholder, needs expansion with 10 new tests per test expectations.

Repo Placement

OK. All work is in ldraney/intelligentstaffingsystems. Keycloak admin calls go through the existing KeycloakAdminService already in this repo. No cross-repo work needed.

Dependencies

  • #54 (Appointment scheduling) -- board item #1824, currently in validation column. Correctly handled as soft dependency: "appointment data is additive, not blocking."
  • #51 (Projects tab) -- done. Project requests model and controller exist.
  • #6 (Keycloak auth) -- done. KeycloakAdminService exists.
  • #14 (Admin user management) -- superseded by this ticket (documented in Lineage).
  • No undocumented blockers found.

Acceptance Criteria

13 acceptance criteria, all verifiable by an agent. Test commands reference real test paths (rails test test/controllers/crm_controller_test.rb). Observations:
  • No explicit AC for Lead.search scope behavior (tests reference it but AC does not).
  • No AC for graceful handling when KeycloakAdminService is not configured (dev environment without Keycloak admin credentials).
  • Promotion audit trail (PaperTrail) has an AC but no corresponding test expectation.

Blast Radius

  • KeycloakAdminService#make_request currently supports only :get and :put. Adding :post changes a shared utility method -- low risk, simple case statement extension.
  • Adding paper_trail gem impacts Docker builds (bundle install in container image). Agent must run bundle install and update Gemfile.lock.
  • Lead.role enum (lead/client/admin) and promoted_at column already exist in schema -- promotion model support is already in place. No migration needed for those fields.
  • No similar promotion patterns exist elsewhere to break. No downstream consumers affected.

Decomposition Assessment

13 ACs across 8+ files in 1 repo. Exceeds the >5 AC threshold. However, per user preference, inclusive tickets are preferred over decomposition. The 8-point sizing is appropriate. All work is in a single repo and a single domain (Rails CRM tab). No decomposition needed.

Recommendations

  • [BODY] Add app/models/lead.rb to File Targets -- needs Lead.search scope per AC #2 ("Search bar filters businesses") and Test Expectations ("Model test: Lead.search scope").
  • [BODY] Add test/controllers/crm_controller_test.rb to File Targets -- existing file (7 stub tests) needs expansion with new tests per Test Expectations section.
  • [BODY] Note in app/services/keycloak_admin_service.rb file target description that make_request needs :post method support for the realm-role-mapping endpoint.
  • [SCOPE] Add "crm" story entry to project-iss user-stories section. The repo has Epic 7: CRM (Admin) in docs/user-stories.md, but the pal-e-docs project page only lists "admin" (Epic 6). The board item correctly uses story:crm per the repo docs.