Gate schedule digest behind feature flag, add Week tab upload button #213

Merged

ldraney merged 1 commit from 212-digest-flag into main 2026-06-14 11:48:38 +00:00

Conversation 2 Commits 1 Files changed 8 +143 -5

ldraney commented 2026-06-14 03:00:45 +00:00

Owner

Copy link

Summary

Gates the schedule digest feature (Claude Vision OCR) behind a schedule_digest feature flag (default OFF) and adds an "Upload Schedule" button to the Week tab so crew leads can upload and digest a schedule photo directly from the weekly planning view.

Changes

• lib/tasks/feature_flags.rake -- added schedule_digest flag (default: false, references #204/#212)
• app/views/uploads/show.html.erb -- wrapped digest-trigger div in feature_enabled?(:schedule_digest) check; wired auto-digest Stimulus controller for auto-submit flow; uses week_start URL param when present
• app/controllers/uploads_controller.rb -- added feature flag guard at top of digest and confirm_digest actions (redirect with "Feature not available." flash when OFF); modified create to redirect to show page with auto_digest=1 when week_start param is present and flag is ON
• app/views/weeks/index.html.erb -- added "Upload Schedule" button (gated behind flag + lead+ role check) with file input that submits to uploads#create with the current week's start date
• app/javascript/controllers/auto_digest_controller.js -- new Stimulus controller that auto-submits the digest form when ?auto_digest=1 is in the URL (used by Week tab upload flow)
• app/assets/stylesheets/application.css -- added .week-upload-form, .week-upload-btn, .week-upload-icon styles matching existing button patterns
• spec/requests/uploads_spec.rb -- added tests for flag-gated digest/confirm_digest actions, show page button visibility, and create redirect behavior with week_start param
• spec/requests/weeks_spec.rb -- added tests for Upload Schedule button visibility when flag is ON/OFF and week start date inclusion

Test Plan

• bundle exec rspec spec/requests/uploads_spec.rb spec/requests/weeks_spec.rb -- 47 examples, 0 failures
• Full suite: 378/379 pass (1 pre-existing failure in work_queue_items_spec.rb unrelated to this PR)
• Manual: toggle schedule_digest flag at /platform/feature_flags and verify button appears/disappears on Week tab and upload show pages

Review Checklist

• Feature flag registered with default OFF
• View gating on upload show page
• Controller gating on digest and confirm_digest actions
• Week tab upload button with role check
• Auto-digest redirect flow from Week tab
• Tests for all flag-gated behavior
• No changes to files listed in "do not touch"

Related Notes

None.

Related

Closes #212

• Parent feature: #204

## Summary Gates the schedule digest feature (Claude Vision OCR) behind a `schedule_digest` feature flag (default OFF) and adds an "Upload Schedule" button to the Week tab so crew leads can upload and digest a schedule photo directly from the weekly planning view. ## Changes - `lib/tasks/feature_flags.rake` -- added `schedule_digest` flag (default: false, references #204/#212) - `app/views/uploads/show.html.erb` -- wrapped digest-trigger div in `feature_enabled?(:schedule_digest)` check; wired `auto-digest` Stimulus controller for auto-submit flow; uses `week_start` URL param when present - `app/controllers/uploads_controller.rb` -- added feature flag guard at top of `digest` and `confirm_digest` actions (redirect with "Feature not available." flash when OFF); modified `create` to redirect to show page with `auto_digest=1` when `week_start` param is present and flag is ON - `app/views/weeks/index.html.erb` -- added "Upload Schedule" button (gated behind flag + lead+ role check) with file input that submits to `uploads#create` with the current week's start date - `app/javascript/controllers/auto_digest_controller.js` -- new Stimulus controller that auto-submits the digest form when `?auto_digest=1` is in the URL (used by Week tab upload flow) - `app/assets/stylesheets/application.css` -- added `.week-upload-form`, `.week-upload-btn`, `.week-upload-icon` styles matching existing button patterns - `spec/requests/uploads_spec.rb` -- added tests for flag-gated digest/confirm_digest actions, show page button visibility, and create redirect behavior with week_start param - `spec/requests/weeks_spec.rb` -- added tests for Upload Schedule button visibility when flag is ON/OFF and week start date inclusion ## Test Plan - `bundle exec rspec spec/requests/uploads_spec.rb spec/requests/weeks_spec.rb` -- 47 examples, 0 failures - Full suite: 378/379 pass (1 pre-existing failure in `work_queue_items_spec.rb` unrelated to this PR) - Manual: toggle `schedule_digest` flag at `/platform/feature_flags` and verify button appears/disappears on Week tab and upload show pages ## Review Checklist - [x] Feature flag registered with default OFF - [x] View gating on upload show page - [x] Controller gating on digest and confirm_digest actions - [x] Week tab upload button with role check - [x] Auto-digest redirect flow from Week tab - [x] Tests for all flag-gated behavior - [x] No changes to files listed in "do not touch" ## Related Notes None. ## Related Closes #212 - Parent feature: #204

ldraney added 1 commit 2026-06-14 03:00:45 +00:00

Gate schedule digest behind feature flag, add Week tab upload button (#212) ... 7332ad0cd7

The schedule digest feature (Claude Vision OCR) is now gated behind a
`schedule_digest` feature flag (default OFF, toggled via /platform/feature_flags).
When the flag is OFF, the "Digest Schedule" button is hidden on upload show pages
and the digest/confirm_digest controller actions redirect with a flash message.

Adds an "Upload Schedule" button to the Week tab that lets leads+ upload a
schedule photo and immediately redirect to the digest confirmation screen,
passing the current week's start date automatically.

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

ldraney commented 2026-06-14 03:02:19 +00:00

Author

Owner

Copy link

PR #213 Review

DOMAIN REVIEW

Stack: Rails 8.1 / Hotwire / Stimulus / importmap / plain CSS / RSpec

Feature flag registration -- Clean. schedule_digest flag registered in feature_flags.rake with enabled: false (default OFF), description references parent tickets. Follows the existing pattern exactly.

Controller gating -- Well implemented. Both digest and confirm_digest actions have early-return guards that redirect with a flash alert when the flag is OFF. The create action correctly checks the flag before redirecting to the auto-digest flow, and falls through to the default redirect when the flag is OFF.

View gating -- uploads/show.html.erb wraps the digest-trigger div in feature_enabled?(:schedule_digest). weeks/index.html.erb wraps the upload button in the same check plus a role gate.

Stimulus controllers -- auto_digest_controller.js is minimal and correct: reads URL param, auto-submits if auto_digest=1. Both controllers are auto-registered via eagerLoadControllersFrom and pin_all_from in importmap -- no manual registration needed.

CSS -- New styles use design tokens (--spacing-md, --spacing-sm) consistently. No magic numbers. Follows existing component comment-block convention.

BLOCKERS

None.

NITS

1. Dead role check in weeks view (line 26): The upload button checks current_user_has_role?(:lead), but WeeksController already has require_role :admin, :super_admin -- leads cannot reach this page at all. The :lead check is dead code. Not harmful, but misleading to future readers who might think leads can see the Week tab.

2. week_start param passed through URL unvalidated in create redirect: In UploadsController#create (line 17), params[:week_start] is passed directly into the redirect URL as a query param. It is validated later in parse_week_start when the digest action runs, so there is no injection risk -- but a malformed week_start value will survive the round-trip before being caught. Very minor.

3. digest_week fallback chain in show view (line 13): The expression params[:week_start].presence || (@upload.week_start || Date.current.beginning_of_week(:monday)).iso8601 mixes param trust with model data. This is fine because the value only goes into a date input's value attribute (not executed), but worth noting the precedence: URL param wins over persisted model data.

SOP COMPLIANCE

• PR body has: Summary, Changes, Test Plan, Related
• Test Plan documents run results (47 examples, 0 failures + full suite count)
• No secrets, .env files, or credentials committed
• No unnecessary file changes -- all 8 files are directly related to the feature
• Commit messages are descriptive
• Review checklist present and complete

TEST COVERAGE

Tests are thorough:

• uploads_spec.rb covers: flag-ON redirect with auto_digest param, flag-OFF fallback to index, show page button visibility (ON/OFF), digest action flag gate, confirm_digest action flag gate
• weeks_spec.rb covers: Upload Schedule button visibility (ON/OFF), week start date inclusion in form

All new behavior paths have corresponding specs. Happy paths and flag-off guard paths are both tested.

PROCESS OBSERVATIONS

• Low risk change: feature is gated behind a flag (default OFF) so no production behavior changes until explicitly enabled
• Clean separation: no changes to existing functionality, only additive gating and the new upload flow
• The 1 pre-existing test failure in work_queue_items_spec.rb is documented and unrelated

VERDICT: APPROVED

## PR #213 Review ### DOMAIN REVIEW **Stack:** Rails 8.1 / Hotwire / Stimulus / importmap / plain CSS / RSpec **Feature flag registration** -- Clean. `schedule_digest` flag registered in `feature_flags.rake` with `enabled: false` (default OFF), description references parent tickets. Follows the existing pattern exactly. **Controller gating** -- Well implemented. Both `digest` and `confirm_digest` actions have early-return guards that redirect with a flash alert when the flag is OFF. The `create` action correctly checks the flag before redirecting to the auto-digest flow, and falls through to the default redirect when the flag is OFF. **View gating** -- `uploads/show.html.erb` wraps the digest-trigger div in `feature_enabled?(:schedule_digest)`. `weeks/index.html.erb` wraps the upload button in the same check plus a role gate. **Stimulus controllers** -- `auto_digest_controller.js` is minimal and correct: reads URL param, auto-submits if `auto_digest=1`. Both controllers are auto-registered via `eagerLoadControllersFrom` and `pin_all_from` in importmap -- no manual registration needed. **CSS** -- New styles use design tokens (`--spacing-md`, `--spacing-sm`) consistently. No magic numbers. Follows existing component comment-block convention. ### BLOCKERS None. ### NITS 1. **Dead role check in weeks view (line 26):** The upload button checks `current_user_has_role?(:lead)`, but `WeeksController` already has `require_role :admin, :super_admin` -- leads cannot reach this page at all. The `:lead` check is dead code. Not harmful, but misleading to future readers who might think leads can see the Week tab. 2. **`week_start` param passed through URL unvalidated in `create` redirect:** In `UploadsController#create` (line 17), `params[:week_start]` is passed directly into the redirect URL as a query param. It is validated later in `parse_week_start` when the `digest` action runs, so there is no injection risk -- but a malformed `week_start` value will survive the round-trip before being caught. Very minor. 3. **`digest_week` fallback chain in show view (line 13):** The expression `params[:week_start].presence || (@upload.week_start || Date.current.beginning_of_week(:monday)).iso8601` mixes param trust with model data. This is fine because the value only goes into a date input's `value` attribute (not executed), but worth noting the precedence: URL param wins over persisted model data. ### SOP COMPLIANCE - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Test Plan documents run results (47 examples, 0 failures + full suite count) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes -- all 8 files are directly related to the feature - [x] Commit messages are descriptive - [x] Review checklist present and complete ### TEST COVERAGE Tests are thorough: - `uploads_spec.rb` covers: flag-ON redirect with `auto_digest` param, flag-OFF fallback to index, show page button visibility (ON/OFF), digest action flag gate, confirm_digest action flag gate - `weeks_spec.rb` covers: Upload Schedule button visibility (ON/OFF), week start date inclusion in form All new behavior paths have corresponding specs. Happy paths and flag-off guard paths are both tested. ### PROCESS OBSERVATIONS - Low risk change: feature is gated behind a flag (default OFF) so no production behavior changes until explicitly enabled - Clean separation: no changes to existing functionality, only additive gating and the new upload flow - The 1 pre-existing test failure in `work_queue_items_spec.rb` is documented and unrelated ### VERDICT: APPROVED

ldraney commented 2026-06-14 10:55:13 +00:00

Author

Owner

Copy link

PR #213 Review

DOMAIN REVIEW

Stack: Rails 7 (ERB views, Stimulus JS, importmap, RSpec request specs)

Feature flag pattern -- clean. The schedule_digest flag follows the established pattern: rake-registered with default OFF, controller guards via feature_enabled?, view gating, cache-busted toggles. Consistent with the existing pending_completion and stripe flags.

Controller changes (uploads_controller.rb):

• digest and confirm_digest get identical guard-then-redirect blocks at the top of each action. This is the right approach -- checking the flag before Upload.find avoids an unnecessary DB hit when the flag is off.
• create redirect logic is sound: when week_start is present AND the flag is ON, redirect to the show page with auto_digest=1; otherwise fall through to the existing redirect. The flag check on the create path prevents the auto-digest redirect when the flag is off, which is correct.

View changes (uploads/show.html.erb):

• The digest_week local variable is well-constructed: URL param takes precedence, then the upload's stored week_start, then the current Monday. Good fallback chain.
• The auto-digest Stimulus controller is wired to the form via data: { controller: "auto-digest" }. This will auto-submit the digest form when ?auto_digest=1 is in the URL.

Week tab upload button (weeks/index.html.erb):

• Role check: !keycloak_configured? || current_user_has_role?(:lead) || current_user_has_role?(:admin) || current_user_has_role?(:super_admin). This is consistent with the UploadsController class-level require_role :lead, :admin, :super_admin. The !keycloak_configured? fallback for dev mode matches the app's convention.
• The hidden week_start input passes @start_date.iso8601 -- this ensures the correct week context flows through the upload-to-digest pipeline.
• The form submits via the existing upload Stimulus controller (data-action="change->upload#submit"), which calls this.element.requestSubmit(). This reuses the existing controller correctly.

Stimulus controller (auto_digest_controller.js):

• Simple and correct: checks URL params on connect(), auto-submits if auto_digest=1 is present.
• Auto-registration via eagerLoadControllersFrom + pin_all_from means no manual registration needed.
• requestSubmit() (vs. submit()) triggers form validation and Turbo properly. Good choice.

CSS:

• New classes (.week-upload-form, .week-upload-btn, .week-upload-icon) use existing design tokens (--spacing-md, --spacing-sm). No magic numbers for spacing. The 1.25rem icon size and 1rem font size are reasonable and consistent with the codebase.

BLOCKERS

None.

NITS

1. Verbose role check in weeks/index.html.erb -- The inline role check (!keycloak_configured? || current_user_has_role?(:lead) || current_user_has_role?(:admin) || current_user_has_role?(:super_admin)) duplicates the roles already declared in UploadsController.require_role. If a role is added/removed from the controller, the view check could drift. Consider extracting to a helper like can_upload? or checking visible_tabs.include?(:uploads). Non-blocking since both are correct today.

2. digest_week variable in show.html.erb -- The params[:week_start].presence usage means user-supplied query params flow directly into the date input's value attribute. The value is used inside an <input type="date" value="..."> which is safe from XSS (the browser's date picker will reject malformed values, and ERB's <%= %> escapes HTML by default). However, if a non-date string is passed, it will show as the input value rather than falling through to the computed default. Not a security issue, but slightly inconsistent UX -- parse_week_start in the controller handles this gracefully, but the view does not.

3. Auto-digest fires on browser back/forward -- The auto_digest_controller checks ?auto_digest=1 in connect(). If a user navigates away from the digest confirmation and hits the browser back button, Turbo's cache restoration will re-run connect() and re-submit the form. This could cause a duplicate API call. A mitigation would be to remove the URL param after submission (e.g., window.history.replaceState in connect() after triggering submit). Low risk since the digest action is idempotent (it just re-calls the Vision API), but worth noting.

4. Test coverage for auto-digest Stimulus controller -- The JS controller itself has no unit test. The integration is covered by the request specs (redirect behavior), but the auto-submit-on-connect behavior is not tested. This is typical for Stimulus controllers in Rails apps without a JS test harness, so noting as a nit rather than a blocker.

SOP COMPLIANCE

• PR body has: Summary, Changes, Test Plan, Related
• No secrets or credentials committed
• No unnecessary file changes (all 8 files are relevant to the feature)
• Tests exist and cover: flag-gated controller actions, view button visibility, redirect behavior with week_start param
• Commit messages are descriptive (PR title matches the feature scope)

PROCESS OBSERVATIONS

• Test suite coverage is thorough: 47 examples, 0 failures. Both flag-on and flag-off paths are tested for every gated behavior.
• The existing with_feature_disabled / FeatureFlag.enable test helpers are used correctly and consistently.
• The feature flag defaults to OFF, which is the correct posture for a new feature that calls an external API (Claude Vision). Production rollout is controlled.
• One pre-existing failure in work_queue_items_spec.rb is noted in the Test Plan and is unrelated -- no regression introduced.

VERDICT: APPROVED

## PR #213 Review ### DOMAIN REVIEW **Stack:** Rails 7 (ERB views, Stimulus JS, importmap, RSpec request specs) **Feature flag pattern** -- clean. The `schedule_digest` flag follows the established pattern: rake-registered with default OFF, controller guards via `feature_enabled?`, view gating, cache-busted toggles. Consistent with the existing `pending_completion` and `stripe` flags. **Controller changes (uploads_controller.rb):** - `digest` and `confirm_digest` get identical guard-then-redirect blocks at the top of each action. This is the right approach -- checking the flag before `Upload.find` avoids an unnecessary DB hit when the flag is off. - `create` redirect logic is sound: when `week_start` is present AND the flag is ON, redirect to the show page with `auto_digest=1`; otherwise fall through to the existing redirect. The flag check on the `create` path prevents the auto-digest redirect when the flag is off, which is correct. **View changes (uploads/show.html.erb):** - The `digest_week` local variable is well-constructed: URL param takes precedence, then the upload's stored `week_start`, then the current Monday. Good fallback chain. - The `auto-digest` Stimulus controller is wired to the form via `data: { controller: "auto-digest" }`. This will auto-submit the digest form when `?auto_digest=1` is in the URL. **Week tab upload button (weeks/index.html.erb):** - Role check: `!keycloak_configured? || current_user_has_role?(:lead) || current_user_has_role?(:admin) || current_user_has_role?(:super_admin)`. This is consistent with the `UploadsController` class-level `require_role :lead, :admin, :super_admin`. The `!keycloak_configured?` fallback for dev mode matches the app's convention. - The hidden `week_start` input passes `@start_date.iso8601` -- this ensures the correct week context flows through the upload-to-digest pipeline. - The form submits via the existing `upload` Stimulus controller (`data-action="change->upload#submit"`), which calls `this.element.requestSubmit()`. This reuses the existing controller correctly. **Stimulus controller (auto_digest_controller.js):** - Simple and correct: checks URL params on `connect()`, auto-submits if `auto_digest=1` is present. - Auto-registration via `eagerLoadControllersFrom` + `pin_all_from` means no manual registration needed. - `requestSubmit()` (vs. `submit()`) triggers form validation and Turbo properly. Good choice. **CSS:** - New classes (`.week-upload-form`, `.week-upload-btn`, `.week-upload-icon`) use existing design tokens (`--spacing-md`, `--spacing-sm`). No magic numbers for spacing. The `1.25rem` icon size and `1rem` font size are reasonable and consistent with the codebase. ### BLOCKERS None. ### NITS 1. **Verbose role check in weeks/index.html.erb** -- The inline role check `(!keycloak_configured? || current_user_has_role?(:lead) || current_user_has_role?(:admin) || current_user_has_role?(:super_admin))` duplicates the roles already declared in `UploadsController.require_role`. If a role is added/removed from the controller, the view check could drift. Consider extracting to a helper like `can_upload?` or checking `visible_tabs.include?(:uploads)`. Non-blocking since both are correct today. 2. **`digest_week` variable in show.html.erb** -- The `params[:week_start].presence` usage means user-supplied query params flow directly into the date input's `value` attribute. The value is used inside an `<input type="date" value="...">` which is safe from XSS (the browser's date picker will reject malformed values, and ERB's `<%= %>` escapes HTML by default). However, if a non-date string is passed, it will show as the input value rather than falling through to the computed default. Not a security issue, but slightly inconsistent UX -- `parse_week_start` in the controller handles this gracefully, but the view does not. 3. **Auto-digest fires on browser back/forward** -- The `auto_digest_controller` checks `?auto_digest=1` in `connect()`. If a user navigates away from the digest confirmation and hits the browser back button, Turbo's cache restoration will re-run `connect()` and re-submit the form. This could cause a duplicate API call. A mitigation would be to remove the URL param after submission (e.g., `window.history.replaceState` in `connect()` after triggering submit). Low risk since the digest action is idempotent (it just re-calls the Vision API), but worth noting. 4. **Test coverage for auto-digest Stimulus controller** -- The JS controller itself has no unit test. The integration is covered by the request specs (redirect behavior), but the auto-submit-on-connect behavior is not tested. This is typical for Stimulus controllers in Rails apps without a JS test harness, so noting as a nit rather than a blocker. ### SOP COMPLIANCE - [x] PR body has: Summary, Changes, Test Plan, Related - [x] No secrets or credentials committed - [x] No unnecessary file changes (all 8 files are relevant to the feature) - [x] Tests exist and cover: flag-gated controller actions, view button visibility, redirect behavior with `week_start` param - [x] Commit messages are descriptive (PR title matches the feature scope) ### PROCESS OBSERVATIONS - Test suite coverage is thorough: 47 examples, 0 failures. Both flag-on and flag-off paths are tested for every gated behavior. - The existing `with_feature_disabled` / `FeatureFlag.enable` test helpers are used correctly and consistently. - The feature flag defaults to OFF, which is the correct posture for a new feature that calls an external API (Claude Vision). Production rollout is controlled. - One pre-existing failure in `work_queue_items_spec.rb` is noted in the Test Plan and is unrelated -- no regression introduced. ### VERDICT: APPROVED

ldraney merged commit fa0ab403a8 into main 2026-06-14 11:48:38 +00:00

ldraney deleted branch 212-digest-flag 2026-06-14 11:48:38 +00:00

ldraney referenced this pull request from a commit 2026-06-14 11:48:39 +00:00

Gate schedule digest behind feature flag, add Week tab upload button (#213)

ldraney referenced this pull request from a commit 2026-06-14 11:51:40 +00:00

Revert "Gate schedule digest behind feature flag, add Week tab upload button (#213)"

ldraney referenced this pull request 2026-06-14 11:51:58 +00:00

Revert schedule digest feature flag and Week upload button #219

ldraney referenced this pull request 2026-06-14 12:10:44 +00:00

Move Assigned to Another Crew to bottom, remove Schedule section #222

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

Move Assigned to Another Crew to bottom, remove Schedule section #222

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!213

No description provided.