fix(tours): let only the latest read set a collection's kept tours

Two reads of the same collection, after two changes by other members or
the first read overlapping one, could answer in either order and an older
list replace a newer one. A refused removal also restored the list from
before it, dropping changes read meanwhile; it now only puts its own tour
back.
This commit is contained in:
Maurice
2026-10-20 16:56:15 +02:00
parent 839d736905
commit 981c372640
2 changed files with 54 additions and 5 deletions
+30 -1
View File
@@ -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<undefined>();
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);
+24 -4
View File
@@ -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<void>;
/** Removes one kept tour; a refusal puts it back and throws. */
remove: (collectionId: number, tourId: number) => Promise<void>;
@@ -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<CollectionTourState>((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 })),
}));
/**