diff --git a/client/src/components/Tours/paddle/paddleCheckModel.test.ts b/client/src/components/Tours/paddle/paddleCheckModel.test.ts index ce146c7ac..3065b218d 100644 --- a/client/src/components/Tours/paddle/paddleCheckModel.test.ts +++ b/client/src/components/Tours/paddle/paddleCheckModel.test.ts @@ -1,4 +1,4 @@ -// PCM-001 to PCM-018: the check bench of a paddle tour in one pill, how the dates of the checklist read, where the +// PCM-001 to PCM-019: the check bench of a paddle tour in one pill, how the dates of the checklist read, where the // camera goes for an entry and the order the checklist lists its rows in (PAD-06, PAD-09). import { Ban, CircleDashed, ListChecks } from 'lucide-react'; import { describe, expect, it } from 'vitest'; @@ -244,8 +244,9 @@ describe('nextPendingEntry', () => { }); const last = obstacleEntry({ key: 'node/5', refs: ['node/5'], along_m: 1700, status: 'decided', decision }); const gone = earlierEntry(); + const offRoute = earlierEntry({ key: 'node/11', refs: ['node/11'], status: 'off_route' }); // Listed out of route order: the next one is found by its metre, not by its place in the list. - const check = obstacleCheck({ entries: [review, last, gone, todo, first, decided] }); + const check = obstacleCheck({ entries: [review, last, gone, offRoute, todo, first, decided] }); it('PCM-016: skips what is decided and takes the next one after the entry, one to check again counted', () => { expect(nextPendingEntry(check, first)).toEqual(todo); @@ -253,13 +254,14 @@ describe('nextPendingEntry', () => { expect(nextPendingEntry(check, todo)).toEqual(review); }); - it('PCM-017: from an earlier decision the first pending entry of the route comes next', () => { - expect(nextPendingEntry(check, gone)).toEqual(first); + it('PCM-017: from an earlier decision outside the rows the first pending entry of the route comes next', () => { + expect(nextPendingEntry(check, offRoute)).toEqual(first); }); - it('PCM-018: after the last pending entry there is none', () => { - expect(nextPendingEntry(check, review)).toBeNull(); - expect(nextPendingEntry(check, last)).toBeNull(); + it('PCM-018: a vanished object, listed after the route, comes after its last entry, and after it there is none', () => { + expect(nextPendingEntry(check, review)).toEqual(gone); + expect(nextPendingEntry(check, last)).toEqual(gone); + expect(nextPendingEntry(check, gone)).toBeNull(); expect(nextPendingEntry(obstacleCheck({ entries: [first] }), first)).toBeNull(); }); }); diff --git a/client/src/components/Tours/paddle/paddleCheckModel.ts b/client/src/components/Tours/paddle/paddleCheckModel.ts index b1b0eadd0..1c45b27ce 100644 --- a/client/src/components/Tours/paddle/paddleCheckModel.ts +++ b/client/src/components/Tours/paddle/paddleCheckModel.ts @@ -108,17 +108,16 @@ export function checklistRows(check: TourObstacleCheck): { rows: ChecklistRow[]; }; } -const PENDING = new Set(['open', 'todo', 'review']); +/** The statuses that still ask for a look, as the bench counts them (a vanished object among them). */ +const PENDING = new Set(['open', 'todo', 'review', 'gone']); /** * The next entry still asking for a look, in the order the checklist lists - * them (route order, entries at one metre in list order); from an earlier - * decision, the first one. + * its rows (route order, entries at one metre in list order, then the + * vanished ones); from an earlier decision outside the rows, the first one. */ export function nextPendingEntry(check: TourObstacleCheck, entry: TourObstacleEntry): TourObstacleEntry | null { - const current = check.entries - .filter((candidate) => candidate.relation !== null) - .sort((a, b) => (a.along_m ?? 0) - (b.along_m ?? 0)); - const from = current.findIndex((candidate) => candidate.key === entry.key); - return current.slice(from + 1).find((candidate) => PENDING.has(candidate.status)) ?? null; + const listed = checklistRows(check).rows.flatMap((row) => (row.type === 'obstacle' ? [row.entry] : [])); + const from = listed.findIndex((candidate) => candidate.key === entry.key); + return listed.slice(from + 1).find((candidate) => PENDING.has(candidate.status)) ?? null; } diff --git a/client/src/components/Tours/paddle/usePaddleChecklist.test.tsx b/client/src/components/Tours/paddle/usePaddleChecklist.test.tsx index 94fd7cf66..e55eef920 100644 --- a/client/src/components/Tours/paddle/usePaddleChecklist.test.tsx +++ b/client/src/components/Tours/paddle/usePaddleChecklist.test.tsx @@ -1,5 +1,5 @@ /** - * UPC-001 to UPC-030: the obstacle checklist of the paddle tour open in the + * UPC-001 to UPC-032: the obstacle checklist of the paddle tour open in the * tours workspace (PAD-06, PAD-09), as its card and the map share it. * * Pinned: off for anything but an open paddle tour in an active workspace, @@ -760,4 +760,42 @@ describe('usePaddleChecklist after review', () => { }); expect(saved).toBe(true); }); + + it('UPC-031: a save on one tour and then on another keep their own pending state until each is answered', async () => { + const other = paddleTour({ place_id: 8, paddle_check: paddleBench({ revision: 'rev-8' }) }); + const otherEntry = obstacleEntry({ key: 'node/8', refs: ['node/8'] }); + const first = deferred(); + const second = deferred(); + repo.saveObstacleReviews.mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + const props = options({ tours: [TOUR, other] }); + const { result, rerender } = await renderReady(props); + act(() => { + void result.current.decide(ENTRY, 'pass'); + }); + repo.obstacleCheck.mockResolvedValue({ + check: obstacleCheck({ revision: 'rev-8', entries: [otherEntry] }), + stale: false, + }); + rerender({ ...props, planner: planner({ mode: { type: 'edit-saved', placeId: 8 } }) }); + await waitFor(() => expect(result.current.check?.entries[0]?.key).toBe('node/8')); + act(() => { + void result.current.decide(otherEntry, 'portage'); + }); + expect(result.current.savingKey).toBe('node/8'); + + await act(async () => { + first.resolve(answer(obstacleCheck({ revision: 'rev-2' }), 'rev-2')); + await first.promise; + }); + expect(result.current.savingKey).toBe('node/8'); + expect(result.current.check?.entries[0]).toMatchObject({ key: 'node/8', decision: { decision: 'portage' } }); + }); + + it('UPC-032: a saved paddle tour the list has not loaded yet is off until it is there, then read', async () => { + const { result, rerender } = renderChecklist(options({ tours: [] })); + expect(result.current.status).toBe('off'); + expect(repo.obstacleCheck).not.toHaveBeenCalled(); + rerender(options()); + await waitFor(() => expect(result.current.status).toBe('ready')); + }); }); diff --git a/client/src/components/Tours/paddle/usePaddleChecklist.ts b/client/src/components/Tours/paddle/usePaddleChecklist.ts index 04c8e3249..0cfa94fc3 100644 --- a/client/src/components/Tours/paddle/usePaddleChecklist.ts +++ b/client/src/components/Tours/paddle/usePaddleChecklist.ts @@ -125,6 +125,20 @@ function waypointBefore(points: TourPlannerController['waypoints'], alongM: numb return points[points.length - 2]?.id ?? null; } +/** + * A new paddle draft, or a saved tour the list shows as another type that the + * planner turned into a paddle tour: no server check exists for either. A + * tour the list does not hold yet (still loading) is neither. + */ +function unsavedPaddleDraft(planner: TourPlannerController, tours: readonly TourListItem[]): boolean { + if (!planner.isPaddle) return false; + const { mode } = planner; + if (mode.type === 'new-draft') return true; + return ( + mode.type === 'edit-saved' && tours.some((item) => item.place_id === mode.placeId && item.tour_type !== 'kayak') + ); +} + /** Where the checklist of the open tour stands. */ function checklistStatus(input: { active: boolean; @@ -234,9 +248,7 @@ export function usePaddleChecklist(options: PaddleChecklistOptions) { const dirty = planner.hasUnsavedChanges || !!trackPaddlePlan?.dirty; const status = checklistStatus({ active, - // A new paddle draft, or a saved tour turned into one and not saved yet: no server check exists for either. - newPaddleDraft: - planner.isPaddle && (planner.mode.type === 'new-draft' || (planner.mode.type === 'edit-saved' && !tour)), + newPaddleDraft: unsavedPaddleDraft(planner, tours), tour, current: read.current, offline: read.offline, @@ -262,6 +274,22 @@ export function usePaddleChecklist(options: PaddleChecklistOptions) { export type PaddleChecklist = ReturnType; +interface PendingSave { + key: string; + check: TourObstacleCheck; +} + +/** The saves on their way without this one, once it is answered; a later save of the same tour stays. */ +function withoutSave( + all: Readonly>, + placeId: number, + done: PendingSave +): Readonly> { + if (all[placeId] !== done) return all; + const { [placeId]: _done, ...rest } = all; + return rest; +} + /** The decisions saved through the offline core: shown at once, the server's answer taken in, a failure undone. */ function useChecklistWrites( options: PaddleChecklistOptions, @@ -272,7 +300,8 @@ function useChecklistWrites( const { t } = useTranslation(); const toast = useToast(); const userId = useAuthStore((state) => state.user?.id ?? null); - const [pending, setPending] = useState<{ placeId: number; key: string; check: TourObstacleCheck } | null>(null); + // The save on its way per tour, so one tour's answer never clears another's. + const [pending, setPending] = useState>>({}); const base = read.current?.check ?? null; const placeId = tour?.place_id ?? null; const { setLoaded, offline } = read; @@ -280,11 +309,11 @@ function useChecklistWrites( const save = useCallback( async (key: string, op: ObstacleReviewOp) => { if (!base || placeId === null) return false; - setPending({ - placeId, + const entry: PendingSave = { key, check: applyObstacleOps(base, [op], { now: new Date().toISOString(), userId, revision: 'pending' }), - }); + }; + setPending((all) => ({ ...all, [placeId]: entry })); try { const result = await tourRepo.saveObstacleReviews(tripId, placeId, { ops: [op] }, userId); const revision = result.tour.paddle_check?.revision ?? null; @@ -305,7 +334,7 @@ function useChecklistWrites( toast.error(t('tours.paddle.marks.failed')); return false; } finally { - setPending(null); + setPending((all) => withoutSave(all, placeId, entry)); } }, [base, offline, onSaved, placeId, setLoaded, t, toast, tripId, userId] @@ -331,6 +360,6 @@ function useChecklistWrites( [save] ); // A save still running for a tour left meanwhile shows nothing on the one open now. - const mine = pending && pending.placeId === placeId ? pending : null; + const mine = placeId === null ? null : (pending[placeId] ?? null); return { optimistic: mine?.check ?? null, savingKey: mine?.key ?? null, actions }; } diff --git a/client/src/repo/tourRepo.obstacles.test.ts b/client/src/repo/tourRepo.obstacles.test.ts index 7443632d7..fb47aa701 100644 --- a/client/src/repo/tourRepo.obstacles.test.ts +++ b/client/src/repo/tourRepo.obstacles.test.ts @@ -1,4 +1,4 @@ -// FE-REPO-TOUR-030 to FE-REPO-TOUR-040 +// FE-REPO-TOUR-030 to FE-REPO-TOUR-041 // A paddle tour's checklist through the offline core (PAD-06, PAD-09): read // from the cache while its revision matches the bench, decisions saved online // and cached, saved offline under a local revision and queued, and a route @@ -234,4 +234,29 @@ describe('after review', () => { expect(flush).toHaveBeenCalled(); expect(read).toMatchObject({ check: { revision: 'rev-5' }, tour: { paddle_check: { revision: 'rev-5' } } }); }); + + it('FE-REPO-TOUR-041: online, a checklist changed here and still queued waits for the queue before it is read', async () => { + await offlineDb.tours.put({ + ...paddleTour({ paddle_check: paddleBench({ revision: 'rev-2' }) }), + trip_id: 3, + waypoints, + obstacle_check: obstacleCheck({ revision: 'local:x' }), + }); + const order: string[] = []; + vi.spyOn(mutationQueue, 'flush').mockImplementation(async () => { + order.push('flush'); + }); + server.use( + http.get('/api/trips/3/tours/40', () => { + order.push('get'); + return HttpResponse.json({ + tour: paddleTour({ paddle_check: paddleBench({ revision: 'rev-3' }) }), + waypoints, + obstacle_check: obstacleCheck({ revision: 'rev-3' }), + }); + }) + ); + await tourRepo.obstacleCheck(3, 40, 'rev-2'); + expect(order).toEqual(['flush', 'get']); + }); }); diff --git a/client/src/repo/tourRepo.ts b/client/src/repo/tourRepo.ts index 1b6bb8661..54c04d91f 100644 --- a/client/src/repo/tourRepo.ts +++ b/client/src/repo/tourRepo.ts @@ -278,8 +278,13 @@ export const tourRepo = { revision: string | null, signal?: AbortSignal, ): Promise<{ check: TourObstacleCheck | null; stale: boolean; tour?: TourListItem }> { - if (revision?.startsWith('local:') && !isEffectivelyOffline()) await mutationQueue.flush() - const cached = await offlineDb.tours.get(placeId) + let cached = await offlineDb.tours.get(placeId) + // A bench or a checklist of a write made here offline waits for the queue to replay once online. + const queuedHere = !!revision?.startsWith('local:') || !!cached?.obstacle_check?.revision.startsWith('local:') + if (queuedHere && !isEffectivelyOffline()) { + await mutationQueue.flush() + cached = await offlineDb.tours.get(placeId) + } if (revision !== null && cached?.obstacle_check?.revision === revision) return { check: cached.obstacle_check, stale: false } const detail = await tourRepo.detail(tripId, placeId, signal) const check = detail.obstacle_check ?? null