From 1eb2cb8eb2f96a1bf04226884adc97a98de8009d Mon Sep 17 00:00:00 2001 From: jubnl <66769052+jubnl@users.noreply.github.com> Date: Mon, 15 Jun 2026 09:25:28 +0200 Subject: [PATCH] fix(store): reset and uniformly hydrate trip-scoped slices in loadTrip (H4, H5) (#1180) loadTrip only replaced the first slice group, so budget/reservations/files from a previous trip stayed visible after switching trips (data exposure on a shared screen). Those three also loaded via separate tab-gated effects, so they never hydrated offline for an unopened tab. - resetTrip() clears every trip-scoped slice (keeps global tags/categories) and runs at the top of loadTrip, so a switch can't leak the prior trip's data - loadTrip now hydrates budget/reservations/files through their repos alongside the rest (non-fatal catches), making offline hydration uniform - useTripPlanner drops the redundant loadFiles + reservations/budget effects; tab-gated lazy reloads stay as on-demand refresh - tests: cross-trip no-leak, uniform hydration, resetTrip --- .../src/pages/tripPlanner/useTripPlanner.ts | 12 +-- client/src/store/tripStore.ts | 34 +++++++- client/tests/unit/tripStore.test.ts | 81 ++++++++++++++++++- 3 files changed, 116 insertions(+), 11 deletions(-) diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 9a757aa5..03d6d094 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -221,11 +221,12 @@ export function useTripPlanner() { } }, [isLoading, places]) - // Load trip + files (needed for place inspector file section) + // Load the trip. loadTrip hydrates every trip-scoped slice (days, places, + // packing, todo, budget, reservations, files) so offline hydration is uniform + // and there's no cross-trip bleed; members/accommodations load alongside. useEffect(() => { if (tripId) { tripActions.loadTrip(tripId).catch(() => { toast.error(t('trip.toast.loadError')); navigate('/dashboard') }) - tripActions.loadFiles(tripId) loadAccommodations() if (!navigator.onLine) { offlineDb.tripMembers.where('tripId').equals(Number(tripId)).toArray() @@ -240,13 +241,6 @@ export function useTripPlanner() { } }, [tripId]) - useEffect(() => { - if (tripId) { - tripActions.loadReservations(tripId) - tripActions.loadBudgetItems?.(tripId) - } - }, [tripId]) - useTripWebSocket(tripId) const [mapCategoryFilter, setMapCategoryFilter] = useState>(new Set()) diff --git a/client/src/store/tripStore.ts b/client/src/store/tripStore.ts index e5f328c4..491cada2 100644 --- a/client/src/store/tripStore.ts +++ b/client/src/store/tripStore.ts @@ -7,6 +7,9 @@ import { dayRepo } from '../repo/dayRepo' import { placeRepo } from '../repo/placeRepo' import { packingRepo } from '../repo/packingRepo' import { todoRepo } from '../repo/todoRepo' +import { budgetRepo } from '../repo/budgetRepo' +import { reservationRepo } from '../repo/reservationRepo' +import { fileRepo } from '../repo/fileRepo' import { createPlacesSlice } from './slices/placesSlice' import { createAssignmentsSlice } from './slices/assignmentsSlice' import { createDaysSlice } from './slices/daysSlice' @@ -61,6 +64,7 @@ export interface TripStoreState setSelectedDay: (dayId: number | null) => void handleRemoteEvent: (event: WebSocketEvent) => void + resetTrip: () => void loadTrip: (tripId: number | string) => Promise refreshDays: (tripId: number | string) => Promise updateTrip: (tripId: number | string, data: Partial) => Promise @@ -89,15 +93,40 @@ export const useTripStore = create((set, get) => ({ handleRemoteEvent: (event: WebSocketEvent) => handleRemoteEvent(set, get, event), + // Clear every trip-scoped slice so switching trips (or losing access to one) + // can never leave a previous trip's data visible. Global tags/categories are + // left intact. Called at the top of loadTrip. + resetTrip: () => set({ + trip: null, + days: [], + places: [], + assignments: {}, + dayNotes: {}, + packingItems: [], + todoItems: [], + budgetItems: [], + files: [], + reservations: [], + selectedDayId: null, + error: null, + }), + loadTrip: async (tripId: number | string) => { + get().resetTrip() set({ isLoading: true, error: null }) try { - const [tripData, daysData, placesData, packingData, todoData, tagsData, categoriesData] = await Promise.all([ + const [tripData, daysData, placesData, packingData, todoData, budgetData, reservationsData, filesData, tagsData, categoriesData] = await Promise.all([ tripRepo.get(tripId), dayRepo.list(tripId), placeRepo.list(tripId), packingRepo.list(tripId), todoRepo.list(tripId), + // Budget / reservations / files are hydrated here too so the offline + // path is uniform (no separate tab-gated effects). Non-fatal: a failure + // in any of these must not blank the whole trip. + budgetRepo.list(tripId).catch(() => ({ items: [] as BudgetItem[] })), + reservationRepo.list(tripId).catch(() => ({ reservations: [] as Reservation[] })), + fileRepo.list(tripId).catch(() => ({ files: [] as TripFile[] })), navigator.onLine ? tagsApi.list().catch(() => offlineDb.tags.toArray().then(tags => ({ tags }))) : offlineDb.tags.toArray().then(tags => ({ tags })), @@ -121,6 +150,9 @@ export const useTripStore = create((set, get) => ({ dayNotes: dayNotesMap, packingItems: packingData.items, todoItems: todoData.items, + budgetItems: budgetData.items, + reservations: reservationsData.reservations, + files: filesData.files, tags: tagsData.tags, categories: categoriesData.categories, isLoading: false, diff --git a/client/tests/unit/tripStore.test.ts b/client/tests/unit/tripStore.test.ts index 7054a2b0..7132ebc0 100644 --- a/client/tests/unit/tripStore.test.ts +++ b/client/tests/unit/tripStore.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { http, HttpResponse } from 'msw'; import { useTripStore } from '../../src/store/tripStore'; import { resetAllStores } from '../helpers/store'; -import { buildTrip, buildDay, buildPlace, buildPackingItem, buildTodoItem, buildTag, buildCategory, buildAssignment, buildDayNote } from '../helpers/factories'; +import { buildTrip, buildDay, buildPlace, buildPackingItem, buildTodoItem, buildTag, buildCategory, buildAssignment, buildDayNote, buildBudgetItem, buildReservation, buildTripFile } from '../helpers/factories'; import { server } from '../helpers/msw/server'; vi.mock('../../src/api/websocket', () => ({ @@ -21,6 +21,28 @@ beforeEach(() => { resetAllStores(); }); +/** Full set of MSW handlers for one trip's loadTrip fan-out. */ +function tripHandlers( + id: number, + data: { + budget?: unknown[]; reservations?: unknown[]; files?: unknown[]; + tags?: unknown[]; categories?: unknown[]; + }, +) { + return [ + http.get(`/api/trips/${id}`, () => HttpResponse.json({ trip: buildTrip({ id }) })), + http.get(`/api/trips/${id}/days`, () => HttpResponse.json({ days: [] })), + http.get(`/api/trips/${id}/places`, () => HttpResponse.json({ places: [] })), + http.get(`/api/trips/${id}/packing`, () => HttpResponse.json({ items: [] })), + http.get(`/api/trips/${id}/todo`, () => HttpResponse.json({ items: [] })), + http.get(`/api/trips/${id}/budget`, () => HttpResponse.json({ items: data.budget ?? [] })), + http.get(`/api/trips/${id}/reservations`, () => HttpResponse.json({ reservations: data.reservations ?? [] })), + http.get(`/api/trips/${id}/files`, () => HttpResponse.json({ files: data.files ?? [] })), + http.get('/api/tags', () => HttpResponse.json({ tags: data.tags ?? [] })), + http.get('/api/categories', () => HttpResponse.json({ categories: data.categories ?? [] })), + ]; +} + describe('tripStore', () => { describe('loadTrip', () => { it('FE-TRIP-001: fires parallel API calls for trips, days, places, packing, todo, tags, categories', async () => { @@ -178,6 +200,63 @@ describe('tripStore', () => { expect(state.isLoading).toBe(false); expect(state.error).not.toBeNull(); }); + + it('FE-TRIP-H5: loadTrip uniformly hydrates budget, reservations and files', async () => { + const budgetItem = buildBudgetItem({ trip_id: 1 }); + const reservation = buildReservation({ trip_id: 1 }); + const file = buildTripFile({ trip_id: 1 }); + server.use(...tripHandlers(1, { budget: [budgetItem], reservations: [reservation], files: [file] })); + + await useTripStore.getState().loadTrip(1); + const state = useTripStore.getState(); + + expect(state.budgetItems).toEqual([budgetItem]); + expect(state.reservations).toEqual([reservation]); + expect(state.files).toEqual([file]); + }); + + it('FE-TRIP-H4: switching trips does not leak budget/reservations/files from the previous trip', async () => { + // Trip 1 has budget/reservations/files; trip 2 has none. + server.use(...tripHandlers(1, { + budget: [buildBudgetItem({ trip_id: 1 })], + reservations: [buildReservation({ trip_id: 1 })], + files: [buildTripFile({ trip_id: 1 })], + })); + await useTripStore.getState().loadTrip(1); + expect(useTripStore.getState().budgetItems).toHaveLength(1); + + server.use(...tripHandlers(2, {})); + await useTripStore.getState().loadTrip(2); + const state = useTripStore.getState(); + + expect(state.trip!.id).toBe(2); + expect(state.budgetItems).toEqual([]); + expect(state.reservations).toEqual([]); + expect(state.files).toEqual([]); + }); + + it('FE-TRIP-H4b: resetTrip clears every trip-scoped slice but keeps tags/categories', async () => { + server.use(...tripHandlers(1, { + budget: [buildBudgetItem({ trip_id: 1 })], + reservations: [buildReservation({ trip_id: 1 })], + files: [buildTripFile({ trip_id: 1 })], + tags: [buildTag()], + })); + await useTripStore.getState().loadTrip(1); + expect(useTripStore.getState().budgetItems).toHaveLength(1); + + useTripStore.getState().resetTrip(); + const state = useTripStore.getState(); + + expect(state.trip).toBeNull(); + expect(state.places).toEqual([]); + expect(state.budgetItems).toEqual([]); + expect(state.reservations).toEqual([]); + expect(state.files).toEqual([]); + expect(state.selectedDayId).toBeNull(); + // Global lookups survive a trip reset. + expect(state.tags).toHaveLength(1); + }); }); describe('refreshDays', () => {