Review: Property detail page: Projects section (depends on #122)

review-1403-2026-06-07 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- present, references #126, #121, #122
  • [x] Repo -- ldraney/landscaping-assistant
  • [x] User Story -- well-formed (crew lead or admin)
  • [x] Context -- clear: admin/lead-facing display, references architecture doc
  • [x] File Targets -- 5 targets listed
  • [x] Feature Flag -- none (correct for read-only display)
  • [x] Acceptance Criteria -- 6 criteria
  • [x] Test Expectations -- present with run command
  • [x] Constraints -- CSS guide, mobile-first, read-only scope
  • [x] Checklist -- present
  • [x] Related -- present with cross-references

Traceability

  • [x] story:auth label -- "auth" story present on board item
  • [x] story note verified -- "auth" story found in project-landscaping-assistant user-stories section (All roles: "Log in with Keycloak, see role-appropriate tabs and features")
  • [x] arch:rails-app label -- present on board item
  • [ ] arch note MISSING -- [SCOPE] No arch-rails-app note found in pal-e-docs. Create architecture note arch-rails-app for the Rails application component.
  • [x] Forgejo issue -- ldraney/landscaping-assistant#176, state: open

File Targets

  • [x] app/views/properties/show.html.erb -- verified: exists, currently shows property details with dl, actions, and address form. No projects section yet. Correct update target.
  • [x] app/views/service_requests/_property_projects.html.erb -- new file: directory app/views/service_requests/ does not exist yet. Will need to be created. This is expected for a new partial.
  • [x] app/assets/stylesheets/application.css -- verified: exists (1630 lines). Has existing design token system, status badges (.role-badge, .status-indicator), and component organization pattern. Good target for new project list styles.
  • [x] app/controllers/properties_controller.rb -- verified: exists. show action currently loads only @property = Property.find(params[:id]). Needs eager-load of service_requests. Property model at HEAD already has has_many :service_requests.
  • [x] spec/requests/properties_spec.rb -- verified: exists (284 lines). Has existing show specs (lines 46-79). Correct update target.

Repo Placement

OK. Issue filed on ldraney/landscaping-assistant, all file targets are in the same repo. Single-repo change.

Dependencies

  • #122 (ServiceRequest model) -- board item #1347, column: qa. Forgejo issue state: closed. Commit 5391056 merged at HEAD. ServiceRequest model, migration, and Property association all verified at HEAD. Dependency satisfied.
  • #126 (CSS refresh) -- board item #1351, column: qa. Commit 26c0e5f merged at HEAD. Property detail page layout is updated. Dependency satisfied.
  • #123 (Client request form) -- board item #1348, column: backlog. Sibling ticket, not a dependency. No conflict.
  • #179 (Admin project workflow) -- board item #1404, column: backlog. Depends ON this ticket (downstream). No conflict.

Acceptance Criteria

6 acceptance criteria. All are verifiable by an agent:
  • [x] "Projects/Requests section visible" -- testable via request spec (check response body)
  • [x] "Shows active requests grouped" -- testable by creating ServiceRequest fixtures with various statuses
  • [x] "Shows completed/declined separately" -- testable
  • [x] "Each request shows description, type badge, status badge, price" -- testable via response body assertions
  • [x] "Empty state" -- testable with no-request property
  • [x] "Works on mobile (44px touch targets)" -- CSS review only; no automated mobile test infra currently. Acceptable at this scope.

Blast Radius

  • No existing service_request references in views or controllers -- this is the first view layer usage. Clean.
  • ServiceRequest model at HEAD has belongs_to :crew_member -- the display will need to handle this association (e.g., showing who the request is assigned to). The ticket doesn't mention displaying crew member info, which is fine for a read-only initial display.
  • No downstream consumers of the show page partial yet.

Decomposition Assessment

5 file targets in 1 repo, 6 acceptance criteria. Borderline on the >5 AC rule, but the work is straightforward: one partial, one controller load, one CSS block, one spec update. Estimated agent time: ~3-4 minutes. No decomposition needed.

Recommendations

  • [BODY] Fix reference to docs/service-requests.md -- this file does not exist in the repo (not at HEAD, not in any branch). The issue body and Constraints section reference it. Either create the doc (separate ticket) or remove the reference and inline the status values: requested, quoted, paid, scheduled, completed, declined (from ServiceRequest::STATUSES). The model itself is the source of truth.
  • [SCOPE] Create architecture note arch-rails-app in pal-e-docs for the Rails application component. This is a platform-wide gap, not specific to this ticket.