Skip to content

fix(ui): enforce future-only planned start date + widen Configure Purchase Term field - #1258

Merged
cristim merged 2 commits into
mainfrom
fix/qa437-purchase-ui-polish
Jul 17, 2026
Merged

fix(ui): enforce future-only planned start date + widen Configure Purchase Term field#1258
cristim merged 2 commits into
mainfrom
fix/qa437-purchase-ui-polish

Conversation

@cristim

@cristim cristim commented Jun 19, 2026

Copy link
Copy Markdown
Member

Summary

Two small Purchase-flow UI polish fixes from the QA sheet, shipped together.

Bug 1 (QA 5.6) - Add Purchases start-date picker allowed past dates

Closes #1249

Root cause: the <input type="date" id="add-purchases-start-date"> in the existing-plan "Add Purchases" modal (frontend/src/plans.ts) had no min attribute, so the browser date-picker let users select dates in the past, which is contradictory for scheduling future purchases.

Fix: after the default value (tomorrow) is assigned, set startDateInput.min to today's ISO date. Past dates are now blocked in the picker while today remains selectable (a same-day start is legitimate). The default value stays tomorrow.

Bug 2 (QA 5.3) - Configure Purchase modal Term field too narrow to read

Closes #1250

Root cause: the Term <select> in the Configure Purchase modal carries class .purchase-row-term (set in recommendations.ts), but no CSS rule existed for it anywhere, so the browser rendered it at content width and clipped both the selected value and the dropdown arrow.

Fix: add .purchase-row-term { min-width: 5rem; } to frontend/src/styles/modals.css, next to the sibling .purchase-modal-* rules. CSS-only; recommendations.ts is untouched (owned by another PR).

Testing

  • Bug 1: regression test added in frontend/src/__tests__/plans-range-validation.test.ts (new describe block "Add Purchases modal: start-date picker enforces future-only dates (QA 5.6)"). Asserts input.min === today's ISO date after the modal opens, plus that the default value stays tomorrow. Confirmed FAIL pre-fix (received ""), PASS post-fix. Full file: 44 pass / 0 fail.
  • npx tsc --noEmit: no errors.
  • Bug 2: CSS-only, not unit-testable in jsdom - needs visual verification in a real browser (Configure Purchase modal Term dropdown should no longer clip the value/arrow).

Note

Stacked on #1254 (pre-commit repair); retarget to main when #1254 merges.

Summary by CodeRabbit

  • New Features

    • Added date validation to the Add Planned Purchases modal, preventing selection of dates before today.
    • Start dates now display using the user’s local calendar date, with tomorrow selected by default.
  • Bug Fixes

    • Fixed date handling for users in time zones west of UTC, ensuring “today” is recognized correctly.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca4d24be-b3bd-4df5-9534-3298efb6a843

📥 Commits

Reviewing files that changed from the base of the PR and between 9595958 and d8e718e.

📒 Files selected for processing (2)
  • frontend/src/__tests__/plans-range-validation.test.ts
  • frontend/src/plans.ts
📝 Walkthrough

Walkthrough

The Add Purchases modal now formats dates from local calendar components, defaults the start date to tomorrow, and prevents selecting dates before today. Tests cover the min attribute, default value, and UTC-boundary behavior.

Changes

Add Purchases start-date validation

Layer / File(s) Summary
Start-date constraint and regression coverage
frontend/src/plans.ts, frontend/src/__tests__/plans-range-validation.test.ts
Adds local YYYY-MM-DD formatting, sets the input minimum to today while retaining tomorrow as the default, and verifies both values plus local-calendar behavior in tests.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title mentions the Term field, but this PR only changes the start-date picker and tests. Rename it to focus on the date-picker fix, e.g. 'fix(ui): enforce local min date for Add Purchases start date'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The date-picker fix and regression test satisfy #1249, and #1250 is already covered in main per the rebased PR context.
Out of Scope Changes check ✅ Passed No unrelated code changes are present beyond the date-picker fix and its regression coverage.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/qa437-purchase-ui-polish

Comment @coderabbitai help to get the list of available commands.

@cristim cristim added triaged Item has been triaged type/bug Defect priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/this-quarter Within the quarter impact/few Limited audience effort/xs Trivial / one-liner labels Jun 19, 2026
@cristim

cristim commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
cristim changed the base branch from fix/1222-precommit-repair to main June 19, 2026 14:51
@cristim

cristim commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@cristim
cristim force-pushed the fix/qa437-purchase-ui-polish branch from 308f47c to 80df799 Compare June 19, 2026 22:02
@cristim

cristim commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 2 minutes and 5 seconds before sending another message.

@cristim

cristim commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@cristim

cristim commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Adversarial review pass — in-scope fix pushed, two follow-ups filed

In-scope fix (just pushed as 43c395440) — UTC vs local-calendar bug in the new min / default value.

Both lines added by this PR used new Date().toISOString().split('T')[0], which returns the UTC calendar day. For any user west of UTC after their local-evening crossover (e.g. US Pacific time after ~5pm local), min resolves to tomorrow's UTC date and the picker would grey out today — directly contradicting the PR body's promise that "today remains selectable (a same-day start is legitimate)" (QA 5.6).

The fix introduces a toLocalDateInputValue(Date) -> YYYY-MM-DD helper that builds the string from local year/month/day components (mirrors the local-midnight pattern isPlanOverdue already uses just above), and applies it to both value and min. The regression test in plans-range-validation.test.ts had the same shape as the production bug — new Date().toISOString().split('T')[0] on both sides — so it would have stayed green with the bug present. It now asserts against a local-component derivation, plus an explicit "local calendar day, not UTC" case documenting the failure shape.

tsc --noEmit clean. plans-range-validation.test.ts: 45 passed (was 42 + 3 new). plans.test.ts: 173 passed combined.

Out-of-scope follow-ups (filed as separate issues per the playbook):

  • Enforce future-only purchase start_date server-side (defense in depth for #1249) #1286 — server-side enforcement of start_date >= today. The title of this PR says "enforce future-only" but the <input type="date" min=...> attribute is only a UA hint (Chrome blocks submit, Firefox/Safari accept; devtools / a direct POST /plans/{id}/purchases bypasses it). parseCreatePurchasesRequest (internal/api/handler_plans.go:327-343) only validates the date format. The scheduler treats a past ScheduledDate as due-now (internal/purchase/manager.go:529), so a bypass would queue the executions for the next tick. P2 / severity/medium / effort/s.
  • Sweep toISOString().split('T')[0] -> toLocalDateInputValue across frontend date inputs #1287 — broader sweep of the same toISOString().split('T')[0] shape across frontend/src/history.ts (4 call sites) and frontend/src/apikeys.ts (1 call site). P3 / severity/low / effort/s.

Kept those out of this PR to honour its effort/xs scope and not delay the QA 5.6 / QA 5.3 polish.

CI UNSTABLE: the three failing checks on this PR (Lint Code, Integration Tests, Security Scanning) are preexisting main-wide failures unrelated to a CSS + date-min change:

  • Lint Code: 2875 Go-side golangci-lint issues (errcheck/godot/misspell/unparam/...) in internal/api/, internal/server/, internal/email/ etc. — none in this PR's files (all three changed files are frontend-only).
  • Integration Tests: role "testuser" does not exist Postgres bootstrap failure (CI infra).
  • Security Scanning: npm audit reporting pre-existing transitive vulns in webpack/svgo/ws/serialize-javascript.

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

@cristim Thanks for the thorough adversarial pass — the local-calendar fix makes sense, and keeping the server-side validation and broader toISOString().split('T')[0] sweep in follow-up issues #1286 and #1287 is a good scope split for this PR.

I’ll run a full review with the updated local-date helper/test coverage in mind.


✏️ Learnings added
Learnt from: cristim
URL: https://github.com/LeanerCloud/CUDly/pull/1258

Timestamp: 2026-06-26T13:56:27.300Z
Learning: In this repository's frontend date-input flows such as `frontend/src/plans.ts`, using `new Date().toISOString().split('T')[0]` is incorrect for local calendar semantics because it derives the UTC day and can disable/select the wrong local date for users west of UTC. Use a local-component helper that formats `YYYY-MM-DD` from local year/month/day for `<input type="date">` `value` and `min`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Action performed

Full review finished.

@cristim

cristim commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
cristim force-pushed the fix/qa437-purchase-ui-polish branch from 43c3954 to a864861 Compare July 17, 2026 07:56
@cristim

cristim commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Rebased onto origin/main (27fdb06). The term-width commit (closes #1250) was superseded by the already-merged #1430; it was resolved as a no-op during rebase -- the CSS rule from #1430 (min-width: 5.5rem) is kept. The remaining two commits (future-only start-date picker, closes #1249) are unique and intact. Gates: go build/vet exit 0; npm test exit 0 (78 suites, 2628 tests).

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@cristim: Understood—I'll review the rebased date-picker changes against the current main baseline, treating the term-width change as already covered by #1430.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
cristim force-pushed the fix/qa437-purchase-ui-polish branch from a864861 to 9595958 Compare July 17, 2026 10:59
@cristim

cristim commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Rebased onto origin/main (89658a3 -> a945344) which includes #1437 (tflint plugin cache + retry) and #1438 (trivy \*\*/.terraform skip). Workflow files confirmed to match main. Gates: npm test 2641 passed. Pre-commit should be fully green.

cristim added 2 commits July 17, 2026 17:03
#1249)

Set `startDateInput.min` to today's ISO date after the default value
(tomorrow) is assigned so the browser date-picker rejects past dates
while still allowing today as a valid same-day start.

Regression test added in plans-range-validation.test.ts: asserts
`input.min === today ISO` after `openAddPurchasesModal` returns (FAIL
pre-fix, PASS post-fix, 44 tests green).
… day

`new Date().toISOString().split('T')[0]` returns the UTC calendar date, not
the user's local one. For any user west of UTC after their local-evening
crossover, the prior `min = toISOString().split('T')[0]` would resolve to
tomorrow's UTC date and grey out today in the picker, directly contradicting
the PR's own claim that "today remains selectable (a same-day start is
legitimate)" (QA 5.6).

Extract a `toLocalDateInputValue(Date) -> YYYY-MM-DD` helper that builds the
ISO string from local year/month/day components (mirrors the local-midnight
pattern already used by `isPlanOverdue` above), and use it for both `value`
(default tomorrow) and `min` (today) in `openAddPurchasesModal`.

Regression test updated in `plans-range-validation.test.ts`: the prior
assertion used the same UTC function as production code, so it would have
gone green even with the bug present. The replacement asserts both `min` and
`value` against the local-component derivation, and adds an explicit
"local calendar day, not UTC" case that documents the failure shape.
@cristim
cristim force-pushed the fix/qa437-purchase-ui-polish branch from 9595958 to d8e718e Compare July 17, 2026 14:04
@cristim
cristim merged commit b09516a into main Jul 17, 2026
20 checks passed
@cristim
cristim deleted the fix/qa437-purchase-ui-polish branch July 27, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/few Limited audience priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

1 participant