pal-e-docs

Review: Communications tab (admin): incoming management

review-1827-2026-07-18 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets (issues found below)
  • [x] Feature Flag — none (acceptable for core admin view)
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:communications label — Epic 5: Communications (US-5.4)
  • [ ] story note MISSING — [SCOPE] The project-iss user-stories section on pal-e-docs has no "communications" key (only "messaging" for Epic 4). Create user story entry "communications" on project-iss covering Epic 5 (US-5.1–5.4).
  • [x] arch:rails label — Rails application component
  • [ ] arch note MISSING — [SCOPE] No arch-rails note found in pal-e-docs. Create architecture note arch-rails for the ISS Rails component.
  • [x] Forgejo issue — ldraney/intelligentstaffingsystems#57, open

File Targets

  • [x] app/controllers/communications_controller.rb — verified: exists, already has admin branch rendering :admin_index
  • [ ] app/views/communications/inbox.html.erb — ISSUE: File does not exist. The controller currently renders :admin_index which maps to admin_index.html.erb (already exists as placeholder). Ticket should target admin_index.html.erb or explicitly document the rename.
  • [ ] app/views/communications/_inbox_card.html.erb — new file (acceptable, to be created)
  • [x] app/assets/stylesheets/communications.css — verified: exists
  • [x] app/views/communications/index.html.erb (NOT touch) — verified: exists, lead/client contact cards
  • [x] app/controllers/messages_controller.rb (NOT touch) — verified: exists, has its own admin inbox at /messages
  • [ ] app/controllers/appointments_controller.rb (NOT touch) — does not exist: no appointment system exists in the codebase

Repo Placement

OK — issue filed on ldraney/intelligentstaffingsystems and all file targets are in that repo.

Dependencies

  • #54 — Appointment scheduling (board item 1824, backlog, sprint:C) — BLOCKER. The ticket expects to show appointment data in inbox cards, filter by Appointments, and provide "view appointment details" quick action. No Appointment model or controller exists. AC4, AC6, and parts of AC2–AC3 depend on this.
  • #55 — Live DM messaging (board item 1825, backlog, sprint:C) — Soft dependency. The existing Message model and messages/inbox.html.erb already provide DM thread data, but #55 adds real-time features. Not a blocker for basic inbox display.
  • #56 — CRM tab (board item 1826, backlog, sprint:C) — AC7 references "jump to CRM business card." CRM ticket is in backlog. Quick action link can be added but destination may not exist.
  • #52 — Communications tab lead/client (done) — Prerequisite met. Controller branching and lead/client view already work.

Acceptance Criteria

8 acceptance criteria total. Assessment:
  • AC1 (admin sees inbox) — verifiable, controller branch already exists
  • AC2 (cards sorted by activity) — partially verifiable; DM activity sortable via Message model, but appointment activity requires #54
  • AC3 (card content) — same dependency on appointments for "appointment summary"
  • AC4 (filter: All/DMs/Appointments) — BLOCKED by #54, no appointment data to filter
  • AC5 (quick action: open DM) — verifiable, messages path exists
  • AC6 (quick action: view appointment) — BLOCKED by #54, no appointment controller
  • AC7 (quick action: jump to CRM) — soft dependency on #56; link target may 404
  • AC8 (lead/client still sees contact buttons) — verifiable, already works
Test commands are real (rails test test/controllers/communications_controller_test.rb). Existing test file already covers role-based access.

Blast Radius

Low. The change is isolated to the communications admin view. The existing messages/inbox.html.erb pattern at /messages already demonstrates the admin inbox pattern. No downstream consumers affected. Existing tests for lead/client communications access will confirm no regression.

Decomposition Assessment

4 file targets in 1 repo, 8 acceptance criteria (exceeds 5 threshold). However, the primary issue is the dependency blocker (#54 appointments), not size. If the ticket is scoped to DM-only inbox first (dropping AC4 appointment filter and AC6 appointment quick action), it fits within a single agent pass at ~5 minutes. No decomposition needed — dependency resolution recommended instead.

Recommendation

  • [BODY] Fix file target: app/views/communications/inbox.html.erb should be app/views/communications/admin_index.html.erb (the controller already renders :admin_index and the placeholder file exists)
  • [BODY] Document dependency on #54 (Appointment scheduling). Options: (a) reduce scope to DM-only inbox initially, adding appointment integration after #54 lands; or (b) mark this ticket as blocked by #54.
  • [BODY] AC4 and AC6 should be deferred or marked as depending on #54
  • [SCOPE] Create user story entry "communications" on project-iss user-stories section (covering Epic 5: US-5.1–5.4)
  • [SCOPE] Create architecture note arch-rails for the ISS Rails component