pal-e-docs

Review: Sort comments by most recent activity (newest/recently replied first)

review-1901-2026-07-25 Review

review ready
Project

Verdict: READY

Re-review of board item #1901. All three issues from the prior NEEDS_REFINEMENT review have been resolved. Scope is solid, file targets verified, traceability complete.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — depends on #303
  • [x] Repo — ldraney/landscaping-assistant
  • [x] User Story — crew lead/admin wants newest-activity-first
  • [x] Context — explains current oldest_first behavior and desired MAX(created_at) sort
  • [x] File Targets — 4 modify targets, 2 do-not-touch targets
  • [x] Feature Flag — gated by comment_threads from #303
  • [x] Acceptance Criteria — 5 testable criteria
  • [x] Test Expectations — 2 unit tests + bundle exec rspec
  • [x] Constraints — dependency ordering, N+1 avoidance, reply sort preserved
  • [x] Checklist — standard PR/tests/no-unrelated
  • [x] Related — project + #303 dependency

Traceability

  • [x] story:comments label — "Threaded discussion on property detail pages with photo attachments, reply threads, and activity-based sort"
  • [x] story note verified — found in project-landscaping-assistant user-stories section; entry explicitly lists #304 as a child ticket
  • [x] arch:rails-app label — shared Rails app deployment pattern
  • [x] arch note verified — arch-rails-app note exists and is active
  • [x] Forgejo issue — ldraney/landscaping-assistant#304, state: open

File Targets

  • [x] app/models/property_comment.rb — verified: has oldest_first scope at line 17, no threading code yet (correct, depends on #303)
  • [x] app/views/properties/show.html.erb — verified: renders @comments at lines 65-80
  • [x] app/controllers/properties_controller.rb — verified: loads comments with oldest_first at line 79, includes eager loading for crew_member, service_request, photos
  • [x] app/views/property_comments/create.turbo_stream.erb — verified: uses turbo_stream.append at line 3, ticket correctly describes need for conditional prepend/append

Repo Placement

OK — issue filed on ldraney/landscaping-assistant, all file targets in same repo. Single-repo change.

Dependencies

  • #303 (threaded replies) — hard dependency, documented in both Lineage and Constraints. Introduces parent_id column and comment_threads feature flag that this ticket requires. #303 is board item #1900, currently in backlog. #303 also lists #304 as downstream in its Dependencies section. Mutual awareness confirmed.
  • Board ordering — both #303 and #304 are in backlog. Board workflow must move #303 to done before #304 moves to in_progress.

Acceptance Criteria

All 5 criteria are agent-verifiable:
  • AC 1-3: testable via unit tests with controlled timestamps — scope returns correct order
  • AC 4: testable by toggling comment_threads flag and asserting fallback to oldest_first
  • AC 5: testable by checking turbo stream response for prepend vs append action
Test command bundle exec rspec matches the arch-rails-app CI pipeline (Woodpecker runs rspec).

Blast Radius

Low. oldest_first scope is only referenced in two places: the model definition (line 17) and the properties controller (line 79). No other controllers, views, or models reference this scope. Comment sort is isolated to the property detail page. No downstream consumers affected.

Decomposition Assessment

No decomposition needed:
  • 4 file targets in 1 repo — under threshold
  • 5 acceptance criteria — at boundary, acceptable
  • Estimated agent time: ~3-4 minutes (add scope, update controller, update view reference, adjust turbo stream) — within 5-minute rule
  • 2 story points — appropriately sized

Prior Review Issues — Resolution Status

  • story:comments label had no backing user story — RESOLVED: "comments" entry now exists in project-landscaping-assistant user-stories section with key "comments", role "All roles", and explicit child reference to #304
  • Wrong test command (rails test vs bundle exec rspec) — FIXED: issue body now reads bundle exec rspec
  • Missing file target for create.turbo_stream.erb — FIXED: file target now listed with conditional prepend/append guidance matching comment_threads flag state

Recommendation

No action needed. Ticket is ready to move to todo once #303 (threaded replies) has progressed.