mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 05:11:46 +00:00
fix(realtime): correct assignment:created echo dedup (H11) (#1183)
When X-Idempotency/X-Socket-Id let an own-echo through, the assignment:created dedup had two bugs: it keyed on place id, so (1) a legitimate second assignment of a place already on the day was silently dropped, and (2) the temp-version reconciliation matched place?.id === placeId, letting undefined === undefined collapse place-less rows onto each other. - dedup now keys on assignment id (exact-id duplicate -> no-op) - temp (negative-id) optimistic rows are reconciled only when a real placeId matches, replacing just that row; a sibling temp of another place is untouched - everything else appends, including a genuine 2nd assignment of the same place - tests: 2nd-of-same-place kept, correct temp picked among siblings, place-less rows don't collapse Note: the broader own-echo suppression relies on X-Socket-Id being sent; this fixes the client-side fallback when an echo slips through.
This commit is contained in:
@@ -193,25 +193,34 @@ export function handleRemoteEvent(set: SetState, get: GetState, event: WebSocket
|
||||
|
||||
// Assignments
|
||||
case 'assignment:created': {
|
||||
const dayKey = String((payload.assignment as Assignment).day_id)
|
||||
const existing = (state.assignments[dayKey] || [])
|
||||
const placeId = (payload.assignment as Assignment).place?.id || (payload.assignment as Assignment).place_id
|
||||
if (existing.some(a => a.id === (payload.assignment as Assignment).id || (placeId && a.place?.id === placeId))) {
|
||||
const hasTempVersion = existing.some(a => a.id < 0 && a.place?.id === placeId)
|
||||
if (hasTempVersion) {
|
||||
return {
|
||||
assignments: {
|
||||
...state.assignments,
|
||||
[dayKey]: existing.map(a => (a.id < 0 && a.place?.id === placeId) ? payload.assignment as Assignment : a),
|
||||
}
|
||||
}
|
||||
const incoming = payload.assignment as Assignment
|
||||
const dayKey = String(incoming.day_id)
|
||||
const existing = state.assignments[dayKey] || []
|
||||
const placeId = incoming.place?.id ?? incoming.place_id
|
||||
|
||||
// Already have this exact assignment id → duplicate broadcast or the
|
||||
// echo of an already-committed assignment. No-op.
|
||||
if (existing.some(a => a.id === incoming.id)) return {}
|
||||
|
||||
// Reconcile our own optimistic create: replace the temp (negative-id)
|
||||
// assignment of the same place on this day with the real one. Guarded on
|
||||
// a real placeId so an assignment with no place can never collapse onto
|
||||
// another place-less one (undefined === undefined).
|
||||
if (placeId != null) {
|
||||
const tempIdx = existing.findIndex(a => a.id < 0 && a.place?.id === placeId)
|
||||
if (tempIdx !== -1) {
|
||||
const next = existing.slice()
|
||||
next[tempIdx] = incoming
|
||||
return { assignments: { ...state.assignments, [dayKey]: next } }
|
||||
}
|
||||
return {}
|
||||
}
|
||||
|
||||
// Genuinely new — including a legitimate second assignment of a place
|
||||
// already on this day (no temp version to reconcile). Append.
|
||||
return {
|
||||
assignments: {
|
||||
...state.assignments,
|
||||
[dayKey]: [...existing, payload.assignment as Assignment],
|
||||
[dayKey]: [...existing, incoming],
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { useTripStore } from '../../../src/store/tripStore';
|
||||
import { resetAllStores } from '../../helpers/store';
|
||||
import { buildDay, buildAssignment, buildPlace } from '../../helpers/factories';
|
||||
import type { Assignment } from '../../../src/types';
|
||||
|
||||
beforeEach(() => {
|
||||
resetAllStores();
|
||||
@@ -50,6 +51,58 @@ describe('remoteEventHandler > assignments', () => {
|
||||
expect(assignments['10'][0].id).toBe(500);
|
||||
});
|
||||
|
||||
it('FE-WSEVT-ASSIGN-003b: a second assignment of an already-present place is NOT suppressed (H11)', () => {
|
||||
const place = buildPlace({ id: 55 });
|
||||
useTripStore.setState({
|
||||
days: [buildDay({ id: 10 })],
|
||||
// A committed (positive-id) assignment of place 55 already on the day.
|
||||
assignments: { '10': [buildAssignment({ id: 100, day_id: 10, place, place_id: place.id })] },
|
||||
});
|
||||
// A legitimately new, distinct assignment of the same place arrives.
|
||||
const second = buildAssignment({ id: 300, day_id: 10, place, place_id: place.id });
|
||||
useTripStore.getState().handleRemoteEvent({ type: 'assignment:created', assignment: second });
|
||||
const { assignments } = useTripStore.getState();
|
||||
expect(assignments['10']).toHaveLength(2);
|
||||
expect(assignments['10'].map(a => a.id).sort((x, y) => x - y)).toEqual([100, 300]);
|
||||
});
|
||||
|
||||
it('FE-WSEVT-ASSIGN-003c: temp reconciliation replaces only the matching place, not a sibling temp (H11)', () => {
|
||||
const place55 = buildPlace({ id: 55 });
|
||||
const place66 = buildPlace({ id: 66 });
|
||||
useTripStore.setState({
|
||||
days: [buildDay({ id: 10 })],
|
||||
assignments: {
|
||||
'10': [
|
||||
buildAssignment({ id: -1, day_id: 10, place: place55, place_id: 55 }),
|
||||
buildAssignment({ id: -2, day_id: 10, place: place66, place_id: 66 }),
|
||||
],
|
||||
},
|
||||
});
|
||||
const real = buildAssignment({ id: 500, day_id: 10, place: place55, place_id: 55 });
|
||||
useTripStore.getState().handleRemoteEvent({ type: 'assignment:created', assignment: real });
|
||||
const { assignments } = useTripStore.getState();
|
||||
const ids = assignments['10'].map(a => a.id);
|
||||
expect(assignments['10']).toHaveLength(2);
|
||||
expect(ids).toContain(500); // temp 55 reconciled to real
|
||||
expect(ids).toContain(-2); // sibling temp 66 untouched
|
||||
expect(ids).not.toContain(-1);
|
||||
});
|
||||
|
||||
it('FE-WSEVT-ASSIGN-003d: place-less assignments do not collapse onto each other (H11)', () => {
|
||||
// Defensive: a malformed event lacking place data must not let the
|
||||
// `place?.id === placeId` reconciliation match undefined === undefined.
|
||||
const placeless = (id: number): Assignment =>
|
||||
({ ...buildAssignment({ id, day_id: 10 }), place: undefined, place_id: undefined } as unknown as Assignment);
|
||||
useTripStore.setState({
|
||||
days: [buildDay({ id: 10 })],
|
||||
assignments: { '10': [placeless(-1)] },
|
||||
});
|
||||
useTripStore.getState().handleRemoteEvent({ type: 'assignment:created', assignment: placeless(700) });
|
||||
const { assignments } = useTripStore.getState();
|
||||
// No placeId → no reconcile; both survive as distinct rows (no collapse).
|
||||
expect(assignments['10']).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('FE-WSEVT-ASSIGN-004: assignment:updated merges updated data into correct day', () => {
|
||||
seedData();
|
||||
const updated = buildAssignment({ id: 100, day_id: 10, notes: 'Updated notes' });
|
||||
|
||||
Reference in New Issue
Block a user