From aaef3e3e769b08b47028069f628db4e676fd1641 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 29 Jul 2026 14:37:28 +0200 Subject: [PATCH] fix(reservations): honour the 24h time setting (#1725) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 12h/24h rules lived in three copies and two of them ignored the setting. They are one function now. splitReservationDateTime also truncated the meridiem instead of converting it, so 3:00 PM became 3:00 — three in the morning. Values already stored that way normalise when a form renders the field, without changing the payload or the stored format. --- .../Planner/DayDetailPanel.test.tsx | 40 +++++++ .../src/components/Planner/DayDetailPanel.tsx | 18 +-- .../shared/CustomTimePicker.test.tsx | 107 ++++++++++++++++++ .../components/shared/CustomTimePicker.tsx | 48 ++++---- client/src/utils/formatters.test.ts | 12 ++ client/src/utils/formatters.ts | 50 ++++++-- client/tests/unit/utils/formatters.test.ts | 64 ++++++++++- 7 files changed, 292 insertions(+), 47 deletions(-) diff --git a/client/src/components/Planner/DayDetailPanel.test.tsx b/client/src/components/Planner/DayDetailPanel.test.tsx index 3401db44c..d1f84e3e4 100644 --- a/client/src/components/Planner/DayDetailPanel.test.tsx +++ b/client/src/components/Planner/DayDetailPanel.test.tsx @@ -1830,3 +1830,43 @@ describe('DayDetailPanel remaining branches, part two', () => { expect(await screen.findByText(/Add accommodation/i)).toBeInTheDocument(); }); }); + +// FE-DDP1725-001 to -003 — the day panel reads times through the shared formatter, so a +// value that was stored with a meridiem still follows the configured format (#1725). +describe('DayDetailPanel time format', () => { + const meridiemRes = (overrides: Record = {}) => + buildReservation({ + id: 9, type: 'event', title: 'Matinee', day_id: 1, + reservation_time: '3:00 PM', reservation_end_time: '11:00 PM', status: 'confirmed', + ...overrides, + }); + + it('FE-DDP1725-001: a reservation stored with a meridiem shows in 24h', async () => { + render(); + expect(await screen.findByText('15:00 – 23:00')).toBeInTheDocument(); + }); + + it('FE-DDP1725-002: the same reservation keeps its afternoon in 12h', async () => { + seedStore(useSettingsStore, { settings: { time_format: '12h', temperature_unit: 'celsius', blur_booking_codes: false } }); + render(); + expect(await screen.findByText('3:00 PM – 11:00 PM')).toBeInTheDocument(); + }); + + it('FE-DDP1725-003: an accommodation check-in stored with a meridiem shows in 24h', async () => { + server.use( + http.get('/api/trips/1/accommodations', () => + HttpResponse.json({ + accommodations: [{ + id: 1, place_id: 5, place_name: 'Grand Hotel', place_address: 'Paris', + start_day_id: 1, end_day_id: 3, check_in: '3:00 PM', check_in_end: null, + check_out: '11:00 AM', confirmation: null, + }], + }), + ), + ); + render(); + + expect(await screen.findByText('15:00')).toBeInTheDocument(); + expect(screen.getByText('11:00')).toBeInTheDocument(); + }); +}); diff --git a/client/src/components/Planner/DayDetailPanel.tsx b/client/src/components/Planner/DayDetailPanel.tsx index e98ab3d54..4b24f3820 100644 --- a/client/src/components/Planner/DayDetailPanel.tsx +++ b/client/src/components/Planner/DayDetailPanel.tsx @@ -16,7 +16,7 @@ import { useSettingsStore } from '../../store/settingsStore' import { getLocaleForLanguage, useTranslation } from '../../i18n' import type { Day, Place, Category, Reservation, AssignmentsMap } from '../../types' import { isDayInAccommodationRange } from '../../utils/dayOrder' -import { splitReservationDateTime } from '../../utils/formatters' +import { formatClockTime, splitReservationDateTime } from '../../utils/formatters' import { useDayDetail } from './useDayDetail' const WEATHER_ICON_MAP = { @@ -36,16 +36,6 @@ function WIcon({ main, size = 14 }: WIconProps) { function cTemp(c, f) { return Math.round(f ? c * 9 / 5 + 32 : c) } -function formatTime12(val, is12h) { - if (!val) return val - const [h, m] = val.split(':').map(Number) - if (isNaN(h) || isNaN(m)) return val - if (!is12h) return val - const period = h >= 12 ? 'PM' : 'AM' - const h12 = h === 0 ? 12 : h > 12 ? h - 12 : h - return `${h12}:${String(m).padStart(2, '0')} ${period}` -} - interface DayDetailPanelProps { day: Day days: Day[] @@ -78,7 +68,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri const fmtTime = (v) => { if (!v) return v if (v.includes('T')) return new Date(v).toLocaleTimeString(locale, { hour: '2-digit', minute: '2-digit', hour12: is12h }) - return formatTime12(v, is12h) + return formatClockTime(v, is12h) } const unit = isFahrenheit ? '°F' : '°C' const collapsed = collapsedProp @@ -313,8 +303,8 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri if (!startTime && !endTime) return null return ( - {startTime ? formatTime12(startTime, is12h) : ''} - {endTime ? ` – ${formatTime12(endTime, is12h)}` : ''} + {startTime ? formatClockTime(startTime, is12h) : ''} + {endTime ? ` – ${formatClockTime(endTime, is12h)}` : ''} ) })()} diff --git a/client/src/components/shared/CustomTimePicker.test.tsx b/client/src/components/shared/CustomTimePicker.test.tsx index 9c6b8d1a8..d136910f2 100644 --- a/client/src/components/shared/CustomTimePicker.test.tsx +++ b/client/src/components/shared/CustomTimePicker.test.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react'; import { render, screen, fireEvent, act } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import CustomTimePicker from './CustomTimePicker'; @@ -455,6 +456,19 @@ describe('CustomTimePicker branches', () => { expect(onChange).toHaveBeenCalledWith('05:30'); }); + it('FE-W5TP-030: a meridiem typed while 24h is configured survives until blur', () => { + render(); + fireEvent.change(screen.getByRole('textbox'), { target: { value: '5:30 pm' } }); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith('5:30 pm'); + }); + + it('FE-W5TP-031: blur reads a meridiem correctly while 24h is configured', () => { + render(); + fireEvent.blur(screen.getByRole('textbox')); + expect(onChange).toHaveBeenCalledWith('17:30'); + }); + it('FE-W5TP-024: the steppers and the clear button highlight on hover', () => { use12h(); render(); @@ -476,3 +490,96 @@ describe('CustomTimePicker branches', () => { expect(clear.style.color).toBe('var(--text-faint)'); }); }); + +// FE-TP1725-001 to -006 — the configured time format decides how a time is shown, +// including values that were stored with a meridiem (booking check-in/check-out +// times entered while 12h was set, or carried in by an import). +describe('CustomTimePicker honours the configured time format', () => { + const onChange = vi.fn(); + const steppers = () => screen.getAllByRole('button').filter(b => b.textContent?.trim() === ''); + + beforeEach(() => { + vi.clearAllMocks(); + resetAllStores(); + }); + + const use = (time_format: string) => seedStore(useSettingsStore, { settings: buildSettings({ time_format }) }); + + it('FE-TP1725-001: shows a stored meridiem value in 24h', () => { + use('24h'); + render(); + expect(screen.getByRole('textbox')).toHaveProperty('value', '15:00'); + }); + + it('FE-TP1725-002: keeps a stored meridiem value in 12h', () => { + use('12h'); + render(); + expect(screen.getByRole('textbox')).toHaveProperty('value', '3:00 PM'); + }); + + it('FE-TP1725-003: converts a 24h value for a 12h user', () => { + use('12h'); + render(); + expect(screen.getByRole('textbox')).toHaveProperty('value', '3:00 PM'); + }); + + it('FE-TP1725-004: the dropdown steppers start from the converted value', () => { + use('24h'); + render(); + fireEvent.click(steppers()[0]); + expect(screen.getByText('15')).toBeInTheDocument(); + expect(screen.getByText('00')).toBeInTheDocument(); + fireEvent.click(steppers()[1]); + expect(onChange).toHaveBeenLastCalledWith('16:00'); + }); + + it('FE-TP1725-005: the empty-field hint follows the setting', () => { + use('24h'); + const { unmount } = render(); + expect(screen.getByRole('textbox')).toHaveProperty('placeholder', '00:00'); + unmount(); + + use('12h'); + render(); + expect(screen.getByRole('textbox')).toHaveProperty('placeholder', '2:30 PM'); + }); + + it('FE-TP1725-006: a 24h value is left alone in 24h', () => { + use('24h'); + render(); + expect(screen.getByRole('textbox')).toHaveProperty('value', '14:30'); + }); + + // Display alone is not enough: a value stored as "3:00 PM" stays that way in the + // database until someone edits the field. The picker hands the parsed HH:MM back to + // the form instead, so the next save writes a clean value. + const Controlled = ({ initial }: { initial: string }) => { + const [v, setV] = useState(initial); + return { onChange(nv); setV(nv); }} />; + }; + + it('FE-TP1725-007: a stored meridiem value is handed back as HH:MM without an edit', () => { + use('24h'); + render(); + expect(onChange).toHaveBeenCalledWith('15:00'); + expect(screen.getByRole('textbox')).toHaveProperty('value', '15:00'); + }); + + it('FE-TP1725-008: typing a meridiem is left alone until the field is left', () => { + use('24h'); + render(); + const input = screen.getByRole('textbox'); + fireEvent.focus(input); + fireEvent.change(input, { target: { value: '5:30 pm' } }); + expect(input).toHaveProperty('value', '5:30 pm'); + fireEvent.blur(input); + expect(onChange).toHaveBeenLastCalledWith('17:30'); + expect(input).toHaveProperty('value', '17:30'); + }); + + it('FE-TP1725-009: a clean value is not written back', () => { + use('24h'); + render(); + expect(onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/client/src/components/shared/CustomTimePicker.tsx b/client/src/components/shared/CustomTimePicker.tsx index 3360c6c55..4b1aa08c5 100644 --- a/client/src/components/shared/CustomTimePicker.tsx +++ b/client/src/components/shared/CustomTimePicker.tsx @@ -2,16 +2,7 @@ import React, { useState, useRef, useEffect } from 'react' import ReactDOM from 'react-dom' import { Clock, ChevronUp, ChevronDown } from 'lucide-react' import { useSettingsStore } from '../../store/settingsStore' - -function formatDisplay(val: string, is12h: boolean): string { - if (!val) return '' - const [h, m] = val.split(':').map(Number) - if (isNaN(h) || isNaN(m)) return val - if (!is12h) return val - const period = h >= 12 ? 'PM' : 'AM' - const h12 = h === 0 ? 12 : h > 12 ? h - 12 : h - return `${h12}:${String(m).padStart(2, '0')} ${period}` -} +import { formatClockTime, parseMeridiemTime } from '../../utils/formatters' interface CustomTimePickerProps { value: string @@ -27,7 +18,7 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00 const ref = useRef(null) const dropRef = useRef(null) - const [h, m] = (value || '').split(':').map(Number) + const [h, m] = (parseMeridiemTime(value) ?? value ?? '').split(':').map(Number) const hour = isNaN(h) ? null : h const minute = isNaN(m) ? null : m @@ -41,6 +32,16 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00 return () => document.removeEventListener('mousedown', handler) }, [open]) + // A value that arrives with a meridiem ("3:00 PM" — stored while 12h was configured, + // or carried in by a booking import) is handed back as HH:MM right away, so the form + // saves a clean value even when the field is never touched (#1725). While the input + // has focus this stays out of the way — handleBlur parses what was typed. + useEffect(() => { + if (inputFocused) return + const norm = parseMeridiemTime(value) + if (norm && norm !== value) onChange(norm) + }, [value, inputFocused]) + const update = (newH: number, newM: number) => { const hh = String(Math.max(0, Math.min(23, newH))).padStart(2, '0') const mm = String(Math.max(0, Math.min(59, newM))).padStart(2, '0') @@ -69,7 +70,9 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00 const handleInput = (e: React.ChangeEvent) => { const raw = e.target.value onChange(raw) - if (is12h) return // let handleBlur parse 12h formats + // Anything with letters in it ("5:30 pm") is left to handleBlur — stripping + // them here would swallow the meridiem and turn 5:30 pm into 05:30. + if (is12h || /[a-z]/i.test(raw)) return const clean = raw.replace(/[^0-9:]/g, '') if (/^\d{2}:\d{2}$/.test(clean)) onChange(clean) else if (/^\d{4}$/.test(clean)) onChange(clean.slice(0, 2) + ':' + clean.slice(2)) @@ -83,18 +86,13 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00 if (!value) return const raw = value.trim() - // Parse 12h input like "5:30 PM", "5:30pm", "530pm" - if (is12h) { - const match12 = raw.match(/^(\d{1,2}):?(\d{2})?\s*(am|pm)$/i) - if (match12) { - let h = parseInt(match12[1]) - const m = match12[2] ? parseInt(match12[2]) : 0 - const isPm = match12[3].toLowerCase() === 'pm' - if (h === 12) h = isPm ? 12 : 0 - else if (isPm) h += 12 - onChange(String(Math.min(23, h)).padStart(2, '0') + ':' + String(Math.min(59, m)).padStart(2, '0')) - return - } + // Parse 12h input like "5:30 PM", "5:30pm", "530pm". A meridiem is + // unambiguous, so it is honoured whatever the configured format is — + // otherwise the cleanup below reads "5:30 PM" as 05:30 (#1725). + const parsed12h = parseMeridiemTime(raw) + if (parsed12h) { + onChange(parsed12h) + return } const clean = raw.replace(/[^0-9:]/g, '') @@ -124,7 +122,7 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00 }}> setInputFocused(true)} onBlur={() => { setInputFocused(false); handleBlur() }} diff --git a/client/src/utils/formatters.test.ts b/client/src/utils/formatters.test.ts index bc94ef5b5..bac3c66aa 100644 --- a/client/src/utils/formatters.test.ts +++ b/client/src/utils/formatters.test.ts @@ -220,4 +220,16 @@ describe('splitReservationDateTime', () => { it('returns nulls for unrecognized string', () => { expect(splitReservationDateTime('garbage')).toEqual({ date: null, time: null }) }) + + // #1725 — slicing the time part to five characters used to swallow the meridiem, + // so an afternoon booking came back as a morning one. + it('converts a meridiem time part instead of cutting it off', () => { + expect(splitReservationDateTime('2026-08-01T3:00 PM')).toEqual({ date: '2026-08-01', time: '15:00' }) + expect(splitReservationDateTime('2026-08-01T12:30 am')).toEqual({ date: '2026-08-01', time: '00:30' }) + }) + + it('converts a bare meridiem time', () => { + expect(splitReservationDateTime('3:00 PM')).toEqual({ date: null, time: '15:00' }) + expect(splitReservationDateTime('3 PM')).toEqual({ date: null, time: '15:00' }) + }) }) diff --git a/client/src/utils/formatters.ts b/client/src/utils/formatters.ts index 91be2c9ee..cbbc86ed8 100644 --- a/client/src/utils/formatters.ts +++ b/client/src/utils/formatters.ts @@ -128,31 +128,67 @@ export function formatDate(dateStr: string | null | undefined, locale: string, t return date.toLocaleDateString(locale, opts) } +// Times are stored as HH:MM, but a value can still carry a meridiem — typed while +// 12h was configured, or handed over by a booking import. Convert those to 24h so +// the configured time format decides how a time is shown, not the way it happens +// to be stored. Returns null when there is no meridiem to convert. +export function parseMeridiemTime(value: string | null | undefined): string | null { + const match = (value || '').trim().match(/^(\d{1,2}):?(\d{2})?\s*(am|pm)$/i) + if (!match) return null + let h = parseInt(match[1]) + const m = match[2] ? parseInt(match[2]) : 0 + const isPm = match[3].toLowerCase() === 'pm' + if (h === 12) h = isPm ? 12 : 0 + else if (isPm) h += 12 + return `${String(Math.min(23, h)).padStart(2, '0')}:${String(Math.min(59, m)).padStart(2, '0')}` +} + +function to12h(h: number, m: number): string { + const period = h >= 12 ? 'PM' : 'AM' + const h12 = h === 0 ? 12 : h > 12 ? h - 12 : h + return `${h12}:${String(m).padStart(2, '0')} ${period}` +} + export function formatTime(timeStr: string | null | undefined, locale: string, timeFormat: string): string { if (!timeStr) return '' try { - const parts = timeStr.split(':') + const parts = (parseMeridiemTime(timeStr) ?? timeStr).split(':') const h = Number(parts[0]) || 0 const m = Number(parts[1]) || 0 if (isNaN(h)) return timeStr - if (timeFormat === '12h') { - const period = h >= 12 ? 'PM' : 'AM' - const h12 = h === 0 ? 12 : h > 12 ? h - 12 : h - return `${h12}:${String(m).padStart(2, '0')} ${period}` - } + if (timeFormat === '12h') return to12h(h, m) const str = `${String(h).padStart(2, '0')}:${String(m).padStart(2, '0')}` return locale?.startsWith('de') ? `${str} Uhr` : str } catch { return timeStr } } +/** + * Clock time for the compact spots that have no locale to work with — the time picker + * input and the dense day/booking rows. Same 12h/24h decision as formatTime, but it + * leaves a 24h value exactly as it is instead of adding the German "Uhr" suffix. + * A stored meridiem is normalised first, so the setting decides in both directions. + */ +export function formatClockTime(value: string | null | undefined, is12h: boolean): string { + if (!value) return '' + const norm = parseMeridiemTime(value) ?? value + const [h, m] = norm.split(':').map(Number) + if (isNaN(h) || isNaN(m)) return value + return is12h ? to12h(h, m) : norm +} + export function splitReservationDateTime(value?: string | null): { date: string | null; time: string | null } { if (!value) return { date: null, time: null } const isoDate = /^\d{4}-\d{2}-\d{2}$/ + // Cutting the time part after five characters would drop the meridiem and turn + // "3:00 PM" into "3:00", i.e. 3 AM — convert it instead (#1725). + const timePart = (t: string) => parseMeridiemTime(t) ?? t.slice(0, 5) if (value.includes('T')) { const [d, t] = value.split('T') - return { date: isoDate.test(d) ? d : null, time: t ? t.slice(0, 5) : null } + return { date: isoDate.test(d) ? d : null, time: t ? timePart(t) : null } } if (isoDate.test(value)) return { date: value, time: null } + const meridiem = parseMeridiemTime(value) + if (meridiem) return { date: null, time: meridiem } if (/^\d{1,2}:\d{2}/.test(value)) return { date: null, time: value.slice(0, 5) } return { date: null, time: null } } diff --git a/client/tests/unit/utils/formatters.test.ts b/client/tests/unit/utils/formatters.test.ts index 4f6cee194..0530f3b71 100644 --- a/client/tests/unit/utils/formatters.test.ts +++ b/client/tests/unit/utils/formatters.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { formatDate, formatTime, dayTotalCost, currencyDecimals } from '../../../src/utils/formatters'; +import { formatDate, formatTime, formatClockTime, dayTotalCost, currencyDecimals, parseMeridiemTime } from '../../../src/utils/formatters'; import type { AssignmentsMap } from '../../../src/types'; // dayTotalCost intentionally exercises edge-case price inputs (string / non-numeric), @@ -60,6 +60,68 @@ describe('formatTime', () => { expect(formatTime('12:00', 'en-US', '12h')).toBe('12:00 PM'); expect(formatTime('01:00', 'en-US', '12h')).toBe('1:00 AM'); }); + + // #1725 — a value stored with a meridiem must still follow the configured format + it('converts a stored meridiem value to the configured format', () => { + expect(formatTime('3:00 PM', 'en-US', '24h')).toBe('15:00'); + expect(formatTime('12:30 am', 'en-US', '24h')).toBe('00:30'); + expect(formatTime('3:00 PM', 'en-US', '12h')).toBe('3:00 PM'); + expect(formatTime('15:00', 'de-DE', '24h')).toBe('15:00 Uhr'); + }); +}); + +describe('parseMeridiemTime', () => { + it('returns null when there is no meridiem to convert', () => { + expect(parseMeridiemTime(null)).toBeNull(); + expect(parseMeridiemTime(undefined)).toBeNull(); + expect(parseMeridiemTime('')).toBeNull(); + expect(parseMeridiemTime('14:30')).toBeNull(); + expect(parseMeridiemTime('later')).toBeNull(); + }); + + it('converts a meridiem time to HH:MM', () => { + expect(parseMeridiemTime('5:30 pm')).toBe('17:30'); + expect(parseMeridiemTime('5:30 AM')).toBe('05:30'); + expect(parseMeridiemTime('530pm')).toBe('17:30'); + expect(parseMeridiemTime('5pm')).toBe('17:00'); + }); + + it('maps 12 AM to midnight and 12 PM to noon', () => { + expect(parseMeridiemTime('12:30 AM')).toBe('00:30'); + expect(parseMeridiemTime('12:30 PM')).toBe('12:30'); + }); +}); + +// The locale-free counterpart of formatTime, shared by the time picker and the dense +// day/booking rows so those two do not carry their own copy of the rules (#1725). +describe('formatClockTime', () => { + it('returns an empty string for a missing value', () => { + expect(formatClockTime(null, false)).toBe(''); + expect(formatClockTime(undefined, true)).toBe(''); + expect(formatClockTime('', true)).toBe(''); + }); + + it('leaves a 24h value untouched in 24h and never adds a locale suffix', () => { + expect(formatClockTime('14:30', false)).toBe('14:30'); + expect(formatClockTime('09:05', false)).toBe('09:05'); + }); + + it('converts a 24h value for a 12h user', () => { + expect(formatClockTime('14:30', true)).toBe('2:30 PM'); + expect(formatClockTime('00:15', true)).toBe('12:15 AM'); + expect(formatClockTime('12:45', true)).toBe('12:45 PM'); + }); + + it('converts a stored meridiem value down to 24h', () => { + expect(formatClockTime('3:00 PM', false)).toBe('15:00'); + expect(formatClockTime('12:30 am', false)).toBe('00:30'); + expect(formatClockTime('3:00 PM', true)).toBe('3:00 PM'); + }); + + it('hands back anything it cannot read', () => { + expect(formatClockTime('later', false)).toBe('later'); + expect(formatClockTime('later', true)).toBe('later'); + }); }); describe('dayTotalCost', () => {