pal-e-docs

Review: Crew/team grouping with shared Today view

review-1573-2026-06-23 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Standalone, session-requested
  • [x] Repo -- ldraney/landscaping-assistant
  • [x] User Story -- present, well-formed
  • [x] Context -- good background on current global scope and motivation
  • [x] File Targets -- 6 files listed (1 new, 5 existing)
  • [x] Feature Flag -- "none" with rationale (data model change, not UI toggle)
  • [x] Acceptance Criteria -- 6 criteria
  • [x] Test Expectations -- 3 test items + run command
  • [x] Constraints -- 4 constraints listed
  • [x] Checklist -- present
  • [x] Related -- docs referenced
All required sections for the Feature template are present.

Traceability

  • [x] story:auth label -- "Auth" user story (login with Keycloak, role-appropriate tabs)
  • [x] story note verified -- "auth" entry found in project-landscaping-assistant user-stories section
  • [x] arch:rails-app label -- Rails application component
  • [ ] arch note MISSING -- [SCOPE] No note found for "arch-rails-app" in pal-e-docs. Create architecture note arch-rails-app for the Rails application component.
  • [x] Forgejo issue -- ldraney/landscaping-assistant#260, state: open

File Targets

  • [x] app/models/team.rb (new) -- confirmed does NOT exist yet, correct to create
  • [x] app/models/crew_member.rb -- verified: exists, currently has no team association. Has has_many :service_requests, validates name/keycloak_username/role. Adding belongs_to :team is appropriate.
  • [x] app/models/work_queue_item.rb -- verified: exists, currently belongs_to :property only. Adding belongs_to :team is appropriate.
  • [x] app/controllers/work_queue_items_controller.rb -- verified: exists, currently queries WorkQueueItem globally with no crew/team scoping. All queries (index, create, update, destroy, mark_other, reorder) need scoping.
  • [x] app/controllers/application_controller.rb -- verified: exists, already has current_crew_member helper. Adding current_team derived from current_crew_member.team is natural.
  • [x] db/migrate/ -- verified: migration directory exists, latest migration is 20260617000000

Repo Placement

OK. Issue filed on ldraney/landscaping-assistant, all file targets are in that repo. No cross-repo work needed.

Dependencies

  • No blocking dependencies found on the board. No items in in_progress or next_up that conflict.
  • Item #1572 (next_up) is a Keycloak issuer mismatch bug -- unrelated, no conflict.
  • Item #1403 (todo, "Property detail page: Projects section") -- unrelated.
  • The ticket correctly identifies that config/initializers/omniauth.rb should NOT be touched (separate bug fix).

Acceptance Criteria

6 AC items, all reasonably testable by an agent:
  • AC 1-4 are verifiable via request specs (create items as one user, verify visibility as another)
  • AC 5 (no regression) is verifiable by running existing test suite
  • AC 6 (seeds/migration) is verifiable by checking migration and seed files
However, AC 1-4 assume multi-user sessions which require Keycloak integration test setup. The test expectations section describes model and controller tests which are more realistic for agent verification.

Blast Radius

CRITICAL: The issue underestimates the blast radius. The File Targets list 4 controllers to modify but the actual codebase shows WorkQueueItem is queried in 4 controllers, not just 1:
  • app/controllers/work_queue_items_controller.rb -- listed in ticket
  • app/controllers/days_controller.rb -- NOT listed. Has 3 independent WorkQueueItem queries (show, add_to_queue, load_previously). All need team scoping.
  • app/controllers/weeks_controller.rb -- NOT listed. Queries WorkQueueItem in index and toggle_assign. All need team scoping.
  • app/controllers/uploads_controller.rb -- NOT listed (indirectly via ScheduleDigester). app/services/schedule_digester.rb creates WorkQueueItems without team_id.
Additionally:
  • app/controllers/crew_controller.rb -- queries CrewMember but does not filter by team. If team scoping is added, the Crew tab should show only the current team's members.
  • The work_queue_items table has a unique index on [work_date, property_id]. With team scoping, this uniqueness constraint may need to become [work_date, property_id, team_id] if different teams can queue the same property on the same day.

Decomposition Assessment

NEEDS DECOMPOSITION
  • File targets: 6 listed + at least 4 additional files not mentioned = ~10 files
  • Acceptance criteria: 6 AC + test expectations = 9 verifiable items
  • Controllers affected: 4 controllers + 1 service, spanning multiple concerns (model layer, scoping layer, view layer)
  • Estimated agent work: >5 minutes -- the migration alone (teams table + 2 FK columns + data backfill + unique index change) is non-trivial, and scoping 4 controllers is substantial
This exceeds the 5-minute rule on all three criteria. Route to skill-decompose-ticket.

Recommendation

  • [BODY] Add missing file targets: app/controllers/days_controller.rb, app/controllers/weeks_controller.rb, app/controllers/crew_controller.rb, app/services/schedule_digester.rb
  • [BODY] Address unique index change: the existing [work_date, property_id] unique index on work_queue_items may need to become [work_date, property_id, team_id] for multi-team support
  • [BODY] Add blast radius note about uploads/schedule_digester creating WorkQueueItems without team_id
  • [SCOPE] Create architecture note arch-rails-app for the Rails application component in pal-e-docs
  • [DECOMPOSE] 10 files across model/controller/service layers, 6+ AC, estimated >5 min agent work -- route to skill-decompose-ticket. Suggested split: (1) Migration + Team model + belongs_to associations, (2) ApplicationController current_team helper + WorkQueueItemsController scoping, (3) DaysController + WeeksController + CrewController scoping, (4) ScheduleDigester team-aware creation + test updates