ldraney/landscaping-assistant
1
0
Fork 0
Code Issues 32 Pull requests 1 Projects Releases Packages Wiki Activity Actions 4

Add last-week same-day queue section to Today tab #102

Merged
ldraney merged 1 commit from 98-show-last-week-s-same-day-queue-on-the-t into main 2026-06-04 12:40:39 +00:00
Conversation 2 Commits 1 Files changed 7 +152
ldraney commented 2026-06-04 12:27:38 +00:00
Owner
Copy link

Summary

Adds a "Last [weekday]" section to the Today tab that shows properties queued on the same weekday 7 days ago, with a one-tap re-queue button to add them to today's queue.

Changes

  • app/controllers/work_queue_items_controller.rb -- Query last-week items in index; look up last-week item in destroy for Turbo Stream sync
  • app/views/work_queue_items/index.html.erb -- Render the last-week section between the queue list and the Recent section (only when items exist)
  • app/views/work_queue_items/_last_week_item.html.erb -- New partial for each last-week row with re-queue button or "Queued" badge
  • app/views/work_queue_items/create.turbo_stream.erb -- Replace last-week row on re-queue to swap button for badge
  • app/views/work_queue_items/destroy.turbo_stream.erb -- Restore re-queue button on last-week row when item removed from today
  • app/assets/stylesheets/application.css -- LastWeekSection and btn-requeue styles following existing design tokens
  • spec/requests/work_queue_items_spec.rb -- 5 new specs covering section rendering, absence, queued state, date param, and turbo stream sync

Test Plan

  • Full suite passes: 93 examples, 0 failures
  • Visit Today tab with no history -- no last-week section appears
  • Queue properties on a given day, then navigate to the same weekday 7 days later -- last-week section shows with re-queue buttons
  • Tap re-queue -- item added to today's queue, button changes to "Queued" badge (Turbo Stream, no full reload)
  • Remove a re-queued item from today -- last-week row restores the re-queue button

Review Checklist

  • No Tailwind -- uses existing design tokens and CSS patterns
  • Re-queue uses existing create action (POST to work_queue_items_path)
  • Turbo Stream updates keep last-week badges in sync on add/remove
  • Section hidden when no last-week items exist
  • 5 new specs, full suite green (93/93)

Related Notes

None.

Related

Closes #98

## Summary Adds a "Last [weekday]" section to the Today tab that shows properties queued on the same weekday 7 days ago, with a one-tap re-queue button to add them to today's queue. ## Changes - `app/controllers/work_queue_items_controller.rb` -- Query last-week items in `index`; look up last-week item in `destroy` for Turbo Stream sync - `app/views/work_queue_items/index.html.erb` -- Render the last-week section between the queue list and the Recent section (only when items exist) - `app/views/work_queue_items/_last_week_item.html.erb` -- New partial for each last-week row with re-queue button or "Queued" badge - `app/views/work_queue_items/create.turbo_stream.erb` -- Replace last-week row on re-queue to swap button for badge - `app/views/work_queue_items/destroy.turbo_stream.erb` -- Restore re-queue button on last-week row when item removed from today - `app/assets/stylesheets/application.css` -- LastWeekSection and btn-requeue styles following existing design tokens - `spec/requests/work_queue_items_spec.rb` -- 5 new specs covering section rendering, absence, queued state, date param, and turbo stream sync ## Test Plan - Full suite passes: 93 examples, 0 failures - Visit Today tab with no history -- no last-week section appears - Queue properties on a given day, then navigate to the same weekday 7 days later -- last-week section shows with re-queue buttons - Tap re-queue -- item added to today's queue, button changes to "Queued" badge (Turbo Stream, no full reload) - Remove a re-queued item from today -- last-week row restores the re-queue button ## Review Checklist - [x] No Tailwind -- uses existing design tokens and CSS patterns - [x] Re-queue uses existing `create` action (POST to work_queue_items_path) - [x] Turbo Stream updates keep last-week badges in sync on add/remove - [x] Section hidden when no last-week items exist - [x] 5 new specs, full suite green (93/93) ## Related Notes None. ## Related Closes #98
Add last-week same-day queue section to Today tab
Some checks failed
CI / scan_ruby (pull_request) Has been cancelled
Details
CI / scan_js (pull_request) Has been cancelled
Details
CI / lint (pull_request) Has been cancelled
Details
d7c08af381 
Show properties that were queued on the same weekday 7 days ago with a
one-tap re-queue button. The section appears between the current queue
and the Recent properties list, and only renders when last-week items
exist. Turbo Stream updates keep the re-queue button and queued badge
in sync when items are added or removed from today's queue.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ldraney commented 2026-06-04 12:28:27 +00:00
Author
Owner
Copy link

QA Review -- PR #102

Scope Check

PR implements Forgejo #98: "Last week" section on the Today tab showing same-weekday items from 7 days ago with a re-queue button. All spec items addressed.

Findings

Controller -- Clean. @last_week_items uses .includes(:property) to avoid N+1. destroy action correctly looks up the last-week item for Turbo Stream sync.

Partial -- _last_week_item.html.erb receives queued_property_ids and date as explicit locals, consistent with _property_item.html.erb pattern. Re-queue button POSTs to the existing create action -- good reuse, no new endpoints.

Turbo Streams -- create.turbo_stream.erb always emits a replace targeting last_week_<property_id>. When the element is absent (item queued from picker, not from last-week section), Turbo silently ignores the no-op replace. Acceptable. destroy.turbo_stream.erb correctly guards with if @last_week_item to avoid rendering the partial when no last-week row exists.

Note on create.turbo_stream.erb: The @item passed to _last_week_item.html.erb is the newly-created today item, not the original last-week WorkQueueItem. This works because the partial only uses item.property_id and item.property (same property either way), but it is slightly misleading -- the item local is a today-dated record being rendered in a "last week" row. No functional bug, just a semantic mismatch in the variable. Not blocking.

CSS -- Follows existing design tokens and component patterns. No Tailwind. btn-requeue mirrors btn-add / btn-icon dimensions and transitions. Font sizes slightly smaller than other list items (0.9rem / 0.75rem vs 0.95rem / 0.8rem for queue-item and property-item) -- intentional to visually de-emphasize the historical section, which reads well.

View -- Section heading "Last [weekday]" is clear. Section conditionally hidden with if @last_week_items.any? -- no empty state noise.

Tests -- 5 new specs covering: section present, section absent, queued badge state, date param, and turbo stream re-queue. Full suite green (93/93). Good coverage.

VERDICT: APPROVE

## QA Review -- PR #102 ### Scope Check PR implements Forgejo #98: "Last week" section on the Today tab showing same-weekday items from 7 days ago with a re-queue button. All spec items addressed. ### Findings **Controller** -- Clean. `@last_week_items` uses `.includes(:property)` to avoid N+1. `destroy` action correctly looks up the last-week item for Turbo Stream sync. **Partial** -- `_last_week_item.html.erb` receives `queued_property_ids` and `date` as explicit locals, consistent with `_property_item.html.erb` pattern. Re-queue button POSTs to the existing `create` action -- good reuse, no new endpoints. **Turbo Streams** -- `create.turbo_stream.erb` always emits a `replace` targeting `last_week_<property_id>`. When the element is absent (item queued from picker, not from last-week section), Turbo silently ignores the no-op replace. Acceptable. `destroy.turbo_stream.erb` correctly guards with `if @last_week_item` to avoid rendering the partial when no last-week row exists. **Note on `create.turbo_stream.erb`**: The `@item` passed to `_last_week_item.html.erb` is the newly-created today item, not the original last-week WorkQueueItem. This works because the partial only uses `item.property_id` and `item.property` (same property either way), but it is slightly misleading -- the `item` local is a today-dated record being rendered in a "last week" row. No functional bug, just a semantic mismatch in the variable. Not blocking. **CSS** -- Follows existing design tokens and component patterns. No Tailwind. `btn-requeue` mirrors `btn-add` / `btn-icon` dimensions and transitions. Font sizes slightly smaller than other list items (0.9rem / 0.75rem vs 0.95rem / 0.8rem for queue-item and property-item) -- intentional to visually de-emphasize the historical section, which reads well. **View** -- Section heading "Last [weekday]" is clear. Section conditionally hidden with `if @last_week_items.any?` -- no empty state noise. **Tests** -- 5 new specs covering: section present, section absent, queued badge state, date param, and turbo stream re-queue. Full suite green (93/93). Good coverage. ### VERDICT: APPROVE
ldraney commented 2026-06-04 12:32:37 +00:00
Author
Owner
Copy link

PR #102 Review

DOMAIN REVIEW

Tech stack: Ruby on Rails 8, Turbo Streams, vanilla CSS with design tokens, RSpec request specs.

Controller (work_queue_items_controller.rb)

  • The index action adds @last_week_items with .includes(:property) -- correct eager loading, no N+1.
  • The destroy action adds @last_week_item = WorkQueueItem.find_by(...) -- appropriate use of find_by (returns nil when no last-week item exists), and the turbo stream correctly guards with if @last_week_item.
  • The @date - 7.days pattern is used consistently in both index and destroy. No hardcoded date math drift.

Turbo Stream sync (create.turbo_stream.erb)

  • The turbo_stream.replace "last_week_#{@item.property_id}" targets a DOM id that only exists when a last-week item is rendered. When no such DOM element exists (property was not in last week's queue), Turbo Stream replace is a silent no-op. This is safe and intentional.
  • Note: the partial is rendered with item: @item where @item is the today queue item, not the last-week item. This works because the partial only reads item.property_id and item.property.* (which are identical for the same property), plus queued_property_ids (which now correctly includes this property). Semantically imprecise but functionally correct.

Turbo Stream sync (destroy.turbo_stream.erb)

  • Correctly guarded with if @last_week_item, and passes the actual last-week item to the partial. The @queued_property_ids is recomputed after destroy, so the re-queue button will correctly reappear.

Partial (_last_week_item.html.erb)

  • Follows the existing pattern from _property_item.html.erb closely: same address display logic, same "No address yet" fallback, same queued-badge class reuse.
  • button_to generates a POST form with CSRF token automatically -- secure by default.
  • The date: date parameter ensures re-queued items go to the correct date, not just today.

CSS (application.css)

  • All spacing and color values use design tokens (--spacing-sm, --color-accent, etc.) -- no magic numbers.
  • .btn-requeue dimensions (2.75rem x 2.75rem) match the existing .btn-icon pattern exactly. This is near-duplicate CSS (see nits).
  • Component comment block follows the existing convention.

Specs (spec/requests/work_queue_items_spec.rb)

  • 5 new specs covering: section presence, section absence, queued badge state, date param override, and turbo stream re-queue sync.
  • Tests cover the key behavioral variations: items exist vs. don't, already queued vs. not, default date vs. explicit date.
  • The turbo stream re-queue test verifies the last_week_ prefix appears in the response body.

Missing test coverage (non-blocking): No spec for the destroy turbo stream restoring the re-queue button on the last-week row. The create path is tested but the inverse (removing from queue restores the button) is not. This is the only untested Turbo Stream path.

BLOCKERS

None. The PR has test coverage for new functionality, no unvalidated user input (all DB queries use parameterized ActiveRecord methods), no secrets or credentials, and no duplicated auth/security logic.

NITS

  1. Near-duplicate CSS for .btn-requeue: The .btn-requeue styles are almost identical to .btn-icon (same dimensions, border-radius, cursor, background, transition). Consider extending .btn-icon and adding only the color override, e.g., .btn-icon.btn-requeue { color: var(--color-accent); }. This would reduce ~15 lines of CSS and keep button styles from diverging.

  2. Semantic item mismatch in create.turbo_stream.erb: The partial _last_week_item is rendered with item: @item where @item is the today-queue item. It works because only property data is read, but a future change to the partial that uses item.work_date or item.position would silently produce wrong data. A comment explaining this would help.

  3. Missing destroy turbo stream spec: Adding a spec that creates a last-week item, queues the property for today, then DELETEs the today item and asserts the turbo stream response includes last_week_ would complete the round-trip coverage.

  4. Section heading could use the target date, not re-derive: index.html.erb computes (@date - 7.days).strftime("%A") -- this repeats the 7-day offset already computed in the controller. Consider passing the weekday name as an instance variable from the controller to reduce logic in the view.

SOP COMPLIANCE

  • Branch named after issue: 98-show-last-week-s-same-day-queue-on-the-t (follows {issue-number}-{kebab-case-purpose} convention)
  • PR body has Summary, Changes, Test Plan, Related sections
  • Related references plan slug -- no plan slug exists (caller confirmed "No plan slug")
  • No secrets committed
  • No unnecessary file changes -- all 7 files are directly related to the feature
  • Tests exist and cover the new functionality (5 new specs)

PROCESS OBSERVATIONS

  • Deployment risk: low. Additive feature -- new section only renders when last-week data exists. No existing behavior is modified. Turbo Stream replace targets are no-ops when the DOM element is absent.
  • Change failure risk: low. The feature relies on a straightforward date offset query with no schema changes. The uniqueness constraint on [property_id, work_date] prevents double-queue issues.
  • Documentation: The PR body is thorough with a manual test plan. No external docs needed for this UI-only addition.

VERDICT: APPROVED

## PR #102 Review ### DOMAIN REVIEW **Tech stack:** Ruby on Rails 8, Turbo Streams, vanilla CSS with design tokens, RSpec request specs. **Controller (`work_queue_items_controller.rb`)** - The `index` action adds `@last_week_items` with `.includes(:property)` -- correct eager loading, no N+1. - The `destroy` action adds `@last_week_item = WorkQueueItem.find_by(...)` -- appropriate use of `find_by` (returns nil when no last-week item exists), and the turbo stream correctly guards with `if @last_week_item`. - The `@date - 7.days` pattern is used consistently in both `index` and `destroy`. No hardcoded date math drift. **Turbo Stream sync (`create.turbo_stream.erb`)** - The `turbo_stream.replace "last_week_#{@item.property_id}"` targets a DOM id that only exists when a last-week item is rendered. When no such DOM element exists (property was not in last week's queue), Turbo Stream `replace` is a silent no-op. This is safe and intentional. - Note: the partial is rendered with `item: @item` where `@item` is the **today** queue item, not the last-week item. This works because the partial only reads `item.property_id` and `item.property.*` (which are identical for the same property), plus `queued_property_ids` (which now correctly includes this property). Semantically imprecise but functionally correct. **Turbo Stream sync (`destroy.turbo_stream.erb`)** - Correctly guarded with `if @last_week_item`, and passes the actual last-week item to the partial. The `@queued_property_ids` is recomputed after destroy, so the re-queue button will correctly reappear. **Partial (`_last_week_item.html.erb`)** - Follows the existing pattern from `_property_item.html.erb` closely: same address display logic, same "No address yet" fallback, same `queued-badge` class reuse. - `button_to` generates a POST form with CSRF token automatically -- secure by default. - The `date: date` parameter ensures re-queued items go to the correct date, not just today. **CSS (`application.css`)** - All spacing and color values use design tokens (`--spacing-sm`, `--color-accent`, etc.) -- no magic numbers. - `.btn-requeue` dimensions (2.75rem x 2.75rem) match the existing `.btn-icon` pattern exactly. This is near-duplicate CSS (see nits). - Component comment block follows the existing convention. **Specs (`spec/requests/work_queue_items_spec.rb`)** - 5 new specs covering: section presence, section absence, queued badge state, date param override, and turbo stream re-queue sync. - Tests cover the key behavioral variations: items exist vs. don't, already queued vs. not, default date vs. explicit date. - The turbo stream re-queue test verifies the `last_week_` prefix appears in the response body. **Missing test coverage (non-blocking):** No spec for the `destroy` turbo stream restoring the re-queue button on the last-week row. The create path is tested but the inverse (removing from queue restores the button) is not. This is the only untested Turbo Stream path. ### BLOCKERS None. The PR has test coverage for new functionality, no unvalidated user input (all DB queries use parameterized ActiveRecord methods), no secrets or credentials, and no duplicated auth/security logic. ### NITS 1. **Near-duplicate CSS for `.btn-requeue`**: The `.btn-requeue` styles are almost identical to `.btn-icon` (same dimensions, border-radius, cursor, background, transition). Consider extending `.btn-icon` and adding only the color override, e.g., `.btn-icon.btn-requeue { color: var(--color-accent); }`. This would reduce ~15 lines of CSS and keep button styles from diverging. 2. **Semantic item mismatch in `create.turbo_stream.erb`**: The partial `_last_week_item` is rendered with `item: @item` where `@item` is the today-queue item. It works because only property data is read, but a future change to the partial that uses `item.work_date` or `item.position` would silently produce wrong data. A comment explaining this would help. 3. **Missing destroy turbo stream spec**: Adding a spec that creates a last-week item, queues the property for today, then DELETEs the today item and asserts the turbo stream response includes `last_week_` would complete the round-trip coverage. 4. **Section heading could use the target date, not re-derive**: `index.html.erb` computes `(@date - 7.days).strftime("%A")` -- this repeats the 7-day offset already computed in the controller. Consider passing the weekday name as an instance variable from the controller to reduce logic in the view. ### SOP COMPLIANCE - [x] Branch named after issue: `98-show-last-week-s-same-day-queue-on-the-t` (follows `{issue-number}-{kebab-case-purpose}` convention) - [x] PR body has Summary, Changes, Test Plan, Related sections - [ ] Related references plan slug -- no plan slug exists (caller confirmed "No plan slug") - [x] No secrets committed - [x] No unnecessary file changes -- all 7 files are directly related to the feature - [x] Tests exist and cover the new functionality (5 new specs) ### PROCESS OBSERVATIONS - **Deployment risk: low.** Additive feature -- new section only renders when last-week data exists. No existing behavior is modified. Turbo Stream replace targets are no-ops when the DOM element is absent. - **Change failure risk: low.** The feature relies on a straightforward date offset query with no schema changes. The uniqueness constraint on `[property_id, work_date]` prevents double-queue issues. - **Documentation:** The PR body is thorough with a manual test plan. No external docs needed for this UI-only addition. ### VERDICT: APPROVED
ldraney deleted branch 98-show-last-week-s-same-day-queue-on-the-t 2026-06-04 12:40:39 +00:00
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
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!102
No description provided.