From e0aedbb2c65d071b07f53ca864a24fc73e590ac4 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 7 Oct 2026 16:11:41 +0200 Subject: [PATCH] fix(trips): count trip dates in calendar days, the same on REST and MCP Filling in a missing end date and keeping a trip's length when its start moved both shifted a local-time Date, so a trip crossing a DST change lost or gained a day. Both now use addIsoDays. The one-date-makes-a-week default moves from the REST controller into TripsService.create, so the MCP create_trip tool no longer stores a half-dated trip, and the trip and its days are written in one transaction. --- .../components/Trips/TripFormModal.test.tsx | 14 ++++++++ client/src/components/Trips/TripFormModal.tsx | 11 +++--- server/src/nest/trips/trips.controller.ts | 9 ++--- server/src/nest/trips/trips.service.ts | 34 +++++++++++++------ .../tests/unit/nest/trips.controller.test.ts | 8 ++--- server/tests/unit/nest/trips.service.test.ts | 18 ++++++++++ 6 files changed, 65 insertions(+), 29 deletions(-) diff --git a/client/src/components/Trips/TripFormModal.test.tsx b/client/src/components/Trips/TripFormModal.test.tsx index ba4fb65b9..25e92f2a9 100644 --- a/client/src/components/Trips/TripFormModal.test.tsx +++ b/client/src/components/Trips/TripFormModal.test.tsx @@ -442,6 +442,20 @@ describe('TripFormModal', () => { })); }); + it('FE-COMP-TRIPFORM-038: moving the start across a DST change keeps the trip length in calendar days', async () => { + const user = userEvent.setup(); + const onSave = vi.fn().mockResolvedValue({}); + const trip = buildTrip({ id: 1, title: 'Spring Trip', start_date: '2026-03-20', end_date: '2026-03-29' }); + render(); + + await changeStartDate(user, '2026-03-25'); + await user.click(screen.getByRole('button', { name: /Update/i })); + await user.click(await screen.findByRole('button', { name: /Update/i })); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ start_date: '2026-03-25', end_date: '2026-04-03' })); + }); + it('FE-COMP-TRIPFORM-036: picking "Shift everything" sends shift_all', async () => { const user = userEvent.setup(); const onSave = vi.fn().mockResolvedValue({}); diff --git a/client/src/components/Trips/TripFormModal.tsx b/client/src/components/Trips/TripFormModal.tsx index 83198335f..3022e7624 100644 --- a/client/src/components/Trips/TripFormModal.tsx +++ b/client/src/components/Trips/TripFormModal.tsx @@ -11,7 +11,7 @@ import { useTranslation } from '../../i18n' import { CustomDatePicker } from '../shared/CustomDateTimePicker' import { normalizeImageFile } from '../../utils/convertHeic' import { getApiErrorMessage, type Trip } from '../../types' -import { MAX_TRIP_DAYS, tripSpanDays, type TripCreateRequest } from '@trek/shared' +import { MAX_TRIP_DAYS, addIsoDays, tripSpanDays, type TripCreateRequest } from '@trek/shared' import { NumericInput } from '../shared/NumericInput' import { currenciesWith, SYMBOLS } from '../Budget/BudgetPanel.constants' import TripDateReview, { dateReviewIsWide } from './TripDateReview' @@ -373,12 +373,9 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp if (!prev.end_date || prev.end_date < value) { next.end_date = value } else if (prev.start_date) { - const oldStart = new Date(prev.start_date + 'T00:00:00Z') - const oldEnd = new Date(prev.end_date + 'T00:00:00Z') - const duration = Math.round((oldEnd.getTime() - oldStart.getTime()) / 86400000) - const newEnd = new Date(value + 'T00:00:00Z') - newEnd.setDate(newEnd.getDate() + duration) - next.end_date = newEnd.toISOString().split('T')[0] + // Keep the trip's length, counted in UTC calendar days: a local-time + // shift across a DST change used to drop or add a day. + next.end_date = addIsoDays(value, tripSpanDays(prev.start_date, prev.end_date) - 1) } } return next diff --git a/server/src/nest/trips/trips.controller.ts b/server/src/nest/trips/trips.controller.ts index e13bcc8f8..b91c4c982 100644 --- a/server/src/nest/trips/trips.controller.ts +++ b/server/src/nest/trips/trips.controller.ts @@ -50,9 +50,6 @@ export const TRIP_COVER_FILE_FILTER: Options['fileFilter'] = (_req, file, cb) => else cb(new Error('Only jpg, png, gif, webp images allowed')); }; -const toDateStr = (d: Date) => d.toISOString().slice(0, 10); -const addDays = (d: Date, n: number) => { const r = new Date(d); r.setDate(r.getDate() + n); return r; }; - /** * /api/trips — the trip aggregate root. * @@ -115,10 +112,8 @@ export class TripsController { } // Presence/shape validation happens in the ZodValidationPipe (tripCreateRequestSchema). const { title, description, currency, reminder_days, day_count } = body; - let start_date: string | null = body.start_date || null; - let end_date: string | null = body.end_date || null; - if (start_date && !end_date) end_date = toDateStr(addDays(new Date(start_date), 6)); - else if (!start_date && end_date) start_date = toDateStr(addDays(new Date(end_date), -6)); + const start_date: string | null = body.start_date || null; + const end_date: string | null = body.end_date || null; if (start_date && end_date && new Date(end_date) < new Date(start_date)) { throw new HttpException({ error: 'End date must be after start date' }, 400); } diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index 2e67d8dfc..fc0f54e2a 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -28,6 +28,7 @@ import { TourWaypoints } from '../../db/entities/TourWaypoints.entity'; import type { TourWaypointRow } from '../../db/repositories/TourWaypoints.repository'; import { MAX_TRIP_DAYS, + addIsoDays, planDayGrid, resolveDayGridRange, tripSpanDays, @@ -405,23 +406,34 @@ export class TripsService { } async create(userId: number, data: CreateTripData) { - if (data.start_date && data.end_date) assertTripSpan(data.start_date, data.end_date); + // One given date makes a week, on REST and MCP alike. Calendar arithmetic + // in UTC: a local-time Date shifted across a DST change lost or gained a day. + let startDate = data.start_date || null; + let endDate = data.end_date || null; + if (startDate && !endDate) endDate = addIsoDays(startDate, 6); + else if (!startDate && endDate) startDate = addIsoDays(endDate, -6); + if (startDate && endDate) assertTripSpan(startDate, endDate); const rd = data.reminder_days !== undefined ? (Number(data.reminder_days) >= 0 && Number(data.reminder_days) <= 30 ? Number(data.reminder_days) : 3) : 3; + const currency = data.currency || (await this.defaultCurrencyFor(userId)); - const tripId = await this.tripsRepo.insertTrip({ // TP18 - user_id: userId, - title: data.title, - description: data.description || null, - start_date: data.start_date || null, - end_date: data.end_date || null, - currency: data.currency || (await this.defaultCurrencyFor(userId)), - reminder_days: rd, + // The trip and its days commit together: a failure in between used to leave + // a dated trip without a single day. + const tripId = await this.uow.transactional(async () => { + const id = await this.tripsRepo.insertTrip({ // TP18 + user_id: userId, + title: data.title, + description: data.description || null, + start_date: startDate, + end_date: endDate, + currency, + reminder_days: rd, + }); + await this.generateDays(id, startDate, endDate, data.day_count); + return id; }); - await this.generateDays(tripId, data.start_date || null, data.end_date || null, data.day_count); - const trip = await this.tripsRepo.findForViewer(tripId, userId); // TP19 — the creator always owns it, so the access predicate is trivially satisfied return { trip, tripId, reminderDays: rd }; } diff --git a/server/tests/unit/nest/trips.controller.test.ts b/server/tests/unit/nest/trips.controller.test.ts index c57f1439e..5779b85b8 100644 --- a/server/tests/unit/nest/trips.controller.test.ts +++ b/server/tests/unit/nest/trips.controller.test.ts @@ -161,10 +161,10 @@ describe('TripsController (parity with the legacy /api/trips route)', async () = expect(tripCreateRequestSchema.safeParse({ title: '' }).success).toBe(false); }); - it('infers end_date from start_date (+6 days) and creates', async () => { + it('passes a single start date on as it is; the service fills in the week', async () => { const create = vi.fn().mockReturnValue({ trip: { id: 9 }, tripId: 9, reminderDays: 0 }); await tc(svc({ create } as Partial)).create(user, { title: 'T', start_date: '2026-07-01' }, req); - expect(create).toHaveBeenCalledWith(1, expect.objectContaining({ start_date: '2026-07-01', end_date: '2026-07-07' })); + expect(create).toHaveBeenCalledWith(1, expect.objectContaining({ start_date: '2026-07-01', end_date: null })); }); it('400 when end_date precedes start_date', async () => { @@ -173,10 +173,10 @@ describe('TripsController (parity with the legacy /api/trips route)', async () = }); }); - it('infers start_date from end_date (-6 days) and parses day_count', async () => { + it('passes a single end date on as it is and parses day_count', async () => { const create = vi.fn().mockReturnValue({ trip: { id: 9 }, tripId: 9, reminderDays: 0 }); await tc(svc({ create } as Partial)).create(user, prePipeCreateBody({ title: 'T', end_date: '2026-07-07', day_count: '40' }), req); - expect(create).toHaveBeenCalledWith(1, expect.objectContaining({ start_date: '2026-07-01', end_date: '2026-07-07', day_count: 40 })); + expect(create).toHaveBeenCalledWith(1, expect.objectContaining({ start_date: null, end_date: '2026-07-07', day_count: 40 })); }); it('clamps a non-numeric day_count to the default of 7', async () => { diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 0c872e5d4..6e889c28b 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -1116,6 +1116,24 @@ describe('folded trip CRUD', () => { expect(getDays(tripId)).toHaveLength(3); }); + it('TRIP-SVC-090: one given date makes a week, counted in calendar days across a DST change', async () => { + const { user } = createUser(testDb); + const fromStart = await svc.create(user.id, { title: 'Spring', start_date: '2026-03-25' }); + expect(fromStart.trip).toMatchObject({ start_date: '2026-03-25', end_date: '2026-03-31' }); + expect(getDays(fromStart.tripId)).toHaveLength(7); + const fromEnd = await svc.create(user.id, { title: 'Autumn', end_date: '2026-10-30' }); + expect(fromEnd.trip).toMatchObject({ start_date: '2026-10-24', end_date: '2026-10-30' }); + }); + + it('TRIP-SVC-091: the trip and its days commit together', async () => { + const { user } = createUser(testDb); + const before = (testDb.prepare('SELECT COUNT(*) AS c FROM trips').get() as { c: number }).c; + const generate = vi.spyOn(svc, 'generateDays').mockRejectedValueOnce(new Error('disk I/O error')); + await expect(svc.create(user.id, { title: 'Half', start_date: '2026-05-01', end_date: '2026-05-03' })).rejects.toThrow('disk I/O error'); + generate.mockRestore(); + expect((testDb.prepare('SELECT COUNT(*) AS c FROM trips').get() as { c: number }).c).toBe(before); + }); + it('TRIP-SVC-080: create without a currency takes the display currency, admin default included, else EUR', async () => { const { user } = createUser(testDb); const setUser = (value: string) =>