pal-e-docs

Review: ScheduleDigester team assignment + test updates (crew grouping 4/4)

review-1577-2026-06-23-r2 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Re-review of board item #1577 after refinement. The 3 [BODY] items from review-1577-2026-06-23 were addressed. One new [BODY] issue found: incomplete spec file list. The [LABEL] and [SCOPE] items from the prior review remain as systemic gaps (not blocking).

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Child of #260, sub-ticket 4/4, depends on sub-ticket 1
  • [x] Repo — ldraney/landscaping-assistant
  • [x] User Story — As an admin, schedule digestion assigns to team
  • [x] Context — explains ScheduleDigester, team_id requirement, uploads_controller call site
  • [x] File Targets — listed with modify/not-touch sections, specific spec files named
  • [x] Feature Flag — none (internal model change, appropriate)
  • [x] Acceptance Criteria — 5 criteria
  • [x] Test Expectations — service test + 5 spec files + run command
  • [x] Constraints — no OCR changes, consistent with current_team fallback, uploads_controller scoped
  • [x] Checklist — present
  • [x] Related — references parent epic, sub-ticket 1, sub-ticket 2

Traceability

  • [ ] story:auth label — MISMATCH (carried from prior review). This work is crew/team grouping, not authentication. [LABEL] Change story:auth to story:crew-grouping or similar. Systemic gap — not blocking this ticket.
  • [ ] story note — No crew-grouping story in project-landscaping-assistant user-stories table. [SCOPE] Create user story entry for crew/team grouping. Systemic gap — not blocking.
  • [x] arch:rails-app label — present on board item
  • [ ] arch note MISSING — no arch-rails-app note found in pal-e-docs. [SCOPE] Create architecture note arch-rails-app. Systemic gap — not blocking.
  • [x] Forgejo issue — ldraney/landscaping-assistant#265, state: open

Prior [BODY] Items — Verification

  • [x] [BODY] Specific spec files — FIXED. Issue now lists 5 named spec files instead of vague catch-all. However, list is incomplete (see below).
  • [x] [BODY] uploads_controller.rb as file target — FIXED. Listed as file target with clear description: "pass current_team.id as team_id: when calling ScheduleDigester.create_work_queue_items (one-line change at line 92)." Verified: line 92 is the correct call site (ScheduleDigester.create_work_queue_items call).
  • [x] [BODY] Unique index ownership — FIXED. "Files NOT to touch" section now states: "Unique index on work_queue_items — owned by sub-ticket 1 (#262)."

File Targets

  • [x] app/services/schedule_digester.rb — verified: exists, create_work_queue_items class method at line 38. WorkQueueItem.create! at line 54 passes work_date, property_id, position only. Correct modification target.
  • [x] app/controllers/uploads_controller.rb — verified: exists, line 92 calls ScheduleDigester.create_work_queue_items(week_start:, day_assignments:). Correct call site for passing team_id.
  • [x] spec/services/schedule_digester_spec.rb — verified: 1 WorkQueueItem.create! call (line 123)
  • [x] spec/requests/weeks_spec.rb — verified: 10 WorkQueueItem.create! calls
  • [x] spec/requests/uploads_spec.rb — verified: 1 WorkQueueItem.create! call (line 231)
  • [x] spec/requests/work_queue_items_spec.rb — verified: 33 WorkQueueItem.create!/new calls
  • [x] spec/models/paper_trail_spec.rb — verified: 1 WorkQueueItem.create! call (line 41)
  • [ ] spec/requests/days_spec.rb — MISSING from file targets. Contains 17 WorkQueueItem.create!/new calls. Will break when team_id becomes NOT NULL. [BODY]
  • [ ] spec/models/work_queue_item_spec.rb — MISSING from file targets. Contains 7 WorkQueueItem.create!/new calls. Will break when team_id becomes NOT NULL. [BODY]
No spec/factories/ directory exists — confirmed. All tests use inline creation.
Files NOT to touch section is clear and accurate: work_queue_items_controller.rb (sub-ticket 2), days_controller.rb/weeks_controller.rb/crew_controller.rb (sub-ticket 3), unique index (sub-ticket 1).

Repo Placement

OK. All file targets are in ldraney/landscaping-assistant. Single-repo change.

Dependencies

  • Hard dependency on sub-ticket 1 (#262, board item 1574) — Team model, team_id FK on work_queue_items. Currently in backlog, not started. This ticket cannot be implemented until #262 is merged.
  • Soft dependency on sub-ticket 2 (#263, board item 1575) — ApplicationController current_team helper. The uploads_controller change references current_team.id, which sub-ticket 2 creates. This is now a hard dependency for the uploads_controller change.
  • Dependencies documented in the issue Lineage section and Related section. Adequate.

Acceptance Criteria

  • [x] "ScheduleDigester.create_work_queue_items accepts and assigns team_id" — testable via service spec
  • [x] "uploads_controller.rb passes current_team.id when calling the digester" — testable, clear call site identified
  • [x] "If no team can be derived, fall back to the default team" — testable
  • [x] "All 5 spec files updated with team association" — testable, but should say "all 7 spec files" (see missing files above)
  • [x] "bundle exec rspec passes with zero failures" — testable
5 AC — within the 5-minute rule threshold.

Blast Radius

  • Missing spec files — spec/requests/days_spec.rb (17 calls) and spec/models/work_queue_item_spec.rb (7 calls) create WorkQueueItems without team_id. When sub-ticket 1 adds NOT NULL on team_id, these tests will fail. They must be listed as file targets.
  • Controller creates (days_controller, weeks_controller, work_queue_items_controller) — correctly scoped to sub-tickets 2 and 3. No overlap concern.
  • db/seeds.rb — 5 WorkQueueItem.create! calls without team_id. Will need updating but is appropriately outside this ticket's scope (seed data follows model changes).

Decomposition Assessment

1 service file + 1 controller file + 7 spec files across 1 repo. 5 acceptance criteria. Estimated agent work under 5 minutes. No decomposition needed.

Recommendation

  • [BODY] Add spec/requests/days_spec.rb (17 WorkQueueItem.create!/new calls) to the File Targets section. This file will break when team_id becomes NOT NULL.
  • [BODY] Add spec/models/work_queue_item_spec.rb (7 WorkQueueItem.create!/new calls) to the File Targets section. This file will break when team_id becomes NOT NULL.
  • [BODY] Update AC #4 from "All 5 spec files" to "All 7 spec files" to match the complete list.
  • [LABEL] Change story:auth to a crew-grouping story label (systemic gap, not blocking).
  • [SCOPE] Create user story entry for crew/team grouping on project-landscaping-assistant (systemic gap, not blocking).
  • [SCOPE] Create architecture note arch-rails-app (systemic gap, not blocking).