From 302101b186e7a0a6b3eeb83981724eda77837a0c Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 23 Sep 2026 20:50:29 +0200 Subject: [PATCH] fix(dayplan): the delete question names every date and night it changes The question before a day goes now also says which day without a date takes over the last date, which stay running across the day loses a night and when it checks out, and what the cancelled booking costs, or that it has no expense at all; further bookings on the same stay are counted. Road trip boundaries follow their own days when the numbering still has a hole from the old delete, and ones left on no day are dropped. A long list scrolls inside the confirm card on both shells with the buttons always in reach, Escape closes only the sheet on top and takes the trip dialog back from its review step to the form, and the panel of a deleted day closes, also when a fellow traveller deletes it. Undo steps on a deleted day are dropped, a restored place skips days that are gone, and an undo that fails says so. The day cache is written without holding up the answer once the server has it, the tip under a new start date points at the dated add button, and two Collab tests find the confirm button by its role instead of an old colour class. --- .../components/Collab/CollabLinks.test.tsx | 6 +- .../components/Collab/CollabNotes.test.tsx | 4 +- .../Planner/DayPlanSidebar.test.tsx | 7 +- .../src/components/Planner/DayPlanSidebar.tsx | 9 +- .../components/Trips/TripFormModal.test.tsx | 48 ++++++- client/src/components/Trips/TripFormModal.tsx | 5 +- .../components/shared/ConfirmDialog.test.tsx | 15 +++ .../src/components/shared/ConfirmDialog.tsx | 8 +- client/src/hooks/usePlannerHistory.ts | 37 +++++- client/src/hooks/useTripRangeGuard.test.ts | 28 +++- client/src/hooks/useTripRangeGuard.ts | 14 +- client/src/mobile/components/MSheet.tsx | 9 +- .../mobile/screens/settings/MConfirmSheet.tsx | 18 ++- client/src/pages/tripPlanner/useDayDelete.ts | 17 ++- .../pages/tripPlanner/useTripPlanner.test.tsx | 120 +++++++++++++++++- .../src/pages/tripPlanner/useTripPlanner.ts | 34 +++-- client/src/repo/dayRepo.test.ts | 34 ++++- client/src/repo/dayRepo.ts | 20 ++- client/src/store/slices/remoteEventHandler.ts | 5 +- client/src/store/tripStore.ts | 4 +- client/src/utils/dayDeleteImpact.test.ts | 93 +++++++++++++- client/src/utils/dayDeleteImpact.ts | 109 ++++++++++++++-- client/src/utils/dayImpactLines.test.ts | 57 ++++++++- client/src/utils/dayImpactLines.ts | 73 ++++++++++- client/src/utils/tripRangeImpact.test.ts | 3 +- .../unit/hooks/usePlannerHistory.test.ts | 41 ++++++ client/tests/unit/mobile/MSheet.test.tsx | 43 +++++++ .../mobile/settings/MConfirmSheet.test.tsx | 46 +++++++ .../unit/mobile/trip/MDaysSheet.test.tsx | 3 +- server/src/nest/days/day-removal.service.ts | 36 ++++-- .../unit/nest/day-removal.service.test.ts | 28 +++- shared/src/i18n/ar/dashboard.ts | 4 +- shared/src/i18n/ar/dayplan.ts | 9 ++ shared/src/i18n/ar/undo.ts | 1 + shared/src/i18n/br/dashboard.ts | 4 +- shared/src/i18n/br/dayplan.ts | 9 ++ shared/src/i18n/br/undo.ts | 1 + shared/src/i18n/ca/dashboard.ts | 4 +- shared/src/i18n/ca/dayplan.ts | 9 ++ shared/src/i18n/ca/undo.ts | 1 + shared/src/i18n/cs/dashboard.ts | 4 +- shared/src/i18n/cs/dayplan.ts | 9 ++ shared/src/i18n/cs/undo.ts | 1 + shared/src/i18n/de/dashboard.ts | 4 +- shared/src/i18n/de/dayplan.ts | 9 ++ shared/src/i18n/de/undo.ts | 1 + shared/src/i18n/en/dashboard.ts | 4 +- shared/src/i18n/en/dayplan.ts | 9 ++ shared/src/i18n/en/undo.ts | 1 + shared/src/i18n/es/dashboard.ts | 4 +- shared/src/i18n/es/dayplan.ts | 9 ++ shared/src/i18n/es/undo.ts | 1 + shared/src/i18n/fr/dashboard.ts | 4 +- shared/src/i18n/fr/dayplan.ts | 9 ++ shared/src/i18n/fr/undo.ts | 1 + shared/src/i18n/gr/dashboard.ts | 4 +- shared/src/i18n/gr/dayplan.ts | 11 ++ shared/src/i18n/gr/undo.ts | 1 + shared/src/i18n/hu/dashboard.ts | 4 +- shared/src/i18n/hu/dayplan.ts | 9 ++ shared/src/i18n/hu/undo.ts | 1 + shared/src/i18n/id/dashboard.ts | 4 +- shared/src/i18n/id/dayplan.ts | 11 ++ shared/src/i18n/id/undo.ts | 1 + shared/src/i18n/it/dashboard.ts | 4 +- shared/src/i18n/it/dayplan.ts | 9 ++ shared/src/i18n/it/undo.ts | 1 + shared/src/i18n/ja/dashboard.ts | 4 +- shared/src/i18n/ja/dayplan.ts | 9 ++ shared/src/i18n/ja/undo.ts | 1 + shared/src/i18n/ko/dashboard.ts | 4 +- shared/src/i18n/ko/dayplan.ts | 8 ++ shared/src/i18n/ko/undo.ts | 1 + shared/src/i18n/nl/dashboard.ts | 4 +- shared/src/i18n/nl/dayplan.ts | 9 ++ shared/src/i18n/nl/undo.ts | 1 + shared/src/i18n/pl/dashboard.ts | 4 +- shared/src/i18n/pl/dayplan.ts | 9 ++ shared/src/i18n/pl/undo.ts | 1 + shared/src/i18n/ru/dashboard.ts | 4 +- shared/src/i18n/ru/dayplan.ts | 9 ++ shared/src/i18n/ru/undo.ts | 1 + shared/src/i18n/sv/dashboard.ts | 4 +- shared/src/i18n/sv/dayplan.ts | 9 ++ shared/src/i18n/sv/undo.ts | 1 + shared/src/i18n/tr/dashboard.ts | 4 +- shared/src/i18n/tr/dayplan.ts | 9 ++ shared/src/i18n/tr/undo.ts | 1 + shared/src/i18n/uk/dashboard.ts | 4 +- shared/src/i18n/uk/dayplan.ts | 9 ++ shared/src/i18n/uk/undo.ts | 1 + shared/src/i18n/vi/dashboard.ts | 4 +- shared/src/i18n/vi/dayplan.ts | 9 ++ shared/src/i18n/vi/undo.ts | 1 + shared/src/i18n/zh-TW/dashboard.ts | 4 +- shared/src/i18n/zh-TW/dayplan.ts | 8 ++ shared/src/i18n/zh-TW/undo.ts | 1 + shared/src/i18n/zh/dashboard.ts | 4 +- shared/src/i18n/zh/dayplan.ts | 8 ++ shared/src/i18n/zh/undo.ts | 1 + wiki/Accommodations.md | 2 +- wiki/Day-Plans-and-Notes.md | 13 +- 102 files changed, 1170 insertions(+), 142 deletions(-) create mode 100644 client/tests/unit/mobile/settings/MConfirmSheet.test.tsx diff --git a/client/src/components/Collab/CollabLinks.test.tsx b/client/src/components/Collab/CollabLinks.test.tsx index 779fd3cdb..ff648ad8d 100644 --- a/client/src/components/Collab/CollabLinks.test.tsx +++ b/client/src/components/Collab/CollabLinks.test.tsx @@ -12,7 +12,7 @@ vi.mock('../../api/websocket', () => ({ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; -import { fireEvent, render, screen, waitFor } from '../../../tests/helpers/render'; +import { fireEvent, render, screen, waitFor, within } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; import { server } from '../../../tests/helpers/msw/server'; @@ -177,7 +177,9 @@ describe('CollabLinks', () => { expect(deleted).toBe(false); expect(screen.getByText('Ferry timetable')).toBeInTheDocument(); - await user.click(document.querySelector('button.bg-red-600') as HTMLElement); + // The confirm button of the question itself, found by its role rather than a colour class. + const question = screen.getByText(/delete link\?|collab\.links\.confirmDeleteTitle/i).closest('.trek-modal-enter') as HTMLElement; + await user.click(within(question).getByRole('button', { name: /^(delete|common\.delete)$/i })); await waitFor(() => expect(deleted).toBe(true)); await waitFor(() => expect(screen.queryByText('Ferry timetable')).not.toBeInTheDocument()); }); diff --git a/client/src/components/Collab/CollabNotes.test.tsx b/client/src/components/Collab/CollabNotes.test.tsx index 576029c1a..3b5e57d84 100644 --- a/client/src/components/Collab/CollabNotes.test.tsx +++ b/client/src/components/Collab/CollabNotes.test.tsx @@ -201,7 +201,9 @@ describe('CollabNotes', () => { // Deleting now asks for confirmation first — the note stays until confirmed. expect(screen.getByText('Delete note?')).toBeInTheDocument(); expect(screen.getByText('Remove Me')).toBeInTheDocument(); - await user.click(document.querySelector('button.bg-red-600') as HTMLElement); + // The confirm button of the question itself, found by its role rather than a colour class. + const question = screen.getByText('Delete note?').closest('.trek-modal-enter') as HTMLElement; + await user.click(within(question).getByRole('button', { name: 'Delete' })); await waitFor(() => expect(screen.queryByText('Remove Me')).not.toBeInTheDocument()); }); diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index bcd440d15..f5e692a95 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -2891,7 +2891,8 @@ describe('DayPlanSidebar', () => { expect(onRouteRefresh).toHaveBeenCalled() // The store carries the new slot per day so the merged list stays stable. expect(useTripStore.getState().reservations[0].day_positions).toEqual({ 10: expect.any(Number) }) - // Undoing restores the original assignment order. + // Undoing restores the original assignment order; the step names its day, so deleting the day drops it. + expect(pushUndo.mock.calls[0][2]).toEqual([10]) const undo = pushUndo.mock.calls[0][1] as () => Promise await undo() expect(reorderAssignments).toHaveBeenCalledWith(1, 10, [11, 12]) @@ -3477,8 +3478,9 @@ describe('DayPlanSidebar', () => { fireEvent.drop(dayHeader('Day 2'), { dataTransfer: { getData: vi.fn(() => '') } }) expect(onMoveToDay).toHaveBeenCalledWith(21, 10, 11, 1) expect(moveAssignment).not.toHaveBeenCalled() - // Still offered back once the planner has made the move. + // Still offered back once the planner has made the move, tagged with both days it touches. await waitFor(() => expect(pushUndo).toHaveBeenCalledTimes(1)) + expect(pushUndo.mock.calls[0][2]).toEqual([11, 10]) }) }) @@ -4376,6 +4378,7 @@ describe('DayPlanSidebar', () => { render() await user.click(screen.getByRole('button', { name: 'Optimize' })) await waitFor(() => expect(onReorder).toHaveBeenCalled()) + expect(pushUndo.mock.calls[0][2]).toEqual([10]) const undo = pushUndo.mock.calls[0][1] as () => Promise await undo() expect(reorderAssignments).toHaveBeenCalledWith(1, 10, [11, 12, 13]) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 54b4f1220..de6ee51dc 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -113,7 +113,8 @@ interface DayPlanSidebarProps { /** Open the place form already pointed at this day, to create a new place there. */ onCreatePlaceForDay?: (dayId: number) => void onExpandedDaysChange?: (expandedDayIds: Set) => void - pushUndo?: (label: string, undoFn: () => Promise | void) => void + /** `dayIds`: the days the step acts on, so deleting one of them drops it. */ + pushUndo?: (label: string, undoFn: () => Promise | void, dayIds?: number[]) => void canUndo?: boolean lastActionLabel?: string | null onUndo?: () => void @@ -898,7 +899,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const capturedPrevIds = prevAssignmentIds pushUndo?.(t('undo.reorder'), async () => { await tripActions.reorderAssignments(tripId, capturedDayId, capturedPrevIds) - }) + }, [capturedDayId]) } } catch (err: unknown) { rollBackReservations() @@ -1073,7 +1074,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const capturedDayId = dayId pushUndo?.(t('undo.optimize'), async () => { await tripActions.reorderAssignments(tripId, capturedDayId, prevIds) - }) + }, [capturedDayId]) } @@ -1097,7 +1098,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { .then(() => { pushUndo?.(t('undo.moveDay'), async () => { await tripActions.moveAssignment(tripId, Number(assignmentId), dayId, capturedFromDayId, capturedOrderIndex) - }) + }, [Number(dayId), Number(capturedFromDayId)]) }) .catch((err: unknown) => toast.error(err instanceof Error ? err.message : t('common.unknownError'))) } else if (noteId && fromDayId !== dayId) { diff --git a/client/src/components/Trips/TripFormModal.test.tsx b/client/src/components/Trips/TripFormModal.test.tsx index 8dbff70af..c53579d6f 100644 --- a/client/src/components/Trips/TripFormModal.test.tsx +++ b/client/src/components/Trips/TripFormModal.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-TRIPFORM-001 to FE-COMP-TRIPFORM-095 +// FE-COMP-TRIPFORM-001 to FE-COMP-TRIPFORM-097 import type { Mock } from 'vitest'; import { render, screen, waitFor, fireEvent, within } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; @@ -8,9 +8,9 @@ import { useTripStore } from '../../store/tripStore'; import { useSettingsStore } from '../../store/settingsStore'; import { usePermissionsStore } from '../../store/permissionsStore'; import { resetAllStores, seedStore } from '../../../tests/helpers/store'; -import { buildUser, buildTrip, buildDay, buildAssignment, buildPlace, buildReservation } from '../../../tests/helpers/factories'; +import { buildUser, buildTrip, buildDay, buildAssignment, buildPlace, buildReservation, buildBudgetItem } from '../../../tests/helpers/factories'; import { server } from '../../../tests/helpers/msw/server'; -import type { Accommodation, Reservation, Trip } from '../../types'; +import type { Accommodation, BudgetItem, Reservation, Trip } from '../../types'; import { MAX_TRIP_DAYS } from '@trek/shared'; import TripFormModal from './TripFormModal'; @@ -1295,7 +1295,7 @@ describe('TripFormModal', () => { /** A week in October with a place on its last two days, and the trip's bookings and stays. */ const shrinkableTrip = () => buildTrip({ id: 1, title: 'Coast week', start_date: '2026-10-01', end_date: '2026-10-07' }); - const seedTripDays = (extras: { reservations?: Reservation[]; accommodations?: Accommodation[]; empty?: boolean } = {}) => { + const seedTripDays = (extras: { reservations?: Reservation[]; accommodations?: Accommodation[]; budget?: BudgetItem[]; empty?: boolean } = {}) => { const days = Array.from({ length: 7 }, (_, i) => buildDay({ id: 300 + i, trip_id: 1, @@ -1307,6 +1307,7 @@ describe('TripFormModal', () => { http.get('/api/trips/:id/days', () => HttpResponse.json({ days })), http.get('/api/trips/:id/reservations', () => HttpResponse.json({ reservations: extras.reservations ?? [] })), http.get('/api/trips/:id/accommodations', () => HttpResponse.json({ accommodations: extras.accommodations ?? [] })), + http.get('/api/trips/:id/budget', () => HttpResponse.json({ items: extras.budget ?? [] })), ); }; const changeEndDate = async (user: ReturnType, iso: string) => { @@ -1320,6 +1321,7 @@ describe('TripFormModal', () => { seedTripDays({ accommodations: [{ id: 9, trip_id: 1, start_day_id: 304, end_day_id: 306, place_name: 'Harbour Hotel' } as Accommodation], reservations: [buildReservation({ id: 70, day_id: 304, type: 'hotel', title: 'Harbour, 2 nights', accommodation_id: 9 })], + budget: [buildBudgetItem({ reservation_id: 70, total_price: 240 })], }); const user = userEvent.setup(); const onSave = vi.fn().mockResolvedValue({}); @@ -1422,6 +1424,44 @@ describe('TripFormModal', () => { expect(onSave).not.toHaveBeenCalled(); }); + it('FE-COMP-TRIPFORM-096: Escape in the warning goes back to the form, like Back, and keeps the edit', async () => { + seedTripDays(); + const user = userEvent.setup(); + const onClose = vi.fn(); + render(); + + const title = screen.getByDisplayValue('Coast week'); + await user.clear(title); + await user.type(title, 'Coast week, shorter'); + await changeEndDate(user, '2026-10-05'); + await user.click(screen.getByRole('button', { name: /Update/i })); + await screen.findByRole('list', { name: 'Remove days?' }); + + fireEvent.keyDown(document, { key: 'Escape' }); + await waitFor(() => expect(screen.queryByRole('list', { name: 'Remove days?' })).not.toBeInTheDocument()); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByDisplayValue('Coast week, shorter')).toBeInTheDocument(); + // Out of the warning, Escape closes the dialog as before. + fireEvent.keyDown(document, { key: 'Escape' }); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('FE-COMP-TRIPFORM-097: a removed stay whose booking carries no expense does not claim one', async () => { + seedTripDays({ + accommodations: [{ id: 9, trip_id: 1, start_day_id: 304, end_day_id: 306, place_name: 'Harbour Hotel' } as Accommodation], + reservations: [buildReservation({ id: 70, day_id: 304, type: 'hotel', title: 'Harbour, 2 nights', accommodation_id: 9 })], + }); + const user = userEvent.setup(); + render(); + + await changeEndDate(user, '2026-10-05'); + await user.click(screen.getByRole('button', { name: /Update/i })); + + const list = await screen.findByRole('list', { name: 'Remove days?' }); + expect(within(list).getByText('Checks in or out on a removed day, so the whole stay is removed. Its booking "Harbour, 2 nights" stays under Bookings.')).toBeInTheDocument(); + expect(within(list).queryByText(/its expense/)).not.toBeInTheDocument(); + }); + it('FE-COMP-TRIPFORM-094: when the days cannot be read it still warns, in general terms', async () => { const spy = vi.spyOn(console, 'error').mockImplementation(() => {}); server.use(http.get('/api/trips/:id/days', () => HttpResponse.json({ error: 'boom' }, { status: 500 }))); diff --git a/client/src/components/Trips/TripFormModal.tsx b/client/src/components/Trips/TripFormModal.tsx index 0ba34d16e..e92b8e7ae 100644 --- a/client/src/components/Trips/TripFormModal.tsx +++ b/client/src/components/Trips/TripFormModal.tsx @@ -403,7 +403,10 @@ export default function TripFormModal({ isOpen, onClose, onSave, trip, onCoverUp return ( { if (!busy) setPendingReview(null) } : onClose} title={pendingReview ? t(pendingReview.askShift ? 'dashboard.dateShiftTitle' : 'dashboard.shrinkTitle') : isEditing ? t('dashboard.editTrip') : t('dashboard.createTrip')} diff --git a/client/src/components/shared/ConfirmDialog.test.tsx b/client/src/components/shared/ConfirmDialog.test.tsx index 07975743c..8a673f56c 100644 --- a/client/src/components/shared/ConfirmDialog.test.tsx +++ b/client/src/components/shared/ConfirmDialog.test.tsx @@ -130,6 +130,21 @@ describe('ConfirmDialog', () => { expect(document.querySelector('.bg-danger-soft')).toBeNull(); }); + it('FE-COMP-CONFIRM-013: a long list scrolls inside the card, and the buttons stay outside it, always in reach', () => { + render( + +
    {Array.from({ length: 40 }, (_, i) =>
  • Row {i}
  • )}
+
+ ); + const scroller = screen.getByRole('list', { name: 'consequences' }).closest('.overflow-y-auto') as HTMLElement; + expect(scroller).not.toBeNull(); + expect(scroller.parentElement).toHaveClass('flex-col'); + expect(scroller.parentElement!.className).toMatch(/max-h-\[/); + const confirm = screen.getByRole('button', { name: /delete/i }); + expect(scroller.contains(confirm)).toBe(false); + expect(confirm.parentElement).toHaveClass('flex-none'); + }); + it('FE-COMP-CONFIRM-008: clicking backdrop calls onClose', async () => { const user = userEvent.setup(); render(); diff --git a/client/src/components/shared/ConfirmDialog.tsx b/client/src/components/shared/ConfirmDialog.tsx index 1cc920ba7..626e8db17 100644 --- a/client/src/components/shared/ConfirmDialog.tsx +++ b/client/src/components/shared/ConfirmDialog.tsx @@ -65,12 +65,14 @@ export default function ConfirmDialog({ style={{ paddingBottom: 'var(--bottom-nav-h)' }} onClick={onClose} > + {/* Capped at the viewport: a long list of what goes along scrolls on its + own, and the two buttons below it always stay in reach. */}
e.stopPropagation()} > -
+
{danger && (
@@ -87,7 +89,7 @@ export default function ConfirmDialog({
-
+