Skip to content

Commit bfa5208

Browse files
fix(sharing): extract producers array from wrapped API response in fetchDrinkData (#286)
* fix(sharing): extract producers array from wrapped API response in fetchDrinkData 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 * 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 </head> without requiring the Cloudflare Workers runtime or any new dependencies. https://claude.ai/code/session_014n8NH3HkZQUkGGjK2geKYg * 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 90904df commit bfa5208

3 files changed

Lines changed: 232 additions & 3 deletions

File tree

functions/_lib/drink-preview.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,5 +62,8 @@ export async function fetchDrinkData(festivalId, category) {
6262
const url = `${DATA_BASE_URL}/${encodeURIComponent(festivalId)}/${encodeURIComponent(category)}.json`;
6363
const response = await fetch(url);
6464
if (!response.ok) return null;
65-
return response.json();
65+
const data = await response.json();
66+
if (Array.isArray(data)) return data;
67+
if (data && Array.isArray(data.producers)) return data.producers;
68+
return null;
6669
}

functions/test/drink-preview.test.js

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { describe, it, expect } from 'vitest';
2-
import { isCrawler, findDrink, buildOgTags } from '../_lib/drink-preview.js';
1+
import { describe, it, expect, vi, afterEach } from 'vitest';
2+
import { isCrawler, findDrink, buildOgTags, fetchDrinkData } from '../_lib/drink-preview.js';
33

44
const TEST_PRODUCERS = [
55
{
@@ -194,3 +194,74 @@ describe('buildOgTags', () => {
194194
});
195195
});
196196

197+
describe('fetchDrinkData', () => {
198+
afterEach(() => {
199+
vi.unstubAllGlobals();
200+
});
201+
202+
it('returns producers array from wrapped API response', async () => {
203+
const producers = [{ id: 'adnams', name: 'Adnams', products: [] }];
204+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
205+
ok: true,
206+
json: () => Promise.resolve({ producers, timestamp: '2025-01-01' }),
207+
}));
208+
const result = await fetchDrinkData('cbf2025', 'beer');
209+
expect(result).toEqual(producers);
210+
});
211+
212+
it('returns array directly when API response is already an array', async () => {
213+
const producers = [{ id: 'adnams', name: 'Adnams', products: [] }];
214+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
215+
ok: true,
216+
json: () => Promise.resolve(producers),
217+
}));
218+
const result = await fetchDrinkData('cbf2025', 'beer');
219+
expect(result).toEqual(producers);
220+
});
221+
222+
it('returns null when response is not ok', async () => {
223+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 404 }));
224+
const result = await fetchDrinkData('cbf2025', 'beer');
225+
expect(result).toBeNull();
226+
});
227+
228+
it('returns null when wrapped response has no producers key', async () => {
229+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
230+
ok: true,
231+
json: () => Promise.resolve({ timestamp: '2025-01-01' }),
232+
}));
233+
const result = await fetchDrinkData('cbf2025', 'beer');
234+
expect(result).toBeNull();
235+
});
236+
237+
it('returns null when response.json() resolves to null', async () => {
238+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
239+
ok: true,
240+
json: () => Promise.resolve(null),
241+
}));
242+
const result = await fetchDrinkData('cbf2025', 'beer');
243+
expect(result).toBeNull();
244+
});
245+
246+
it('returns null when producers field exists but is not an array', async () => {
247+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
248+
ok: true,
249+
json: () => Promise.resolve({ producers: 'not-an-array' }),
250+
}));
251+
const result = await fetchDrinkData('cbf2025', 'beer');
252+
expect(result).toBeNull();
253+
});
254+
255+
it('fetches the correct URL for a given festival and category', async () => {
256+
const mockFetch = vi.fn().mockResolvedValue({
257+
ok: true,
258+
json: () => Promise.resolve({ producers: [] }),
259+
});
260+
vi.stubGlobal('fetch', mockFetch);
261+
await fetchDrinkData('cbf2025', 'beer');
262+
expect(mockFetch).toHaveBeenCalledWith(
263+
'https://data.cambeerfestival.app/cbf2025/beer.json',
264+
);
265+
});
266+
});
267+

functions/test/handler.test.js

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2+
import { onRequest } from '../[festivalId]/drink/[category]/[drinkId].js';
3+
4+
// Minimal HTMLRewriter mock: collects appended HTML and inserts it before </head>.
5+
class MockHTMLRewriter {
6+
constructor() {
7+
this._headHandler = null;
8+
}
9+
on(selector, handler) {
10+
if (selector === 'head') this._headHandler = handler;
11+
return this;
12+
}
13+
async transform(response) {
14+
const text = await response.text();
15+
let injected = '';
16+
if (this._headHandler) {
17+
// Second arg ({ html: true }) is intentionally ignored in this mock.
18+
const element = { append: (html) => { injected += html; } };
19+
this._headHandler.element(element);
20+
}
21+
return new Response(text.replace('</head>', injected + '</head>'), {
22+
status: response.status,
23+
headers: response.headers,
24+
});
25+
}
26+
}
27+
28+
const SPA_HTML = '<!DOCTYPE html><html><head><title>CBF</title></head><body></body></html>';
29+
30+
const TEST_PRODUCERS = [
31+
{
32+
id: 'adnams',
33+
name: 'Adnams',
34+
location: 'Southwold',
35+
products: [
36+
{ id: 'broadside', name: 'Broadside', style: 'Strong Bitter', abv: 6.3, category: 'beer' },
37+
],
38+
},
39+
];
40+
41+
function makeSpaResponse() {
42+
return new Response(SPA_HTML, { headers: { 'Content-Type': 'text/html' } });
43+
}
44+
45+
function makeContext({ ua, festivalId = 'cbf2025', category = 'beer', drinkId = 'broadside' } = {}) {
46+
const url = `https://cambeerfestival.app/${festivalId}/drink/${category}/${drinkId}`;
47+
return {
48+
request: new Request(url, {
49+
headers: { 'User-Agent': ua ?? 'Mozilla/5.0 Chrome/120' },
50+
}),
51+
env: {
52+
ASSETS: { fetch: vi.fn().mockResolvedValue(makeSpaResponse()) },
53+
},
54+
params: { festivalId, category, drinkId },
55+
};
56+
}
57+
58+
beforeEach(() => {
59+
vi.stubGlobal('HTMLRewriter', MockHTMLRewriter);
60+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
61+
ok: true,
62+
json: () => Promise.resolve({ producers: TEST_PRODUCERS }),
63+
}));
64+
});
65+
66+
afterEach(() => {
67+
vi.unstubAllGlobals();
68+
});
69+
70+
describe('onRequest — non-crawler passthrough', () => {
71+
it('calls ASSETS.fetch and returns immediately for a regular browser', async () => {
72+
const ctx = makeContext({ ua: 'Mozilla/5.0 Chrome/120' });
73+
await onRequest(ctx);
74+
expect(ctx.env.ASSETS.fetch).toHaveBeenCalledWith(ctx.request);
75+
expect(global.fetch).not.toHaveBeenCalled();
76+
});
77+
78+
it('does not inject OG tags for a regular browser', async () => {
79+
const ctx = makeContext({ ua: 'Mozilla/5.0 Chrome/120' });
80+
const response = await onRequest(ctx);
81+
const text = await response.text();
82+
expect(text).not.toContain('og:title');
83+
});
84+
});
85+
86+
describe('onRequest — crawler OG injection', () => {
87+
it('injects og:title with drink name and brewery for Googlebot', async () => {
88+
const ctx = makeContext({ ua: 'Googlebot/2.1' });
89+
const response = await onRequest(ctx);
90+
const text = await response.text();
91+
expect(text).toContain('og:title');
92+
expect(text).toContain('Broadside — Adnams');
93+
});
94+
95+
it('injects og:url with the canonical drink URL', async () => {
96+
const ctx = makeContext({ ua: 'Googlebot/2.1', festivalId: 'cbf2025', category: 'beer', drinkId: 'broadside' });
97+
const response = await onRequest(ctx);
98+
const text = await response.text();
99+
expect(text).toContain('https://cambeerfestival.app/cbf2025/drink/beer/broadside');
100+
});
101+
102+
it('injects OG tags for WhatsApp crawler', async () => {
103+
const ctx = makeContext({ ua: 'WhatsApp/2.19.81 A' });
104+
const response = await onRequest(ctx);
105+
const text = await response.text();
106+
expect(text).toContain('og:title');
107+
});
108+
109+
it('fetches drink data from the correct festival and category', async () => {
110+
const ctx = makeContext({ ua: 'Googlebot/2.1', festivalId: 'cbf2024', category: 'cider', drinkId: 'broadside' });
111+
const mockFetch = vi.fn().mockResolvedValue({
112+
ok: true,
113+
json: () => Promise.resolve({ producers: TEST_PRODUCERS }),
114+
});
115+
vi.stubGlobal('fetch', mockFetch);
116+
await onRequest(ctx);
117+
expect(mockFetch).toHaveBeenCalledWith(
118+
expect.stringContaining('cbf2024/cider.json'),
119+
);
120+
});
121+
122+
it('preserves the rest of the SPA HTML when injecting', async () => {
123+
const ctx = makeContext({ ua: 'Googlebot/2.1' });
124+
const response = await onRequest(ctx);
125+
const text = await response.text();
126+
expect(text).toContain('<title>CBF</title>');
127+
expect(text).toContain('<body></body>');
128+
});
129+
});
130+
131+
describe('onRequest — crawler fallback paths', () => {
132+
it('returns unmodified SPA when API responds with non-ok status', async () => {
133+
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false, status: 404 }));
134+
const ctx = makeContext({ ua: 'Googlebot/2.1' });
135+
const response = await onRequest(ctx);
136+
const text = await response.text();
137+
expect(text).not.toContain('og:title');
138+
expect(text).toBe(SPA_HTML);
139+
});
140+
141+
it('returns unmodified SPA when fetch throws a network error', async () => {
142+
vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('Network failure')));
143+
const ctx = makeContext({ ua: 'Googlebot/2.1' });
144+
const response = await onRequest(ctx);
145+
const text = await response.text();
146+
expect(text).not.toContain('og:title');
147+
});
148+
149+
it('returns unmodified SPA when the drink ID is not found', async () => {
150+
const ctx = makeContext({ ua: 'Googlebot/2.1', drinkId: 'nonexistent' });
151+
const response = await onRequest(ctx);
152+
const text = await response.text();
153+
expect(text).not.toContain('og:title');
154+
});
155+
});

0 commit comments

Comments
 (0)