mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-20 05:56:53 +00:00
fix(offline): a missing driving-settings row no longer discards the trip bundle
syncTrip read the roadtrip preferences before writing anything and rethrew everything but a 404. That read has an offline fallback which throws a bare Error when there is no cached row, which is the state a first sync is in and the state a logout leaves behind, so the whole downloaded bundle for that trip was dropped while the run still counted it. Settings reported "N trips stored" over an empty database, and the trips were gone the moment the connection was. The read moved to the end and is never fatal now: it belongs to an optional addon and the addon answers 404 when it is off. Two things that fed it, from the same trace. A sync run started on navigator.onLine alone, so work-offline downloaded over the real connection while every repo read answered from cache. And work-offline itself survived a logout, in localStorage, while step six of the same logout deleted the database it reads from, so the next account came up believing it was offline with nothing to be offline with. Also in this pass: deleting a hotel booking released its day stop after deleting the booking row, which now clears the stop's pointer by trigger. The release goes first, or the stop stands with nothing left to reach it.
This commit is contained in:
@@ -7,6 +7,7 @@ import { getApiErrorMessage } from '../types'
|
||||
import { tripSyncManager } from '../sync/tripSyncManager'
|
||||
import { reopenForUser, deleteCurrentUserDb } from '../db/offlineDb'
|
||||
import { setAuthed } from '../sync/authGate'
|
||||
import { setForcedOffline } from '../sync/networkMode'
|
||||
import { registerSyncTriggers, unregisterSyncTriggers } from '../sync/syncTriggers'
|
||||
import { useSystemNoticeStore } from './systemNoticeStore.js'
|
||||
import { clearAppearanceSnapshot } from '../theme/applyAppearance'
|
||||
@@ -252,6 +253,11 @@ export const useAuthStore = create<AuthState>()(
|
||||
// browser language is one TREK ships, so otherwise the next user here stays
|
||||
// in the previous account's language, launch after launch.
|
||||
forgetServerLanguage()
|
||||
// And work-offline, for the same reason with sharper teeth: the switch lives
|
||||
// in localStorage, step 6 below deletes the offline database it reads from,
|
||||
// and the next account would come up believing it is offline over a working
|
||||
// connection, with nothing cached to answer from.
|
||||
setForcedOffline(false)
|
||||
// 4. Tell server to clear the httpOnly cookie (best-effort).
|
||||
await fetch('/api/auth/logout', { method: 'POST', credentials: 'include' }).catch(() => {})
|
||||
// 5. Clear service worker caches containing sensitive data.
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
import { prefetchPlacesForTrip } from './placePrefetcher'
|
||||
import { prefetchTilesForTrip } from './tilePrefetcher'
|
||||
import { isAuthed } from './authGate'
|
||||
import { isEffectivelyOffline } from './networkMode'
|
||||
import { getOfflinePrefs, isTripOfflineEnabled, isTripPinned } from './offlinePrefs'
|
||||
import { useSettingsStore } from '../store/settingsStore'
|
||||
import type { Trip, Day, Place, PackingItem, TodoItem, BudgetItem, Reservation, TripFile, Accommodation, TripMember } from '../types'
|
||||
@@ -115,9 +116,6 @@ function isVideo(file: TripFile): boolean {
|
||||
async function syncTrip(tripId: number): Promise<void> {
|
||||
const bundle = await tripsApi.bundle(tripId) as TripBundle
|
||||
|
||||
await roadtripPreferencesRepo.read(tripId).catch((error: unknown) => {
|
||||
if ((error as { response?: { status?: number } }).response?.status !== 404) throw error
|
||||
})
|
||||
await upsertTrip(bundle.trip)
|
||||
await upsertDays(bundle.days)
|
||||
await upsertPlaces(bundle.places)
|
||||
@@ -144,6 +142,14 @@ async function syncTrip(tripId: number): Promise<void> {
|
||||
tilesBbox: null,
|
||||
filesCachedCount: 0,
|
||||
})
|
||||
|
||||
// Last, and never fatal. Driving settings belong to an optional addon: the
|
||||
// route answers 404 when it is off, and the repo's own offline fallback throws
|
||||
// a bare Error when there is no cached row yet, which is exactly the state a
|
||||
// first sync is in. Ahead of the writes and rethrowing anything but a 404, it
|
||||
// threw the whole downloaded bundle away for that trip while the run still
|
||||
// reported it stored, so Settings said "N trips ready" over an empty database.
|
||||
await roadtripPreferencesRepo.read(tripId).catch(() => { /* optional addon, optional cache */ })
|
||||
}
|
||||
|
||||
/** Cache non-photo file blobs for a trip. Fire-and-forget safe. */
|
||||
@@ -222,7 +228,11 @@ export type SyncOutcome =
|
||||
|
||||
function skipReason(): SyncOutcome | null {
|
||||
if (_syncing) return { status: 'skipped', reason: 'busy' }
|
||||
if (!navigator.onLine) return { status: 'skipped', reason: 'offline' }
|
||||
// isEffectivelyOffline, not navigator.onLine: work-offline is a switch the user
|
||||
// holds, and it survives a logout. A run started under it downloads over the
|
||||
// real connection while every repo read answers from a cache that logging out
|
||||
// just deleted, which is a sync that reports trips it did not store.
|
||||
if (isEffectivelyOffline()) return { status: 'skipped', reason: 'offline' }
|
||||
if (!isAuthed()) return { status: 'skipped', reason: 'signed-out' }
|
||||
return null
|
||||
}
|
||||
|
||||
@@ -884,18 +884,22 @@ export class ReservationsService {
|
||||
let accommodationDeleted = false;
|
||||
let stayMirror = noStayMirror();
|
||||
if (reservation.accommodation_id) {
|
||||
// trip_id in the WHERE, not just the reservation's own scope: a row
|
||||
// written before referencesOutsideTrip existed can still carry a
|
||||
// foreign accommodation_id, and the cascade must not follow it.
|
||||
const removed = this.db.run(
|
||||
'DELETE FROM day_accommodations WHERE id = ? AND trip_id = ?', reservation.accommodation_id, tripId,
|
||||
// trip_id in the check, not just the reservation's own scope: a row written
|
||||
// before referencesOutsideTrip existed can still carry a foreign
|
||||
// accommodation_id, and the cascade must not follow it. The stops go by
|
||||
// accommodation id alone, which is exactly the reach that guard denies.
|
||||
const ownStay = this.db.get<{ id: number }>(
|
||||
'SELECT id FROM day_accommodations WHERE id = ? AND trip_id = ?', reservation.accommodation_id, tripId,
|
||||
);
|
||||
accommodationDeleted = removed.changes > 0;
|
||||
// Only once that DELETE actually took a row on THIS trip. The stops go by
|
||||
// accommodation id alone, which is exactly the reach the trip_id guard above
|
||||
// is there to deny a foreign id. A stop left carrying a dead booking's id is
|
||||
// one the day list hides and nobody can reach to remove.
|
||||
if (accommodationDeleted) stayMirror = this.accommodations.dropStayStops(reservation.accommodation_id);
|
||||
if (ownStay) {
|
||||
// Released before the row goes, not after: the release looks the stops up
|
||||
// by accommodation id, and that pointer is cleared the moment the stay is
|
||||
// deleted. Reversed, the stop stands with nothing left to remove it, and
|
||||
// the day list hides it for carrying a booking id.
|
||||
stayMirror = this.accommodations.dropStayStops(reservation.accommodation_id);
|
||||
this.db.run('DELETE FROM day_accommodations WHERE id = ? AND trip_id = ?', reservation.accommodation_id, tripId);
|
||||
accommodationDeleted = true;
|
||||
}
|
||||
}
|
||||
|
||||
const linkedBudget = this.db.get<{ id: number }>('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id);
|
||||
|
||||
@@ -874,6 +874,30 @@ describe('the day stop a booking implies', () => {
|
||||
expect(mirror).toEqual({ created: null, moved: null, updated: [], removed: [], stamped: null });
|
||||
});
|
||||
|
||||
it('ACC-029b a night sitting behind a later afternoon is re-seated, keeping its row', () => {
|
||||
// The state the migration backfill leaves: every pre-existing stay's stop was
|
||||
// appended last, whatever its check-in says. Giving the booking an hour has to
|
||||
// move it ahead of the evening it was booked around.
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const dinner = createPlace(testDb, trip.id, { name: 'Osteria' });
|
||||
const hotel = createPlace(testDb, trip.id, { name: 'Hotel Adlon' });
|
||||
testDb.prepare("UPDATE places SET place_time = '19:00' WHERE id = ?").run(dinner.id);
|
||||
createDayAssignment(testDb, day.id, dinner.id);
|
||||
const { accommodation } = book(trip.id, hotel.id, day.id, day.id);
|
||||
const stop = stopsOn(day.id).find((row: { place_id: number }) => row.place_id === hotel.id)!;
|
||||
expect(stop.order_index).toBe(1);
|
||||
|
||||
const existing = svc.getAccommodation(accommodation.id, trip.id)!;
|
||||
const { mirror } = svc.updateAccommodation(accommodation.id, existing, { check_in: '15:00' }) as any;
|
||||
|
||||
expect(stopsOn(day.id).map((row: { place_id: number }) => row.place_id)).toEqual([hotel.id, dinner.id]);
|
||||
// Re-seated, not rebuilt: the same row, one place further forward.
|
||||
expect(mirror.moved).toMatchObject({ oldDayId: day.id, assignment: { id: stop.id } });
|
||||
expect(mirror.removed).toEqual([]);
|
||||
});
|
||||
|
||||
it('ACC-030 a stay whose stop belongs to the traveller does not get a second one', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
Reference in New Issue
Block a user