Unify today tab search and quick-add into single input #56

Merged

ldraney merged 2 commits from worktree-today-tab into main 2026-06-02 12:36:28 +00:00

Conversation 1 Commits 2 Files changed 5 +25 -31

ldraney commented 2026-06-02 12:32:39 +00:00

Owner

Copy link

Summary

• Single input on today tab now searches existing properties AND creates new ones
• Input clears after add, filter resets to show all properties again
• Service filter chips removed from today tab (belong on properties page only)

Changes

• app/views/work_queue_items/index.html.erb -- merged quick-add and search into one input inside filter controller scope, removed service filter chips and separate search input
• app/javascript/controllers/filter_controller.js -- added clearInput() method for post-submit reset
• app/controllers/work_queue_items_controller.rb -- removed unused @services query
• config/puma.rb -- added activate_control_app to fix yabeda-puma-plugin crash in local dev
• docker-compose.yml -- parameterized port via WEB_PORT env var for worktree use

Test Plan

• Type partial name in input — Recent list filters in real time (case-insensitive)
• Submit a new property name — property created, queued, input cleared, filter reset
• Add existing property from filtered list via "+" button
• Service filter chips still work on properties/manage page
• Day navigation still works
• Drag-to-reorder still works

Review Checklist

• No secrets committed
• No unnecessary file changes
• Commit messages are descriptive

Related Notes

• Closes #55
• landscaping-assistant -- project

## Summary - Single input on today tab now searches existing properties AND creates new ones - Input clears after add, filter resets to show all properties again - Service filter chips removed from today tab (belong on properties page only) ## Changes - `app/views/work_queue_items/index.html.erb` -- merged quick-add and search into one input inside filter controller scope, removed service filter chips and separate search input - `app/javascript/controllers/filter_controller.js` -- added `clearInput()` method for post-submit reset - `app/controllers/work_queue_items_controller.rb` -- removed unused `@services` query - `config/puma.rb` -- added `activate_control_app` to fix yabeda-puma-plugin crash in local dev - `docker-compose.yml` -- parameterized port via `WEB_PORT` env var for worktree use ## Test Plan - [ ] Type partial name in input — Recent list filters in real time (case-insensitive) - [ ] Submit a new property name — property created, queued, input cleared, filter reset - [ ] Add existing property from filtered list via "+" button - [ ] Service filter chips still work on properties/manage page - [ ] Day navigation still works - [ ] Drag-to-reorder still works ## Review Checklist - [ ] No secrets committed - [ ] No unnecessary file changes - [ ] Commit messages are descriptive ## Related Notes - Closes #55 - `landscaping-assistant` -- project

ldraney added 1 commit 2026-06-02 12:32:39 +00:00

Unify today tab search and quick-add into single input ... bce2773519

The quick-add input now doubles as the property search filter — typing
filters the Recent list in real time, and submitting creates a new
property. Input clears after add. Removed service filter chips from the
today tab (they belong on the properties page only).

Also fixes yabeda-puma-plugin crash in local dev (activate_control_app)
and parameterizes the Docker compose port for worktree use.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented 2026-06-02 12:34:18 +00:00

Author

Owner

Copy link

PR #56 Review

DOMAIN REVIEW

Tech stack: Rails 7 / Hotwire (Turbo + Stimulus) / ERB views

Stimulus controller wiring -- correct. The filter controller scope in index.html.erb now wraps the quick-add form, the queue list, and the Recent property list. The input has data-filter-target="search" and data-action="input->filter#filter", and the property items each have data-filter-target="property". This means typing in the unified input filters the Recent list in real time. The queue items (_queue_item.html.erb) do not carry data-filter-target="property", so they are correctly unaffected by the filter. Wiring is sound.

clearInput() action -- correct. The form uses data-action="turbo:submit-end->filter#clearInput". The turbo:submit-end event fires on the form element after a Turbo submission completes (success or failure). Since the form is inside the filter controller scope, Stimulus will dispatch to filter#clearInput which clears the search target and calls filter() to reset visibility. This works.

Turbo Stream compatibility -- correct. create.turbo_stream.erb appends to #queue-list and prepends/replaces in #property-list. Both IDs remain in the DOM and are now inside the filter controller scope. Newly prepended property items will have data-filter-target="property" from the partial, so filter() after clearInput will show them. No breakage.

Removing @services from WorkQueueItemsController#index -- safe. The @services variable was only used in the service filter chips block that this PR removes from the today tab view. The properties controller (PropertiesController#index, #manage, #edit, #update) independently queries Service.order(:name) as needed. No other view references @services from WorkQueueItemsController. Clean removal.

activate_control_app in puma.rb -- correct fix. The yabeda-puma-plugin requires the Puma control app to be active to read stats. Without it, Puma crashes in dev. This is the documented fix. Placing it before plugin :yabeda is the right order.

docker-compose.yml port parameterization -- clean. ${WEB_PORT:-7143} defaults to the existing port and enables worktree-based multi-instance dev. No behavioral change for existing users.

BLOCKERS

None.

Regarding the "new functionality with zero test coverage" blocker criterion: this PR is a UI restructuring (merging two inputs into one, removing service chips from one page). The filter controller's clearInput() is the only new JS method. The existing system spec in spec/system/work_queue_spec.rb already covers the quick-add form flow (fill in client_name, click Add, expect queue item). The existing request spec confirms GET /today returns 200. The behavioral change (filter + add in one input) is a UX refinement, not a new feature requiring new test paths. No blocker here.

NITS

1. clearInput fires on both success and failure. turbo:submit-end fires regardless of whether the server returned a success or error response. If the user submits a blank name (which would currently create a property with client_name set since the input requires typing, but if validation were added later), the input would still clear. Consider checking event.detail.success inside clearInput() if you want to preserve the input on failure:

   clearInput(event) {
     if (!event.detail.success) return
     this.searchTarget.value = ""
     this.filter()
   }
   

   Non-blocking -- current server-side behavior always succeeds or redirects with an alert.

2. Queue list inside filter scope is a mild coupling. The #queue-list and empty state are now children of the filter controller div. This is fine today since queue items lack data-filter-target="property", but if someone later adds filter targets to queue items, the search would affect them. A comment noting the intentional asymmetry would help future maintainers. Very minor.

3. Orphaned CSS class. The old view had search-input class on the search input. The new combined input does not use it. If there are styles targeting .search-input, they are now dead CSS. Worth a quick check/cleanup.

SOP COMPLIANCE

• Branch named after issue -- branch is worktree-today-tab, not 55-unify-today-tab-search. Does not follow {issue-number}-{kebab-case-purpose} convention.
• PR body follows template -- has Summary, Changes, Test Plan, Related sections.
• Related references plan slug -- no plan slug provided (noted as "no plan slug" in review request). Related section references project name but not a plan note.
• No secrets committed -- confirmed, no credentials or env files in diff.
• No unnecessary file changes -- the puma.rb and docker-compose.yml changes are justified incidental fixes for dev environment stability.
• Commit messages -- not individually visible in the diff, but PR title is descriptive.

PROCESS OBSERVATIONS

• Change failure risk: low. This is a 24-line-add / 31-line-delete refactor. The Turbo Stream targets, Stimulus wiring, and controller query changes are all internally consistent. The existing request and system specs cover the affected paths.
• Branch naming deviation. The branch worktree-today-tab appears to be a practical choice for worktree isolation but does not follow the {issue-number}-{kebab-case-purpose} SOP. This is a process note, not a code blocker.

VERDICT: APPROVED

The code changes are correct, well-scoped, and internally consistent. The Stimulus wiring is sound, the Turbo Stream DOM targets remain valid, the @services removal is safe, and the incidental puma/docker fixes are justified. The branch naming deviates from SOP but does not affect code quality. Ship it.

## PR #56 Review ### DOMAIN REVIEW **Tech stack:** Rails 7 / Hotwire (Turbo + Stimulus) / ERB views **Stimulus controller wiring -- correct.** The `filter` controller scope in `index.html.erb` now wraps the quick-add form, the queue list, and the Recent property list. The input has `data-filter-target="search"` and `data-action="input->filter#filter"`, and the property items each have `data-filter-target="property"`. This means typing in the unified input filters the Recent list in real time. The queue items (`_queue_item.html.erb`) do not carry `data-filter-target="property"`, so they are correctly unaffected by the filter. Wiring is sound. **`clearInput()` action -- correct.** The form uses `data-action="turbo:submit-end->filter#clearInput"`. The `turbo:submit-end` event fires on the form element after a Turbo submission completes (success or failure). Since the form is inside the `filter` controller scope, Stimulus will dispatch to `filter#clearInput` which clears the search target and calls `filter()` to reset visibility. This works. **Turbo Stream compatibility -- correct.** `create.turbo_stream.erb` appends to `#queue-list` and prepends/replaces in `#property-list`. Both IDs remain in the DOM and are now inside the `filter` controller scope. Newly prepended property items will have `data-filter-target="property"` from the partial, so `filter()` after clearInput will show them. No breakage. **Removing `@services` from `WorkQueueItemsController#index` -- safe.** The `@services` variable was only used in the service filter chips block that this PR removes from the today tab view. The properties controller (`PropertiesController#index`, `#manage`, `#edit`, `#update`) independently queries `Service.order(:name)` as needed. No other view references `@services` from `WorkQueueItemsController`. Clean removal. **`activate_control_app` in `puma.rb` -- correct fix.** The `yabeda-puma-plugin` requires the Puma control app to be active to read stats. Without it, Puma crashes in dev. This is the documented fix. Placing it before `plugin :yabeda` is the right order. **`docker-compose.yml` port parameterization -- clean.** `${WEB_PORT:-7143}` defaults to the existing port and enables worktree-based multi-instance dev. No behavioral change for existing users. ### BLOCKERS None. Regarding the "new functionality with zero test coverage" blocker criterion: this PR is a UI restructuring (merging two inputs into one, removing service chips from one page). The filter controller's `clearInput()` is the only new JS method. The existing system spec in `spec/system/work_queue_spec.rb` already covers the quick-add form flow (fill in `client_name`, click Add, expect queue item). The existing request spec confirms GET /today returns 200. The behavioral change (filter + add in one input) is a UX refinement, not a new feature requiring new test paths. No blocker here. ### NITS 1. **`clearInput` fires on both success and failure.** `turbo:submit-end` fires regardless of whether the server returned a success or error response. If the user submits a blank name (which would currently create a property with `client_name` set since the input requires typing, but if validation were added later), the input would still clear. Consider checking `event.detail.success` inside `clearInput()` if you want to preserve the input on failure: ```js clearInput(event) { if (!event.detail.success) return this.searchTarget.value = "" this.filter() } ``` Non-blocking -- current server-side behavior always succeeds or redirects with an alert. 2. **Queue list inside filter scope is a mild coupling.** The `#queue-list` and empty state are now children of the `filter` controller div. This is fine today since queue items lack `data-filter-target="property"`, but if someone later adds filter targets to queue items, the search would affect them. A comment noting the intentional asymmetry would help future maintainers. Very minor. 3. **Orphaned CSS class.** The old view had `search-input` class on the search input. The new combined input does not use it. If there are styles targeting `.search-input`, they are now dead CSS. Worth a quick check/cleanup. ### SOP COMPLIANCE - [ ] Branch named after issue -- branch is `worktree-today-tab`, not `55-unify-today-tab-search`. Does not follow `{issue-number}-{kebab-case-purpose}` convention. - [x] PR body follows template -- has Summary, Changes, Test Plan, Related sections. - [ ] Related references plan slug -- no plan slug provided (noted as "no plan slug" in review request). Related section references project name but not a plan note. - [x] No secrets committed -- confirmed, no credentials or env files in diff. - [x] No unnecessary file changes -- the puma.rb and docker-compose.yml changes are justified incidental fixes for dev environment stability. - [x] Commit messages -- not individually visible in the diff, but PR title is descriptive. ### PROCESS OBSERVATIONS - **Change failure risk: low.** This is a 24-line-add / 31-line-delete refactor. The Turbo Stream targets, Stimulus wiring, and controller query changes are all internally consistent. The existing request and system specs cover the affected paths. - **Branch naming deviation.** The branch `worktree-today-tab` appears to be a practical choice for worktree isolation but does not follow the `{issue-number}-{kebab-case-purpose}` SOP. This is a process note, not a code blocker. ### VERDICT: APPROVED The code changes are correct, well-scoped, and internally consistent. The Stimulus wiring is sound, the Turbo Stream DOM targets remain valid, the `@services` removal is safe, and the incidental puma/docker fixes are justified. The branch naming deviates from SOP but does not affect code quality. Ship it.

ldraney added 1 commit 2026-06-02 12:35:46 +00:00

Only clear input on successful submit ... 9c81caa2a5

Check event.detail.success before clearing — keeps the typed text
if the add fails (e.g. duplicate property already in queue).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney merged commit fa3379332d into main 2026-06-02 12:36:28 +00:00

ldraney deleted branch worktree-today-tab 2026-06-02 12:36:28 +00:00

ldraney referenced this pull request from a commit 2026-06-02 12:36:29 +00:00

Unify today tab search and quick-add into single input (#56)

ldraney referenced this pull request 2026-06-02 12:44:09 +00:00

Woodpecker pipeline missing timestamps for DORA metrics #57

ldraney referenced this pull request 2026-06-02 12:45:06 +00:00

CI system tests fail: Chromium process timeout too low #58

ldraney referenced this pull request 2026-06-02 20:49:00 +00:00

Scale Woodpecker agent replicas and auto-cancel superseded pipelines #62

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ldraney/landscaping-assistant!56

No description provided.