From d682cda399f68def5d78e013d45000445599fdbf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 17 May 2026 15:25:17 +0000 Subject: [PATCH 1/3] fix(sharing): extract producers array from wrapped API response in fetchDrinkData MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The data API returns {"producers": [...], "timestamp": ...} but fetchDrinkData was returning the whole object. findDrink then called for...of on a plain object, which throws TypeError: not iterable — silently falling back to the unmodified SPA for all crawlers. Fix: extract data.producers when the response is an object, fall back to treating the response as an array for forward-compatibility. Add 5 missing unit tests for fetchDrinkData covering the wrapped format, bare-array format, 404 responses, missing producers key, and URL construction. https://claude.ai/code/session_014n8NH3HkZQUkGGjK2geKYg --- functions/_lib/drink-preview.js | 3 +- functions/test/drink-preview.test.js | 56 +++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/functions/_lib/drink-preview.js b/functions/_lib/drink-preview.js index cea56633..d5118e97 100644 --- a/functions/_lib/drink-preview.js +++ b/functions/_lib/drink-preview.js @@ -62,5 +62,6 @@ export async function fetchDrinkData(festivalId, category) { const url = `${DATA_BASE_URL}/${encodeURIComponent(festivalId)}/${encodeURIComponent(category)}.json`; const response = await fetch(url); if (!response.ok) return null; - return response.json(); + const data = await response.json(); + return Array.isArray(data) ? data : (data.producers ?? null); } diff --git a/functions/test/drink-preview.test.js b/functions/test/drink-preview.test.js index 75d18e52..14463045 100644 --- a/functions/test/drink-preview.test.js +++ b/functions/test/drink-preview.test.js @@ -1,5 +1,5 @@ -import { describe, it, expect } from 'vitest'; -import { isCrawler, findDrink, buildOgTags } from '../_lib/drink-preview.js'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { isCrawler, findDrink, buildOgTags, fetchDrinkData } from '../_lib/drink-preview.js'; const TEST_PRODUCERS = [ { @@ -194,3 +194,55 @@ describe('buildOgTags', () => { }); }); +describe('fetchDrinkData', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns producers array from wrapped API response', async () => { + const producers = [{ id: 'adnams', name: 'Adnams', products: [] }]; + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ producers, timestamp: '2025-01-01' }), + }); + const result = await fetchDrinkData('cbf2025', 'beer'); + expect(result).toEqual(producers); + }); + + it('returns array directly when API response is already an array', async () => { + const producers = [{ id: 'adnams', name: 'Adnams', products: [] }]; + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve(producers), + }); + const result = await fetchDrinkData('cbf2025', 'beer'); + expect(result).toEqual(producers); + }); + + it('returns null when response is not ok', async () => { + global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 404 }); + const result = await fetchDrinkData('cbf2025', 'beer'); + expect(result).toBeNull(); + }); + + it('returns null when wrapped response has no producers key', async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ timestamp: '2025-01-01' }), + }); + const result = await fetchDrinkData('cbf2025', 'beer'); + expect(result).toBeNull(); + }); + + it('fetches the correct URL for a given festival and category', async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ producers: [] }), + }); + await fetchDrinkData('cbf2025', 'beer'); + expect(global.fetch).toHaveBeenCalledWith( + 'https://data.cambeerfestival.app/cbf2025/beer.json', + ); + }); +}); + From c98aaaef2f8f077942918079486a586d6401d951 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 17 May 2026 15:30:17 +0000 Subject: [PATCH 2/3] test(sharing): add handler integration tests for drink OG preview function Tests the full onRequest pipeline that was previously untested: - Non-crawler passthrough (ASSETS.fetch called, no data fetch, no OG tags) - Crawler OG injection (og:title, og:url, correct festival/category URL) - Fallback paths (non-ok API, network error, drink not found) Uses a minimal MockHTMLRewriter that appends to without requiring the Cloudflare Workers runtime or any new dependencies. https://claude.ai/code/session_014n8NH3HkZQUkGGjK2geKYg --- functions/test/handler.test.js | 155 +++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 functions/test/handler.test.js diff --git a/functions/test/handler.test.js b/functions/test/handler.test.js new file mode 100644 index 00000000..c526dd41 --- /dev/null +++ b/functions/test/handler.test.js @@ -0,0 +1,155 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { onRequest } from '../[festivalId]/drink/[category]/[drinkId].js'; + +// Minimal HTMLRewriter mock: collects appended HTML and inserts it before . +class MockHTMLRewriter { + constructor() { + this._headHandler = null; + } + on(selector, handler) { + if (selector === 'head') this._headHandler = handler; + return this; + } + async transform(response) { + const text = await response.text(); + let injected = ''; + if (this._headHandler) { + // Second arg ({ html: true }) is intentionally ignored in this mock. + const element = { append: (html) => { injected += html; } }; + this._headHandler.element(element); + } + return new Response(text.replace('', injected + ''), { + status: response.status, + headers: response.headers, + }); + } +} + +const SPA_HTML = 'CBF'; + +const TEST_PRODUCERS = [ + { + id: 'adnams', + name: 'Adnams', + location: 'Southwold', + products: [ + { id: 'broadside', name: 'Broadside', style: 'Strong Bitter', abv: 6.3, category: 'beer' }, + ], + }, +]; + +function makeSpaResponse() { + return new Response(SPA_HTML, { headers: { 'Content-Type': 'text/html' } }); +} + +function makeContext({ ua, festivalId = 'cbf2025', category = 'beer', drinkId = 'broadside' } = {}) { + const url = `https://cambeerfestival.app/${festivalId}/drink/${category}/${drinkId}`; + return { + request: new Request(url, { + headers: { 'User-Agent': ua ?? 'Mozilla/5.0 Chrome/120' }, + }), + env: { + ASSETS: { fetch: vi.fn().mockResolvedValue(makeSpaResponse()) }, + }, + params: { festivalId, category, drinkId }, + }; +} + +beforeEach(() => { + global.HTMLRewriter = MockHTMLRewriter; + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ producers: TEST_PRODUCERS }), + }); +}); + +afterEach(() => { + vi.restoreAllMocks(); + delete global.HTMLRewriter; +}); + +describe('onRequest — non-crawler passthrough', () => { + it('calls ASSETS.fetch and returns immediately for a regular browser', async () => { + const ctx = makeContext({ ua: 'Mozilla/5.0 Chrome/120' }); + await onRequest(ctx); + expect(ctx.env.ASSETS.fetch).toHaveBeenCalledWith(ctx.request); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it('does not inject OG tags for a regular browser', async () => { + const ctx = makeContext({ ua: 'Mozilla/5.0 Chrome/120' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).not.toContain('og:title'); + }); +}); + +describe('onRequest — crawler OG injection', () => { + it('injects og:title with drink name and brewery for Googlebot', async () => { + const ctx = makeContext({ ua: 'Googlebot/2.1' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).toContain('og:title'); + expect(text).toContain('Broadside — Adnams'); + }); + + it('injects og:url with the canonical drink URL', async () => { + const ctx = makeContext({ ua: 'Googlebot/2.1', festivalId: 'cbf2025', category: 'beer', drinkId: 'broadside' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).toContain('https://cambeerfestival.app/cbf2025/drink/beer/broadside'); + }); + + it('injects OG tags for WhatsApp crawler', async () => { + const ctx = makeContext({ ua: 'WhatsApp/2.19.81 A' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).toContain('og:title'); + }); + + it('fetches drink data from the correct festival and category', async () => { + const ctx = makeContext({ ua: 'Googlebot/2.1', festivalId: 'cbf2024', category: 'cider', drinkId: 'broadside' }); + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ producers: TEST_PRODUCERS }), + }); + await onRequest(ctx); + expect(global.fetch).toHaveBeenCalledWith( + expect.stringContaining('cbf2024/cider.json'), + ); + }); + + it('preserves the rest of the SPA HTML when injecting', async () => { + const ctx = makeContext({ ua: 'Googlebot/2.1' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).toContain('CBF'); + expect(text).toContain(''); + }); +}); + +describe('onRequest — crawler fallback paths', () => { + it('returns unmodified SPA when API responds with non-ok status', async () => { + global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 404 }); + const ctx = makeContext({ ua: 'Googlebot/2.1' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).not.toContain('og:title'); + expect(text).toBe(SPA_HTML); + }); + + it('returns unmodified SPA when fetch throws a network error', async () => { + global.fetch = vi.fn().mockRejectedValue(new Error('Network failure')); + const ctx = makeContext({ ua: 'Googlebot/2.1' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).not.toContain('og:title'); + }); + + it('returns unmodified SPA when the drink ID is not found', async () => { + const ctx = makeContext({ ua: 'Googlebot/2.1', drinkId: 'nonexistent' }); + const response = await onRequest(ctx); + const text = await response.text(); + expect(text).not.toContain('og:title'); + }); +}); From 8acc8dbf897a345fe9f77b7cdecda0053bebfd7e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 17 May 2026 15:38:07 +0000 Subject: [PATCH 3/3] fix(sharing): harden fetchDrinkData shape validation and fix global mock leak fetchDrinkData now guards against data.producers being non-array and data being null, returning null in both cases rather than returning an uninitable value that would make findDrink throw. Test hygiene: replace global.fetch/HTMLRewriter direct assignment with vi.stubGlobal/vi.unstubAllGlobals so stubs are properly restored between test files without relying on vi.restoreAllMocks (which doesn't restore globalThis properties set by direct assignment). Two new fetchDrinkData cases: null JSON body and non-array producers field. https://claude.ai/code/session_014n8NH3HkZQUkGGjK2geKYg --- functions/_lib/drink-preview.js | 4 ++- functions/test/drink-preview.test.js | 39 +++++++++++++++++++++------- functions/test/handler.test.js | 18 ++++++------- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/functions/_lib/drink-preview.js b/functions/_lib/drink-preview.js index d5118e97..0266624c 100644 --- a/functions/_lib/drink-preview.js +++ b/functions/_lib/drink-preview.js @@ -63,5 +63,7 @@ export async function fetchDrinkData(festivalId, category) { const response = await fetch(url); if (!response.ok) return null; const data = await response.json(); - return Array.isArray(data) ? data : (data.producers ?? null); + if (Array.isArray(data)) return data; + if (data && Array.isArray(data.producers)) return data.producers; + return null; } diff --git a/functions/test/drink-preview.test.js b/functions/test/drink-preview.test.js index 14463045..8a9004c3 100644 --- a/functions/test/drink-preview.test.js +++ b/functions/test/drink-preview.test.js @@ -196,51 +196,70 @@ describe('buildOgTags', () => { describe('fetchDrinkData', () => { afterEach(() => { - vi.restoreAllMocks(); + vi.unstubAllGlobals(); }); it('returns producers array from wrapped API response', async () => { const producers = [{ id: 'adnams', name: 'Adnams', products: [] }]; - global.fetch = vi.fn().mockResolvedValue({ + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve({ producers, timestamp: '2025-01-01' }), - }); + })); const result = await fetchDrinkData('cbf2025', 'beer'); expect(result).toEqual(producers); }); it('returns array directly when API response is already an array', async () => { const producers = [{ id: 'adnams', name: 'Adnams', products: [] }]; - global.fetch = vi.fn().mockResolvedValue({ + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve(producers), - }); + })); const result = await fetchDrinkData('cbf2025', 'beer'); expect(result).toEqual(producers); }); it('returns null when response is not ok', async () => { - global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 404 }); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 404 })); const result = await fetchDrinkData('cbf2025', 'beer'); expect(result).toBeNull(); }); it('returns null when wrapped response has no producers key', async () => { - global.fetch = vi.fn().mockResolvedValue({ + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve({ timestamp: '2025-01-01' }), - }); + })); + const result = await fetchDrinkData('cbf2025', 'beer'); + expect(result).toBeNull(); + }); + + it('returns null when response.json() resolves to null', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve(null), + })); + const result = await fetchDrinkData('cbf2025', 'beer'); + expect(result).toBeNull(); + }); + + it('returns null when producers field exists but is not an array', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ producers: 'not-an-array' }), + })); const result = await fetchDrinkData('cbf2025', 'beer'); expect(result).toBeNull(); }); it('fetches the correct URL for a given festival and category', async () => { - global.fetch = vi.fn().mockResolvedValue({ + const mockFetch = vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve({ producers: [] }), }); + vi.stubGlobal('fetch', mockFetch); await fetchDrinkData('cbf2025', 'beer'); - expect(global.fetch).toHaveBeenCalledWith( + expect(mockFetch).toHaveBeenCalledWith( 'https://data.cambeerfestival.app/cbf2025/beer.json', ); }); diff --git a/functions/test/handler.test.js b/functions/test/handler.test.js index c526dd41..3f72247b 100644 --- a/functions/test/handler.test.js +++ b/functions/test/handler.test.js @@ -56,16 +56,15 @@ function makeContext({ ua, festivalId = 'cbf2025', category = 'beer', drinkId = } beforeEach(() => { - global.HTMLRewriter = MockHTMLRewriter; - global.fetch = vi.fn().mockResolvedValue({ + vi.stubGlobal('HTMLRewriter', MockHTMLRewriter); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve({ producers: TEST_PRODUCERS }), - }); + })); }); afterEach(() => { - vi.restoreAllMocks(); - delete global.HTMLRewriter; + vi.unstubAllGlobals(); }); describe('onRequest — non-crawler passthrough', () => { @@ -109,12 +108,13 @@ describe('onRequest — crawler OG injection', () => { it('fetches drink data from the correct festival and category', async () => { const ctx = makeContext({ ua: 'Googlebot/2.1', festivalId: 'cbf2024', category: 'cider', drinkId: 'broadside' }); - global.fetch = vi.fn().mockResolvedValue({ + const mockFetch = vi.fn().mockResolvedValue({ ok: true, json: () => Promise.resolve({ producers: TEST_PRODUCERS }), }); + vi.stubGlobal('fetch', mockFetch); await onRequest(ctx); - expect(global.fetch).toHaveBeenCalledWith( + expect(mockFetch).toHaveBeenCalledWith( expect.stringContaining('cbf2024/cider.json'), ); }); @@ -130,7 +130,7 @@ describe('onRequest — crawler OG injection', () => { describe('onRequest — crawler fallback paths', () => { it('returns unmodified SPA when API responds with non-ok status', async () => { - global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 404 }); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 404 })); const ctx = makeContext({ ua: 'Googlebot/2.1' }); const response = await onRequest(ctx); const text = await response.text(); @@ -139,7 +139,7 @@ describe('onRequest — crawler fallback paths', () => { }); it('returns unmodified SPA when fetch throws a network error', async () => { - global.fetch = vi.fn().mockRejectedValue(new Error('Network failure')); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('Network failure'))); const ctx = makeContext({ ua: 'Googlebot/2.1' }); const response = await onRequest(ctx); const text = await response.text();