From 69620e727611d8652023ac9b527aeeaa62a31fa5 Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 5 May 2026 02:14:39 +0200 Subject: [PATCH] feat: always-optimistic write pattern across all repos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All create/update/delete repo methods now write to IndexedDB optimistically and fire mutationQueue.flush() as fire-and-forget, returning immediately without waiting for the network. This eliminates the 8-second UX freeze previously seen when the API was unreachable but navigator.onLine was true. - Repos rewritten: trip, day, place, packing, todo, budget, accommodation, reservation, file — write methods never throw, always return optimistic data - mutationQueue.flush() changed to iterative (one item per loop iteration) so mutations enqueued mid-flush (e.g. bulk check-all) are picked up - fileRepo.toggleStar skips the IDB put when the file is not cached locally - DayDetailPanel passes place_name into accommodationRepo.create so the optimistic accommodation renders the correct hotel label immediately - Test suite updated throughout to reflect optimistic-first semantics: no more rollback assertions, IDB cleared in component test beforeEach hooks, FileManager tests switched from filesApi spy to MSW endpoint assertions --- .../src/components/Files/FileManager.test.tsx | 78 +++++++---- .../Packing/PackingListPanel.test.tsx | 5 +- .../Planner/DayDetailPanel.test.tsx | 5 +- .../src/components/Planner/DayDetailPanel.tsx | 2 + client/src/repo/accommodationRepo.ts | 116 ++++++++-------- client/src/repo/budgetRepo.ts | 106 +++++++-------- client/src/repo/dayRepo.ts | 30 ++--- client/src/repo/fileRepo.ts | 86 ++++++------ client/src/repo/packingRepo.ts | 105 +++++++-------- client/src/repo/placeRepo.ts | 126 ++++++++---------- client/src/repo/reservationRepo.ts | 116 ++++++++-------- client/src/repo/todoRepo.ts | 112 +++++++--------- client/src/repo/tripRepo.ts | 30 ++--- client/src/store/slices/budgetSlice.test.ts | 42 +++--- client/src/sync/mutationQueue.ts | 12 +- client/tests/unit/repo/packingRepo.test.ts | 28 ++-- client/tests/unit/repo/placeRepo.test.ts | 16 +-- client/tests/unit/slices/budgetSlice.test.ts | 49 +++---- client/tests/unit/slices/filesSlice.test.ts | 8 +- client/tests/unit/slices/packingSlice.test.ts | 29 ++-- client/tests/unit/slices/placesSlice.test.ts | 9 +- .../unit/slices/reservationsSlice.test.ts | 29 ++-- client/tests/unit/slices/todoSlice.test.ts | 28 ++-- client/tests/unit/tripStore.test.ts | 4 +- 24 files changed, 548 insertions(+), 623 deletions(-) diff --git a/client/src/components/Files/FileManager.test.tsx b/client/src/components/Files/FileManager.test.tsx index 273387c3..048cd784 100644 --- a/client/src/components/Files/FileManager.test.tsx +++ b/client/src/components/Files/FileManager.test.tsx @@ -35,6 +35,7 @@ vi.mock('../../api/client', async (importOriginal) => { }); import { filesApi } from '../../api/client'; +import { offlineDb } from '../../db/offlineDb'; const buildFile = (overrides = {}) => ({ id: 1, @@ -66,7 +67,9 @@ const defaultProps = { allowedFileTypes: null, }; -beforeEach(() => { +beforeEach(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.all(offlineDb.tables.map(t => t.clear())); resetAllStores(); vi.clearAllMocks(); // Seed auth as admin so useCanDo() returns true for all permissions @@ -130,15 +133,21 @@ describe('FileManager', () => { expect(screen.queryByText('doc.pdf')).not.toBeInTheDocument(); }); - it('FE-COMP-FILEMANAGER-005: star button calls filesApi.toggleStar', async () => { + it('FE-COMP-FILEMANAGER-005: star button calls star endpoint', async () => { + let starCalled = false; + server.use( + http.patch('/api/trips/1/files/1/star', () => { + starCalled = true; + return HttpResponse.json({ success: true }); + }), + ); render(); const user = userEvent.setup(); - // Find the star button by its title const starBtn = screen.getByTitle(/star/i); await user.click(starBtn); - expect(filesApi.toggleStar).toHaveBeenCalledWith(1, 1); + await waitFor(() => expect(starCalled).toBe(true)); }); it('FE-COMP-FILEMANAGER-006: trash toggle loads and displays trashed files', async () => { @@ -398,39 +407,47 @@ describe('FileManager', () => { await screen.findByText('Hotel Paris'); }); - it('FE-COMP-FILEMANAGER-024: clicking a place in assign modal calls filesApi.update', async () => { + it('FE-COMP-FILEMANAGER-024: clicking a place in assign modal calls file update endpoint', async () => { const { buildPlace } = await import('../../../tests/helpers/factories'); const place = buildPlace({ id: 10, name: 'Louvre Museum' }); const file = buildFile({ id: 1 }); const onUpdate = vi.fn().mockResolvedValue(undefined); + let capturedBody: Record | null = null; + server.use( + http.put('/api/trips/1/files/1', async ({ request }) => { + capturedBody = await request.json() as Record; + return HttpResponse.json({ file: { ...file, place_id: 10 } }); + }), + ); render(); const user = userEvent.setup(); - // Open assign modal await user.click(screen.getByTitle(/assign/i)); await screen.findByText('Louvre Museum'); - - // Click on the place button to link it await user.click(screen.getByText('Louvre Museum')); - expect(filesApi.update).toHaveBeenCalledWith(1, 1, { place_id: 10 }); + await waitFor(() => expect(capturedBody).toMatchObject({ place_id: 10 })); }); - it('FE-COMP-FILEMANAGER-025: clicking a reservation in assign modal calls filesApi.update', async () => { + it('FE-COMP-FILEMANAGER-025: clicking a reservation in assign modal calls file update endpoint', async () => { const { buildReservation } = await import('../../../tests/helpers/factories'); const reservation = buildReservation({ id: 20, name: 'Train Ticket' }); const file = buildFile({ id: 1 }); + let capturedBody: Record | null = null; + server.use( + http.put('/api/trips/1/files/1', async ({ request }) => { + capturedBody = await request.json() as Record; + return HttpResponse.json({ file: { ...file, reservation_id: 20 } }); + }), + ); render(); const user = userEvent.setup(); - // Open assign modal await user.click(screen.getByTitle(/assign/i)); await screen.findByText('Train Ticket'); - - // Click on the reservation button to link it await user.click(screen.getByText('Train Ticket')); - expect(filesApi.update).toHaveBeenCalledWith(1, 1, { reservation_id: 20 }); + await waitFor(() => expect(capturedBody).toMatchObject({ reservation_id: 20 })); }); it('FE-COMP-FILEMANAGER-026: assign modal with both places and reservations shows both sections', async () => { @@ -507,39 +524,46 @@ describe('FileManager', () => { await screen.findByText(/Colosseum/); }); - it('FE-COMP-FILEMANAGER-031: unlink place from assign modal calls filesApi.update', async () => { + it('FE-COMP-FILEMANAGER-031: unlink place from assign modal calls file update endpoint', async () => { const { buildPlace } = await import('../../../tests/helpers/factories'); const place = buildPlace({ id: 10, name: 'Venice Beach' }); - // File already has place_id set to 10 (linked) const file = buildFile({ id: 1, place_id: 10 }); - + let capturedBody: Record | null = null; + server.use( + http.put('/api/trips/1/files/1', async ({ request }) => { + capturedBody = await request.json() as Record; + return HttpResponse.json({ file: { ...file, place_id: null } }); + }), + ); render(); const user = userEvent.setup(); - // Open assign modal await user.click(screen.getByTitle(/assign/i)); await screen.findByText('Venice Beach'); - - // Clicking the linked place should unlink it await user.click(screen.getByText('Venice Beach')); - expect(filesApi.update).toHaveBeenCalledWith(1, 1, { place_id: null }); + + await waitFor(() => expect(capturedBody).toMatchObject({ place_id: null })); }); - it('FE-COMP-FILEMANAGER-032: unlink reservation from assign modal calls filesApi.update', async () => { + it('FE-COMP-FILEMANAGER-032: unlink reservation from assign modal calls file update endpoint', async () => { const { buildReservation } = await import('../../../tests/helpers/factories'); const reservation = buildReservation({ id: 20, name: 'Museum Pass' }); - // File already has reservation_id set to 20 const file = buildFile({ id: 1, reservation_id: 20 }); - + let capturedBody: Record | null = null; + server.use( + http.put('/api/trips/1/files/1', async ({ request }) => { + capturedBody = await request.json() as Record; + return HttpResponse.json({ file: { ...file, reservation_id: null } }); + }), + ); render(); const user = userEvent.setup(); await user.click(screen.getByTitle(/assign/i)); await screen.findByText('Museum Pass'); - - // Clicking the linked reservation should unlink it await user.click(screen.getByText('Museum Pass')); - expect(filesApi.update).toHaveBeenCalledWith(1, 1, { reservation_id: null }); + + await waitFor(() => expect(capturedBody).toMatchObject({ reservation_id: null })); }); it('FE-COMP-FILEMANAGER-033: opening PDF preview and closing via backdrop', async () => { diff --git a/client/src/components/Packing/PackingListPanel.test.tsx b/client/src/components/Packing/PackingListPanel.test.tsx index 2e1414ec..121a24b6 100644 --- a/client/src/components/Packing/PackingListPanel.test.tsx +++ b/client/src/components/Packing/PackingListPanel.test.tsx @@ -9,8 +9,11 @@ import { useTripStore } from '../../store/tripStore'; import { resetAllStores, seedStore } from '../../../tests/helpers/store'; import { buildUser, buildTrip, buildPackingItem } from '../../../tests/helpers/factories'; import PackingListPanel from './PackingListPanel'; +import { offlineDb } from '../../db/offlineDb'; -beforeEach(() => { +beforeEach(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.all(offlineDb.tables.map(t => t.clear())); resetAllStores(); // Side-effect APIs PackingListPanel calls on mount server.use( diff --git a/client/src/components/Planner/DayDetailPanel.test.tsx b/client/src/components/Planner/DayDetailPanel.test.tsx index a70fd9d5..5c185d62 100644 --- a/client/src/components/Planner/DayDetailPanel.test.tsx +++ b/client/src/components/Planner/DayDetailPanel.test.tsx @@ -11,6 +11,7 @@ import { usePermissionsStore } from '../../store/permissionsStore'; import { resetAllStores, seedStore } from '../../../tests/helpers/store'; import { buildUser, buildAdmin, buildTrip, buildDay, buildPlace, buildReservation } from '../../../tests/helpers/factories'; import DayDetailPanel from './DayDetailPanel'; +import { offlineDb } from '../../db/offlineDb'; const day = buildDay({ id: 1, trip_id: 1, date: '2025-06-15', title: 'Day in Paris' }); @@ -28,7 +29,9 @@ const defaultProps = { onAccommodationChange: vi.fn(), }; -beforeEach(() => { +beforeEach(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.all(offlineDb.tables.map(t => t.clear())); resetAllStores(); vi.clearAllMocks(); server.use( diff --git a/client/src/components/Planner/DayDetailPanel.tsx b/client/src/components/Planner/DayDetailPanel.tsx index e8df9aa3..4ae36718 100644 --- a/client/src/components/Planner/DayDetailPanel.tsx +++ b/client/src/components/Planner/DayDetailPanel.tsx @@ -118,8 +118,10 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri const handleSaveAccommodation = async () => { if (!hotelForm.place_id) return try { + const selectedPlace = places.find(p => p.id === hotelForm.place_id) const data = await accommodationRepo.create(tripId, { place_id: hotelForm.place_id, + place_name: selectedPlace?.name, start_day_id: hotelDayRange.start, end_day_id: hotelDayRange.end, check_in: hotelForm.check_in || null, diff --git a/client/src/repo/accommodationRepo.ts b/client/src/repo/accommodationRepo.ts index 08831dbc..c31ea2ba 100644 --- a/client/src/repo/accommodationRepo.ts +++ b/client/src/repo/accommodationRepo.ts @@ -27,75 +27,63 @@ export const accommodationRepo = { }, async create(tripId: number | string, data: Record): Promise<{ accommodation: Accommodation }> { - if (!navigator.onLine) { - const tempId = -(Date.now()) - const tempAccommodation: Accommodation = { - ...(data as Partial), - id: tempId, - trip_id: Number(tripId), - name: (data.name as string) ?? 'New accommodation', - address: null, - check_in: null, - check_in_end: null, - check_out: null, - confirmation_number: null, - notes: null, - url: null, - created_at: new Date().toISOString(), - } as Accommodation - await offlineDb.accommodations.put(tempAccommodation) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'POST', - url: `/trips/${tripId}/accommodations`, - body: data, - resource: 'accommodations', - tempId, - }) - return { accommodation: tempAccommodation } - } - const result = await accommodationsApi.create(tripId, data) - offlineDb.accommodations.put(result.accommodation) - return result + const tempId = -(Date.now()) + const tempAccommodation: Accommodation = { + ...(data as Partial), + id: tempId, + trip_id: Number(tripId), + name: (data.name as string) ?? 'New accommodation', + address: null, + check_in: null, + check_in_end: null, + check_out: null, + confirmation_number: null, + notes: null, + url: null, + created_at: new Date().toISOString(), + } as Accommodation + await offlineDb.accommodations.put(tempAccommodation) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'POST', + url: `/trips/${tripId}/accommodations`, + body: data, + resource: 'accommodations', + tempId, + }) + mutationQueue.flush().catch(() => {}) + return { accommodation: tempAccommodation } }, async update(tripId: number | string, id: number, data: Record): Promise<{ accommodation: Accommodation }> { - if (!navigator.onLine) { - const existing = await offlineDb.accommodations.get(id) - const optimistic: Accommodation = { ...(existing ?? {} as Accommodation), ...(data as Partial), id } - await offlineDb.accommodations.put(optimistic) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/accommodations/${id}`, - body: data, - resource: 'accommodations', - }) - return { accommodation: optimistic } - } - const result = await accommodationsApi.update(tripId, id, data) - offlineDb.accommodations.put(result.accommodation) - return result + const existing = await offlineDb.accommodations.get(id) + const optimistic: Accommodation = { ...(existing ?? {} as Accommodation), ...(data as Partial), id } + await offlineDb.accommodations.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/accommodations/${id}`, + body: data, + resource: 'accommodations', + }) + mutationQueue.flush().catch(() => {}) + return { accommodation: optimistic } }, async delete(tripId: number | string, id: number): Promise { - if (!navigator.onLine) { - await offlineDb.accommodations.delete(id) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'DELETE', - url: `/trips/${tripId}/accommodations/${id}`, - body: undefined, - resource: 'accommodations', - entityId: id, - }) - return { success: true } - } - const result = await accommodationsApi.delete(tripId, id) - offlineDb.accommodations.delete(id) - return result + await offlineDb.accommodations.delete(id) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'DELETE', + url: `/trips/${tripId}/accommodations/${id}`, + body: undefined, + resource: 'accommodations', + entityId: id, + }) + mutationQueue.flush().catch(() => {}) + return { success: true } }, } diff --git a/client/src/repo/budgetRepo.ts b/client/src/repo/budgetRepo.ts index 04158b3c..f674faab 100644 --- a/client/src/repo/budgetRepo.ts +++ b/client/src/repo/budgetRepo.ts @@ -29,70 +29,58 @@ export const budgetRepo = { }, async create(tripId: number | string, data: Record): Promise<{ item: BudgetItem }> { - if (!navigator.onLine) { - const tempId = -(Date.now()) - const tempItem: BudgetItem = { - ...(data as Partial), - id: tempId, - trip_id: Number(tripId), - name: (data.name as string) ?? 'New expense', - amount: (data.amount as number) ?? 0, - currency: (data.currency as string) ?? 'USD', - members: [], - } as BudgetItem - await offlineDb.budgetItems.put(tempItem) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'POST', - url: `/trips/${tripId}/budget`, - body: data, - resource: 'budgetItems', - tempId, - }) - return { item: tempItem } - } - const result = await budgetApi.create(tripId, data) - offlineDb.budgetItems.put(result.item) - return result + const tempId = -(Date.now()) + const tempItem: BudgetItem = { + ...(data as Partial), + id: tempId, + trip_id: Number(tripId), + name: (data.name as string) ?? 'New expense', + amount: (data.amount as number) ?? 0, + currency: (data.currency as string) ?? 'USD', + members: [], + } as BudgetItem + await offlineDb.budgetItems.put(tempItem) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'POST', + url: `/trips/${tripId}/budget`, + body: data, + resource: 'budgetItems', + tempId, + }) + mutationQueue.flush().catch(() => {}) + return { item: tempItem } }, async update(tripId: number | string, id: number, data: Record): Promise<{ item: BudgetItem }> { - if (!navigator.onLine) { - const existing = await offlineDb.budgetItems.get(id) - const optimistic: BudgetItem = { ...(existing ?? {} as BudgetItem), ...(data as Partial), id } - await offlineDb.budgetItems.put(optimistic) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/budget/${id}`, - body: data, - resource: 'budgetItems', - }) - return { item: optimistic } - } - const result = await budgetApi.update(tripId, id, data) - offlineDb.budgetItems.put(result.item) - return result + const existing = await offlineDb.budgetItems.get(id) + const optimistic: BudgetItem = { ...(existing ?? {} as BudgetItem), ...(data as Partial), id } + await offlineDb.budgetItems.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/budget/${id}`, + body: data, + resource: 'budgetItems', + }) + mutationQueue.flush().catch(() => {}) + return { item: optimistic } }, async delete(tripId: number | string, id: number): Promise { - if (!navigator.onLine) { - await offlineDb.budgetItems.delete(id) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'DELETE', - url: `/trips/${tripId}/budget/${id}`, - body: undefined, - resource: 'budgetItems', - entityId: id, - }) - return { success: true } - } - const result = await budgetApi.delete(tripId, id) - offlineDb.budgetItems.delete(id) - return result + await offlineDb.budgetItems.delete(id) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'DELETE', + url: `/trips/${tripId}/budget/${id}`, + body: undefined, + resource: 'budgetItems', + entityId: id, + }) + mutationQueue.flush().catch(() => {}) + return { success: true } }, } diff --git a/client/src/repo/dayRepo.ts b/client/src/repo/dayRepo.ts index efbf9efe..74129707 100644 --- a/client/src/repo/dayRepo.ts +++ b/client/src/repo/dayRepo.ts @@ -29,22 +29,18 @@ export const dayRepo = { }, async update(tripId: number | string, dayId: number | string, data: Record): Promise<{ day: Day }> { - if (!navigator.onLine) { - const existing = await offlineDb.days.get(Number(dayId)) - const optimistic: Day = { ...(existing ?? {} as Day), ...(data as Partial), id: Number(dayId) } - await offlineDb.days.put(optimistic) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/days/${dayId}`, - body: data, - resource: 'days', - }) - return { day: optimistic } - } - const result = await daysApi.update(tripId, dayId, data) - offlineDb.days.put(result.day) - return result + const existing = await offlineDb.days.get(Number(dayId)) + const optimistic: Day = { ...(existing ?? {} as Day), ...data, id: Number(dayId) } + await offlineDb.days.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/days/${dayId}`, + body: data, + resource: 'days', + }) + mutationQueue.flush().catch(() => {}) + return { day: optimistic } }, } diff --git a/client/src/repo/fileRepo.ts b/client/src/repo/fileRepo.ts index ae685515..030ca815 100644 --- a/client/src/repo/fileRepo.ts +++ b/client/src/repo/fileRepo.ts @@ -28,60 +28,50 @@ export const fileRepo = { return { files: fresh.files, refresh: Promise.resolve(fresh) } }, - async update(tripId: number | string, id: number, data: Record): Promise { - if (!navigator.onLine) { - const existing = await offlineDb.tripFiles.get(id) - if (existing) await offlineDb.tripFiles.put({ ...existing, ...(data as Partial) }) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/files/${id}`, - body: data, - resource: 'tripFiles', - }) - return { success: true } - } - const result = await filesApi.update(tripId, id, data) - const file = (result as { file?: TripFile }).file - if (file) offlineDb.tripFiles.put(file) - return result + async update(tripId: number | string, id: number, data: Record): Promise<{ file: TripFile }> { + const existing = await offlineDb.tripFiles.get(id) + const optimistic: TripFile = { ...(existing ?? {} as TripFile), ...(data as Partial), id: Number(id) } + await offlineDb.tripFiles.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/files/${id}`, + body: data, + resource: 'tripFiles', + }) + mutationQueue.flush().catch(() => {}) + return { file: optimistic } }, async toggleStar(tripId: number | string, id: number): Promise { - if (!navigator.onLine) { - const existing = await offlineDb.tripFiles.get(id) - if (existing) { - await offlineDb.tripFiles.put({ ...existing, starred: existing.starred ? 0 : 1 }) - } - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PATCH', - url: `/trips/${tripId}/files/${id}/star`, - body: undefined, - }) - return { success: true } + const existing = await offlineDb.tripFiles.get(id) + if (existing) { + await offlineDb.tripFiles.put({ ...existing, starred: existing.starred ? 0 : 1 }) } - return filesApi.toggleStar(tripId, id) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PATCH', + url: `/trips/${tripId}/files/${id}/star`, + body: undefined, + }) + mutationQueue.flush().catch(() => {}) + return { success: true } }, async delete(tripId: number | string, id: number): Promise { - if (!navigator.onLine) { - await offlineDb.tripFiles.delete(id) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'DELETE', - url: `/trips/${tripId}/files/${id}`, - body: undefined, - resource: 'tripFiles', - entityId: id, - }) - return { success: true } - } - const result = await filesApi.delete(tripId, id) - offlineDb.tripFiles.delete(id) - return result + await offlineDb.tripFiles.delete(id) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'DELETE', + url: `/trips/${tripId}/files/${id}`, + body: undefined, + resource: 'tripFiles', + entityId: id, + }) + mutationQueue.flush().catch(() => {}) + return { success: true } }, } diff --git a/client/src/repo/packingRepo.ts b/client/src/repo/packingRepo.ts index 9d1ed060..49130813 100644 --- a/client/src/repo/packingRepo.ts +++ b/client/src/repo/packingRepo.ts @@ -29,71 +29,56 @@ export const packingRepo = { }, async create(tripId: number | string, data: Record): Promise<{ item: PackingItem }> { - if (!navigator.onLine) { - const tempId = -(Date.now()) - const tempItem: PackingItem = { - ...(data as Partial), - id: tempId, - trip_id: Number(tripId), - name: (data.name as string) ?? 'New item', - checked: 0, - } as PackingItem - await offlineDb.packingItems.put(tempItem) - const id = generateUUID() - await mutationQueue.enqueue({ - id, - tripId: Number(tripId), - method: 'POST', - url: `/trips/${tripId}/packing`, - body: data, - resource: 'packingItems', - tempId, - }) - return { item: tempItem } - } - const result = await packingApi.create(tripId, data) - offlineDb.packingItems.put(result.item) - return result + const tempId = -(Date.now()) + const tempItem: PackingItem = { + ...(data as Partial), + id: tempId, + trip_id: Number(tripId), + name: (data.name as string) ?? 'New item', + checked: 0, + } as PackingItem + await offlineDb.packingItems.put(tempItem) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'POST', + url: `/trips/${tripId}/packing`, + body: data, + resource: 'packingItems', + tempId, + }) + mutationQueue.flush().catch(() => {}) + return { item: tempItem } }, async update(tripId: number | string, id: number, data: Record): Promise<{ item: PackingItem }> { - if (!navigator.onLine) { - const existing = await offlineDb.packingItems.get(id) - const optimistic: PackingItem = { ...(existing ?? {} as PackingItem), ...(data as Partial), id } - await offlineDb.packingItems.put(optimistic) - const mutId = generateUUID() - await mutationQueue.enqueue({ - id: mutId, - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/packing/${id}`, - body: data, - resource: 'packingItems', - }) - return { item: optimistic } - } - const result = await packingApi.update(tripId, id, data) - offlineDb.packingItems.put(result.item) - return result + const existing = await offlineDb.packingItems.get(id) + const optimistic: PackingItem = { ...(existing ?? {} as PackingItem), ...(data as Partial), id } + await offlineDb.packingItems.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/packing/${id}`, + body: data, + resource: 'packingItems', + }) + mutationQueue.flush().catch(() => {}) + return { item: optimistic } }, async delete(tripId: number | string, id: number): Promise { - if (!navigator.onLine) { - await offlineDb.packingItems.delete(id) - const mutId = generateUUID() - await mutationQueue.enqueue({ - id: mutId, - tripId: Number(tripId), - method: 'DELETE', - url: `/trips/${tripId}/packing/${id}`, - body: undefined, - resource: 'packingItems', - entityId: id, - }) - return { success: true } - } - const result = await packingApi.delete(tripId, id) - offlineDb.packingItems.delete(id) - return result + await offlineDb.packingItems.delete(id) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'DELETE', + url: `/trips/${tripId}/packing/${id}`, + body: undefined, + resource: 'packingItems', + entityId: id, + }) + mutationQueue.flush().catch(() => {}) + return { success: true } }, } diff --git a/client/src/repo/placeRepo.ts b/client/src/repo/placeRepo.ts index dc1f776b..320f44b9 100644 --- a/client/src/repo/placeRepo.ts +++ b/client/src/repo/placeRepo.ts @@ -29,92 +29,72 @@ export const placeRepo = { }, async create(tripId: number | string, data: Record): Promise<{ place: Place }> { - if (!navigator.onLine) { - const tempId = -(Date.now()) - const tempPlace: Place = { - ...(data as Partial), - id: tempId, - trip_id: Number(tripId), - name: (data.name as string) ?? 'New place', - } as Place - await offlineDb.places.put(tempPlace) - const id = generateUUID() - await mutationQueue.enqueue({ - id, - tripId: Number(tripId), - method: 'POST', - url: `/trips/${tripId}/places`, - body: data, - resource: 'places', - tempId, - }) - return { place: tempPlace } - } - const result = await placesApi.create(tripId, data) - offlineDb.places.put(result.place) - return result + const tempId = -(Date.now()) + const tempPlace: Place = { + ...(data as Partial), + id: tempId, + trip_id: Number(tripId), + name: (data.name as string) ?? 'New place', + } as Place + await offlineDb.places.put(tempPlace) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'POST', + url: `/trips/${tripId}/places`, + body: data, + resource: 'places', + tempId, + }) + mutationQueue.flush().catch(() => {}) + return { place: tempPlace } }, async update(tripId: number | string, id: number | string, data: Record): Promise<{ place: Place }> { - if (!navigator.onLine) { - const existing = await offlineDb.places.get(Number(id)) - const optimistic: Place = { ...(existing ?? {} as Place), ...(data as Partial), id: Number(id) } - await offlineDb.places.put(optimistic) - const mutId = generateUUID() - await mutationQueue.enqueue({ - id: mutId, - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/places/${id}`, - body: data, - resource: 'places', - }) - return { place: optimistic } - } - const result = await placesApi.update(tripId, id, data) - offlineDb.places.put(result.place) - return result + const existing = await offlineDb.places.get(Number(id)) + const optimistic: Place = { ...(existing ?? {} as Place), ...(data as Partial), id: Number(id) } + await offlineDb.places.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/places/${id}`, + body: data, + resource: 'places', + }) + mutationQueue.flush().catch(() => {}) + return { place: optimistic } }, async delete(tripId: number | string, id: number | string): Promise { - if (!navigator.onLine) { - await offlineDb.places.delete(Number(id)) - const mutId = generateUUID() + await offlineDb.places.delete(Number(id)) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'DELETE', + url: `/trips/${tripId}/places/${id}`, + body: undefined, + resource: 'places', + entityId: Number(id), + }) + mutationQueue.flush().catch(() => {}) + return { success: true } + }, + + async deleteMany(tripId: number | string, ids: number[]): Promise { + await offlineDb.places.bulkDelete(ids) + for (const id of ids) { await mutationQueue.enqueue({ - id: mutId, + id: generateUUID(), tripId: Number(tripId), method: 'DELETE', url: `/trips/${tripId}/places/${id}`, body: undefined, resource: 'places', - entityId: Number(id), + entityId: id, }) - return { success: true } } - const result = await placesApi.delete(tripId, id) - offlineDb.places.delete(Number(id)) - return result - }, - - async deleteMany(tripId: number | string, ids: number[]): Promise { - if (!navigator.onLine) { - await offlineDb.places.bulkDelete(ids) - for (const id of ids) { - const mutId = generateUUID() - await mutationQueue.enqueue({ - id: mutId, - tripId: Number(tripId), - method: 'DELETE', - url: `/trips/${tripId}/places/${id}`, - body: undefined, - resource: 'places', - entityId: id, - }) - } - return { deleted: ids, count: ids.length } - } - const result = await placesApi.bulkDelete(tripId, ids) - await offlineDb.places.bulkDelete(ids) - return result + mutationQueue.flush().catch(() => {}) + return { deleted: ids, count: ids.length } }, } diff --git a/client/src/repo/reservationRepo.ts b/client/src/repo/reservationRepo.ts index a480968e..d0a4759c 100644 --- a/client/src/repo/reservationRepo.ts +++ b/client/src/repo/reservationRepo.ts @@ -29,75 +29,63 @@ export const reservationRepo = { }, async create(tripId: number | string, data: Record): Promise<{ reservation: Reservation }> { - if (!navigator.onLine) { - const tempId = -(Date.now()) - const tempReservation: Reservation = { - ...(data as Partial), - id: tempId, - trip_id: Number(tripId), - name: (data.name as string) ?? 'New reservation', - type: (data.type as string) ?? 'other', - status: 'pending', - date: (data.date as string) ?? null, - time: null, - confirmation_number: null, - notes: null, - url: null, - created_at: new Date().toISOString(), - } as Reservation - await offlineDb.reservations.put(tempReservation) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'POST', - url: `/trips/${tripId}/reservations`, - body: data, - resource: 'reservations', - tempId, - }) - return { reservation: tempReservation } - } - const result = await reservationsApi.create(tripId, data) - offlineDb.reservations.put(result.reservation) - return result + const tempId = -(Date.now()) + const tempReservation: Reservation = { + ...(data as Partial), + id: tempId, + trip_id: Number(tripId), + name: (data.name as string) ?? 'New reservation', + type: (data.type as string) ?? 'other', + status: 'pending', + date: (data.date as string) ?? null, + time: null, + confirmation_number: null, + notes: null, + url: null, + created_at: new Date().toISOString(), + } as Reservation + await offlineDb.reservations.put(tempReservation) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'POST', + url: `/trips/${tripId}/reservations`, + body: data, + resource: 'reservations', + tempId, + }) + mutationQueue.flush().catch(() => {}) + return { reservation: tempReservation } }, async update(tripId: number | string, id: number, data: Record): Promise<{ reservation: Reservation }> { - if (!navigator.onLine) { - const existing = await offlineDb.reservations.get(id) - const optimistic: Reservation = { ...(existing ?? {} as Reservation), ...(data as Partial), id } - await offlineDb.reservations.put(optimistic) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/reservations/${id}`, - body: data, - resource: 'reservations', - }) - return { reservation: optimistic } - } - const result = await reservationsApi.update(tripId, id, data) - offlineDb.reservations.put(result.reservation) - return result + const existing = await offlineDb.reservations.get(id) + const optimistic: Reservation = { ...(existing ?? {} as Reservation), ...(data as Partial), id } + await offlineDb.reservations.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/reservations/${id}`, + body: data, + resource: 'reservations', + }) + mutationQueue.flush().catch(() => {}) + return { reservation: optimistic } }, async delete(tripId: number | string, id: number): Promise { - if (!navigator.onLine) { - await offlineDb.reservations.delete(id) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'DELETE', - url: `/trips/${tripId}/reservations/${id}`, - body: undefined, - resource: 'reservations', - entityId: id, - }) - return { success: true } - } - const result = await reservationsApi.delete(tripId, id) - offlineDb.reservations.delete(id) - return result + await offlineDb.reservations.delete(id) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'DELETE', + url: `/trips/${tripId}/reservations/${id}`, + body: undefined, + resource: 'reservations', + entityId: id, + }) + mutationQueue.flush().catch(() => {}) + return { success: true } }, } diff --git a/client/src/repo/todoRepo.ts b/client/src/repo/todoRepo.ts index f0f9df46..27b54db6 100644 --- a/client/src/repo/todoRepo.ts +++ b/client/src/repo/todoRepo.ts @@ -29,73 +29,61 @@ export const todoRepo = { }, async create(tripId: number | string, data: Record): Promise<{ item: TodoItem }> { - if (!navigator.onLine) { - const tempId = -(Date.now()) - const tempItem: TodoItem = { - ...(data as Partial), - id: tempId, - trip_id: Number(tripId), - name: (data.name as string) ?? 'New todo', - checked: 0, - sort_order: 0, - due_date: null, - description: null, - assigned_user_id: null, - priority: 0, - } as TodoItem - await offlineDb.todoItems.put(tempItem) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'POST', - url: `/trips/${tripId}/todo`, - body: data, - resource: 'todoItems', - tempId, - }) - return { item: tempItem } - } - const result = await todoApi.create(tripId, data) - offlineDb.todoItems.put(result.item) - return result + const tempId = -(Date.now()) + const tempItem: TodoItem = { + ...(data as Partial), + id: tempId, + trip_id: Number(tripId), + name: (data.name as string) ?? 'New todo', + checked: 0, + sort_order: 0, + due_date: null, + description: null, + assigned_user_id: null, + priority: 0, + } as TodoItem + await offlineDb.todoItems.put(tempItem) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'POST', + url: `/trips/${tripId}/todo`, + body: data, + resource: 'todoItems', + tempId, + }) + mutationQueue.flush().catch(() => {}) + return { item: tempItem } }, async update(tripId: number | string, id: number, data: Record): Promise<{ item: TodoItem }> { - if (!navigator.onLine) { - const existing = await offlineDb.todoItems.get(id) - const optimistic: TodoItem = { ...(existing ?? {} as TodoItem), ...(data as Partial), id } - await offlineDb.todoItems.put(optimistic) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}/todo/${id}`, - body: data, - resource: 'todoItems', - }) - return { item: optimistic } - } - const result = await todoApi.update(tripId, id, data) - offlineDb.todoItems.put(result.item) - return result + const existing = await offlineDb.todoItems.get(id) + const optimistic: TodoItem = { ...(existing ?? {} as TodoItem), ...(data as Partial), id } + await offlineDb.todoItems.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}/todo/${id}`, + body: data, + resource: 'todoItems', + }) + mutationQueue.flush().catch(() => {}) + return { item: optimistic } }, async delete(tripId: number | string, id: number): Promise { - if (!navigator.onLine) { - await offlineDb.todoItems.delete(id) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'DELETE', - url: `/trips/${tripId}/todo/${id}`, - body: undefined, - resource: 'todoItems', - entityId: id, - }) - return { success: true } - } - const result = await todoApi.delete(tripId, id) - offlineDb.todoItems.delete(id) - return result + await offlineDb.todoItems.delete(id) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'DELETE', + url: `/trips/${tripId}/todo/${id}`, + body: undefined, + resource: 'todoItems', + entityId: id, + }) + mutationQueue.flush().catch(() => {}) + return { success: true } }, } diff --git a/client/src/repo/tripRepo.ts b/client/src/repo/tripRepo.ts index 6b32ee27..07efc624 100644 --- a/client/src/repo/tripRepo.ts +++ b/client/src/repo/tripRepo.ts @@ -60,22 +60,18 @@ export const tripRepo = { }, async update(tripId: number | string, data: Partial): Promise<{ trip: Trip }> { - if (!navigator.onLine) { - const existing = await offlineDb.trips.get(Number(tripId)) - const optimistic: Trip = { ...(existing ?? {} as Trip), ...(data as Partial), id: Number(tripId) } - await offlineDb.trips.put(optimistic) - await mutationQueue.enqueue({ - id: generateUUID(), - tripId: Number(tripId), - method: 'PUT', - url: `/trips/${tripId}`, - body: data as Record, - resource: 'trips', - }) - return { trip: optimistic } - } - const result = await tripsApi.update(tripId, data as Record) - upsertTrip(result.trip) - return result + const existing = await offlineDb.trips.get(Number(tripId)) + const optimistic: Trip = { ...(existing ?? {} as Trip), ...(data as Partial), id: Number(tripId) } + await offlineDb.trips.put(optimistic) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: Number(tripId), + method: 'PUT', + url: `/trips/${tripId}`, + body: data as Record, + resource: 'trips', + }) + mutationQueue.flush().catch(() => {}) + return { trip: optimistic } }, } diff --git a/client/src/store/slices/budgetSlice.test.ts b/client/src/store/slices/budgetSlice.test.ts index 7b379e68..48033d69 100644 --- a/client/src/store/slices/budgetSlice.test.ts +++ b/client/src/store/slices/budgetSlice.test.ts @@ -37,25 +37,28 @@ describe('budgetSlice', () => { expect(useTripStore.getState().budgetItems).toEqual([]); }); - it('FE-STORE-BUDGET-003: addBudgetItem appends to store and returns item', async () => { - const newItem = buildBudgetItem({ name: 'Hotel', trip_id: 1 }); + it('FE-STORE-BUDGET-003: addBudgetItem appends to store optimistically', async () => { server.use( http.post('/api/trips/1/budget', () => - HttpResponse.json({ item: newItem }) + HttpResponse.json({ item: buildBudgetItem({ name: 'Hotel', trip_id: 1 }) }) ) ); const result = await useTripStore.getState().addBudgetItem(1, { name: 'Hotel' }); - expect(result.id).toBe(newItem.id); - expect(useTripStore.getState().budgetItems).toContainEqual(newItem); + expect(result.name).toBe('Hotel'); + const items = useTripStore.getState().budgetItems; + expect(items).toHaveLength(1); + expect(items[0].name).toBe('Hotel'); }); - it('FE-STORE-BUDGET-004: addBudgetItem throws on API error', async () => { + it('FE-STORE-BUDGET-004: addBudgetItem adds item optimistically even on API error', async () => { server.use( http.post('/api/trips/1/budget', () => HttpResponse.json({ error: 'Validation failed' }, { status: 422 }) ) ); - await expect(useTripStore.getState().addBudgetItem(1, {})).rejects.toThrow(); + const result = await useTripStore.getState().addBudgetItem(1, { name: 'Item' }); + expect(result.name).toBe('Item'); + expect(useTripStore.getState().budgetItems).toHaveLength(1); }); it('FE-STORE-BUDGET-005: updateBudgetItem replaces item in store', async () => { @@ -74,24 +77,21 @@ describe('budgetSlice', () => { expect(items[0].name).toBe('New'); }); - it('FE-STORE-BUDGET-006: updateBudgetItem calls loadReservations when reservation_id + total_price provided', async () => { - const existing = buildBudgetItem({ id: 20, trip_id: 1 }); + it('FE-STORE-BUDGET-006: updateBudgetItem resolves and updates store optimistically', async () => { + const existing = buildBudgetItem({ id: 20, trip_id: 1, amount: 100 }); seedStore(useTripStore, { budgetItems: [existing] }); - const loadReservations = vi.fn().mockResolvedValue(undefined); - seedStore(useTripStore, { loadReservations }); - - const itemWithReservation = { ...existing, reservation_id: 99 }; server.use( http.put('/api/trips/1/budget/20', () => - HttpResponse.json({ item: itemWithReservation }) + HttpResponse.json({ item: { ...existing, amount: 50 } }) ) ); - await useTripStore.getState().updateBudgetItem(1, 20, { total_price: 50 }); - expect(loadReservations).toHaveBeenCalledWith(1); + const result = await useTripStore.getState().updateBudgetItem(1, 20, { amount: 50 }); + expect(result.amount).toBe(50); + expect(useTripStore.getState().budgetItems[0].amount).toBe(50); }); - it('FE-STORE-BUDGET-007: deleteBudgetItem optimistically removes and rolls back on error', async () => { + it('FE-STORE-BUDGET-007: deleteBudgetItem removes item permanently even on API error', async () => { const item = buildBudgetItem({ id: 5, trip_id: 1 }); seedStore(useTripStore, { budgetItems: [item] }); @@ -100,11 +100,9 @@ describe('budgetSlice', () => { HttpResponse.json({ error: 'forbidden' }, { status: 403 }) ) ); - // The item is removed immediately (optimistic), then restored on error - const deletePromise = useTripStore.getState().deleteBudgetItem(1, 5); - await expect(deletePromise).rejects.toThrow(); - // After rollback, item is back - expect(useTripStore.getState().budgetItems).toContainEqual(item); + await useTripStore.getState().deleteBudgetItem(1, 5); + // Permanently removed (queued for sync, no rollback) + expect(useTripStore.getState().budgetItems).toHaveLength(0); }); it('FE-STORE-BUDGET-008: setBudgetItemMembers updates members on matching item', async () => { diff --git a/client/src/sync/mutationQueue.ts b/client/src/sync/mutationQueue.ts index d25b6b5b..93dc0f1c 100644 --- a/client/src/sync/mutationQueue.ts +++ b/client/src/sync/mutationQueue.ts @@ -73,12 +73,14 @@ export const mutationQueue = { if (_flushing || !navigator.onLine) return _flushing = true try { - const pending = await offlineDb.mutationQueue - .where('status') - .equals('pending') - .sortBy('createdAt') + while (true) { + const pending = await offlineDb.mutationQueue + .where('status') + .equals('pending') + .sortBy('createdAt') + const mutation = pending[0] + if (!mutation) break - for (const mutation of pending) { // Mark as syncing so UI can show progress await offlineDb.mutationQueue.update(mutation.id, { status: 'syncing' }) diff --git a/client/tests/unit/repo/packingRepo.test.ts b/client/tests/unit/repo/packingRepo.test.ts index 4c25ada2..97c9b44b 100644 --- a/client/tests/unit/repo/packingRepo.test.ts +++ b/client/tests/unit/repo/packingRepo.test.ts @@ -66,38 +66,28 @@ describe('packingRepo.list', () => { }); describe('packingRepo.create', () => { - it('calls REST and caches created item in Dexie', async () => { - const item = buildPackingItem({ trip_id: 1, name: 'Sunscreen' }); - server.use( - http.post('/api/trips/1/packing', () => HttpResponse.json({ item })), - ); - + it('writes item optimistically to Dexie immediately', async () => { const result = await packingRepo.create(1, { name: 'Sunscreen' }); expect(result.item.name).toBe('Sunscreen'); + // tempId is negative (-(Date.now())) + expect(result.item.id).toBeLessThan(0); - await new Promise(r => setTimeout(r, 0)); - const cached = await offlineDb.packingItems.get(item.id); - expect(cached).toBeDefined(); - expect(cached!.name).toBe('Sunscreen'); + const cached = await offlineDb.packingItems.where('trip_id').equals(1).toArray(); + expect(cached).toHaveLength(1); + expect(cached[0].name).toBe('Sunscreen'); }); }); describe('packingRepo.update', () => { - it('calls REST and updates Dexie cache', async () => { + it('writes optimistic update to Dexie immediately', async () => { const original = buildPackingItem({ trip_id: 1, name: 'Jacket', checked: 0 }); await offlineDb.packingItems.put(original); - const updated = { ...original, checked: 1 }; - server.use( - http.put(`/api/trips/1/packing/${original.id}`, () => HttpResponse.json({ item: updated })), - ); - const result = await packingRepo.update(1, original.id, { checked: true }); - expect(result.item.checked).toBe(1); + expect(result.item.checked).toBeTruthy(); - await new Promise(r => setTimeout(r, 0)); const cached = await offlineDb.packingItems.get(original.id); - expect(cached!.checked).toBe(1); + expect(cached!.checked).toBeTruthy(); }); }); diff --git a/client/tests/unit/repo/placeRepo.test.ts b/client/tests/unit/repo/placeRepo.test.ts index 45387841..9ee808ff 100644 --- a/client/tests/unit/repo/placeRepo.test.ts +++ b/client/tests/unit/repo/placeRepo.test.ts @@ -67,19 +67,15 @@ describe('placeRepo.list', () => { }); describe('placeRepo.create', () => { - it('calls REST and caches created place in Dexie', async () => { - const place = buildPlace({ trip_id: 1, name: 'Eiffel Tower' }); - server.use( - http.post('/api/trips/1/places', () => HttpResponse.json({ place })), - ); - + it('writes place optimistically to Dexie immediately', async () => { const result = await placeRepo.create(1, { name: 'Eiffel Tower' }); expect(result.place.name).toBe('Eiffel Tower'); + // tempId is negative (-(Date.now())) + expect(result.place.id).toBeLessThan(0); - await new Promise(r => setTimeout(r, 0)); - const cached = await offlineDb.places.get(place.id); - expect(cached).toBeDefined(); - expect(cached!.name).toBe('Eiffel Tower'); + const cached = await offlineDb.places.where('trip_id').equals(1).toArray(); + expect(cached).toHaveLength(1); + expect(cached[0].name).toBe('Eiffel Tower'); }); }); diff --git a/client/tests/unit/slices/budgetSlice.test.ts b/client/tests/unit/slices/budgetSlice.test.ts index e847dd86..6a9074f2 100644 --- a/client/tests/unit/slices/budgetSlice.test.ts +++ b/client/tests/unit/slices/budgetSlice.test.ts @@ -2,8 +2,9 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { http, HttpResponse } from 'msw'; import { useTripStore } from '../../../src/store/tripStore'; import { resetAllStores, seedStore } from '../../helpers/store'; -import { buildBudgetItem, buildReservation } from '../../helpers/factories'; +import { buildBudgetItem } from '../../helpers/factories'; import { server } from '../../helpers/msw/server'; +import { offlineDb } from '../../../src/db/offlineDb'; vi.mock('../../../src/api/websocket', () => ({ connect: vi.fn(), @@ -17,7 +18,9 @@ vi.mock('../../../src/api/websocket', () => ({ setPreReconnectHook: vi.fn(), })); -beforeEach(() => { +beforeEach(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.all(offlineDb.tables.map(t => t.clear())); resetAllStores(); }); @@ -49,16 +52,18 @@ describe('budgetSlice', () => { expect(useTripStore.getState().budgetItems).toHaveLength(2); }); - it('FE-BUDGET-003: addBudgetItem on failure throws', async () => { + it('FE-BUDGET-003: addBudgetItem always adds item optimistically (no throw on API error)', async () => { server.use( http.post('/api/trips/1/budget', () => HttpResponse.json({ message: 'Error' }, { status: 500 }) ), ); - await expect( - useTripStore.getState().addBudgetItem(1, { name: 'Fail' }) - ).rejects.toThrow(); + const result = await useTripStore.getState().addBudgetItem(1, { name: 'Fail' }); + + expect(result.name).toBe('Fail'); + expect(useTripStore.getState().budgetItems).toHaveLength(1); + expect(useTripStore.getState().budgetItems[0].name).toBe('Fail'); }); }); @@ -80,38 +85,26 @@ describe('budgetSlice', () => { expect(useTripStore.getState().budgetItems[0].name).toBe('Updated'); }); - it('FE-BUDGET-005: updateBudgetItem with total_price triggers loadReservations when reservation_id present', async () => { - const item = buildBudgetItem({ id: 10, trip_id: 1, amount: 100 }); - const initialReservation = buildReservation({ trip_id: 1 }); - const newReservation = buildReservation({ trip_id: 1, name: 'Refreshed Reservation' }); - seedStore(useTripStore, { - budgetItems: [item], - reservations: [initialReservation], - }); + it('FE-BUDGET-005: updateBudgetItem resolves and updates store optimistically', async () => { + const item = buildBudgetItem({ id: 10, trip_id: 1, name: 'Old', amount: 100 }); + seedStore(useTripStore, { budgetItems: [item] }); server.use( http.put('/api/trips/1/budget/10', async ({ request }) => { const body = await request.json() as Record; - // Return item with reservation_id to trigger loadReservations return HttpResponse.json({ item: { ...item, ...body, reservation_id: 42 } }); }), - http.get('/api/trips/1/reservations', () => - HttpResponse.json({ reservations: [newReservation] }) - ), ); - await useTripStore.getState().updateBudgetItem(1, 10, { total_price: 200 } as Record); + const result = await useTripStore.getState().updateBudgetItem(1, 10, { amount: 200 } as Record); - // Wait for the async loadReservations to complete - await new Promise(resolve => setTimeout(resolve, 50)); - - expect(useTripStore.getState().reservations).toHaveLength(1); - expect(useTripStore.getState().reservations[0].name).toBe('Refreshed Reservation'); + expect(result.amount).toBe(200); + expect(useTripStore.getState().budgetItems[0].amount).toBe(200); }); }); describe('deleteBudgetItem', () => { - it('FE-BUDGET-006: deleteBudgetItem optimistically removes item, rolls back on failure', async () => { + it('FE-BUDGET-006: deleteBudgetItem removes item permanently even on API error', async () => { const item = buildBudgetItem({ id: 10, trip_id: 1 }); seedStore(useTripStore, { budgetItems: [item] }); @@ -121,10 +114,10 @@ describe('budgetSlice', () => { ), ); - await expect(useTripStore.getState().deleteBudgetItem(1, 10)).rejects.toThrow(); + await useTripStore.getState().deleteBudgetItem(1, 10); - expect(useTripStore.getState().budgetItems).toHaveLength(1); - expect(useTripStore.getState().budgetItems[0].id).toBe(10); + // Permanently removed (queued for sync, no rollback) + expect(useTripStore.getState().budgetItems).toHaveLength(0); }); it('FE-BUDGET-006b: deleteBudgetItem success removes item', async () => { diff --git a/client/tests/unit/slices/filesSlice.test.ts b/client/tests/unit/slices/filesSlice.test.ts index 7f5adc8c..814eceaa 100644 --- a/client/tests/unit/slices/filesSlice.test.ts +++ b/client/tests/unit/slices/filesSlice.test.ts @@ -100,7 +100,7 @@ describe('filesSlice', () => { expect(files[0].id).toBe(20); }); - it('FE-FILES-006: deleteFile on failure throws', async () => { + it('FE-FILES-006: deleteFile removes file permanently even on API error', async () => { const file = buildTripFile({ id: 10, trip_id: 1 }); seedStore(useTripStore, { files: [file] }); @@ -110,10 +110,10 @@ describe('filesSlice', () => { ), ); - await expect(useTripStore.getState().deleteFile(1, 10)).rejects.toThrow(); + await useTripStore.getState().deleteFile(1, 10); - // File remains since server-first (only removes after success) - expect(useTripStore.getState().files).toHaveLength(1); + // Permanently removed (queued for sync, no rollback) + expect(useTripStore.getState().files).toHaveLength(0); }); }); }); diff --git a/client/tests/unit/slices/packingSlice.test.ts b/client/tests/unit/slices/packingSlice.test.ts index 901c0a08..587b8379 100644 --- a/client/tests/unit/slices/packingSlice.test.ts +++ b/client/tests/unit/slices/packingSlice.test.ts @@ -4,6 +4,7 @@ import { useTripStore } from '../../../src/store/tripStore'; import { resetAllStores, seedStore } from '../../helpers/store'; import { buildPackingItem } from '../../helpers/factories'; import { server } from '../../helpers/msw/server'; +import { offlineDb } from '../../../src/db/offlineDb'; vi.mock('../../../src/api/websocket', () => ({ connect: vi.fn(), @@ -17,7 +18,9 @@ vi.mock('../../../src/api/websocket', () => ({ setPreReconnectHook: vi.fn(), })); -beforeEach(() => { +beforeEach(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.all(offlineDb.tables.map(t => t.clear())); resetAllStores(); }); @@ -36,16 +39,18 @@ describe('packingSlice', () => { expect(items[items.length - 1].name).toBe('Toothbrush'); }); - it('FE-PACKING-002: addPackingItem on failure throws', async () => { + it('FE-PACKING-002: addPackingItem always adds item optimistically (no throw on API error)', async () => { server.use( http.post('/api/trips/1/packing', () => HttpResponse.json({ message: 'Error' }, { status: 500 }) ), ); - await expect( - useTripStore.getState().addPackingItem(1, { name: 'Fail item' }) - ).rejects.toThrow(); + const result = await useTripStore.getState().addPackingItem(1, { name: 'Fail item' }); + + expect(result.name).toBe('Fail item'); + expect(useTripStore.getState().packingItems).toHaveLength(1); + expect(useTripStore.getState().packingItems[0].name).toBe('Fail item'); }); }); @@ -69,7 +74,7 @@ describe('packingSlice', () => { }); describe('deletePackingItem', () => { - it('FE-PACKING-004: deletePackingItem optimistically removes item, rollback on failure', async () => { + it('FE-PACKING-004: deletePackingItem removes item permanently even on API error', async () => { const item = buildPackingItem({ id: 10, trip_id: 1 }); seedStore(useTripStore, { packingItems: [item] }); @@ -79,10 +84,9 @@ describe('packingSlice', () => { ), ); - await expect(useTripStore.getState().deletePackingItem(1, 10)).rejects.toThrow(); + await useTripStore.getState().deletePackingItem(1, 10); - expect(useTripStore.getState().packingItems).toHaveLength(1); - expect(useTripStore.getState().packingItems[0].id).toBe(10); + expect(useTripStore.getState().packingItems).toHaveLength(0); }); it('FE-PACKING-004b: deletePackingItem success removes item', async () => { @@ -115,7 +119,7 @@ describe('packingSlice', () => { expect(useTripStore.getState().packingItems[0].checked).toBe(1); }); - it('FE-PACKING-006: togglePackingItem rolls back checked on API failure', async () => { + it('FE-PACKING-006: togglePackingItem preserves optimistic checked state even on API failure', async () => { const item = buildPackingItem({ id: 10, trip_id: 1, checked: 0 }); seedStore(useTripStore, { packingItems: [item] }); @@ -125,11 +129,10 @@ describe('packingSlice', () => { ), ); - // toggle does NOT throw on error (silent rollback) await useTripStore.getState().togglePackingItem(1, 10, true); - // Should be rolled back to original value - expect(useTripStore.getState().packingItems[0].checked).toBe(0); + // Optimistic state preserved — no rollback (queued for sync) + expect(useTripStore.getState().packingItems[0].checked).toBe(1); }); }); }); diff --git a/client/tests/unit/slices/placesSlice.test.ts b/client/tests/unit/slices/placesSlice.test.ts index 19df3a9b..8a90e363 100644 --- a/client/tests/unit/slices/placesSlice.test.ts +++ b/client/tests/unit/slices/placesSlice.test.ts @@ -38,7 +38,7 @@ describe('placesSlice', () => { expect(places[0].name).toBe('New Place'); // prepended }); - it('FE-PLACES-002: addPlace on failure throws and places remain unchanged', async () => { + it('FE-PLACES-002: addPlace always adds place optimistically (no throw on API error)', async () => { const existing = buildPlace({ trip_id: 1 }); seedStore(useTripStore, { places: [existing] }); @@ -48,8 +48,11 @@ describe('placesSlice', () => { ), ); - await expect(useTripStore.getState().addPlace(1, { name: 'Fail' })).rejects.toThrow(); - expect(useTripStore.getState().places).toEqual([existing]); + const result = await useTripStore.getState().addPlace(1, { name: 'Fail' }); + + expect(result.name).toBe('Fail'); + expect(useTripStore.getState().places).toHaveLength(2); + expect(useTripStore.getState().places[0].name).toBe('Fail'); }); }); diff --git a/client/tests/unit/slices/reservationsSlice.test.ts b/client/tests/unit/slices/reservationsSlice.test.ts index b0b5e134..021b8e21 100644 --- a/client/tests/unit/slices/reservationsSlice.test.ts +++ b/client/tests/unit/slices/reservationsSlice.test.ts @@ -4,6 +4,7 @@ import { useTripStore } from '../../../src/store/tripStore'; import { resetAllStores, seedStore } from '../../helpers/store'; import { buildReservation } from '../../helpers/factories'; import { server } from '../../helpers/msw/server'; +import { offlineDb } from '../../../src/db/offlineDb'; vi.mock('../../../src/api/websocket', () => ({ connect: vi.fn(), @@ -17,7 +18,9 @@ vi.mock('../../../src/api/websocket', () => ({ setPreReconnectHook: vi.fn(), })); -beforeEach(() => { +beforeEach(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.all(offlineDb.tables.map(t => t.clear())); resetAllStores(); }); @@ -58,16 +61,18 @@ describe('reservationsSlice', () => { expect(reservations[0].name).toBe('New Hotel'); }); - it('FE-RESERV-003: addReservation on failure throws', async () => { + it('FE-RESERV-003: addReservation always adds optimistically (no throw on API error)', async () => { server.use( http.post('/api/trips/1/reservations', () => HttpResponse.json({ message: 'Error' }, { status: 500 }) ), ); - await expect( - useTripStore.getState().addReservation(1, { name: 'Fail' }) - ).rejects.toThrow(); + const result = await useTripStore.getState().addReservation(1, { name: 'Fail' }); + + expect(result.name).toBe('Fail'); + expect(useTripStore.getState().reservations).toHaveLength(1); + expect(useTripStore.getState().reservations[0].name).toBe('Fail'); }); }); @@ -123,7 +128,7 @@ describe('reservationsSlice', () => { expect(useTripStore.getState().reservations[0].status).toBe('confirmed'); }); - it('FE-RESERV-007: toggleReservationStatus rolls back on API failure (silent)', async () => { + it('FE-RESERV-007: toggleReservationStatus preserves optimistic status even on API failure', async () => { const reservation = buildReservation({ id: 10, trip_id: 1, status: 'confirmed' }); seedStore(useTripStore, { reservations: [reservation] }); @@ -133,10 +138,10 @@ describe('reservationsSlice', () => { ), ); - // Does NOT throw (silent rollback) await useTripStore.getState().toggleReservationStatus(1, 10); - expect(useTripStore.getState().reservations[0].status).toBe('confirmed'); + // Optimistic state preserved — no rollback (queued for sync) + expect(useTripStore.getState().reservations[0].status).toBe('pending'); }); it('FE-RESERV-008: toggleReservationStatus does nothing if reservation not found', async () => { @@ -162,7 +167,7 @@ describe('reservationsSlice', () => { expect(reservations[0].id).toBe(20); }); - it('FE-RESERV-010: deleteReservation on failure throws (no optimistic, server-first)', async () => { + it('FE-RESERV-010: deleteReservation removes permanently even on API error', async () => { const reservation = buildReservation({ id: 10, trip_id: 1 }); seedStore(useTripStore, { reservations: [reservation] }); @@ -172,10 +177,10 @@ describe('reservationsSlice', () => { ), ); - await expect(useTripStore.getState().deleteReservation(1, 10)).rejects.toThrow(); + await useTripStore.getState().deleteReservation(1, 10); - // Still in state since server-first (only removes after success) - expect(useTripStore.getState().reservations).toHaveLength(1); + // Permanently removed (queued for sync, no rollback) + expect(useTripStore.getState().reservations).toHaveLength(0); }); }); }); diff --git a/client/tests/unit/slices/todoSlice.test.ts b/client/tests/unit/slices/todoSlice.test.ts index 123426bc..257f02cf 100644 --- a/client/tests/unit/slices/todoSlice.test.ts +++ b/client/tests/unit/slices/todoSlice.test.ts @@ -4,6 +4,7 @@ import { useTripStore } from '../../../src/store/tripStore'; import { resetAllStores, seedStore } from '../../helpers/store'; import { buildTodoItem } from '../../helpers/factories'; import { server } from '../../helpers/msw/server'; +import { offlineDb } from '../../../src/db/offlineDb'; vi.mock('../../../src/api/websocket', () => ({ connect: vi.fn(), @@ -17,7 +18,9 @@ vi.mock('../../../src/api/websocket', () => ({ setPreReconnectHook: vi.fn(), })); -beforeEach(() => { +beforeEach(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + await Promise.all(offlineDb.tables.map(t => t.clear())); resetAllStores(); }); @@ -34,16 +37,18 @@ describe('todoSlice', () => { expect(items).toHaveLength(2); }); - it('FE-TODO-002: addTodoItem on failure throws', async () => { + it('FE-TODO-002: addTodoItem always adds item optimistically (no throw on API error)', async () => { server.use( http.post('/api/trips/1/todo', () => HttpResponse.json({ message: 'Error' }, { status: 500 }) ), ); - await expect( - useTripStore.getState().addTodoItem(1, { name: 'Fail' }) - ).rejects.toThrow(); + const result = await useTripStore.getState().addTodoItem(1, { name: 'Fail' }); + + expect(result.name).toBe('Fail'); + expect(useTripStore.getState().todoItems).toHaveLength(1); + expect(useTripStore.getState().todoItems[0].name).toBe('Fail'); }); }); @@ -69,7 +74,7 @@ describe('todoSlice', () => { }); describe('deleteTodoItem', () => { - it('FE-TODO-004: deleteTodoItem optimistically removes item, rollback on failure', async () => { + it('FE-TODO-004: deleteTodoItem removes item permanently even on API error', async () => { const item = buildTodoItem({ id: 10, trip_id: 1 }); seedStore(useTripStore, { todoItems: [item] }); @@ -79,10 +84,9 @@ describe('todoSlice', () => { ), ); - await expect(useTripStore.getState().deleteTodoItem(1, 10)).rejects.toThrow(); + await useTripStore.getState().deleteTodoItem(1, 10); - expect(useTripStore.getState().todoItems).toHaveLength(1); - expect(useTripStore.getState().todoItems[0].id).toBe(10); + expect(useTripStore.getState().todoItems).toHaveLength(0); }); it('FE-TODO-004b: deleteTodoItem success removes item from array', async () => { @@ -115,7 +119,7 @@ describe('todoSlice', () => { expect(useTripStore.getState().todoItems[0].checked).toBe(1); }); - it('FE-TODO-006: toggleTodoItem rolls back checked on API failure (silent)', async () => { + it('FE-TODO-006: toggleTodoItem preserves optimistic checked state even on API failure', async () => { const item = buildTodoItem({ id: 10, trip_id: 1, checked: 0 }); seedStore(useTripStore, { todoItems: [item] }); @@ -125,10 +129,10 @@ describe('todoSlice', () => { ), ); - // Does NOT throw await useTripStore.getState().toggleTodoItem(1, 10, true); - expect(useTripStore.getState().todoItems[0].checked).toBe(0); + // Optimistic state preserved — no rollback (queued for sync) + expect(useTripStore.getState().todoItems[0].checked).toBe(1); }); it('FE-TODO-007: toggleTodoItem preserves sort_order field', async () => { diff --git a/client/tests/unit/tripStore.test.ts b/client/tests/unit/tripStore.test.ts index 2d03a1e7..0aefe291 100644 --- a/client/tests/unit/tripStore.test.ts +++ b/client/tests/unit/tripStore.test.ts @@ -219,8 +219,8 @@ describe('tripStore', () => { const result = await useTripStore.getState().updateTrip(1, { name: 'Updated Trip' }); - expect(result).toEqual(updatedTrip); - expect(useTripStore.getState().trip).toEqual(updatedTrip); + expect(result.name).toBe('Updated Trip'); + expect(useTripStore.getState().trip?.name).toBe('Updated Trip'); }); });