pal-e-docs

Review: Bug: Camera photo on property comments crashes the app

review-1899-2026-07-25-r2 Review

review approved
Project

Verdict: APPROVED

Re-review of board item #1899 — ldraney/landscaping-assistant#302. All 7 issues from the first review (review-1899-2026-07-25, verdict NEEDS_REFINEMENT) have been addressed. The issue body now accurately reflects the codebase state, all required template sections are present, and traceability is complete.

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — "Standalone — discovered during usage session."
  • [x] Repo — ldraney/landscaping-assistant
  • [x] What Broke — present (titled "What happened")
  • [x] Repro Steps — present (titled "Steps to reproduce")
  • [x] Expected Behavior — present, correctly notes photo upload feature IS implemented
  • [x] Environment — present: iOS turbo-ios wrapper (primary), web browser (secondary), crash reports noted
  • [x] Investigation Notes — present, accurately documents the full photo stack (model, view, controller, feature flag)
  • [x] File Targets — present, split by repo with cross-repo dependency noted
  • [x] Blast Radius — present, documents weeks/index.html.erb:33
  • [x] Acceptance Criteria — present, 5 ACs focused on crash fix
  • [x] Test Expectations — present with manual test steps and bundle exec rspec
  • [x] Constraints — present
  • [x] Checklist — present
  • [x] Related — present

Traceability

  • [x] story:comments label — present on board item
  • [x] story note verified — "comments" entry found in project-landscaping-assistant user-stories section: "Threaded discussion on property detail pages with photo attachments, reply threads, and activity-based sort." References #302 as child of umbrella #303.
  • [x] arch:rails-app label — present on board item
  • [x] arch note verified — arch-rails-app note exists and is active in pal-e-docs
  • [x] Forgejo issue — ldraney/landscaping-assistant#302, state: open

File Targets

  • [x] app/views/property_comments/_form.html.erb — verified: line 8 has <input type="file" name="property_comment[photos][]" multiple accept="image/*"> gated by feature_enabled?(:photo_uploads) at line 5. Issue accurately describes this.
  • [x] app/controllers/property_comments_controller.rb — verified: line 17 has @comment.photos.attach(comment_params[:photos]) if feature_enabled?(:photo_uploads) && comment_params[:photos].present?. Strong params permit photos: [] at line 45. Issue accurate.
  • [x] app/models/property_comment.rb — verified: line 12 has has_many_attached :photos. ALLOWED_CONTENT_TYPES, MAX_FILE_SIZE (20MB), MAX_PHOTOS (5) all present. Issue accurate.
  • [x] app/views/weeks/index.html.erb:33 (blast radius) — verified: line 33 has <input type="file" name="upload[photo]" accept="image/*"> with Stimulus upload controller.
  • [x] iOS repo (ldraney/landscaping-assistant-ios) — cross-repo dependency clearly documented. Specific files to investigate listed: Info.plist, WKWebView config, turbo-ios session config.
  • [x] lib/tasks/feature_flags.rake line 12 — verified: photo_uploads flag seeded enabled: true.

Repo Placement

OK. Issue filed on ldraney/landscaping-assistant (Rails). Cross-repo dependency with ldraney/landscaping-assistant-ios is clearly documented in File Targets and Acceptance Criteria. AC #5 explicitly requires a linked PR in the iOS repo if changes are needed there.

Dependencies

No blocking dependencies. No items in in_progress column. Related board items in backlog:
  • #1900 — backlog, type:feature, story:comments (sibling under umbrella #303)
  • #1901 — backlog, type:feature, story:comments (sibling under umbrella #303)
  • #1405 — backlog, "Property comments: timestamped discussion for all roles" (parent feature)

Acceptance Criteria

5 ACs, all verifiable:
  • AC1: No crash on "Add photos" — manual test on iOS, clear pass/fail
  • AC2: No crash on "Upload Schedule" (blast radius) — manual test on iOS, clear pass/fail
  • AC3: Photo uploads end-to-end — manual test: select, submit, verify display
  • AC4: Crash reports examined and root cause documented — verifiable in PR body
  • AC5: Cross-repo PR if iOS changes needed — verifiable by checking for linked PR
Test Expectations section provides manual test steps and bundle exec rspec for automated suite.

Blast Radius

Correctly documented. app/views/weeks/index.html.erb:33 uses the same accept="image/*" pattern on a file input. AC #2 explicitly covers this. The Stimulus controller (data-action="change->upload#submit") on the schedule upload may behave differently but the underlying <input type="file"> trigger is identical.

Decomposition Assessment

3 Rails file targets (investigation, not modification) + iOS cross-repo dependency. 5 acceptance criteria (at threshold, not over). Core work is focused: examine crash reports, identify root cause in turbo-ios/WKWebView layer, fix. Single agent pass is feasible (<5 min for the Rails-side investigation; iOS fix may extend but is scoped by AC #5 to a linked PR). No decomposition needed.

First Review Issues — Verification

# Issue Status
1 [BODY] Rewrite investigation notes — photo support IS implemented FIXED. Notes now accurately document model, view, controller, and feature flag.
2 [BODY] Add Lineage section FIXED. "Standalone — discovered during usage session."
3 [BODY] Add Environment section FIXED. iOS turbo-ios primary, web browser secondary, crash reports noted.
4 [BODY] Fix file targets — cross-repo dependency FIXED. iOS repo identified with specific files. Cross-repo note added.
5 [BODY] Rewrite Acceptance Criteria FIXED. 5 ACs focused on crash fix, covering blast radius and cross-repo.
6 [SCOPE] Create user story "comments" on project page FIXED. Story entry exists with summary and child issue references.
7 [BODY] Document blast radius FIXED. Dedicated section with weeks/index.html.erb:33 reference.

Recommendation

No action needed. Ticket is ready for development.