pal-e-docs

Review: Communications tab (lead/client): contact methods

review-1822-2026-07-17 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Board item #1822 — Forgejo issue ldraney/intelligentstaffingsystems#52. Type: Feature. Sprint B, 3 points.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, replaces old Messages tab #12
  • [x] Repo — ldraney/intelligentstaffingsystems
  • [x] User Story — lead/client wants clear contact methods
  • [x] Context — three action buttons (DM, Email, Appointment), role gating
  • [x] File Targets — 5 files listed (create or modify)
  • [x] Feature Flag — none
  • [x] Acceptance Criteria — 9 items
  • [x] Test Expectations — 5 items with run command
  • [x] Constraints — Stimulus, importmap, no Node
  • [x] Checklist — present
  • [x] Related — project-iss, #12
All required sections present.

Traceability

  • [x] story:communications label — present on board item
  • [ ] story note MISSING — communications is not listed in project-iss user-stories section. Closest key is messaging. Three board items (#1822, #1824, #1827) use this label, so a new story entry should be created rather than relabeling. [SCOPE] Create user story entry communications on project-iss user-stories section.
  • [x] arch:rails label — present on board item
  • [ ] arch note MISSING — no arch-rails note found in pal-e-docs. [SCOPE] Create architecture note arch-rails for the Rails application component.
  • [x] Forgejo issue — ldraney/intelligentstaffingsystems#52, open

File Targets

  • [x] app/controllers/communications_controller.rb — verified: EXISTS as stub from #49 (empty index action, 16 lines). Ticket says "create or modify" but should say "modify — add role branching".
  • [x] app/views/communications/index.html.erb — verified: EXISTS as stub from #49 (57 lines, already renders three static contact cards with SVG icons). Ticket should acknowledge this stub and describe what to change (add role-conditional rendering, extract partial).
  • [x] app/views/communications/_contact_cards.html.erb — verified: DOES NOT EXIST. Correctly identified as new file.
  • [ ] app/assets/stylesheets/communications.css — ISSUE: Does not exist yet (correctly), but the ticket does not mention that contact-card styles already live in app/assets/stylesheets/tabs.css (lines 87-133). tabs.css is staged for deletion in the current working tree. The ticket should state that existing styles should be migrated from tabs.css to communications.css and augmented with locked/disabled state styling.
  • [x] app/javascript/controllers/clipboard_controller.js — verified: DOES NOT EXIST. Correctly identified as new file. Importmap already has pin_all_from "app/javascript/controllers" so auto-discovery will work.
Existing tests at test/controllers/communications_controller_test.rb (55 lines) already cover basic access and card rendering from #49 stub. The ticket's test expectations build on these with role-specific assertions.

Repo Placement

OK — all work is in ldraney/intelligentstaffingsystems, matching the Forgejo issue repo.

Dependencies

  • #49 (tab bar restructure, board #1829) — in validation. Provides the stub controller, view, route, and nav entry this ticket builds on. This ticket depends on #49 being merged first.
  • #12 (messaging model, board #1792) — in backlog, decomposed. Scope overlap noted in issue. This ticket renders DM as a placeholder; #12's decomposed children (#40, #42) already shipped messaging CRUD and Turbo Streams.
  • #54 (admin communications, board #1824, sprint:C) — depends on the role branching this ticket introduces.
  • #55 (live DM, board #1825, sprint:C) — depends on the DM placeholder this ticket creates.
  • #57 (board #1827, sprint:C, story:communications) — depends on this ticket.
Dependencies are documented in the Related section but should explicitly note the #49 dependency as a blocker.

Acceptance Criteria

9 AC items, all verifiable by an agent:
  • AC 1-2: testable via endpoint assertions (already partially covered by existing tests)
  • AC 3: clipboard copy + toast — testable via Stimulus controller unit test or manual verification (noted in test expectations)
  • AC 4-5: DM locked/active by role — testable via assert_select with role-specific sign-in
  • AC 6: appointment link — testable via assert_select
  • AC 7: icon + label + description — testable via CSS class assertions
  • AC 8: responsive/mobile-first — testable via CSS inspection (grid breakpoint)
  • AC 9: admin different view — testable via role-specific rendering assertion
All criteria are concrete and testable. No missing criteria detected.

Blast Radius

  • tabs.css is staged for deletion — the contact-card styles living there need to be migrated to communications.css before or as part of this ticket. If tabs.css deletion lands first (from another PR), the existing stub view will lose its styling.
  • The messages controller (app/controllers/messages_controller.rb) already implements an admin? helper for role branching — this ticket should follow the same pattern for consistency.
  • No downstream API consumers affected. The iOS Turbo Native app will pick up UI changes automatically.

Decomposition Assessment

  • 5 file targets in 1 repo — below the >3 files across >2 repos threshold
  • 9 acceptance criteria — ABOVE the >5 threshold
  • Estimated agent work — borderline 5 minutes
  • However: work is cohesive (single controller/view/partial/CSS/stimulus in one feature area), existing stubs reduce scope, and all files are in the same domain
Borderline. The 9 AC count formally triggers the decomposition rule, but the work is cohesive enough that a single agent pass is feasible. Recommend keeping as-is but could split into (a) controller+view role branching + tests and (b) Stimulus clipboard + CSS if desired.

Recommendations

  • [SCOPE] Create user story entry communications on project-iss user-stories section. Three board items use this label. Suggested row: Key=communications, Backing="Epic 4 (broadened from messaging)", Role="Lead/Client/Admin", Success metric="Three contact methods displayed; DM gated behind client role".
  • [SCOPE] Create architecture note arch-rails for the Rails application component.
  • [BODY] File Targets: note that communications_controller.rb and index.html.erb already exist as stubs from #49. Describe what specifically needs to change (add role branching, extract partial, add interactivity) rather than "create or modify".
  • [BODY] File Targets: add note that contact-card styles currently live in tabs.css (lines 87-133) and must be migrated to communications.css.
  • [BODY] Related section: explicitly note #49 as a blocker dependency (stub controller/view/route must be merged first).