DateTime field should be truncated to minutes #4645 - #4646
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the DateTime/Instant form inputs and descriptors so the UI and default-value handling operate at minute precision (dropping seconds and fractional seconds), aligning with the minute-granular pickers.
Changes:
- Truncate DateTime/Instant default values (including relative expressions like
now,+1d) to minute precision. - Render Instant/DateTime inputs at HH:mm precision, and update tests to cover truncation behavior.
- Tighten public surface area by changing component barrel exports to avoid re-exporting test-only helpers.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/assets/admin/common/js/form2/descriptor/InstantDescriptor.ts | Truncates parsed instants/offsets/relative defaults to minute precision before creating Value. |
| src/main/resources/assets/admin/common/js/form2/descriptor/InstantDescriptor.test.ts | Updates and adds tests asserting minute-precision defaults for Instant parsing. |
| src/main/resources/assets/admin/common/js/form2/descriptor/DateTimeDescriptor.ts | Adds truncateToMinutes() and uses it to drop seconds/fractions for DateTime defaults. |
| src/main/resources/assets/admin/common/js/form2/descriptor/DateTimeDescriptor.test.ts | Adds tests for minute truncation and for the new helper. |
| src/main/resources/assets/admin/common/js/form2/components/instant-input/InstantInput.tsx | Displays Instant values in minutes (HH:mm); exports helper functions for tests. |
| src/main/resources/assets/admin/common/js/form2/components/instant-input/InstantInput.test.ts | Switches to testing exported helpers; adds assertions that seconds/fractions aren’t displayed. |
| src/main/resources/assets/admin/common/js/form2/components/instant-input/index.ts | Changes barrel export to avoid re-exporting helper functions. |
| src/main/resources/assets/admin/common/js/form2/components/date-time-input/index.ts | Changes barrel export to avoid re-exporting helper functions. |
| src/main/resources/assets/admin/common/js/form2/components/date-time-input/DateTimeInput.tsx | Truncates displayed DateTime values to minutes via shared helper. |
| src/main/resources/assets/admin/common/js/form2/components/date-time-input/DateTimeInput.test.ts | Adds tests asserting valueToDisplay() truncates values with seconds/fractions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| export function displayToStorage(s: string): string { | ||
| const date = new Date(s.replace(' ', 'T')); | ||
| if (Number.isNaN(date.getTime())) return `${s.replace(' ', 'T')}Z`; | ||
| const y = date.getUTCFullYear(); |
There was a problem hiding this comment.
Good catch — fixed in 05b8856.
displayToStorage() no longer appends Z when new Date(...) fails. That Z asserted "this is a valid UTC instant", which is exactly what an unparseable string is not: 2025-99-99 14:30 became 2025-99-99T14:30Z, which passes ValueTypeDateTime.isConvertible() (regex only) and then throws in DateTime.fromString():
Error: Invalid date string for Instant: 2025-99-99T14:30Z
Without the Z the fallback no longer matches the instant pattern, so newValue() returns a null value and the field shows the usual invalid-value validation instead of crashing. No regression is possible here: the fallback only runs when new Date() returns NaN, i.e. when no correct storage value exists for that input at all.
Went with this over wrapping newValue() in try/catch, since the latter would leave the same trap for every other caller.
Tests added for an impossible date (2025-99-99 14:30) and an out-of-range time (2025-06-15 25:99), plus the existing fallback test updated.
Note the root cause is broader: ValueTypeDateTime.isValid() validates by regex only, so any YYYY-99-99THH:mmZ string reaching newValue() throws regardless of the caller. Left out of this PR as it touches a widely used value type.
Drafted with AI assistance
Stopped appending `Z` to unparseable input in `displayToStorage`, so an impossible date no longer satisfies the instant pattern and makes `ValueTypeDateTime.newValue()` throw. Added tests for an impossible date and an out-of-range time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Validated typed input with `DateHelper.parseDateTime`, so impossible dates no longer throw in `LocalDateTime.fromString` and no longer roll over to another date in Instant Rejected input carrying seconds, so a typed value can no longer differ from the minute precision the field shows Replaced `DISPLAY_PATTERN`, `parseDateFromDisplay`, `parseTimeFromDisplay` and `displayToStorage` with a single `parseDateTime` call shared by the input and the picker prefill Exported `displayToValue` and `formatTimezoneLabel`, dropping the local copies of implementation the tests were pinning instead of the shipped code Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Pushed 4ba2e2a — scope grew a little past display truncation. Typed input in both fields is now validated with
Drafted with AI assistance |
No description provided.