refactor: one radius-search fetcher behind the three merchant-list reducers (#1171) (#1196)

escapedcat · claude · web-flow · commit bf5b4f7efab0 · 2026-08-04T14:48:15.000+02:00
* refactor(merchant-list): one radius-search fetcher behind the three list reducers #1171 fetchAndReplaceList, fetchCountOnly, and fetchEnrichedDetails each built the same /v4/places/search/ URL with their own copy of the transport policy and validation. A private searchPlacesInRadius now owns the URL shape, the 10s timeout, the array-shape validation, and the invalid-row drop; each reducer keeps its own row policy and failure loudness. The enrichment path gets the same explicit array-shape check as the other two — previously a non-array response there failed only via an incidental TypeError inside filterValidPlaces (same observable outcome: caught, warned, cache untouched); the explicit invariant plus a new test pin the behavior instead of relying on the accident. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(merchant-list): actionable message for non-array search responses #1171 Copilot follow-up: say what was expected and what actually arrived (typeof only — no payload dump) so the console warning is debuggable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(merchant-list): restore the console.warn spy even on failure #1171 qodo follow-up: the suite's beforeEach clears mocks but doesn't restore spied globals, so an assertion failing before mockRestore() would leave console.warn mocked for later tests. try/finally guarantees restoration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
diff --git a/src/lib/merchantListStore.test.ts b/src/lib/merchantListStore.test.ts
@@ -674,6 +674,30 @@ describe("merchantListStore", () => {
 
 			await fetchPromise;
 		});
+
+		it("should leave the cache untouched on a non-array response", async () => {
+			// try/finally: a failing assertion must not leave console.warn
+			// mocked for later tests (beforeEach clears, but doesn't restore)
+			const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+			try {
+				// Seed the cache through the list path
+				const initialPlace = createMockPlace({ id: 1 });
+				(api.get as Mock).mockResolvedValueOnce({ data: [initialPlace] });
+				await merchantList.fetchAndReplaceList({ lat: 0, lon: 0 }, 10);
+
+				// An HTML error page served with 200 must not poison the cache
+				(api.get as Mock).mockResolvedValueOnce({ data: "<html>oops</html>" });
+				await merchantList.fetchEnrichedDetails({ lat: 0, lon: 0 }, 10);
+
+				const state = get(merchantList);
+				expect(state.placeDetailsCache.has(1)).toBe(true);
+				expect(state.placeDetailsCache.size).toBe(1);
+				expect(state.isEnrichingDetails).toBe(false);
+				expect(warnSpy).toHaveBeenCalled();
+			} finally {
+				warnSpy.mockRestore();
+			}
+		});
 	});
 
 	describe("request cancellation", () => {
diff --git a/src/lib/merchantListStore.ts b/src/lib/merchantListStore.ts
@@ -128,6 +128,30 @@ function filterValidPlaces<T extends { id?: unknown }>(items: T[]): T[] {
 	return items.filter((item): item is T => typeof item?.id === "number");
 }
 
+// The one radius-search fetcher behind the three list reducers
+// (fetchAndReplaceList, fetchCountOnly, fetchEnrichedDetails). Owns the URL
+// shape, the 10s transport policy, the array-shape validation (the API can
+// return an HTML error page), and the dropping of rows without a numeric id.
+// What each reducer does with the rows — and how loudly it fails — stays
+// that reducer's own policy.
+async function searchPlacesInRadius<T extends { id?: unknown }>(
+	center: { lat: number; lon: number },
+	radiusKm: number,
+	fields: string,
+	signal: AbortSignal,
+): Promise<T[]> {
+	const response = await api.get<T[]>(
+		`${API_BASE}/v4/places/search/?lat=${center.lat}&lon=${center.lon}&radius_km=${radiusKm}&fields=${fields}`,
+		{ timeout: 10000, signal },
+	);
+	if (!Array.isArray(response.data)) {
+		throw new Error(
+			`Radius search returned invalid data: expected an array, got ${typeof response.data}`,
+		);
+	}
+	return filterValidPlaces(response.data);
+}
+
 function createMerchantListStore() {
 	const store = writable<MerchantListState>(initialState);
 	const { subscribe, set, update } = store;
@@ -229,20 +253,13 @@ function createMerchantListStore() {
 			update((state) => ({ ...state, isLoadingList: true }));
 
 			try {
-				const fields = buildFieldsParam(PLACE_FIELD_SETS.LIST_ITEM);
-				const response = await api.get<Place[]>(
-					`${API_BASE}/v4/places/search/?lat=${center.lat}&lon=${center.lon}&radius_km=${radiusKm}&fields=${fields}`,
-					{ timeout: 10000, signal: listAbortController.signal },
+				const validPlaces = await searchPlacesInRadius<Place>(
+					center,
+					radiusKm,
+					buildFieldsParam(PLACE_FIELD_SETS.LIST_ITEM),
+					listAbortController.signal,
 				);
 
-				// Validate response is an array (API may return HTML error page)
-				if (!Array.isArray(response.data)) {
-					throw new Error("API returned invalid data format");
-				}
-
-				// Filter out invalid items missing required id field
-				const validPlaces = filterValidPlaces(response.data);
-
 				// Build cache for enriched display (icons, addresses, etc.)
 				const placeDetailsCache = new Map<number, Place>();
 				validPlaces.forEach((place) => placeDetailsCache.set(place.id, place));
@@ -342,18 +359,9 @@ function createMerchantListStore() {
 			try {
 				// Typed to the payload actually requested — these rows are not
 				// full Places and must not be handed to anything expecting one.
-				const response = await api.get<Pick<Place, "id" | "verified_at">[]>(
-					`${API_BASE}/v4/places/search/?lat=${center.lat}&lon=${center.lon}&radius_km=${radiusKm}&fields=${fields}`,
-					{ timeout: 10000, signal: listAbortController.signal },
-				);
-
-				// Validate response is an array (API may return HTML error page)
-				if (!Array.isArray(response.data)) {
-					throw new Error("API returned invalid data format");
-				}
-
-				// Filter out invalid items missing required id field
-				const validItems = filterValidPlaces(response.data);
+				const validItems = await searchPlacesInRadius<
+					Pick<Place, "id" | "verified_at">
+				>(center, radiusKm, fields, listAbortController.signal);
 				const recencyPlaces = filterPlacesByRecency(
 					validItems,
 					verifiedWithinYears,
@@ -397,14 +405,14 @@ function createMerchantListStore() {
 			update((state) => ({ ...state, isEnrichingDetails: true }));
 
 			try {
-				const fields = buildFieldsParam(PLACE_FIELD_SETS.LIST_ITEM);
-				const response = await api.get<Place[]>(
-					`${API_BASE}/v4/places/search/?lat=${center.lat}&lon=${center.lon}&radius_km=${radiusKm}&fields=${fields}`,
-					{ timeout: 10000, signal: detailsAbortController.signal },
+				const validPlaces = await searchPlacesInRadius<Place>(
+					center,
+					radiusKm,
+					buildFieldsParam(PLACE_FIELD_SETS.LIST_ITEM),
+					detailsAbortController.signal,
 				);
 
-				// Filter out invalid items and merge into existing cache
-				const validPlaces = filterValidPlaces(response.data);
+				// Merge into existing cache
 				update((state) => {
 					const mergedCache = new Map(state.placeDetailsCache);
 					validPlaces.forEach((place) => mergedCache.set(place.id, place));
