diff --git a/client/src/components/Settings/OfflineTab.test.tsx b/client/src/components/Settings/OfflineTab.test.tsx index 74c8613db..c86d2b136 100644 --- a/client/src/components/Settings/OfflineTab.test.tsx +++ b/client/src/components/Settings/OfflineTab.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-OFFLINETAB-001 to FE-COMP-OFFLINETAB-028 +// FE-COMP-OFFLINETAB-001 to FE-COMP-OFFLINETAB-031 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; @@ -500,6 +500,22 @@ describe('OfflineTab', () => { expect(screen.queryByText('No trips cached yet. Connect to the internet to sync.')).not.toBeInTheDocument(); }); + it('FE-COMP-OFFLINETAB-031: an archived trip still stored here gets a switch that takes it off', async () => { + const user = userEvent.setup(); + const soon = new Date(Date.now() + 7 * 86_400_000).toISOString().slice(0, 10); + const archived = buildTrip({ id: 9, title: 'Rome', start_date: '2025-01-01', end_date: soon, is_archived: 1 }); + // The server's list leaves archived trips out; the device still holds Rome. + h.tripsToArray.mockResolvedValue([archived, paris]); + render(); + + const toggle = await screen.findByRole('button', { name: 'Rome' }); + expect(toggle).toHaveAttribute('aria-pressed', 'true'); + expect(screen.getAllByRole('button', { name: 'Paris' })).toHaveLength(1); + + await user.click(toggle); + expect(h.clearTripData).toHaveBeenCalledWith(9); + }); + it('FE-COMP-OFFLINETAB-020: the per-trip section disappears when there are no trips at all', async () => { server.use(http.get('/api/trips', () => HttpResponse.json({ trips: [] }))); render(); diff --git a/client/src/components/Settings/useOfflineSettings.ts b/client/src/components/Settings/useOfflineSettings.ts index e76223782..a53729513 100644 --- a/client/src/components/Settings/useOfflineSettings.ts +++ b/client/src/components/Settings/useOfflineSettings.ts @@ -26,6 +26,21 @@ import { useNetworkMode } from '../../hooks/useNetworkMode' import type { SyncMeta, QueuedMutation } from '../../db/offlineDb' import type { Trip } from '../../types' +/** + * The trips the per-trip switches cover: the server's list plus the trips still + * stored here that it leaves out. Those are archived trips (a deleted one goes + * with the next sync), which the trip sync keeps but never refreshes, so this + * switch is the user's way to take one off the device before the date rule does. + */ +async function switchableTrips(): Promise { + if (isEffectivelyOffline()) return offlineDb.trips.toArray() + const listed = await tripsApi.list().then(r => (r as { trips: Trip[] }).trips).catch(() => null) + if (!listed) return offlineDb.trips.toArray() + const ids = new Set(listed.map(t => t.id)) + const stored = await offlineDb.trips.toArray().catch(() => [] as Trip[]) + return [...listed, ...stored.filter(t => t.id > 0 && !ids.has(t.id))] +} + export interface CachedTripRow { trip: Trip meta: SyncMeta @@ -128,9 +143,7 @@ export function useOfflineSettings() { // The per-trip storage toggles are driven by the FULL trip list, not just // the cached ones, so a trip turned off stays visible and re-enableable. try { - const trips = isEffectivelyOffline() - ? await offlineDb.trips.toArray() - : await tripsApi.list().then(r => (r as { trips: Trip[] }).trips).catch(() => offlineDb.trips.toArray()) + const trips = await switchableTrips() trips.sort((a, b) => (a.start_date ?? '').localeCompare(b.start_date ?? '')) setAllTrips(trips) } catch { diff --git a/client/src/sync/tripSyncManager.ts b/client/src/sync/tripSyncManager.ts index 069ecf802..e54e1e53c 100644 --- a/client/src/sync/tripSyncManager.ts +++ b/client/src/sync/tripSyncManager.ts @@ -211,10 +211,10 @@ export interface PrepareProgress { let _syncing = false -/** Ids of the user's archived trips, or null when the list could not be read. */ -function archivedTripIds(): Promise | null> { +/** The user's archived trips, or null when the list could not be read. */ +function archivedTrips(): Promise { return (tripsApi.list({ archived: 1 }) as Promise<{ trips: Trip[] }>) - .then(r => new Set(r.trips.map(t => t.id))) + .then(r => r.trips) .catch(() => null) } @@ -223,21 +223,26 @@ function archivedTripIds(): Promise | null> { * and the user's per-trip offline choices (#1135 ask 2). Returns the trips to * sync; clears Dexie for stale or user-disabled trips as a side effect. */ -async function reconcileTrips(trips: Trip[], archivedIds: Set | null): Promise { +async function reconcileTrips(trips: Trip[], archived: Trip[] | null): Promise { // A cached trip the server lists neither as active nor as archived was // deleted, or this user was removed from it. Its data has no business staying // on the device. The plain list leaves archived trips out, so without the - // archived ids every archived trip would read as gone; when that list could + // archived list every archived trip would read as gone; when that list could // not be read, nothing is treated as gone this round. - if (archivedIds) { - const listed = new Set([...trips.map(t => t.id), ...archivedIds]) - const gone = (await offlineDb.trips.toArray()).filter(t => t.id > 0 && !listed.has(t.id)) + const cached = await offlineDb.trips.toArray() + if (archived) { + const listed = new Set([...trips, ...archived].map(t => t.id)) + const gone = cached.filter(t => t.id > 0 && !listed.has(t.id)) await Promise.all(gone.map(t => clearTripData(t.id).catch(console.error))) } - const stale = trips.filter(isStale) + // An archived trip on the device is kept but never synced again, so the date + // rule and the user's switch are the only things that ever take it off. + const cachedIds = new Set(cached.map(t => t.id)) + const known = [...trips, ...(archived ?? []).filter(t => cachedIds.has(t.id))] + const stale = known.filter(isStale) // Trips the user turned off explicitly are evicted regardless of date. - const disabled = trips.filter(t => !isTripOfflineEnabled(t.id)) + const disabled = known.filter(t => !isTripOfflineEnabled(t.id)) await Promise.all([...stale, ...disabled].map(t => clearTripData(t.id).catch(console.error))) return trips.filter(t => shouldCache(t) && isTripOfflineEnabled(t.id)) } @@ -274,8 +279,8 @@ export const tripSyncManager = { if (skipped) return skipped _syncing = true try { - const [{ trips }, archivedIds] = await Promise.all([tripsApi.list() as Promise<{ trips: Trip[] }>, archivedTripIds()]) - const toSync = await reconcileTrips(trips, archivedIds) + const [{ trips }, archived] = await Promise.all([tripsApi.list() as Promise<{ trips: Trip[] }>, archivedTrips()]) + const toSync = await reconcileTrips(trips, archived) for (const trip of toSync) { // The gate is re-read per trip: a logout halfway through must not keep @@ -347,8 +352,8 @@ export const tripSyncManager = { if (skipped) return skipped _syncing = true try { - const [{ trips }, archivedIds] = await Promise.all([tripsApi.list() as Promise<{ trips: Trip[] }>, archivedTripIds()]) - const toSync = await reconcileTrips(trips, archivedIds) + const [{ trips }, archived] = await Promise.all([tripsApi.list() as Promise<{ trips: Trip[] }>, archivedTrips()]) + const toSync = await reconcileTrips(trips, archived) const total = toSync.length // 1) Trip bundles (structured data). diff --git a/client/tests/unit/sync/tripSyncManager.test.ts b/client/tests/unit/sync/tripSyncManager.test.ts index cdc772fd0..340338264 100644 --- a/client/tests/unit/sync/tripSyncManager.test.ts +++ b/client/tests/unit/sync/tripSyncManager.test.ts @@ -10,7 +10,7 @@ import { server } from '../../helpers/msw/server'; import { http, HttpResponse } from 'msw'; import { tripSyncManager } from '../../../src/sync/tripSyncManager'; import { setAuthed } from '../../../src/sync/authGate'; -import { setTripPinned, _resetOfflinePrefs } from '../../../src/sync/offlinePrefs'; +import { setTripPinned, setTripOfflineEnabled, _resetOfflinePrefs } from '../../../src/sync/offlinePrefs'; import { offlineDb, clearAll, upsertTrip } from '../../../src/db/offlineDb'; import { useAddonStore } from '../../../src/store/addonStore'; import { @@ -546,6 +546,56 @@ describe('tripSyncManager.syncAll: deletions made elsewhere', () => { expect(await offlineDb.mutationQueue.get('parked-601')).toBeDefined(); }); + /** The server lists `active` as the user's trips and `archived` behind ?archived=1. */ + function listTrips(active: ReturnType[], archived: ReturnType[]) { + return http.get('/api/trips', ({ request }) => HttpResponse.json({ + trips: new URL(request.url).searchParams.get('archived') === '1' ? archived : active, + })); + } + + it('evicts an archived trip that ended more than 7 days ago, and keeps its parked changes', async () => { + const archived = buildTrip({ id: 605, end_date: dateOffset(-30), is_archived: 1 }); + await upsertTrip(archived); + await offlineDb.places.put(buildPlace({ trip_id: 605, id: 8105 })); + await offlineDb.mutationQueue.put({ + id: 'parked-605', tripId: 605, method: 'PUT', url: '/trips/605/places/8105', body: { name: 'X' }, + createdAt: 1, status: 'failed', attempts: 8, lastError: 'boom', resource: 'places', entityId: 8105, + }); + + server.use(listTrips([], [archived])); + await tripSyncManager.syncAll(); + + expect(await offlineDb.trips.get(605)).toBeUndefined(); + expect(await offlineDb.places.get(8105)).toBeUndefined(); + expect(await offlineDb.mutationQueue.get('parked-605')).toBeDefined(); + }); + + it('evicts an archived trip the user switched off, however recent', async () => { + const archived = buildTrip({ id: 606, end_date: dateOffset(5), is_archived: 1 }); + await upsertTrip(archived); + setTripOfflineEnabled(606, false); + + server.use(listTrips([], [archived])); + await tripSyncManager.syncAll(); + + expect(await offlineDb.trips.get(606)).toBeUndefined(); + }); + + it('keeps an archived trip that is still within the date rule, without syncing it', async () => { + const archived = buildTrip({ id: 607, end_date: dateOffset(5), is_archived: 1 }); + await upsertTrip(archived); + let bundles = 0; + + server.use( + listTrips([], [archived]), + http.get('/api/trips/607/bundle', () => { bundles++; return HttpResponse.json({ ...makeBundle(607), trip: archived }); }), + ); + await tripSyncManager.syncAll(); + + expect(await offlineDb.trips.get(607)).toBeDefined(); + expect(bundles).toBe(0); + }); + it('treats nothing as gone when the archived list cannot be read', async () => { const archived = buildTrip({ id: 602, end_date: dateOffset(5) }); await upsertTrip(archived); diff --git a/wiki/Offline-Mode-and-PWA.md b/wiki/Offline-Mode-and-PWA.md index 53f7efef4..200eacb63 100644 --- a/wiki/Offline-Mode-and-PWA.md +++ b/wiki/Offline-Mode-and-PWA.md @@ -68,7 +68,7 @@ On login, when the browser comes back online, and when you lift **Force offline - Ongoing and future trips are cached (trips whose `end_date` is today or later, or has no end date). - Trips that ended more than 7 days ago are automatically evicted from IndexedDB on the next sync. -- A trip that was deleted, or that you were removed from, is cleared from the device on the next sync. Archiving a trip does not count as either: archived trips stay. +- A trip that was deleted, or that you were removed from, is cleared from the device on the next sync. Archiving a trip does not count as either: an archived trip stays on the device as it was last synced, until the 7-day rule above applies to it or you switch it off under what to store offline. - Evicting or clearing a trip never throws away a change that has not reached the server. Queued changes and conflicts stay, and failed changes stay under **Failed changes** in Settings → Offline until you try them again or discard them. - A finished trip is cached too when you switch it on yourself under **Settings → Offline → What to store offline**, and it is then kept regardless of its dates.