From d2e044869a539f64b1e0b48374e5584e2cabb216 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 17:08:28 +0200 Subject: [PATCH] fix(days): a day without a date keeps no date when it is moved, so it can go to the front of a dated trip Reordering handed out the dates by position, so moving a day added without a date to the front gave it the first date and took the date of the last dated day. The undated row stayed at the end and the move looked like it did nothing, by drag and by the arrows alike. The dates now go to the dated days only, in their new order; the rule lives in shared so the server, the planner's optimistic list and the MCP tool apply the same one. --- client/src/store/slices/daysSlice.test.ts | 23 +++++++++++++ client/src/store/slices/daysSlice.ts | 5 +-- server/src/nest/days/days.mcp.ts | 2 +- server/src/nest/days/days.service.ts | 13 +++----- server/tests/integration/dayReorder.test.ts | 12 +++++++ shared/src/trip/day-grid.spec.ts | 36 +++++++++++++++++++++ shared/src/trip/day-grid.ts | 26 +++++++++++++++ 7 files changed, 106 insertions(+), 11 deletions(-) diff --git a/client/src/store/slices/daysSlice.test.ts b/client/src/store/slices/daysSlice.test.ts index 0b36cbaa4..af182e7d3 100644 --- a/client/src/store/slices/daysSlice.test.ts +++ b/client/src/store/slices/daysSlice.test.ts @@ -42,6 +42,29 @@ describe('daysSlice', () => { expect(optimistic[0].title).toBe('Departure'); }); + it('FE-TSLICE-DAYS-001b: a day without a date moves to the front and keeps no date', async () => { + const days = [ + buildDay({ id: 1, trip_id: 1, day_number: 1, date: '2025-06-01' }), + buildDay({ id: 2, trip_id: 1, day_number: 2, date: '2025-06-02' }), + buildDay({ id: 3, trip_id: 1, day_number: 3, date: null }), + ]; + seedStore(useTripStore, { days }); + + let optimistic: Day[] = []; + server.use( + http.put('/api/trips/1/days/reorder', () => { + optimistic = useTripStore.getState().days; + return HttpResponse.json({ success: true }); + }), + http.get('/api/trips/1/days', () => HttpResponse.json({ days })), + ); + + await useTripStore.getState().reorderDays(1, [3, 1, 2]); + + expect(optimistic.map(d => d.id)).toEqual([3, 1, 2]); + expect(optimistic.map(d => d.date)).toEqual([null, '2025-06-01', '2025-06-02']); + }); + it('FE-TSLICE-DAYS-002: sends the requested order and refreshes days plus bookings', async () => { seedStore(useTripStore, { days: datedDays(), reservations: [] }); diff --git a/client/src/store/slices/daysSlice.ts b/client/src/store/slices/daysSlice.ts index 4820b2706..30b2c0477 100644 --- a/client/src/store/slices/daysSlice.ts +++ b/client/src/store/slices/daysSlice.ts @@ -4,6 +4,7 @@ import type { StoreApi } from 'zustand' import type { TripStoreState } from '../tripStore' import type { Day } from '../../types' import { getApiErrorMessage } from '../../types' +import { reorderedDayDates } from '@trek/shared' type SetState = StoreApi['setState'] type GetState = StoreApi['getState'] @@ -47,12 +48,12 @@ export const createDaysSlice = (set: SetState, get: GetState): DaysSlice => ({ reorderDays: async (tripId, orderedIds) => { const prevDays = get().days const byId = new Map(prevDays.map(d => [d.id, d])) - const sortedDates = prevDays.map(d => d.date).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)) + const dates = reorderedDayDates(prevDays, orderedIds) const optimistic = orderedIds .map((id, i) => { const d = byId.get(id) if (!d) return null - return { ...d, day_number: i + 1, date: sortedDates.length ? (sortedDates[i] ?? null) : d.date } + return { ...d, day_number: i + 1, date: dates.get(id) ?? null } }) .filter((d): d is NonNullable => d !== null) diff --git a/server/src/nest/days/days.mcp.ts b/server/src/nest/days/days.mcp.ts index f1cfc52d0..b453a2683 100644 --- a/server/src/nest/days/days.mcp.ts +++ b/server/src/nest/days/days.mcp.ts @@ -157,7 +157,7 @@ export class DaysMcp { @Tool({ name: 'reorder_days', description: - 'Reorder the days of a trip by listing every one of its day IDs in the desired order. This moves whole days of the itinerary; to move places around inside a single day use reorder_day_assignments instead. Each day keeps its places, notes, stays and bookings, and on a dated trip the calendar dates stay pinned to their slots, so the content moves across the dates.', + 'Reorder the days of a trip by listing every one of its day IDs in the desired order. This moves whole days of the itinerary; to move places around inside a single day use reorder_day_assignments instead. Each day keeps its places, notes, stays and bookings, and on a dated trip the calendar dates stay pinned to their slots, so the content moves across the dates; a day without a date keeps none wherever it is moved.', inputSchema: { tripId: idSchema, orderedIds: dayReorderRequestSchema.shape.orderedIds diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index aa3fb095c..a0b94be55 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -28,7 +28,7 @@ import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { RealtimeService } from '../realtime/realtime.service'; import { InjectRepository } from '@mikro-orm/nestjs'; import { Injectable } from '@nestjs/common'; -import { addIsoDays, MAX_TRIP_DAYS, planDatedAppend } from '@trek/shared'; +import { addIsoDays, MAX_TRIP_DAYS, planDatedAppend, reorderedDayDates } from '@trek/shared'; import type { RoadtripDayBoundary, TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; type Trip = TripAccess; @@ -594,12 +594,9 @@ export class DaysService { } const oldDateById = new Map(rows.map((r) => [r.id, r.date])); - // Dates stay pinned to slots: position i keeps the i-th date (ascending). - const sortedDates = rows - .map((r) => r.date) - .filter((d): d is string => !!d) - .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); - const isDated = sortedDates.length > 0; + // Dates stay pinned to the slots of the dated days; a day without a date keeps none. + const newDates = reorderedDayDates(rows, orderedIds); + const isDated = rows.some((r) => !!r.date); await this.uow.transactional(async () => { // Two-phase renumber to dodge UNIQUE(trip_id, day_number) collisions — @@ -610,7 +607,7 @@ export class DaysService { } const newDateById = new Map(); for (const [i, id] of orderedIds.entries()) { - const date = isDated ? (sortedDates[i] ?? null) : null; + const date = newDates.get(id) ?? null; await this.daysRepo.setDayNumberAndDate(id, i + 1, date); newDateById.set(id, date); } diff --git a/server/tests/integration/dayReorder.test.ts b/server/tests/integration/dayReorder.test.ts index 42a78ff11..732cae356 100644 --- a/server/tests/integration/dayReorder.test.ts +++ b/server/tests/integration/dayReorder.test.ts @@ -121,6 +121,18 @@ describe('reorderDays', () => { expect(r?.reservation_time).toBe('2026-03-01T19:00'); }); + it('moves a day without a date to the front of a dated trip without giving it a date', async () => { + const trip = createTrip(testDb, userId, { start_date: '2026-03-01', end_date: '2026-03-02' }); + const [d1, d2] = await orderedDays(trip.id); + const extra = createDay(testDb, trip.id); // no date, appended as day 3 + + await reorderDays(trip.id, [extra.id, d1.id, d2.id]); + + const after = await orderedDays(trip.id); + expect(after.map((d) => d.id)).toEqual([extra.id, d1.id, d2.id]); + expect(after.map((d) => d.date)).toEqual([null, '2026-03-01', '2026-03-02']); + }); + it('rejects an orderedIds list that is not a permutation of the trip days', async () => { const trip = createTrip(testDb, userId, { start_date: '2026-03-01', end_date: '2026-03-03' }); const [d1, d2] = await orderedDays(trip.id); diff --git a/shared/src/trip/day-grid.spec.ts b/shared/src/trip/day-grid.spec.ts index b081b6f3c..a0df2cc32 100644 --- a/shared/src/trip/day-grid.spec.ts +++ b/shared/src/trip/day-grid.spec.ts @@ -5,6 +5,7 @@ import { nextTripDate, planDatedAppend, planDayGrid, + reorderedDayDates, resolveDayGridRange, } from './day-grid'; import { MAX_TRIP_DAYS } from './trip.schema'; @@ -238,3 +239,38 @@ describe('planDayGrid', () => { }); }); }); + +describe('reorderedDayDates', () => { + const days = [ + { id: 1, date: '2026-10-01' }, + { id: 2, date: '2026-10-02' }, + { id: 3, date: '2026-10-03' }, + { id: 4, date: null }, + ]; + + it('keeps the dates on their slots while dated days change places', () => { + expect([...reorderedDayDates(days, [3, 1, 2, 4])]).toEqual([ + [3, '2026-10-01'], + [1, '2026-10-02'], + [2, '2026-10-03'], + [4, null], + ]); + }); + + it('moves a day without a date to the front without handing it a date', () => { + expect([...reorderedDayDates(days, [4, 1, 2, 3])]).toEqual([ + [4, null], + [1, '2026-10-01'], + [2, '2026-10-02'], + [3, '2026-10-03'], + ]); + }); + + it('leaves every day without a date on a trip without dates', () => { + const undated = [{ id: 1, date: null }, { id: 2 }]; + expect([...reorderedDayDates(undated, [2, 1])]).toEqual([ + [2, null], + [1, null], + ]); + }); +}); diff --git a/shared/src/trip/day-grid.ts b/shared/src/trip/day-grid.ts index 59ac677aa..048418d24 100644 --- a/shared/src/trip/day-grid.ts +++ b/shared/src/trip/day-grid.ts @@ -66,6 +66,32 @@ export function planDatedAppend( return { date, fits: tripSpanDays(trip.start_date, date) <= MAX_TRIP_DAYS }; } +/** + * The date every day carries after the days are put in a new order. The dates stay + * with the positions, so content moves across the calendar, but only the dated days + * take part: a day without a date keeps none wherever it is moved, and the dated days + * take the trip's dates in ascending order as they now come. Handing out the dates by + * plain position instead gave a day added without a date the first date when it was + * moved to the front and took the date of the last dated day, so the undated row + * stayed at the end and the move looked as if nothing had happened. + */ +export function reorderedDayDates( + days: readonly { id: number; date?: string | null }[], + orderedIds: readonly number[], +): Map { + const dated = new Set(days.filter((d) => !!d.date).map((d) => d.id)); + const sortedDates = days + .map((d) => d.date) + .filter((d): d is string => !!d) + .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + const result = new Map(); + let next = 0; + for (const id of orderedIds) { + result.set(id, dated.has(id) ? (sortedDates[next++] ?? null) : null); + } + return result; +} + /** The dates and day count an update leaves a trip with, and whether its day rows are rebuilt. */ export interface DayGridRange { newStart: string | null | undefined;