pal-e-docs

Review: Threaded replies on property comments

review-1900-2026-07-25 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, discovered during property details UX review
  • [x] Repo — ldraney/landscaping-assistant
  • [x] User Story — well-formed (crew lead / admin wants threaded replies)
  • [x] Context — detailed background on flat comment problem
  • [x] File Targets — 8 modify/create targets + 2 exclusions
  • [x] Feature Flag — comment_threads, global, disabled by default, all roles, removal after 2 weeks
  • [x] Acceptance Criteria — 7 items
  • [x] Test Expectations — 4 items + run command
  • [x] Constraints — 3 constraints documented
  • [x] Checklist — present
  • [x] Related — present
  • [x] Dependencies — present (extra section, not in template but valuable)
All required template sections present. No gaps.

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 lists #303 as umbrella issue with children #302 and #304.
  • [x] arch:rails-app label — shared Rails app deployment pattern
  • [x] arch note verified — arch-rails-app note exists in pal-e-docs (architecture note, status: active)
  • [x] Forgejo issue — ldraney/landscaping-assistant#303, state: open
All three traceability legs verified with backing notes.

File Targets

  • [x] db/migrate/xxx_add_parent_id_to_property_comments.rb — new file (migration to create), expected not to exist yet
  • [x] app/models/property_comment.rb — verified: currently has belongs_to :property, :crew_member, :service_request. No parent reference. Correct starting state.
  • [x] app/views/property_comments/_comment.html.erb — verified: renders flat <li> items with comment-item class, author name, timestamp, service request badge, body, and photos
  • [x] app/views/property_comments/_form.html.erb — verified: posts to property_property_comments_path, no parent_id field present
  • [x] app/controllers/property_comments_controller.rb — verified: permits only :body and photos:[]. Needs parent_id added to permitted params.
  • [x] app/views/properties/show.html.erb — verified: line 68-71 renders all @comments in comments-list ul without filtering
  • [x] app/views/property_comments/create.turbo_stream.erb — verified: appends all new comments to comments-list. Issue correctly identifies this needs conditional targeting for replies.
  • [x] app/controllers/properties_controller.rb line 79 — verified: exact match. Line 79 is @comments = @property.property_comments.oldest_first.includes(:crew_member, :service_request).with_attached_photos. No parent_id filtering. Issue description accurate.
All 8 file targets verified. Every existing file exists and contains exactly what the issue describes. Line references are accurate.

Repo Placement

OK. Issue filed on ldraney/landscaping-assistant, all file targets are in the same repo. No cross-repo concerns.

Dependencies

  • #302 (board item #1899) — Bug: camera photo on property comments crashes the app. Column: backlog, type:bug, story:comments, 3 points. Touches same views (_comment.html.erb, _form.html.erb). Merge order coordination documented in issue. No hard blocker but merge conflict risk.
  • #304 (board item #1901) — Sort comments by most recent activity. Column: backlog, type:feature, story:comments, 2 points. Depends on parent_id column from this ticket. Must land after #303. Correctly sequenced.
Dependencies are documented and correctly identified. Neither blocks #303 from starting, but #302 should ideally land first to avoid view conflicts.

Acceptance Criteria

7 acceptance criteria, all concrete and agent-verifiable:
  • AC 1-5: testable via model scopes, view rendering, and validation checks
  • AC 6: testable via Turbo Stream response inspection (integration test)
  • AC 7: testable via feature flag toggle (existing feature_enabled? infrastructure confirmed in codebase)
Test expectations align with AC. Run command bundle exec rspec is correct for this Rails project. Feature flag infrastructure exists (FeatureFlag model, platform controller, feature_enabled? helper already used in comment views).

Blast Radius

Contained. PropertyComment is referenced in:
  • property.rb (has_many :property_comments) — no change needed
  • crew_member.rb (has_many :property_comments) — no change needed
  • profile_controller.rb — references in comments only (lines 57, 61), no active code
No threading/parent_id/reply code exists anywhere in the codebase. Clean greenfield addition. No downstream consumers beyond the views already listed in file targets.

Decomposition Assessment

  • File targets: 8 files in 1 repo — does not trigger cross-repo threshold (>3 files across >2 repos)
  • Acceptance criteria: 7 — exceeds >5 threshold
  • Estimated agent work: migration + model associations + model validation + controller params + 3 view templates + feature flag wiring + tests = estimated 10-15 minutes — exceeds 5-minute threshold
NEEDS DECOMPOSITION — 7 AC and estimated work time both exceed thresholds. Suggested split:
  • Sub-ticket A (backend): migration, model (associations + validation + scope), controller (permit parent_id, scope reply creation), feature flag registration. AC: 1, 5, 7. ~5 min.
  • Sub-ticket B (frontend): _comment.html.erb (reply link, nested rendering), _form.html.erb (parent_id hidden field), show.html.erb (top-level filter), create.turbo_stream.erb (conditional targeting). AC: 2, 3, 4, 6. ~5 min. Depends on Sub-ticket A.

Recommendation

  • [DECOMPOSE] 7 AC across 8 files, estimated 10-15 min agent work. Route to skill-decompose-ticket. Suggested split: backend (migration + model + controller + flag) and frontend (views + turbo stream).