diff --git a/client/src/components/Tours/ToursSidebar.tsx b/client/src/components/Tours/ToursSidebar.tsx index f61b1a907..b7c741aca 100644 --- a/client/src/components/Tours/ToursSidebar.tsx +++ b/client/src/components/Tours/ToursSidebar.tsx @@ -2,7 +2,7 @@ import React, { useEffect, useRef, useState } from 'react' import { Check, ChevronDown, FileDown } from 'lucide-react' import type { TourListItem } from '@trek/shared' import type { Day } from '../../types' -import { toursApi } from '../../api/client' +import { tourRepo } from '../../repo/tourRepo' import { useTripStore } from '../../store/tripStore' import { useTranslation } from '../../i18n' import { useToast } from '../shared/Toast' @@ -63,7 +63,7 @@ export default function ToursSidebar({ tripId, days, tours, loading = false, sel setImporting(true) try { - const result = await toursApi.importGpx(tripId, file) + const result = await tourRepo.importGpx(tripId, file) if (result.tours.length > 0) { // The server publishes place:created with sender echo suppression. // Reload the importing client's trip so new places and route geometry diff --git a/client/src/components/Tours/planner/useTourPlanner.test.tsx b/client/src/components/Tours/planner/useTourPlanner.test.tsx index c649983d0..08ca12dd9 100644 --- a/client/src/components/Tours/planner/useTourPlanner.test.tsx +++ b/client/src/components/Tours/planner/useTourPlanner.test.tsx @@ -17,7 +17,7 @@ const { routeWalkingTour, enrichTourElevations, createTour, detailTour, updateTo })) vi.mock('./tourRouting', () => ({ routeWalkingTour, enrichTourElevations })) -vi.mock('../../../api/client', () => ({ toursApi: { create: createTour, detail: detailTour, update: updateTour } })) +vi.mock('../../../repo/tourRepo', () => ({ tourRepo: { create: createTour, detail: detailTour, update: updateTour } })) vi.mock('../useTourPermissions', () => ({ useTourPermissions: ({ canEdit = true, canAssign = true }: { canEdit?: boolean; canAssign?: boolean }) => ({ canEdit, canAssign }) })) import { useTourPlanner } from './useTourPlanner' diff --git a/client/src/components/Tours/planner/useTourPlanner.ts b/client/src/components/Tours/planner/useTourPlanner.ts index f8642f419..2355f905d 100644 --- a/client/src/components/Tours/planner/useTourPlanner.ts +++ b/client/src/components/Tours/planner/useTourPlanner.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import type { TourCreateResponse, TourListItem, TourWaypointRole, TourMaxHikingDifficulty } from '@trek/shared' -import { toursApi } from '../../../api/client' +import { tourRepo } from '../../../repo/tourRepo' import { randomId } from '../../../utils/randomId' import type { TourBaseLayer } from '../../Map/MapLayerSwitcher' import { analyzeRouteGeometry, type RouteProfileFocus } from '../../../utils/routeGeometry' @@ -412,7 +412,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe setOpeningTourId(tour.place_id) setError(null) try { - const result = await toursApi.detail(tripId, tour.place_id, controller.signal) + const result = await tourRepo.detail(tripId, tour.place_id, controller.signal) if (!requestIsCurrent() || saveInFlightRef.current) return false if (result.waypoints.length < 2) { setError('open') @@ -591,8 +591,8 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe && draftRevisionRef.current === submittedRevision try { const result = submittedPlaceId === null - ? await toursApi.create(tripId, payload) - : await toursApi.update(tripId, submittedPlaceId, payload) + ? await tourRepo.create(tripId, payload) + : await tourRepo.update(tripId, submittedPlaceId, payload) if (stillCurrent()) { localStorage.removeItem(storageKey) setHasUnsavedChanges(false) diff --git a/client/src/components/Tours/toursPermissions.test.tsx b/client/src/components/Tours/toursPermissions.test.tsx index cd58dd6d2..da075d7ae 100644 --- a/client/src/components/Tours/toursPermissions.test.tsx +++ b/client/src/components/Tours/toursPermissions.test.tsx @@ -22,7 +22,7 @@ const state = vi.hoisted(() => ({ vi.mock('../../store/tripStore', () => ({ useTripStore: (selector: (store: unknown) => unknown) => selector({ trip: state.trip, loadTrip: state.loadTrip }) })) vi.mock('../../store/authStore', () => ({ useAuthStore: (selector: (store: unknown) => unknown) => selector({ user: state.user }) })) vi.mock('../../store/settingsStore', () => ({ useSettingsStore: (selector: (store: unknown) => unknown) => selector({ settings: { distance_unit: 'metric' } }) })) -vi.mock('../../api/client', () => ({ toursApi: { importGpx: state.importGpx, create: state.create, update: state.update, detail: state.detail } })) +vi.mock('../../repo/tourRepo', () => ({ tourRepo: { importGpx: state.importGpx, create: state.create, update: state.update, detail: state.detail } })) vi.mock('./planner/tourRouting', () => ({ routeWalkingTour: state.route, enrichTourElevations: state.elevation })) vi.mock('../../i18n', () => ({ useTranslation: () => ({ t: (key: string) => key, locale: 'en' }), translateApiError: () => 'error' })) vi.mock('../shared/Toast', () => ({ useToast: () => ({ success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() }) })) diff --git a/client/src/components/shared/CustomSelect.consumers.test.tsx b/client/src/components/shared/CustomSelect.consumers.test.tsx index 2a3e0fee6..dc0135923 100644 --- a/client/src/components/shared/CustomSelect.consumers.test.tsx +++ b/client/src/components/shared/CustomSelect.consumers.test.tsx @@ -9,7 +9,7 @@ import ToursSidebar from '../Tours/ToursSidebar' import type { Day } from '../../types' import CustomSelect from './CustomSelect' -vi.mock('../../api/client', () => ({ toursApi: {} })) +vi.mock('../../repo/tourRepo', () => ({ tourRepo: {} })) vi.mock('../../store/settingsStore', () => ({ useSettingsStore: (selector: (state: { settings: { language: string; distance_unit: string } }) => unknown) => selector({ settings: { language: 'en', distance_unit: 'metric' } }), diff --git a/client/src/db/offlineDb.ts b/client/src/db/offlineDb.ts index a12f72291..2f7ff86cb 100644 --- a/client/src/db/offlineDb.ts +++ b/client/src/db/offlineDb.ts @@ -1,4 +1,4 @@ -import type { RoadtripPreferences } from '@trek/shared'; +import type { RoadtripPreferences, TourListItem, TourWaypoint } from '@trek/shared'; import Dexie, { type Table } from 'dexie'; import type { Trip, Day, Place, PackingItem, TodoItem, BudgetItem, Reservation, TripFile, Accommodation, TripMember, Tag, Category } from '../types'; @@ -7,6 +7,16 @@ export interface CachedTripMember extends TripMember { tripId: number; } +/** + * A tour as the Tours list reads it, with the trip it belongs to (the list item + * carries none) and, once the route editor opened it online, its control + * points, so the tour still opens offline. + */ +export interface CachedTour extends TourListItem { + trip_id: number; + waypoints?: TourWaypoint[]; +} + // ── Queue + sync types ──────────────────────────────────────────────────────── // 'conflict' is terminal-until-resolved: the server rejected the replay because @@ -185,6 +195,7 @@ class TrekOfflineDb extends Dexie { blobCache!: Table; importFiles!: Table; areaPlaces!: Table; + tours!: Table; constructor(name: string = ANON_DB_NAME) { super(name); @@ -252,6 +263,10 @@ class TrekOfflineDb extends Dexie { delete row.areaPlacesKey; }); }); + + // v9: the Tours facet, keyed like the place it belongs to, so a Tour still + // reads as one offline and can be edited there. + this.version(9).stores({ tours: 'place_id, trip_id' }); } } @@ -367,6 +382,33 @@ export async function replaceTripRows(table: TripScope }); } +/** + * {@link replaceTripRows} for the Tours facet, which is keyed on `place_id`. + * A tour's cached control points survive the refresh only while its route + * reads the same: the list carries no waypoints, and a route edited elsewhere + * must not open offline with the old ones. + */ +export async function replaceTripTours(tripId: number, tours: TourListItem[]): Promise { + await offlineDb.transaction('rw', offlineDb.tours, async () => { + const cached = new Map((await offlineDb.tours.where('trip_id').equals(tripId).toArray()).map(t => [t.place_id, t])); + const keep = new Set(tours.map(t => t.place_id)); + await offlineDb.tours + .where('trip_id') + .equals(tripId) + .filter(t => t.place_id > 0 && !keep.has(t.place_id)) + .delete(); + await offlineDb.tours.bulkPut(tours.map(tour => { + const previous = cached.get(tour.place_id); + const sameRoute = previous !== undefined + && previous.distance === tour.distance + && previous.elevation_gain === tour.elevation_gain + && previous.elevation_loss === tour.elevation_loss + && previous.has_waypoints === tour.has_waypoints; + return { ...tour, trip_id: tripId, ...(sameRoute && previous.waypoints ? { waypoints: previous.waypoints } : {}) }; + })); + }); +} + export async function upsertTripMembers(tripId: number, members: TripMember[]): Promise { const rows: CachedTripMember[] = members.map(m => ({ ...m, tripId })); const keep = new Set(rows.map(r => r.id)); @@ -504,6 +546,7 @@ export async function clearTripData(tripId: number): Promise { offlineDb.blobCache, offlineDb.areaPlaces, offlineDb.roadtripPreferences, + offlineDb.tours, ], async () => { await offlineDb.roadtripPreferences.delete(tripId); @@ -515,6 +558,7 @@ export async function clearTripData(tripId: number): Promise { await offlineDb.reservations.where('trip_id').equals(tripId).delete(); await offlineDb.tripFiles.where('trip_id').equals(tripId).delete(); await offlineDb.accommodations.where('trip_id').equals(tripId).delete(); + await offlineDb.tours.where('trip_id').equals(tripId).delete(); await offlineDb.tripMembers.where('tripId').equals(tripId).delete(); // Keep pending/syncing/conflict mutations — only purge dead 'failed' rows. await offlineDb.mutationQueue.where('tripId').equals(tripId).and(m => m.status === 'failed').delete(); diff --git a/client/src/hooks/useTourPlaceIds.test.ts b/client/src/hooks/useTourPlaceIds.test.ts index a27601825..3e9c9f792 100644 --- a/client/src/hooks/useTourPlaceIds.test.ts +++ b/client/src/hooks/useTourPlaceIds.test.ts @@ -1,11 +1,11 @@ import { act, renderHook, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { TourListItem } from '@trek/shared' -import { toursApi } from '../api/client' +import { tourRepo } from '../repo/tourRepo' import { useTourPlaceIds } from './useTourPlaceIds' -vi.mock('../api/client', () => ({ - toursApi: { list: vi.fn() }, +vi.mock('../repo/tourRepo', () => ({ + tourRepo: { list: vi.fn() }, })) const tour: TourListItem = { @@ -40,7 +40,7 @@ function tourFor(placeId: number, name: string): TourListItem { beforeEach(() => { vi.clearAllMocks() - vi.mocked(toursApi.list).mockResolvedValue({ tours: [tour] }) + vi.mocked(tourRepo.list).mockResolvedValue({ tours: [tour] }) }) describe('useTourPlaceIds', () => { @@ -50,7 +50,22 @@ describe('useTourPlaceIds', () => { await waitFor(() => expect(result.current.tourDataReady).toBe(true)) expect(result.current.tours).toEqual([tour]) expect(result.current.tourPlaceIds).toEqual(new Set([42])) - expect(toursApi.list).toHaveBeenCalledWith(7) + expect(tourRepo.list).toHaveBeenCalledWith(7) + }) + + it('hands out the same Set and list until something changes, enabled or not', async () => { + const { result, rerender } = renderHook(({ enabled }) => useTourPlaceIds(7, enabled), { initialProps: { enabled: true } }) + await waitFor(() => expect(result.current.tours).toEqual([tour])) + const { tourPlaceIds, tours } = result.current + + rerender({ enabled: true }) + expect(result.current.tourPlaceIds).toBe(tourPlaceIds) + expect(result.current.tours).toBe(tours) + + rerender({ enabled: false }) + const disabled = result.current.tourPlaceIds + rerender({ enabled: false }) + expect(result.current.tourPlaceIds).toBe(disabled) }) it('hides facet identity while disabled without fetching or deleting its cached data', async () => { @@ -61,7 +76,7 @@ describe('useTourPlaceIds', () => { expect(result.current.tourDataReady).toBe(true) expect(result.current.tours).toEqual([]) expect(result.current.tourPlaceIds.size).toBe(0) - expect(toursApi.list).toHaveBeenCalledTimes(1) + expect(tourRepo.list).toHaveBeenCalledTimes(1) }) it('inserts a fresh save immediately and replaces it after an edit', async () => { @@ -81,7 +96,7 @@ describe('useTourPlaceIds', () => { const { result } = renderHook(() => useTourPlaceIds(7, true)) await waitFor(() => expect(result.current.tours).toEqual([tour])) const refresh = deferred<{ tours: TourListItem[] }>() - vi.mocked(toursApi.list).mockReturnValueOnce(refresh.promise) + vi.mocked(tourRepo.list).mockReturnValueOnce(refresh.promise) act(() => result.current.invalidateTourPlaceIds({ removedPlaceIds: [tour.place_id] })) @@ -94,7 +109,7 @@ describe('useTourPlaceIds', () => { it('shows only the new trip Tours after a successful trip switch', async () => { const tripTwoTour = tourFor(84, 'Trip two tour') - vi.mocked(toursApi.list).mockImplementation(async tripId => ({ + vi.mocked(tourRepo.list).mockImplementation(async tripId => ({ tours: [tripId === 7 ? tour : tripTwoTour], })) const { result, rerender } = renderHook(({ tripId }) => useTourPlaceIds(tripId, true), { @@ -112,7 +127,7 @@ describe('useTourPlaceIds', () => { }) it('keeps previous-trip data hidden when the new trip refresh fails', async () => { - vi.mocked(toursApi.list).mockImplementation(async tripId => { + vi.mocked(tourRepo.list).mockImplementation(async tripId => { if (tripId === 8) throw new Error('Trip two unavailable') return { tours: [tour] } }) @@ -135,7 +150,7 @@ describe('useTourPlaceIds', () => { const tripOne = deferred<{ tours: TourListItem[] }>() const tripTwoTour = tourFor(84, 'Trip two tour') const tripTwo = deferred<{ tours: TourListItem[] }>() - vi.mocked(toursApi.list).mockImplementation(tripId => tripId === 7 ? tripOne.promise : tripTwo.promise) + vi.mocked(tourRepo.list).mockImplementation(tripId => tripId === 7 ? tripOne.promise : tripTwo.promise) const { result, rerender } = renderHook(({ tripId }) => useTourPlaceIds(tripId, true), { initialProps: { tripId: 7 }, }) @@ -151,7 +166,7 @@ describe('useTourPlaceIds', () => { it('lets only the latest request commit after rapid trip changes', async () => { const requests = new Map>>() - vi.mocked(toursApi.list).mockImplementation(tripId => { + vi.mocked(tourRepo.list).mockImplementation(tripId => { const request = deferred<{ tours: TourListItem[] }>() requests.set(tripId, request) return request.promise @@ -177,7 +192,7 @@ describe('useTourPlaceIds', () => { await waitFor(() => expect(result.current.tours).toEqual([tour])) const beforeMutation = deferred<{ tours: TourListItem[] }>() const afterMutation = deferred<{ tours: TourListItem[] }>() - vi.mocked(toursApi.list).mockReset() + vi.mocked(tourRepo.list).mockReset() .mockReturnValueOnce(beforeMutation.promise) .mockReturnValueOnce(afterMutation.promise) @@ -200,7 +215,7 @@ describe('useTourPlaceIds', () => { it('retains same-trip last-good data and exposes a failed refresh', async () => { const { result } = renderHook(() => useTourPlaceIds(7, true)) await waitFor(() => expect(result.current.tourDataReady).toBe(true)) - vi.mocked(toursApi.list).mockRejectedValueOnce(new Error('Refresh unavailable')) + vi.mocked(tourRepo.list).mockRejectedValueOnce(new Error('Refresh unavailable')) await act(async () => { await result.current.reloadTourPlaceIds() }) @@ -213,7 +228,7 @@ describe('useTourPlaceIds', () => { it('does not let a pending request update state after unmount', async () => { const request = deferred<{ tours: TourListItem[] }>() - vi.mocked(toursApi.list).mockReturnValue(request.promise) + vi.mocked(tourRepo.list).mockReturnValue(request.promise) const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) const { unmount } = renderHook(() => useTourPlaceIds(7, true)) unmount() @@ -224,13 +239,13 @@ describe('useTourPlaceIds', () => { }) it('distinguishes an empty successful list from a failed refresh', async () => { - vi.mocked(toursApi.list).mockResolvedValueOnce({ tours: [] }) + vi.mocked(tourRepo.list).mockResolvedValueOnce({ tours: [] }) const { result } = renderHook(() => useTourPlaceIds(7, true)) await waitFor(() => expect(result.current.tourDataReady).toBe(true)) expect(result.current.tours).toEqual([]) expect(result.current.tourLoadError).toBe(false) - vi.mocked(toursApi.list).mockRejectedValueOnce(new Error('Refresh unavailable')) + vi.mocked(tourRepo.list).mockRejectedValueOnce(new Error('Refresh unavailable')) await act(async () => { await result.current.reloadTourPlaceIds() }) expect(result.current.tours).toEqual([]) expect(result.current.tourDataReady).toBe(true) @@ -241,7 +256,7 @@ describe('useTourPlaceIds', () => { const { result } = renderHook(() => useTourPlaceIds(7, true)) await waitFor(() => expect(result.current.tourDataReady).toBe(true)) const refresh = deferred<{ tours: TourListItem[] }>() - vi.mocked(toursApi.list).mockReturnValueOnce(refresh.promise) + vi.mocked(tourRepo.list).mockReturnValueOnce(refresh.promise) act(() => { void result.current.reloadTourPlaceIds() }) await waitFor(() => expect(result.current.toursLoading).toBe(true)) diff --git a/client/src/hooks/useTourPlaceIds.ts b/client/src/hooks/useTourPlaceIds.ts index 608cccdb4..b3e6d8d38 100644 --- a/client/src/hooks/useTourPlaceIds.ts +++ b/client/src/hooks/useTourPlaceIds.ts @@ -1,6 +1,8 @@ import { useEffect, useState, useCallback, useMemo, useRef } from 'react' import type { TourListItem } from '@trek/shared' -import { toursApi } from '../api/client' +import { tourRepo } from '../repo/tourRepo' + +const NO_TOURS: TourListItem[] = [] export interface ToursInvalidation { placeIds?: number[] @@ -37,7 +39,7 @@ export function useTourPlaceIds(tripId: number, enabled: boolean) { const requestId = ++requestIdRef.current setRequestState({ tripId, loading: true, error: false }) try { - const response = await toursApi.list(tripId) + const response = await tourRepo.list(tripId) if (!mountedRef.current || requestId !== requestIdRef.current) return setTourData({ tripId, tours: response.tours }) setPendingPlaceIds(current => current?.tripId === tripId ? null : current) @@ -92,14 +94,18 @@ export function useTourPlaceIds(tripId: number, enabled: boolean) { }, [enabled, tripId]) const tourDataReady = !enabled || tourData?.tripId === tripId - const visibleTours = enabled && tourData?.tripId === tripId ? tourData.tours : [] - const pendingIds = enabled && pendingPlaceIds?.tripId === tripId ? pendingPlaceIds.ids : [] + // Memoised on the stored state, not on the derived values: an empty list + // made fresh in the render is a new dependency every time. + const visibleTours = useMemo( + () => enabled && tourData?.tripId === tripId ? tourData.tours : NO_TOURS, + [enabled, tourData, tripId], + ) const currentRequest = requestState?.tripId === tripId ? requestState : null // One Set per change, not per render: consumers memoise on it. - const tourPlaceIds = useMemo( - () => new Set([...visibleTours.map(tour => tour.place_id), ...pendingIds]), - [visibleTours, pendingIds], - ) + const tourPlaceIds = useMemo(() => { + const pending = enabled && pendingPlaceIds?.tripId === tripId ? pendingPlaceIds.ids : [] + return new Set([...visibleTours.map(tour => tour.place_id), ...pending]) + }, [visibleTours, enabled, pendingPlaceIds, tripId]) return { tours: visibleTours, diff --git a/client/src/hooks/useTripWebSocket.test.ts b/client/src/hooks/useTripWebSocket.test.ts index dc1a3270d..0b1431d14 100644 --- a/client/src/hooks/useTripWebSocket.test.ts +++ b/client/src/hooks/useTripWebSocket.test.ts @@ -4,7 +4,7 @@ import type { TourListItem } from '@trek/shared' const mocks = vi.hoisted(() => { const listeners = new Set<(event: Record) => void>() - const toursApi = { list: vi.fn() } + const tourRepo = { list: vi.fn() } const handleRemoteEvent = vi.fn() const loadFiles = vi.fn() const useTripStore = Object.assign(vi.fn(() => ({ loadFiles })), { @@ -12,7 +12,7 @@ const mocks = vi.hoisted(() => { }) return { listeners, - toursApi, + tourRepo, handleRemoteEvent, loadFiles, useTripStore, @@ -23,7 +23,7 @@ const mocks = vi.hoisted(() => { } }) -vi.mock('../api/client', () => ({ toursApi: mocks.toursApi })) +vi.mock('../repo/tourRepo', () => ({ tourRepo: mocks.tourRepo })) vi.mock('../api/websocket', () => ({ joinTrip: mocks.joinTrip, leaveTrip: mocks.leaveTrip, @@ -81,14 +81,14 @@ beforeEach(() => { mocks.leaveTrip.mockClear() mocks.handleRemoteEvent.mockClear() mocks.loadFiles.mockClear() - mocks.toursApi.list.mockReset().mockResolvedValue({ tours: [baseTour] }) + mocks.tourRepo.list.mockReset().mockResolvedValue({ tours: [baseTour] }) }) describe('Tours websocket invalidation', () => { it('refreshes the active trip read model and Places exclusion after tours:changed', async () => { const nextTour = { ...baseTour, place_id: 84, name: 'Trip 7 new tour' } const refresh = deferred<{ tours: TourListItem[] }>() - mocks.toursApi.list.mockResolvedValueOnce({ tours: [baseTour] }).mockReturnValueOnce(refresh.promise) + mocks.tourRepo.list.mockResolvedValueOnce({ tours: [baseTour] }).mockReturnValueOnce(refresh.promise) const { result } = renderTrip(7) await waitFor(() => expect(result.current.tourDataReady).toBe(true)) expect(result.current.tourPlaceIds).toEqual(new Set([42])) @@ -99,7 +99,7 @@ describe('Tours websocket invalidation', () => { await act(async () => refresh.resolve({ tours: [nextTour] })) await waitFor(() => expect(result.current.tourPlaceIds).toEqual(new Set([84]))) expect(result.current.tours).toEqual([nextTour]) - expect(mocks.toursApi.list).toHaveBeenCalledTimes(2) + expect(mocks.tourRepo.list).toHaveBeenCalledTimes(2) }) it('ignores events for another trip and events without an explicit trip id', async () => { @@ -111,7 +111,7 @@ describe('Tours websocket invalidation', () => { emit({ type: 'tours:changed', placeIds: [84] }) }) - expect(mocks.toursApi.list).toHaveBeenCalledOnce() + expect(mocks.tourRepo.list).toHaveBeenCalledOnce() expect(result.current.tourPlaceIds).toEqual(new Set([42])) }) @@ -122,7 +122,7 @@ describe('Tours websocket invalidation', () => { act(() => emit({ type, tripId: 7, placeId: 42 })) - await waitFor(() => expect(mocks.toursApi.list).toHaveBeenCalledTimes(2)) + await waitFor(() => expect(mocks.tourRepo.list).toHaveBeenCalledTimes(2)) }, ) @@ -133,7 +133,7 @@ describe('Tours websocket invalidation', () => { act(() => emit({ type, tripId: 7 })) - await waitFor(() => expect(mocks.toursApi.list).toHaveBeenCalledTimes(2)) + await waitFor(() => expect(mocks.tourRepo.list).toHaveBeenCalledTimes(2)) }, ) @@ -141,7 +141,7 @@ describe('Tours websocket invalidation', () => { const first = deferred<{ tours: TourListItem[] }>() const second = deferred<{ tours: TourListItem[] }>() const newest = { ...baseTour, place_id: 99, name: 'Newest' } - mocks.toursApi.list.mockResolvedValueOnce({ tours: [baseTour] }) + mocks.tourRepo.list.mockResolvedValueOnce({ tours: [baseTour] }) .mockReturnValueOnce(first.promise) .mockReturnValueOnce(second.promise) const { result } = renderTrip(7) @@ -158,13 +158,13 @@ describe('Tours websocket invalidation', () => { expect(result.current.tours).toEqual([newest]) expect(result.current.tourPlaceIds).toEqual(new Set([99])) - expect(mocks.toursApi.list).toHaveBeenCalledTimes(3) + expect(mocks.tourRepo.list).toHaveBeenCalledTimes(3) }) it('removes a pending Tour exclusion when its Place is deleted before refresh completes', async () => { const staleRefresh = deferred<{ tours: TourListItem[] }>() const deleteRefresh = deferred<{ tours: TourListItem[] }>() - mocks.toursApi.list.mockResolvedValueOnce({ tours: [] }) + mocks.tourRepo.list.mockResolvedValueOnce({ tours: [] }) .mockReturnValueOnce(staleRefresh.promise) .mockReturnValueOnce(deleteRefresh.promise) const { result } = renderTrip(7) @@ -178,13 +178,13 @@ describe('Tours websocket invalidation', () => { await act(async () => deleteRefresh.resolve({ tours: [] })) await act(async () => staleRefresh.resolve({ tours: [{ ...baseTour, place_id: 84 }] })) expect(result.current.tourPlaceIds).toEqual(new Set()) - expect(mocks.toursApi.list).toHaveBeenCalledTimes(3) + expect(mocks.tourRepo.list).toHaveBeenCalledTimes(3) }) it('keeps same-trip last-good Tours and exposes error on refresh failure', async () => { const { result } = renderTrip(7) await waitFor(() => expect(result.current.tourDataReady).toBe(true)) - mocks.toursApi.list.mockRejectedValueOnce(new Error('refresh unavailable')) + mocks.tourRepo.list.mockRejectedValueOnce(new Error('refresh unavailable')) act(() => emit({ type: 'tours:changed', tripId: 7, placeIds: [84] })) @@ -201,7 +201,7 @@ describe('Tours websocket invalidation', () => { act(() => emit({ type: 'tours:changed', tripId: 7 })) expect(mocks.handleRemoteEvent).toHaveBeenCalledWith(expect.objectContaining({ type: 'place:created', tripId: 7 })) - expect(mocks.toursApi.list).not.toHaveBeenCalled() + expect(mocks.tourRepo.list).not.toHaveBeenCalled() expect(result.current.tours).toEqual([]) expect(result.current.tourPlaceIds.size).toBe(0) }) diff --git a/client/src/repo/placeRepo.ts b/client/src/repo/placeRepo.ts index 800666785..3d9f75e83 100644 --- a/client/src/repo/placeRepo.ts +++ b/client/src/repo/placeRepo.ts @@ -83,7 +83,9 @@ export const placeRepo = { async delete(tripId: number | string, id: number | string): Promise<{ success?: boolean; tourPlaceIds?: number[] }> { if (isEffectivelyOffline()) { + // A tour is a place plus its facet; the server's cascade drops both. await offlineDb.places.delete(Number(id)) + await offlineDb.tours.delete(Number(id)) const mutId = generateUUID() const isTemp = Number(id) < 0 await mutationQueue.enqueue({ @@ -100,12 +102,14 @@ export const placeRepo = { } const result = await placesApi.delete(tripId, id) offlineDb.places.delete(Number(id)) + offlineDb.tours.delete(Number(id)) return result }, async deleteMany(tripId: number | string, ids: number[]): Promise<{ deleted?: number[]; count?: number; tourPlaceIds?: number[] }> { if (isEffectivelyOffline()) { await offlineDb.places.bulkDelete(ids) + await offlineDb.tours.bulkDelete(ids) for (const id of ids) { const mutId = generateUUID() const isTemp = id < 0 @@ -124,6 +128,7 @@ export const placeRepo = { } const result = await placesApi.bulkDelete(tripId, ids) await offlineDb.places.bulkDelete(ids) + await offlineDb.tours.bulkDelete(ids) return result }, diff --git a/client/src/repo/tourRepo.test.ts b/client/src/repo/tourRepo.test.ts new file mode 100644 index 000000000..dff7a1d0b --- /dev/null +++ b/client/src/repo/tourRepo.test.ts @@ -0,0 +1,211 @@ +// FE-REPO-TOUR-001 to FE-REPO-TOUR-012 +// Tours through the offline core: read-through to Dexie, offline create and +// edit written there and queued, and the online paths keeping the copy fresh. +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import 'fake-indexeddb/auto' +import { http, HttpResponse } from 'msw' +import type { TourCreateRequest, TourListItem } from '@trek/shared' +import { server } from '../../tests/helpers/msw/server' +import { tourRepo } from './tourRepo' +import { offlineDb, clearAll, replaceTripTours } from '../db/offlineDb' +import { buildPlace } from '../../tests/helpers/factories' + +function setOnline(v: boolean): void { + Object.defineProperty(navigator, 'onLine', { value: v, writable: true, configurable: true }) +} + +function tour(overrides: Partial = {}): TourListItem { + return { + place_id: 40, name: 'Ridge walk', tour_type: 'hike', distance: 8, elevation_gain: 600, elevation_loss: 600, + duration: 180, difficulty: null, wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, + planned: false, caution: false, has_waypoints: true, ...overrides, + } +} + +const body: TourCreateRequest = { + name: 'Lake loop', + tour_type: 'hike', + route_geometry: [[47, 11, 1000], [47.01, 11, 1100], [47.02, 11, 1050]], + waypoints: [ + { lat: 47, lng: 11, role: 'start', sequence: 0 }, + { lat: 47.02, lng: 11, role: 'end', sequence: 1 }, + ], + max_hiking_difficulty: 3, + duration_seconds: 5430, +} + +beforeEach(async () => { + await clearAll() + setOnline(true) +}) + +afterEach(() => { + vi.restoreAllMocks() + setOnline(true) +}) + +describe('tourRepo.list', () => { + it('FE-REPO-TOUR-001: online, the answer replaces the trip\'s cached tours', async () => { + await offlineDb.tours.put({ ...tour({ place_id: 41 }), trip_id: 3 }) + server.use(http.get('/api/trips/3/tours', () => HttpResponse.json({ tours: [tour()] }))) + + const result = await tourRepo.list(3) + + expect(result.tours.map(t => t.place_id)).toEqual([40]) + await vi.waitFor(async () => expect((await offlineDb.tours.toArray()).map(t => t.place_id)).toEqual([40])) + expect((await offlineDb.tours.get(40))!.trip_id).toBe(3) + }) + + it('FE-REPO-TOUR-002: offline, the cached tours come back newest first, unsynced ones on top, without cache-only fields', async () => { + setOnline(false) + await offlineDb.tours.bulkPut([ + { ...tour({ place_id: 10 }), trip_id: 3 }, + { ...tour({ place_id: 12 }), trip_id: 3, waypoints: body.waypoints }, + { ...tour({ place_id: -7 }), trip_id: 3 }, + { ...tour({ place_id: 11 }), trip_id: 4 }, + ]) + + const { tours } = await tourRepo.list('3') + + expect(tours.map(t => t.place_id)).toEqual([-7, 12, 10]) + expect(tours[1]).not.toHaveProperty('trip_id') + expect(tours[1]).not.toHaveProperty('waypoints') + }) +}) + +describe('tourRepo.detail', () => { + it('FE-REPO-TOUR-003: online, the control points are cached with the tour', async () => { + server.use(http.get('/api/trips/3/tours/40', () => HttpResponse.json({ tour: tour(), waypoints: body.waypoints }))) + + await tourRepo.detail(3, 40) + + expect((await offlineDb.tours.get(40))).toMatchObject({ trip_id: 3, waypoints: body.waypoints }) + }) + + it('FE-REPO-TOUR-004: offline, a tour opens from its cached control points', async () => { + setOnline(false) + await offlineDb.tours.put({ ...tour(), trip_id: 3, waypoints: body.waypoints }) + + const result = await tourRepo.detail(3, 40) + + expect(result.waypoints).toEqual(body.waypoints) + expect(result.tour).toEqual(tour()) + }) + + it('FE-REPO-TOUR-005: offline without control points, the route\'s ends stand in, like the server does for GPX tours', async () => { + setOnline(false) + await offlineDb.tours.put({ ...tour({ has_waypoints: false }), trip_id: 3 }) + await offlineDb.places.put(buildPlace({ id: 40, trip_id: 3, route_geometry: JSON.stringify([[1, 2, 3], [4, 5, 6], [7, 8, 9]]) })) + + const { waypoints } = await tourRepo.detail(3, 40) + + expect(waypoints).toEqual([ + { lat: 1, lng: 2, role: 'start', sequence: 0 }, + { lat: 7, lng: 8, role: 'end', sequence: 1 }, + ]) + }) + + it('FE-REPO-TOUR-006: offline, a tour of another trip or one never cached is not invented', async () => { + setOnline(false) + await offlineDb.tours.put({ ...tour(), trip_id: 4 }) + + await expect(tourRepo.detail(3, 40)).rejects.toThrow('Tour not available offline') + await expect(tourRepo.detail(3, 99)).rejects.toThrow('Tour not available offline') + }) + + it('FE-REPO-TOUR-007: an aborted request is passed on, not answered from the cache', async () => { + await offlineDb.tours.put({ ...tour(), trip_id: 3, waypoints: body.waypoints }) + server.use(http.get('/api/trips/3/tours/40', () => HttpResponse.json({ tour: tour(), waypoints: body.waypoints }))) + const controller = new AbortController() + controller.abort() + + await expect(tourRepo.detail(3, 40, controller.signal)).rejects.toThrow() + }) +}) + +describe('tourRepo.create', () => { + it('FE-REPO-TOUR-008: offline, place and facet share one temp id, the metrics match the server\'s, and a POST is queued', async () => { + setOnline(false) + + const result = await tourRepo.create(3, body) + const id = result.tour.place_id + + expect(id).toBeLessThan(0) + expect(result.waypoints).toEqual(body.waypoints) + expect(result.tour).toMatchObject({ name: 'Lake loop', elevation_gain: 100, elevation_loss: 50, duration: 91, max_hiking_difficulty: 3, has_waypoints: true, planned: false }) + expect(result.tour.distance).toBeGreaterThan(2) + expect(await offlineDb.places.get(id)).toMatchObject({ trip_id: 3, name: 'Lake loop', lat: 47, lng: 11, route_geometry: JSON.stringify(body.route_geometry) }) + expect(await offlineDb.tours.get(id)).toMatchObject({ trip_id: 3, waypoints: body.waypoints }) + + const [mutation] = await offlineDb.mutationQueue.toArray() + expect(mutation).toMatchObject({ method: 'POST', url: '/trips/3/tours', resource: 'tours', tempId: id, body }) + }) + + it('FE-REPO-TOUR-009: online, the saved tour is cached with its control points', async () => { + server.use(http.post('/api/trips/3/tours', () => HttpResponse.json({ tour: tour({ place_id: 77 }), waypoints: body.waypoints }))) + + const result = await tourRepo.create(3, body) + + expect(result.tour.place_id).toBe(77) + expect(await offlineDb.tours.get(77)).toMatchObject({ trip_id: 3, waypoints: body.waypoints }) + expect(await offlineDb.mutationQueue.count()).toBe(0) + }) +}) + +describe('tourRepo.update', () => { + it('FE-REPO-TOUR-010: offline, the cached tour and its place take the new route, and a PUT is queued', async () => { + setOnline(false) + await offlineDb.tours.put({ ...tour({ planned: true }), trip_id: 3 }) + await offlineDb.places.put(buildPlace({ id: 40, trip_id: 3, name: 'Ridge walk', notes: 'keep me' })) + + const result = await tourRepo.update(3, 40, body) + + expect(result.tour).toMatchObject({ place_id: 40, name: 'Lake loop', planned: true, elevation_gain: 100 }) + expect(await offlineDb.places.get(40)).toMatchObject({ name: 'Lake loop', notes: 'keep me', lat: 47 }) + const [mutation] = await offlineDb.mutationQueue.toArray() + expect(mutation).toMatchObject({ method: 'PUT', url: '/trips/3/tours/40', resource: 'tours', entityId: 40 }) + expect(mutation.tempEntityId).toBeUndefined() + }) + + it('FE-REPO-TOUR-011: offline, an edit of a tour that has not synced waits for its id', async () => { + setOnline(false) + const created = await tourRepo.create(3, body) + + await tourRepo.update(3, created.tour.place_id, { ...body, name: 'Renamed' }) + + const queued = (await offlineDb.mutationQueue.toArray()).sort((a, b) => a.createdAt - b.createdAt) + expect(queued[1]).toMatchObject({ url: '/trips/3/tours/{id}', tempEntityId: created.tour.place_id }) + }) + + it('FE-REPO-TOUR-012: online edits and GPX imports refresh the cache', async () => { + server.use( + http.put('/api/trips/3/tours/40', () => HttpResponse.json({ tour: tour({ name: 'Edited' }), waypoints: body.waypoints })), + http.post('/api/trips/3/tours/import/gpx', () => HttpResponse.json({ tours: [tour({ place_id: 50 })], caution: false, skipped: 0 })), + ) + + await tourRepo.update(3, 40, body) + await tourRepo.importGpx(3, new File([''], 'a.gpx')) + + expect(await offlineDb.tours.get(40)).toMatchObject({ name: 'Edited', waypoints: body.waypoints }) + expect(await offlineDb.tours.get(50)).toMatchObject({ trip_id: 3 }) + }) +}) + +describe('replaceTripTours', () => { + it('FE-REPO-TOUR-013: keeps control points only while the route reads the same, drops what the server no longer lists, keeps unsynced tours', async () => { + await offlineDb.tours.bulkPut([ + { ...tour({ place_id: 1 }), trip_id: 3, waypoints: body.waypoints }, + { ...tour({ place_id: 2 }), trip_id: 3, waypoints: body.waypoints }, + { ...tour({ place_id: 3 }), trip_id: 3 }, + { ...tour({ place_id: -4 }), trip_id: 3 }, + { ...tour({ place_id: 5 }), trip_id: 9 }, + ]) + + await replaceTripTours(3, [tour({ place_id: 1 }), tour({ place_id: 2, distance: 99 })]) + + const rows = await offlineDb.tours.orderBy('place_id').toArray() + expect(rows.map(r => r.place_id)).toEqual([-4, 1, 2, 5]) + expect(rows.find(r => r.place_id === 1)!.waypoints).toEqual(body.waypoints) + expect(rows.find(r => r.place_id === 2)!.waypoints).toBeUndefined() + }) +}) diff --git a/client/src/repo/tourRepo.ts b/client/src/repo/tourRepo.ts new file mode 100644 index 000000000..c2b5a73b5 --- /dev/null +++ b/client/src/repo/tourRepo.ts @@ -0,0 +1,168 @@ +import { computeTourMetrics, type GeometryPoint, type TourCreateRequest, type TourCreateResponse, type TourDetailResponse, type TourImportGpxResponse, type TourListItem, type TourWaypoint } from '@trek/shared' +import { toursApi } from '../api/client' +import { offlineDb, replaceTripTours, type CachedTour } from '../db/offlineDb' +import { mutationQueue, generateUUID, nextTempId } from '../sync/mutationQueue' +import { isEffectivelyOffline } from '../sync/networkMode' +import { onlineThenCache } from './withOfflineFallback' +import type { Place } from '../types' + +/** The list item a cached row stands for: the cache-only fields stripped. */ +function toListItem({ trip_id: _tripId, waypoints: _waypoints, ...tour }: CachedTour): TourListItem { + return tour +} + +/** Newest first, as the server lists them: offline creates (negative ids) on top. */ +function newestFirst(a: CachedTour, b: CachedTour): number { + if ((a.place_id < 0) !== (b.place_id < 0)) return a.place_id < 0 ? -1 : 1 + return a.place_id < 0 ? a.place_id - b.place_id : b.place_id - a.place_id +} + +/** A route's first and last point as start and end controls, the server's fallback for GPX tours. */ +function endpointsOf(routeGeometry: unknown): TourWaypoint[] { + let points: unknown + try { points = typeof routeGeometry === 'string' ? JSON.parse(routeGeometry) : routeGeometry } catch { return [] } + if (!Array.isArray(points) || points.length < 2) return [] + const start = points[0] as number[] + const end = points[points.length - 1] as number[] + return [ + { lat: start[0], lng: start[1], role: 'start', sequence: 0 }, + { lat: end[0], lng: end[1], role: 'end', sequence: 1 }, + ] +} + +/** + * What the server will derive from a create or edit, worked out here for the + * copy shown until the queued write replays: the same metrics, the duration + * in minutes, full confidence because the route editor drew it. + */ +function routeFields(body: TourCreateRequest) { + const metrics = computeTourMetrics(body.route_geometry as GeometryPoint[]) + return { + name: body.name, + tour_type: body.tour_type, + distance: metrics.distanceKm, + elevation_gain: metrics.elevationGainM, + elevation_loss: metrics.elevationLossM, + duration: body.duration_seconds == null ? null : Math.round(body.duration_seconds / 60), + match_confidence: 1, + caution: false, + max_hiking_difficulty: body.max_hiking_difficulty, + has_waypoints: true, + } +} + +/** The owning place's route columns, which a create or edit replaces with the tour's. */ +function routePlaceFields(body: TourCreateRequest) { + const start = body.route_geometry[0] + return { name: body.name, lat: start[0], lng: start[1], route_geometry: JSON.stringify(body.route_geometry) } +} + +/** + * Tours through the offline core: reads fall back to the Dexie copy, and a + * create or route edit made offline is written there and queued, like a place. + * A tour is a place plus its facet, so an offline create writes both under one + * temporary id, which the queue swaps for the real one on replay. + * + * The GPX import uploads a file and stays online-only, like every upload. + */ +export const tourRepo = { + async list(tripId: number | string): Promise<{ tours: TourListItem[] }> { + return onlineThenCache( + async () => { + const result = await toursApi.list(tripId) + void replaceTripTours(Number(tripId), result.tours) + return result + }, + async () => { + const rows = await offlineDb.tours.where('trip_id').equals(Number(tripId)).toArray() + return { tours: rows.sort(newestFirst).map(toListItem) } + }, + ) + }, + + async detail(tripId: number | string, placeId: number, signal?: AbortSignal): Promise { + return onlineThenCache( + async () => { + const result = await toursApi.detail(tripId, placeId, signal) + await offlineDb.tours.put({ ...result.tour, trip_id: Number(tripId), waypoints: result.waypoints }) + return result + }, + async () => { + // An aborted request is not a lost connection: the caller moved on. + signal?.throwIfAborted() + const cached = await offlineDb.tours.get(placeId) + if (!cached || cached.trip_id !== Number(tripId)) throw new Error('Tour not available offline') + const waypoints = cached.waypoints?.length + ? cached.waypoints + : endpointsOf((await offlineDb.places.get(placeId))?.route_geometry) + return { tour: toListItem(cached), waypoints } + }, + ) + }, + + async create(tripId: number | string, body: TourCreateRequest): Promise { + if (isEffectivelyOffline()) { + const tid = Number(tripId) + const tempId = nextTempId() + const tour: CachedTour = { + place_id: tempId, trip_id: tid, difficulty: null, wanderer_ref: null, planned: false, + ...routeFields(body), waypoints: body.waypoints, + } + const place = { id: tempId, trip_id: tid, ...routePlaceFields(body) } as Place + await offlineDb.transaction('rw', offlineDb.places, offlineDb.tours, async () => { + await offlineDb.places.put(place) + await offlineDb.tours.put(tour) + }) + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: tid, + method: 'POST', + url: `/trips/${tripId}/tours`, + body, + resource: 'tours', + tempId, + }) + return { tour: toListItem(tour), waypoints: body.waypoints } + } + const result = await toursApi.create(tripId, body) + await offlineDb.tours.put({ ...result.tour, trip_id: Number(tripId), waypoints: result.waypoints }) + return result + }, + + async update(tripId: number | string, placeId: number, body: TourCreateRequest): Promise { + if (isEffectivelyOffline()) { + const tid = Number(tripId) + const existing = await offlineDb.tours.get(placeId) + const tour: CachedTour = { + difficulty: null, wanderer_ref: null, planned: false, + ...existing, ...routeFields(body), place_id: placeId, trip_id: tid, waypoints: body.waypoints, + } + const place = await offlineDb.places.get(placeId) + await offlineDb.transaction('rw', offlineDb.places, offlineDb.tours, async () => { + await offlineDb.places.put({ ...(place ?? {} as Place), id: placeId, trip_id: tid, ...routePlaceFields(body) }) + await offlineDb.tours.put(tour) + }) + const isTemp = placeId < 0 + await mutationQueue.enqueue({ + id: generateUUID(), + tripId: tid, + method: 'PUT', + url: isTemp ? `/trips/${tripId}/tours/{id}` : `/trips/${tripId}/tours/${placeId}`, + body, + resource: 'tours', + entityId: placeId, + ...(isTemp ? { tempEntityId: placeId } : {}), + }) + return { tour: toListItem(tour), waypoints: body.waypoints } + } + const result = await toursApi.update(tripId, placeId, body) + await offlineDb.tours.put({ ...result.tour, trip_id: Number(tripId), waypoints: result.waypoints }) + return result + }, + + async importGpx(tripId: number | string, file: File): Promise { + const result = await toursApi.importGpx(tripId, file) + await offlineDb.tours.bulkPut(result.tours.map(tour => ({ ...tour, trip_id: Number(tripId) }))) + return result + }, +} diff --git a/client/src/sync/mutationQueue.ts b/client/src/sync/mutationQueue.ts index 83854c83c..99b4ee613 100644 --- a/client/src/sync/mutationQueue.ts +++ b/client/src/sync/mutationQueue.ts @@ -13,7 +13,7 @@ import { getOfflinePrefs } from './offlinePrefs' import { randomId } from '../utils/randomId' import type { QueuedMutation } from '../db/offlineDb' import type { Table } from 'dexie' -import { roadtripPreferencesResponseSchema, assignmentSchema } from '@trek/shared' +import { roadtripPreferencesResponseSchema, assignmentSchema, tourCreateResponseSchema, type TourCreateResponse } from '@trek/shared' import { cacheAssignment } from '../db/cacheAssignment' // Map Dexie table names used in `resource` field → actual Dexie tables. @@ -97,6 +97,60 @@ async function applyServerEntity(mutation: QueuedMutation, server: unknown): Pro if (table) await table.put(server) } +/** + * Roll back what an offline CREATE wrote under its temporary id, once the + * server refused it for good. A tour is a place plus its facet, so both go. + */ +async function dropTempRows(mutation: QueuedMutation): Promise { + if (mutation.method === 'DELETE' || mutation.tempId === undefined || !mutation.resource) return + if (mutation.resource === 'tours') { + await offlineDb.places.delete(mutation.tempId) + await offlineDb.tours.delete(mutation.tempId) + return + } + const table = getTable(mutation.resource) + if (table) await table.delete(mutation.tempId) +} + +/** + * A CREATE replayed and the server answered with the real id: later queued + * writes still aimed at the temporary one are pointed at it, in memory for + * this flush and durably for the next. + */ +async function remapTempId(mutation: QueuedMutation, realId: number, idMap: Map): Promise { + if (mutation.tempId === undefined || mutation.tempId === realId) return + idMap.set(mutation.tempId, realId) + await offlineDb.mutationQueue + .where('tripId') + .equals(mutation.tripId) + .filter(m => m.tempEntityId === mutation.tempId) + .modify(m => { + m.url = m.url.replace('{id}', String(realId)) + m.entityId = realId + m.tempEntityId = undefined + }) +} + +/** + * A replayed tour write. The answer names the tour by its place, so the + * offline place and facet move from the temporary id to the real one. The + * place keeps its offline copy until the next list replaces it: the server + * created it from the same route. + */ +async function reconcileTour(mutation: QueuedMutation, saved: TourCreateResponse, idMap: Map): Promise { + const realId = saved.tour.place_id + await offlineDb.transaction('rw', offlineDb.places, offlineDb.tours, async () => { + if (mutation.tempId !== undefined && mutation.tempId !== realId) { + const temp = await offlineDb.places.get(mutation.tempId) + await offlineDb.places.delete(mutation.tempId) + await offlineDb.tours.delete(mutation.tempId) + if (temp && !(await offlineDb.places.get(realId))) await offlineDb.places.put({ ...temp, id: realId }) + } + await offlineDb.tours.put({ ...saved.tour, trip_id: mutation.tripId, waypoints: saved.waypoints }) + }) + await remapTempId(mutation, realId, idMap) +} + export const mutationQueue = { /** * Add a mutation to the queue. @@ -222,7 +276,9 @@ export const mutationQueue = { if (mutation.resource === 'assignments') { await cacheAssignment(assignmentSchema.parse(response.data.assignment)) } - if (mutation.method !== 'DELETE' && mutation.resource) { + if (mutation.resource === 'tours') { + await reconcileTour(mutation, tourCreateResponseSchema.parse(response.data), idMap) + } else if (mutation.method !== 'DELETE' && mutation.resource) { const table = getTable(mutation.resource) if (table && response.data && typeof response.data === 'object') { // Server returns { place: {...} } or { item: {...} } — grab first value @@ -234,17 +290,7 @@ export const mutationQueue = { // remap any queued mutations that still target the negative id. if (mutation.tempId !== undefined && mutation.tempId !== realId) { await table.delete(mutation.tempId) - idMap.set(mutation.tempId, realId) - // Durable rewrite so dependents survive a flush boundary / reload. - await offlineDb.mutationQueue - .where('tripId') - .equals(mutation.tripId) - .filter(m => m.tempEntityId === mutation.tempId) - .modify(m => { - m.url = m.url.replace('{id}', String(realId)) - m.entityId = realId - m.tempEntityId = undefined - }) + await remapTempId(mutation, realId, idMap) } await table.put(entity) // Advance the base-version token of any other queued edits to the @@ -328,10 +374,7 @@ export const mutationQueue = { if (isTerminal) { // Permanent client error — roll back the phantom optimistic CREATE so // it can't masquerade as synced, then mark failed and continue. - if (mutation.method !== 'DELETE' && mutation.tempId !== undefined && mutation.resource) { - const table = getTable(mutation.resource) - if (table) await table.delete(mutation.tempId) - } + await dropTempRows(mutation) await offlineDb.mutationQueue.update(mutation.id, { status: 'failed', attempts: mutation.attempts + 1, @@ -343,10 +386,7 @@ export const mutationQueue = { // parked as failed instead of holding every later write forever. const attempts = mutation.attempts + 1 if (attempts >= MAX_SERVER_ERROR_ATTEMPTS) { - if (mutation.method !== 'DELETE' && mutation.tempId !== undefined && mutation.resource) { - const table = getTable(mutation.resource) - if (table) await table.delete(mutation.tempId) - } + await dropTempRows(mutation) await offlineDb.mutationQueue.update(mutation.id, { status: 'failed', attempts, lastError: String(err), retryAfter: undefined }) } else { await offlineDb.mutationQueue.update(mutation.id, { diff --git a/client/src/sync/tripSyncManager.ts b/client/src/sync/tripSyncManager.ts index c4e942693..3e185c99a 100644 --- a/client/src/sync/tripSyncManager.ts +++ b/client/src/sync/tripSyncManager.ts @@ -11,11 +11,12 @@ import { roadtripPreferencesRepo } from '../repo/roadtripPreferencesRepo' * - trip list refresh (DashboardPage) * - WS reconnect (phase 7) */ -import { tripsApi, tagsApi, categoriesApi } from '../api/client' +import { tripsApi, tagsApi, categoriesApi, toursApi } from '../api/client' import { offlineDb, upsertTrip, replaceTripRows, + replaceTripTours, upsertTripMembers, upsertTags, upsertCategories, @@ -29,6 +30,7 @@ import { isAuthed } from './authGate' import { isEffectivelyOffline } from './networkMode' import { getOfflinePrefs, isTripOfflineEnabled, isTripPinned } from './offlinePrefs' import { useSettingsStore } from '../store/settingsStore' +import { useAddonStore } from '../store/addonStore' import type { Trip, Day, Place, PackingItem, TodoItem, BudgetItem, Reservation, TripFile, Accommodation, TripMember } from '../types' // ── Types ───────────────────────────────────────────────────────────────────── @@ -145,6 +147,17 @@ async function syncTrip(tripId: number): Promise { // threw the whole downloaded bundle away for that trip while the run still // reported it stored, so Settings said "N trips ready" over an empty database. await roadtripPreferencesRepo.read(tripId).catch(() => { /* optional addon, optional cache */ }) + + // Tours, the same way: optional, and never fatal. Without them every Tour + // reads as a plain place offline. Asked only while the addon is on, so a + // sync of many trips does not spend a refused request on each. + if (useAddonStore.getState().isEnabled('tours')) { + try { + await replaceTripTours(tripId, (await toursApi.list(tripId)).tours) + } catch (err) { + console.warn(`[sync] tours for trip ${tripId} not cached:`, err) + } + } } /** Cache non-photo file blobs for a trip. Fire-and-forget safe. */ diff --git a/client/tests/unit/sync/mutationQueue.tours.test.ts b/client/tests/unit/sync/mutationQueue.tours.test.ts new file mode 100644 index 000000000..07bcc094c --- /dev/null +++ b/client/tests/unit/sync/mutationQueue.tours.test.ts @@ -0,0 +1,101 @@ +/** + * mutationQueue replay of offline tour writes: a tour is a place plus its + * facet, so a create moves both from the temporary id to the real one, points + * queued edits at it, and a refused create takes both back out. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import 'fake-indexeddb/auto' +import { http, HttpResponse } from 'msw' +import type { TourCreateRequest, TourListItem } from '@trek/shared' +import { server } from '../../helpers/msw/server' +import { setAuthed } from '../../../src/sync/authGate' +import { mutationQueue } from '../../../src/sync/mutationQueue' +import { offlineDb, clearAll } from '../../../src/db/offlineDb' +import { tourRepo } from '../../../src/repo/tourRepo' +import { placeRepo } from '../../../src/repo/placeRepo' + +function setOnline(v: boolean): void { + Object.defineProperty(navigator, 'onLine', { value: v, writable: true, configurable: true }) +} + +const body: TourCreateRequest = { + name: 'Lake loop', + tour_type: 'hike', + route_geometry: [[47, 11, 1000], [47.02, 11, 1050]], + waypoints: [ + { lat: 47, lng: 11, role: 'start', sequence: 0 }, + { lat: 47.02, lng: 11, role: 'end', sequence: 1 }, + ], + max_hiking_difficulty: 2, + duration_seconds: null, +} + +function savedTour(placeId: number, name = 'Lake loop'): TourListItem { + return { + place_id: placeId, name, tour_type: 'hike', distance: 2.2, elevation_gain: 50, elevation_loss: 0, duration: null, + difficulty: null, wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, planned: false, caution: false, + has_waypoints: true, + } +} + +beforeEach(async () => { + await clearAll() + mutationQueue._resetFlushing() + setAuthed(true) + setOnline(false) +}) + +afterEach(() => { + vi.restoreAllMocks() + setAuthed(false) + setOnline(true) +}) + +describe('mutationQueue.flush — tours', () => { + it('FE-SYNC-TOUR-001: a replayed create moves place and facet to the real id and aims the queued edit at it', async () => { + const { tour } = await tourRepo.create(3, body) + const tempId = tour.place_id + await tourRepo.update(3, tempId, { ...body, name: 'Renamed' }) + const puts: string[] = [] + server.use( + http.post('/api/trips/3/tours', () => HttpResponse.json({ tour: savedTour(501), waypoints: body.waypoints })), + http.put('/api/trips/3/tours/:id', ({ params }) => { + puts.push(String(params.id)) + return HttpResponse.json({ tour: savedTour(501, 'Renamed'), waypoints: body.waypoints }) + }), + ) + setOnline(true) + + await mutationQueue.flush() + + expect(puts).toEqual(['501']) + expect(await offlineDb.mutationQueue.count()).toBe(0) + expect(await offlineDb.tours.get(tempId)).toBeUndefined() + expect(await offlineDb.places.get(tempId)).toBeUndefined() + expect(await offlineDb.tours.get(501)).toMatchObject({ trip_id: 3, name: 'Renamed', waypoints: body.waypoints }) + // The server built its place from the same route; it stays on the map until the next list. + expect(await offlineDb.places.get(501)).toMatchObject({ trip_id: 3, name: 'Renamed' }) + }) + + it('FE-SYNC-TOUR-002: a create the server refuses takes its place and facet back out', async () => { + const { tour } = await tourRepo.create(3, body) + server.use(http.post('/api/trips/3/tours', () => HttpResponse.json({ error: 'Tour type is not available' }, { status: 400 }))) + setOnline(true) + + await mutationQueue.flush() + + expect(await offlineDb.tours.get(tour.place_id)).toBeUndefined() + expect(await offlineDb.places.get(tour.place_id)).toBeUndefined() + expect((await offlineDb.mutationQueue.toArray())[0]).toMatchObject({ status: 'failed' }) + }) + + it('FE-SYNC-TOUR-003: deleting a tour offline drops its facet with the place', async () => { + await offlineDb.tours.put({ ...savedTour(40), trip_id: 3 }) + + await placeRepo.delete(3, 40) + await offlineDb.tours.put({ ...savedTour(41), trip_id: 3 }) + await placeRepo.deleteMany(3, [41]) + + expect(await offlineDb.tours.count()).toBe(0) + }) +}) diff --git a/client/tests/unit/sync/tripSyncManager.test.ts b/client/tests/unit/sync/tripSyncManager.test.ts index c233bf855..1c4cd1c7e 100644 --- a/client/tests/unit/sync/tripSyncManager.test.ts +++ b/client/tests/unit/sync/tripSyncManager.test.ts @@ -12,6 +12,7 @@ import { tripSyncManager } from '../../../src/sync/tripSyncManager'; import { setAuthed } from '../../../src/sync/authGate'; import { setTripPinned, _resetOfflinePrefs } from '../../../src/sync/offlinePrefs'; import { offlineDb, clearAll, upsertTrip } from '../../../src/db/offlineDb'; +import { useAddonStore } from '../../../src/store/addonStore'; import { buildTrip, buildDay, @@ -294,6 +295,55 @@ describe('tripSyncManager.syncAll — bundle upsert', () => { }); }); +describe('tripSyncManager.syncAll — tours', () => { + const tour = { + place_id: 900, name: 'Ridge walk', tour_type: 'hike', distance: 8, elevation_gain: 600, elevation_loss: 600, + duration: 180, difficulty: null, wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, + planned: false, caution: false, has_waypoints: true, + }; + + function serve(tripId: number, tours: () => Response) { + let asked = 0; + server.use( + http.get('/api/trips', () => HttpResponse.json({ trips: [buildTrip({ id: tripId, end_date: dateOffset(5) })] })), + http.get(`/api/trips/${tripId}/bundle`, () => HttpResponse.json(makeBundle(tripId))), + http.get(`/api/trips/${tripId}/tours`, () => { asked++; return tours(); }), + ); + return () => asked; + } + + afterEach(() => useAddonStore.setState({ addons: [] })); + + it("caches the trip's tours while the addon is on", async () => { + useAddonStore.setState({ addons: [{ id: 'tours', enabled: true }] as never }); + serve(310, () => HttpResponse.json({ tours: [tour] })); + + await tripSyncManager.syncAll(); + + expect(await offlineDb.tours.get(900)).toMatchObject({ trip_id: 310, name: 'Ridge walk' }); + }); + + it('does not ask while the addon is off', async () => { + const asked = serve(311, () => HttpResponse.json({ tours: [tour] })); + + await tripSyncManager.syncAll(); + + expect(asked()).toBe(0); + expect(await offlineDb.tours.count()).toBe(0); + }); + + it('a failed tours request leaves the rest of the trip stored', async () => { + useAddonStore.setState({ addons: [{ id: 'tours', enabled: true }] as never }); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + serve(312, () => HttpResponse.json({ error: 'boom' }, { status: 500 })); + + await tripSyncManager.syncAll(); + + expect(await offlineDb.places.where('trip_id').equals(312).count()).toBe(1); + expect(await offlineDb.syncMeta.get(312)).toBeDefined(); + }); +}); + // ── file blob caching ────────────────────────────────────────────────────────── describe('tripSyncManager — file blob caching', () => { diff --git a/server/src/nest/common/geo.ts b/server/src/nest/common/geo.ts index 20a4d2247..f563a4417 100644 --- a/server/src/nest/common/geo.ts +++ b/server/src/nest/common/geo.ts @@ -11,19 +11,7 @@ export function haversineKm(lat1: number, lng1: number, lat2: number, lng2: numb } /** - * The same distance in metres. - * - * There were three implementations of this in the tree: this one, a copy in - * maps.helpers.ts and a private one in places.helpers.ts that differed by a - * single clamp. The clamped form is the one kept — `asin` of a value a hair over - * 1, which floating point does produce for near-antipodal points, is NaN, and a - * NaN distance silently fails every comparison it feeds rather than throwing. + * The same distance in metres. The clamped form lives in `@trek/shared`, where + * the client's tour metrics read it too. */ -export function haversineMetres(aLat: number, aLng: number, bLat: number, bLng: number): number { - const toRad = (deg: number) => (deg * Math.PI) / 180; - const dLat = toRad(bLat - aLat); - const dLng = toRad(bLng - aLng); - const h = - Math.sin(dLat / 2) ** 2 + Math.cos(toRad(aLat)) * Math.cos(toRad(bLat)) * Math.sin(dLng / 2) ** 2; - return 2 * 6371000 * Math.asin(Math.min(1, Math.sqrt(h))); -} +export { haversineMetres } from '@trek/shared'; diff --git a/server/src/nest/tours/tours.helpers.ts b/server/src/nest/tours/tours.helpers.ts index 69c0ef85e..4b9d258be 100644 --- a/server/src/nest/tours/tours.helpers.ts +++ b/server/src/nest/tours/tours.helpers.ts @@ -3,52 +3,10 @@ * PlacesService prepares and persists imported places; these helpers derive * metrics from the resulting [[lat,lng,ele?]] geometry. */ -import { haversineMetres } from '../common/geo'; +import type { GeometryPoint } from '@trek/shared'; -/** One decoded `route_geometry` point: `[lat, lng]` or `[lat, lng, ele]`. */ -export type GeometryPoint = [number, number] | [number, number, number]; - -export interface TourMetrics { - /** Sum of segment haversine distances, in km. */ - distanceKm: number; - /** Sum of positive elevation deltas, in metres. Null when the track carries no elevation. */ - elevationGainM: number | null; - /** Sum of negative elevation deltas (as a positive number), in metres. Null when no elevation. */ - elevationLossM: number | null; - /** Whether every point in the geometry carried a 3rd (elevation) value. */ - hasElevation: boolean; -} - -/** - * Derives distance/elevation from a parsed `route_geometry` array. Never throws — - * A track with fewer than 2 points, or with missing/mixed - * elevation, still yields a usable (if partial) result rather than blocking import. - */ -export function computeTourMetrics(points: GeometryPoint[]): TourMetrics { - if (points.length < 2) { - return { distanceKm: 0, elevationGainM: null, elevationLossM: null, hasElevation: false }; - } - const hasElevation = points.every(p => p.length === 3 && Number.isFinite(p[2])); - - let distanceKm = 0; - let gain = 0; - let loss = 0; - for (let i = 1; i < points.length; i++) { - distanceKm += haversineMetres(points[i - 1][0], points[i - 1][1], points[i][0], points[i][1]) / 1000; - if (hasElevation) { - const delta = (points[i][2] as number) - (points[i - 1][2] as number); - if (delta > 0) gain += delta; - else loss += -delta; - } - } - - return { - distanceKm, - elevationGainM: hasElevation ? gain : null, - elevationLossM: hasElevation ? loss : null, - hasElevation, - }; -} +// The metrics are shared with the client, which computes them for a tour saved offline. +export { computeTourMetrics, type GeometryPoint, type TourMetrics } from '@trek/shared'; /** * Parses a `places.route_geometry` JSON string into typed points. Returns an empty diff --git a/shared/src/geo/haversine.ts b/shared/src/geo/haversine.ts new file mode 100644 index 000000000..27049db54 --- /dev/null +++ b/shared/src/geo/haversine.ts @@ -0,0 +1,14 @@ +/** + * Great-circle distance in metres. + * + * The clamp keeps `asin` defined: floating point can push the argument a hair + * over 1 for near-antipodal points, and a NaN distance silently fails every + * comparison it feeds rather than throwing. + */ +export function haversineMetres(aLat: number, aLng: number, bLat: number, bLng: number): number { + const toRad = (deg: number) => (deg * Math.PI) / 180; + const dLat = toRad(bLat - aLat); + const dLng = toRad(bLng - aLng); + const h = Math.sin(dLat / 2) ** 2 + Math.cos(toRad(aLat)) * Math.cos(toRad(bLat)) * Math.sin(dLng / 2) ** 2; + return 2 * 6371000 * Math.asin(Math.min(1, Math.sqrt(h))); +} diff --git a/shared/src/index.ts b/shared/src/index.ts index 67b06aaae..03ca72c4c 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -48,6 +48,8 @@ export * from './place/place-website'; export * from './roadtrip/roadtrip.schema'; export * from './place/track-colors'; export * from './tours/tours.schema'; +export * from './tours/tours.metrics'; +export * from './geo/haversine'; export * from './collection/collection.schema'; export * from './collection/collection-file.schema'; export * from './trip/trip.schema'; diff --git a/shared/src/tours/tours.metrics.test.ts b/shared/src/tours/tours.metrics.test.ts new file mode 100644 index 000000000..cae2b4eac --- /dev/null +++ b/shared/src/tours/tours.metrics.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest'; +import { computeTourMetrics } from './tours.metrics'; +import { haversineMetres } from '../geo/haversine'; + +describe('computeTourMetrics', () => { + it('sums distance and both elevation directions over a 3D track', () => { + const m = computeTourMetrics([ + [47, 11, 1000], + [47.01, 11, 1100], + [47.02, 11, 1050], + ]); + expect(m.hasElevation).toBe(true); + expect(m.elevationGainM).toBe(100); + expect(m.elevationLossM).toBe(50); + expect(m.distanceKm).toBeCloseTo((haversineMetres(47, 11, 47.01, 11) * 2) / 1000, 6); + }); + + it('reports no elevation when one point lacks it', () => { + const m = computeTourMetrics([ + [47, 11, 1000], + [47.01, 11], + ]); + expect(m).toMatchObject({ hasElevation: false, elevationGainM: null, elevationLossM: null }); + expect(m.distanceKm).toBeGreaterThan(1); + }); + + it('yields a zero-metric result below two points', () => { + expect(computeTourMetrics([[47, 11, 1000]])).toEqual({ + distanceKm: 0, + elevationGainM: null, + elevationLossM: null, + hasElevation: false, + }); + }); +}); + +describe('haversineMetres', () => { + it('stays finite for antipodal points', () => { + expect(haversineMetres(0, 0, 0, 180)).toBeCloseTo(Math.PI * 6371000, 0); + }); +}); diff --git a/shared/src/tours/tours.metrics.ts b/shared/src/tours/tours.metrics.ts new file mode 100644 index 000000000..1dda3cb09 --- /dev/null +++ b/shared/src/tours/tours.metrics.ts @@ -0,0 +1,49 @@ +import { haversineMetres } from '../geo/haversine'; + +/** One decoded `route_geometry` point: `[lat, lng]` or `[lat, lng, ele]`. */ +export type GeometryPoint = [number, number] | [number, number, number]; + +export interface TourMetrics { + /** Sum of segment haversine distances, in km. */ + distanceKm: number; + /** Sum of positive elevation deltas, in metres. Null when the track carries no elevation. */ + elevationGainM: number | null; + /** Sum of negative elevation deltas (as a positive number), in metres. Null when no elevation. */ + elevationLossM: number | null; + /** Whether every point in the geometry carried a 3rd (elevation) value. */ + hasElevation: boolean; +} + +/** + * Derives distance/elevation from a parsed `route_geometry` array. Never throws: + * a track with fewer than 2 points, or with missing/mixed elevation, still + * yields a usable (if partial) result rather than blocking import. + * + * The server stores these numbers on every tour; the client computes the same + * ones for a tour saved offline, so both read them from here. + */ +export function computeTourMetrics(points: readonly GeometryPoint[]): TourMetrics { + if (points.length < 2) { + return { distanceKm: 0, elevationGainM: null, elevationLossM: null, hasElevation: false }; + } + const hasElevation = points.every((p) => p.length === 3 && Number.isFinite(p[2])); + + let distanceKm = 0; + let gain = 0; + let loss = 0; + for (let i = 1; i < points.length; i++) { + distanceKm += haversineMetres(points[i - 1][0], points[i - 1][1], points[i][0], points[i][1]) / 1000; + if (hasElevation) { + const delta = (points[i][2] as number) - (points[i - 1][2] as number); + if (delta > 0) gain += delta; + else loss += -delta; + } + } + + return { + distanceKm, + elevationGainM: hasElevation ? gain : null, + elevationLossM: hasElevation ? loss : null, + hasElevation, + }; +} diff --git a/wiki/Offline-Mode-and-PWA.md b/wiki/Offline-Mode-and-PWA.md index c270923fb..9152c47f3 100644 --- a/wiki/Offline-Mode-and-PWA.md +++ b/wiki/Offline-Mode-and-PWA.md @@ -57,6 +57,7 @@ TREK uses Workbox service-worker caching plus an IndexedDB database (Dexie) for On login, when the browser comes back online, and when you lift **Force offline mode**, TREK runs a background sync that writes full trip bundles into IndexedDB — you can also start one by hand with **Re-sync now**, with **Download for offline use** for a progress-tracked run, or by re-enabling a trip's offline toggle: - Trips, days, places, packing items, to-dos, budget items, reservations, accommodations, trip members, tags, and categories. +- With the Tours addon on, each trip's tours, so a tour still reads as a tour offline. A tour you opened in the route editor while online keeps its control points, so it opens there offline too. - File attachments that are neither photos nor videos (PDFs, documents, etc.) are downloaded and stored as blobs in IndexedDB. Videos are deliberately skipped — a single clip can be hundreds of megabytes and would evict the trip's real documents. - Map tiles are pre-fetched into the service-worker `map-tiles` cache for zoom levels 0 to 16 across each trip's bounding box, stopping at the zoom level that would push the total past 12 288 tiles (roughly 180 MB). If the browser refused persistent storage, prefetching stops at zoom 12 so the app shell cannot be evicted. - The places around each trip, up to 3000 from the [TREK Places API](TREK-Places-API) in one request (about a megabyte for a city), so place search and suggestions still answer offline. They are downloaded whether or not **Store map tiles offline** is on, refreshed only when the trip's area changes, and removed with the trip. See [Searching offline](Places-and-Search#searching-offline). @@ -96,7 +97,7 @@ The stats panel shows cached trips, pending changes, conflicts and failed change ## Limitations -- Offline **editing** is supported for places and packing items (with conflict detection), plus a visit's start and end time, its **End the day here** flag and the road trip **Driving settings** (queued, without conflict detection). Other entities — budget, to-dos, reservations, days — require connectivity to edit; while forced offline those edits still go to the live server when a connection is actually present. +- Offline **editing** is supported for places and packing items (with conflict detection), plus a visit's start and end time, its **End the day here** flag and the road trip **Driving settings** (queued, without conflict detection). A tour can be saved or edited offline as well (queued, without conflict detection), but drawing a new route needs a connection, since routing and elevation come from the routing server, and the GPX import is an upload. Other entities — budget, to-dos, reservations, days — require connectivity to edit; while forced offline those edits still go to the live server when a connection is actually present. - A change you made offline that **deletes** an item wins over a concurrent server edit of that same item ("delete wins"); only edit-vs-edit conflicts are surfaced for resolution. - The conflict token has one-second resolution, so two edits to the same field within the same second can't be told apart and fall back to last-write-wins (only relevant to sub-second races; normal offline windows are unaffected). - Creating a trip requires connectivity. Trip creation is not queued, so a new trip cannot be started while offline.