diff --git a/client/src/store/collectionTourStore.test.ts b/client/src/store/collectionTourStore.test.ts index 2fa976cc1..60960fcb6 100644 --- a/client/src/store/collectionTourStore.test.ts +++ b/client/src/store/collectionTourStore.test.ts @@ -8,7 +8,7 @@ import { OnlineOnlyError } from '../repo/onlineOnly'; import type { WebSocketEvent } from '../types'; import { useCollectionTourStore, watchCollectionTours } from './collectionTourStore'; -// FE-STORE-COLLECTION-TOURS-001 to FE-STORE-COLLECTION-TOURS-005 +// FE-STORE-COLLECTION-TOURS-001 to FE-STORE-COLLECTION-TOURS-007 const tour = (id: number, name: string) => ({ id, collection_id: 3, name }) as CollectionTourItem; @@ -69,6 +69,35 @@ describe('collectionTourStore', () => { expect(useCollectionTourStore.getState().tours.map((kept) => kept.id)).toEqual([2]); }); + it('FE-STORE-COLLECTION-TOURS-006: of two reads of the same collection, an older answer coming last counts for nothing', async () => { + const older = later<{ tours: CollectionTourItem[] }>(); + const newer = later<{ tours: CollectionTourItem[] }>(); + vi.spyOn(collectionToursRepo, 'list').mockReturnValueOnce(older.promise).mockReturnValueOnce(newer.promise); + const first = useCollectionTourStore.getState().load(3); + const second = useCollectionTourStore.getState().load(3); + newer.settle({ tours: [tour(1, 'Ridge'), tour(2, 'Lake')] }); + await second; + older.settle({ tours: [tour(1, 'Ridge')] }); + await first; + expect(useCollectionTourStore.getState().tours.map((kept) => kept.id)).toEqual([1, 2]); + }); + + it('FE-STORE-COLLECTION-TOURS-007: a refused removal puts back only its tour, keeping a list read meanwhile', async () => { + const both = [tour(1, 'Ridge'), tour(2, 'Lake')]; + const list = vi.spyOn(collectionToursRepo, 'list').mockResolvedValue({ tours: both }); + await useCollectionTourStore.getState().load(3); + const refusal = later(); + const refused = refusal.promise.then(() => Promise.reject(new Error('403'))); + vi.spyOn(collectionToursRepo, 'remove').mockReturnValueOnce(refused); + const removing = useCollectionTourStore.getState().remove(3, 1); + // Another member kept a tour meanwhile; the list read for it no longer holds the tour being removed. + list.mockResolvedValueOnce({ tours: [tour(2, 'Lake'), tour(3, 'Summit')] }); + await useCollectionTourStore.getState().load(3); + refusal.settle(undefined); + await expect(removing).rejects.toThrow('403'); + expect(useCollectionTourStore.getState().tours.map((kept) => kept.id)).toEqual([1, 2, 3]); + }); + it('FE-STORE-COLLECTION-TOURS-005: reads again when a member changes the open collection, and stops listening', async () => { const list = vi.spyOn(collectionToursRepo, 'list').mockResolvedValue({ tours: [] }); await useCollectionTourStore.getState().load(3); diff --git a/client/src/store/collectionTourStore.ts b/client/src/store/collectionTourStore.ts index 9e3d34185..43fc22fe8 100644 --- a/client/src/store/collectionTourStore.ts +++ b/client/src/store/collectionTourStore.ts @@ -13,6 +13,8 @@ interface CollectionTourState { collectionId: number | null; tours: CollectionTourItem[]; status: CollectionTourStatus; + /** Raised by every read: only the answer to the latest one is taken in. */ + loadSeq: number; load: (collectionId: number) => Promise; /** Removes one kept tour; a refusal puts it back and throws. */ remove: (collectionId: number, tourId: number) => Promise; @@ -23,32 +25,50 @@ interface CollectionTourState { * LIB-03: the whole tours kept in the collection that is open. Read when it * opens, again when another member keeps or removes one (watchCollectionTours), * and never shown under another collection, not even while that one loads. + * Reads can overlap (two members' changes in a row, the first read and a + * change); only the latest one's answer counts, so an older list never + * replaces a newer one. */ + +/** The list with a tour put back where it stood, unless it is there already. */ +function withPutBack(tours: CollectionTourItem[], tour: CollectionTourItem, at: number): CollectionTourItem[] { + if (tours.some((item) => item.id === tour.id)) return tours; + const index = Math.min(Math.max(at, 0), tours.length); + return [...tours.slice(0, index), tour, ...tours.slice(index)]; +} + export const useCollectionTourStore = create((set, get) => ({ collectionId: null, tours: [], status: 'idle', + loadSeq: 0, load: async (collectionId) => { + const seq = get().loadSeq + 1; + set({ loadSeq: seq }); // Another collection's tours never show under this one. if (get().collectionId !== collectionId) set({ collectionId, tours: [], status: 'loading' }); + const current = () => get().loadSeq === seq && get().collectionId === collectionId; try { const { tours } = await collectionToursRepo.list(collectionId); - if (get().collectionId === collectionId) set({ tours, status: 'ready' }); + if (current()) set({ tours, status: 'ready' }); } catch (err: unknown) { - if (get().collectionId === collectionId) set({ status: err instanceof OnlineOnlyError ? 'offline' : 'error' }); + if (current()) set({ status: err instanceof OnlineOnlyError ? 'offline' : 'error' }); } }, remove: async (collectionId, tourId) => { const before = get().tours; + const at = before.findIndex((tour) => tour.id === tourId); + const removed = before[at]; if (get().collectionId === collectionId) set({ tours: before.filter((tour) => tour.id !== tourId) }); try { await collectionToursRepo.remove(collectionId, tourId); } catch (err) { - if (get().collectionId === collectionId) set({ tours: before }); + // Only the removed tour comes back: a list read meanwhile keeps the other members' changes. + if (get().collectionId === collectionId && removed) set({ tours: withPutBack(get().tours, removed, at) }); throw err; } }, - clear: () => set({ collectionId: null, tours: [], status: 'idle' }), + clear: () => set((state) => ({ collectionId: null, tours: [], status: 'idle', loadSeq: state.loadSeq + 1 })), })); /**