From 44cc376e1b0398f8824a196252c1425fda4e8b32 Mon Sep 17 00:00:00 2001 From: Nico Date: Thu, 4 Jun 2026 04:40:09 -0700 Subject: [PATCH] fix: ignore stale map load responses --- frontend/src/pages/LocationMapManager.jsx | 12 +- frontend/tests/location-map-manager.spec.ts | 115 +++++++++++++++++++- 2 files changed, 124 insertions(+), 3 deletions(-) diff --git a/frontend/src/pages/LocationMapManager.jsx b/frontend/src/pages/LocationMapManager.jsx index e21bc53..a4b8fdc 100644 --- a/frontend/src/pages/LocationMapManager.jsx +++ b/frontend/src/pages/LocationMapManager.jsx @@ -76,6 +76,7 @@ export default function LocationMapManager() { const [pendingLeave, setPendingLeave] = useState(false); const svgRef = useRef(null); const toastRef = useRef(toast); + const loadRequestRef = useRef(0); const location = mapState?.location || stores.find((store) => String(store.id) === String(locationId)); const canManage = Boolean(mapState?.can_manage ?? ["owner", "admin"].includes(activeHousehold?.role)); @@ -158,6 +159,9 @@ export default function LocationMapManager() { }, []); const loadMap = useCallback(async () => { + const requestId = loadRequestRef.current + 1; + loadRequestRef.current = requestId; + if (!activeHousehold?.id || !locationId) { setLoading(false); return; @@ -167,6 +171,8 @@ export default function LocationMapManager() { setLoadError(null); try { const response = await getLocationMap(activeHousehold.id, locationId); + if (loadRequestRef.current !== requestId) return; + const nextState = response.data; const nextCanManage = Boolean( nextState.can_manage ?? ["owner", "admin"].includes(activeHousehold?.role) @@ -218,6 +224,8 @@ export default function LocationMapManager() { !nextState.published_map ); } catch (error) { + if (loadRequestRef.current !== requestId) return; + const message = getApiErrorMessage(error, "Failed to load map"); toastRef.current.error("Load map failed", message); setMapState(null); @@ -228,7 +236,9 @@ export default function LocationMapManager() { setHasUnsavedChanges(false); setLoadError(message); } finally { - setLoading(false); + if (loadRequestRef.current === requestId) { + setLoading(false); + } } }, [activeHousehold?.id, activeHousehold?.role, locationId, syncMapDraftFromState]); diff --git a/frontend/tests/location-map-manager.spec.ts b/frontend/tests/location-map-manager.spec.ts index 0fa01d3..9a56d10 100644 --- a/frontend/tests/location-map-manager.spec.ts +++ b/frontend/tests/location-map-manager.spec.ts @@ -12,6 +12,12 @@ const storeLocation = { display_name: "Costco - Eastvale", is_default: true, }; +const secondStoreLocation = { + ...storeLocation, + id: 11, + location_name: "Ontario", + display_name: "Costco - Ontario", +}; const zones = [ { id: 501, name: "Bakery", sort_order: 10, item_count: 2 }, { id: 502, name: "Frozen Foods", sort_order: 20, item_count: 1 }, @@ -146,7 +152,11 @@ function draftAndPublishedMapState( }; } -async function mockMapShell(page: Page, household = adminHousehold) { +async function mockMapShell( + page: Page, + household = adminHousehold, + locations: Array = [storeLocation] +) { await seedAuthStorage(page, { username: "map-user", role: household.role }); await mockConfig(page); @@ -162,7 +172,7 @@ async function mockMapShell(page: Page, household = adminHousehold) { await route.fulfill({ status: 200, contentType: "application/json", - body: JSON.stringify([storeLocation]), + body: JSON.stringify(locations), }); }); @@ -470,6 +480,107 @@ test("load failure shows retryable error instead of no-map setup", async ({ page expect(attempts).toBe(2); }); +test("ignores stale map responses after switching locations", async ({ page }) => { + await mockMapShell(page, adminHousehold, [storeLocation, secondStoreLocation]); + + const oldLocationState = publishedMapState([ + { + id: 471, + location_map_id: 901, + zone_id: 501, + zone_name: "Bakery", + type: "zone", + label: "Old Bakery", + x: 40, + y: 40, + width: 260, + height: 160, + rotation: 0, + locked: false, + visible: true, + sort_order: 1, + }, + ], true); + const nextLocationBaseState = publishedMapState([ + { + id: 472, + location_map_id: 902, + zone_id: 502, + zone_name: "Produce", + type: "zone", + label: "Ontario Produce", + x: 80, + y: 80, + width: 280, + height: 160, + rotation: 0, + locked: false, + visible: true, + sort_order: 1, + }, + ], true); + const nextPublishedMap = { + ...nextLocationBaseState.published_map, + id: 902, + store_location_id: secondStoreLocation.id, + }; + const nextLocationState = { + ...nextLocationBaseState, + location: secondStoreLocation, + map: nextPublishedMap, + published_map: nextPublishedMap, + }; + + let resolveFirstRequestStarted: () => void = () => {}; + let releaseFirstResponse: () => void = () => {}; + let resolveFirstResponseSent: () => void = () => {}; + const firstRequestStarted = new Promise((resolve) => { + resolveFirstRequestStarted = resolve; + }); + const firstResponseGate = new Promise((resolve) => { + releaseFirstResponse = resolve; + }); + const firstResponseSent = new Promise((resolve) => { + resolveFirstResponseSent = resolve; + }); + + await page.route("**/households/1/locations/10/map", async (route) => { + resolveFirstRequestStarted(); + await firstResponseGate; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify(oldLocationState), + }); + resolveFirstResponseSent(); + }); + await page.route("**/households/1/locations/11/map", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify(nextLocationState), + }); + }); + + await page.goto("/stores/100/locations/10/map"); + await firstRequestStarted; + + await page.evaluate(() => { + window.history.pushState({}, "", "/stores/100/locations/11/map"); + window.dispatchEvent(new PopStateEvent("popstate")); + }); + + await expect(page.getByRole("button", { name: "Map area Ontario Produce" })).toBeVisible(); + await expect(page.locator(".location-map-title span")).toHaveText("Ontario"); + + releaseFirstResponse(); + await firstResponseSent; + + await expect(page.getByRole("button", { name: "Map area Ontario Produce" })).toBeVisible(); + await expect(page.getByRole("button", { name: "Map area Old Bakery" })).toHaveCount(0); + await expect(page.locator(".location-map-title span")).toHaveText("Ontario"); +}); + test("admin can add the first area from a blank map canvas", async ({ page }) => { await mockMapShell(page);