From 237f82c0b66cb56edc6ac58a706b8e15c4248aa8 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 11:20:11 +0200 Subject: [PATCH] fix(offline): an archived trip is not a deleted one, and eviction keeps parked changes The trip sync clears every cached trip the server no longer lists, but it read only GET /trips, which leaves archived trips out. Every sync (login, reconnect, the periodic run) therefore wiped every archived trip from the device, pinned ones included, and with it their parked failed changes. A trip now only counts as gone when it is missing from both the active and the archived list, and when the archived list cannot be read nothing is treated as gone that round. clearTripData no longer deletes 'failed' rows. Stale eviction, a trip switched off in the storage settings and a trip that is really gone all go through it, and none of them is the user deciding to give up a change. A parked change of a deleted trip, or one the user was removed from, now stays in the queue: Settings > Offline counts it under failed changes and offers Try again (the server answers 403/404, so it parks again) or Discard. Dropping it silently would lose the only copy of an edit the user may still want to look at or redo elsewhere, and keeping it costs nothing more than a counter the user can clear in one tap. --- client/src/db/offlineDb.test.ts | 4 +- client/src/db/offlineDb.ts | 21 +++--- client/src/sync/tripSyncManager.ts | 35 ++++++---- client/tests/unit/db/offlineDb.test.ts | 7 +- .../tests/unit/sync/tripSyncManager.test.ts | 68 +++++++++++++++++++ wiki/Offline-Mode-and-PWA.md | 2 + 6 files changed, 108 insertions(+), 29 deletions(-) diff --git a/client/src/db/offlineDb.test.ts b/client/src/db/offlineDb.test.ts index c0abb2dd6..142886367 100644 --- a/client/src/db/offlineDb.test.ts +++ b/client/src/db/offlineDb.test.ts @@ -267,7 +267,7 @@ describe('offlineDb — clearTripData', () => { expect((await offlineDb.blobCache.toArray()).map(e => e.url)).toEqual(['/f2']) }) - it('FE-DB-OFFLINE-019: keeps unsynced work and only purges dead failed mutations', async () => { + it('FE-DB-OFFLINE-019: keeps unsynced work, conflicts and parked failed changes', async () => { await offlineDb.mutationQueue.bulkPut([ queued('m-pending', 1, 'pending'), queued('m-syncing', 1, 'syncing'), @@ -279,7 +279,7 @@ describe('offlineDb — clearTripData', () => { await clearTripData(1) const left = (await offlineDb.mutationQueue.toArray()).map(m => m.id).sort() - expect(left).toEqual(['m-conflict', 'm-other-trip', 'm-pending', 'm-syncing']) + expect(left).toEqual(['m-conflict', 'm-failed', 'm-other-trip', 'm-pending', 'm-syncing']) }) }) diff --git a/client/src/db/offlineDb.ts b/client/src/db/offlineDb.ts index 2f7ff86cb..9587c0cfb 100644 --- a/client/src/db/offlineDb.ts +++ b/client/src/db/offlineDb.ts @@ -519,14 +519,16 @@ export async function enforceBlobBudget( // ── Eviction / cleanup ──────────────────────────────────────────────────────── /** - * Delete one trip's cached READ data (eviction, per-trip opt-out). The offline - * write queue is deliberately preserved except for already-dropped 'failed' rows: - * a trip can be evicted for being stale, or turned off in the storage settings, - * while it still holds unsynced offline edits (pending/syncing) or unresolved - * conflicts — those must survive so the user's work is not silently lost (#1135). - * The replay only needs the queued REST request, not the cached entities, and a - * successful flush re-adds the canonical row. The full "Clear cache" wipe goes - * through clearAll(), which intentionally drops everything. + * Delete one trip's cached READ data (eviction, per-trip opt-out, a trip the + * server no longer lists). The offline write queue is left alone entirely: a + * trip can be evicted for being stale, turned off in the storage settings, or + * deleted elsewhere while it still holds unsynced offline edits (pending/syncing), + * unresolved conflicts or parked 'failed' changes. Those must survive so the + * user's work is not silently lost (#1135); a parked change only goes when the + * user discards it in Settings > Offline. The replay only needs the queued REST + * request, not the cached entities, and a successful flush re-adds the + * canonical row. The full "Clear cache" wipe goes through clearAll(), which + * intentionally drops everything. */ export async function clearTripData(tripId: number): Promise { await offlineDb.transaction( @@ -541,7 +543,6 @@ export async function clearTripData(tripId: number): Promise { offlineDb.tripFiles, offlineDb.accommodations, offlineDb.tripMembers, - offlineDb.mutationQueue, offlineDb.syncMeta, offlineDb.blobCache, offlineDb.areaPlaces, @@ -560,8 +561,6 @@ export async function clearTripData(tripId: number): Promise { 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(); await offlineDb.syncMeta.where('tripId').equals(tripId).delete(); await offlineDb.blobCache.where('tripId').equals(tripId).delete(); // The cached places around this trip's area go with it. They are searched diff --git a/client/src/sync/tripSyncManager.ts b/client/src/sync/tripSyncManager.ts index 3e185c99a..069ecf802 100644 --- a/client/src/sync/tripSyncManager.ts +++ b/client/src/sync/tripSyncManager.ts @@ -211,20 +211,29 @@ 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> { + return (tripsApi.list({ archived: 1 }) as Promise<{ trips: Trip[] }>) + .then(r => new Set(r.trips.map(t => t.id))) + .catch(() => null) +} + /** * Decide which trips to cache and which to drop, honouring both the date rule * 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[]): Promise { - // A cached trip the server no longer lists was deleted, or this user was - // removed from it. Its data has no business staying on the device. - const listed = new Set(trips.map(t => t.id)) - const gone = (await offlineDb.trips.toArray()).filter(t => t.id > 0 && !listed.has(t.id)) - await Promise.all(gone.map(async t => { - await clearTripData(t.id).catch(console.error) - await offlineDb.trips.delete(t.id) - })) +async function reconcileTrips(trips: Trip[], archivedIds: Set | 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 + // 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)) + await Promise.all(gone.map(t => clearTripData(t.id).catch(console.error))) + } const stale = trips.filter(isStale) // Trips the user turned off explicitly are evicted regardless of date. @@ -265,8 +274,8 @@ export const tripSyncManager = { if (skipped) return skipped _syncing = true try { - const { trips } = await tripsApi.list() as { trips: Trip[] } - const toSync = await reconcileTrips(trips) + const [{ trips }, archivedIds] = await Promise.all([tripsApi.list() as Promise<{ trips: Trip[] }>, archivedTripIds()]) + const toSync = await reconcileTrips(trips, archivedIds) for (const trip of toSync) { // The gate is re-read per trip: a logout halfway through must not keep @@ -338,8 +347,8 @@ export const tripSyncManager = { if (skipped) return skipped _syncing = true try { - const { trips } = await tripsApi.list() as { trips: Trip[] } - const toSync = await reconcileTrips(trips) + const [{ trips }, archivedIds] = await Promise.all([tripsApi.list() as Promise<{ trips: Trip[] }>, archivedTripIds()]) + const toSync = await reconcileTrips(trips, archivedIds) const total = toSync.length // 1) Trip bundles (structured data). diff --git a/client/tests/unit/db/offlineDb.test.ts b/client/tests/unit/db/offlineDb.test.ts index 3d5147538..d8c711b39 100644 --- a/client/tests/unit/db/offlineDb.test.ts +++ b/client/tests/unit/db/offlineDb.test.ts @@ -343,7 +343,7 @@ describe('offlineDb — clearTripData', () => { expect(await offlineDb.areaPlaces.where('tripId').equals(2).count()).toBe(1); }); - it('preserves unsynced (pending/conflict) writes but drops dead failed ones (#1135)', async () => { + it('preserves unsynced, conflicting and parked failed writes (#1135)', async () => { await upsertTrip(makeTrip(1)); await offlineDb.mutationQueue.bulkPut([ { id: 'p1', tripId: 1, method: 'PUT', url: '/trips/1/places/10', body: { name: 'X' }, createdAt: 1, status: 'pending', attempts: 0, lastError: null, resource: 'places', entityId: 10 }, @@ -353,10 +353,11 @@ describe('offlineDb — clearTripData', () => { await clearTripData(1); - // The trip's cached read data is gone, but the unsynced work survives. + // The trip's cached read data is gone, but the unsynced work survives. A + // parked change waits for the user's Try again or Discard. expect(await offlineDb.mutationQueue.get('p1')).toBeDefined(); expect(await offlineDb.mutationQueue.get('c1')).toBeDefined(); - expect(await offlineDb.mutationQueue.get('f1')).toBeUndefined(); + expect(await offlineDb.mutationQueue.get('f1')).toBeDefined(); }); }); diff --git a/client/tests/unit/sync/tripSyncManager.test.ts b/client/tests/unit/sync/tripSyncManager.test.ts index 1c4cd1c7e..cdc772fd0 100644 --- a/client/tests/unit/sync/tripSyncManager.test.ts +++ b/client/tests/unit/sync/tripSyncManager.test.ts @@ -521,4 +521,72 @@ describe('tripSyncManager.syncAll: deletions made elsewhere', () => { expect(await offlineDb.trips.get(500)).toBeUndefined(); expect(await offlineDb.places.where('trip_id').equals(500).count()).toBe(0); }); + + it('keeps an archived trip: the plain list leaves it out, the archived list does not', async () => { + const active = buildTrip({ id: 600, end_date: dateOffset(5) }); + const archived = buildTrip({ id: 601, end_date: dateOffset(-400), is_archived: 1 }); + await upsertTrip(archived); + setTripPinned(601, true); + await offlineDb.places.put(buildPlace({ trip_id: 601, id: 8101 })); + await offlineDb.mutationQueue.put({ + id: 'parked-601', tripId: 601, method: 'PUT', url: '/trips/601/places/8101', body: { name: 'X' }, + createdAt: 1, status: 'failed', attempts: 8, lastError: 'boom', resource: 'places', entityId: 8101, + }); + + server.use( + http.get('/api/trips', ({ request }) => HttpResponse.json({ + trips: new URL(request.url).searchParams.get('archived') === '1' ? [archived] : [active], + })), + http.get('/api/trips/600/bundle', () => HttpResponse.json({ ...makeBundle(600), trip: active })), + ); + await tripSyncManager.syncAll(); + + expect(await offlineDb.trips.get(601)).toBeDefined(); + expect(await offlineDb.places.get(8101)).toBeDefined(); + expect(await offlineDb.mutationQueue.get('parked-601')).toBeDefined(); + }); + + 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); + + server.use( + http.get('/api/trips', ({ request }) => new URL(request.url).searchParams.get('archived') === '1' + ? HttpResponse.json({ error: 'boom' }, { status: 500 }) + : HttpResponse.json({ trips: [] })), + ); + await tripSyncManager.syncAll(); + + expect(await offlineDb.trips.get(602)).toBeDefined(); + }); + + it('keeps the parked and queued changes of a trip that is really gone, for the user to discard', async () => { + await upsertTrip(buildTrip({ id: 603, end_date: dateOffset(5) })); + await offlineDb.mutationQueue.bulkPut([ + { id: 'parked-603', tripId: 603, method: 'PUT', url: '/trips/603/places/1', body: {}, createdAt: 1, status: 'failed', attempts: 8, lastError: 'boom', resource: 'places', entityId: 1 }, + { id: 'conflict-603', tripId: 603, method: 'PUT', url: '/trips/603/places/2', body: {}, createdAt: 2, status: 'conflict', attempts: 1, lastError: 'conflict', resource: 'places', entityId: 2 }, + { id: 'pending-603', tripId: 603, method: 'PUT', url: '/trips/603/places/3', body: {}, createdAt: 3, status: 'pending', attempts: 0, lastError: null, resource: 'places', entityId: 3 }, + ]); + + server.use(http.get('/api/trips', () => HttpResponse.json({ trips: [] }))); + await tripSyncManager.syncAll(); + + expect(await offlineDb.trips.get(603)).toBeUndefined(); + expect((await offlineDb.mutationQueue.toArray()).map(m => m.id).sort()).toEqual(['conflict-603', 'parked-603', 'pending-603']); + }); + + it('a stale or switched-off trip keeps its parked changes too', async () => { + const stale = buildTrip({ id: 604, end_date: dateOffset(-30) }); + await upsertTrip(stale); + await offlineDb.mutationQueue.put({ + id: 'parked-604', tripId: 604, method: 'PUT', url: '/trips/604/places/1', body: {}, createdAt: 1, + status: 'failed', attempts: 8, lastError: 'boom', resource: 'places', entityId: 1, + }); + + server.use(http.get('/api/trips', () => HttpResponse.json({ trips: [stale] }))); + await tripSyncManager.syncAll(); + + expect(await offlineDb.trips.get(604)).toBeUndefined(); + expect(await offlineDb.mutationQueue.get('parked-604')).toBeDefined(); + }); }); diff --git a/wiki/Offline-Mode-and-PWA.md b/wiki/Offline-Mode-and-PWA.md index 9152c47f3..e279a770b 100644 --- a/wiki/Offline-Mode-and-PWA.md +++ b/wiki/Offline-Mode-and-PWA.md @@ -68,6 +68,8 @@ 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. +- 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. ## Settings → Offline