pal-e-docs

Review: Per-property photos, project tracking, and photo optimization

review-1338-2026-07-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [ ] Feature Flag -- MISSING. This adds user-visible functionality (per-property photo gallery, project tracking). The repo has docs/feature-flags.md. Section required by template-issue-feature.
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:property-crud label -- "Add/edit/delete client properties" found in project-landscaping-assistant user-stories section
  • [x] story note verified -- found in project-landscaping-assistant user-stories section
  • [x] arch:rails-app label -- Rails App (Shared Pattern)
  • [x] arch note verified -- arch-rails-app note exists in pal-e-docs
  • [x] Forgejo issue -- ldraney/landscaping-assistant#116, open

File Targets

  • [x] app/views/layouts/application.html.erb -- verified exists. BUT: no Photos tab in bottom nav to remove (see Recommendation).
  • [x] app/views/properties/show.html.erb -- verified exists. No upload/photo content yet, correct target for adding gallery.
  • [x] app/models/upload.rb -- verified exists. Currently has no belongs_to :property. Correct target.
  • [x] app/models/property.rb -- verified exists. Currently has no has_many :uploads. Correct target.
  • [x] db/migrate/ -- verified exists. Uploads table currently has no property_id column. Migration needed.
  • [x] app/controllers/uploads_controller.rb -- verified exists. Currently standalone, not scoped to property.
  • [x] config/routes.rb -- verified exists. Uploads currently standalone: resources :uploads, not nested under properties.
  • [x] app/models/project.rb (new) -- does not exist yet, correctly marked as new file.

Repo Placement

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

Dependencies

  • Board item #1403 "Property detail page: Projects section (depends on #122)" (3pt, backlog) covers project tracking on the property page. This ticket (#1338) also includes "per-property project tracking." Potential overlap not documented in either ticket.
  • No items currently in_progress that block this ticket.
  • Ticket #107 (auth/tab visibility) is referenced in Related but already landed.

Acceptance Criteria

7 acceptance criteria. Most are verifiable by an agent (model associations, route scoping, file size checks). However:
  • AC #1 ("Photos tab removed from bottom nav") targets UI that does not exist. Current bottom nav tabs are: Today, Week, Profile, Properties, Crew. There is no Photos tab.
  • AC #7 ("Existing photos migrated to property association") is ambiguous -- existing uploads are schedule photos (with week_start column) from the Week view. Migrating them to a property doesn't make semantic sense. The issue should specify whether these become orphaned, remain standalone, or are handled differently.

Blast Radius

  • Week view schedule uploads will break. The Week view (app/views/weeks/index.html.erb) has a form posting to uploads_path for schedule photo uploads (behind the schedule_digest feature flag). If uploads are nested under properties (/properties/:id/uploads), the standalone uploads_path helper disappears, breaking this form. The issue does not mention the schedule upload workflow at all.
  • Existing upload specs. spec/requests/uploads_spec.rb and spec/models/upload_spec.rb all test standalone upload behavior. Route nesting will require updating all request specs.
  • Property comments already support photo attachments. property_comments (nested under properties in routes) already accept image uploads. The relationship between comment photos and the new per-property photo gallery should be clarified to avoid UX confusion.

Decomposition Assessment

7 AC, 8 file targets, 1 repo. No decomposition needed.

Recommendation

  • [BODY] Add ### Feature Flag section -- new user-visible gallery and project tracking should be flagged. Consult docs/feature-flags.md for conventions.
  • [BODY] Fix Context section -- "The current Photos tab" does not exist. Current bottom nav has 5 tabs (Today, Week, Profile, Properties, Crew). The upload functionality lives at /uploads as a standalone page and via the "Upload Schedule" button on the Week view. Rewrite to reflect actual current state.
  • [BODY] Fix AC #1 -- "Photos tab removed from bottom nav" targets nonexistent UI. Replace with accurate criterion (e.g., remove standalone /uploads index route, or clarify what actually changes in nav).
  • [BODY] Address schedule upload blast radius -- the Week view's "Upload Schedule" form posts to standalone uploads_path. If uploads are nested under properties, specify how schedule uploads (which have no property association) should work. Options: keep a standalone route for schedule uploads, or make schedule uploads a separate model.
  • [BODY] Clarify AC #7 migration strategy -- existing uploads are schedule photos with week_start data. Specify whether they become orphaned, stay standalone, or get a nullable property_id.
  • [BODY] Document relationship with #1403 -- board item #1403 "Property detail page: Projects section" also covers project tracking on the property page. Clarify whether #1338 supersedes #1403, or whether #1403 builds on #1338's project model.