mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-24 16:03:02 +00:00
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.
This commit is contained in:
@@ -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'])
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
+10
-11
@@ -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<void> {
|
||||
await offlineDb.transaction(
|
||||
@@ -541,7 +543,6 @@ export async function clearTripData(tripId: number): Promise<void> {
|
||||
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<void> {
|
||||
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
|
||||
|
||||
@@ -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<Set<number> | 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<Trip[]> {
|
||||
// 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<number> | null): Promise<Trip[]> {
|
||||
// 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).
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user