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