From 8e18c0b37c04afaa2cdfe036ccf0bac3ea7ca87a Mon Sep 17 00:00:00 2001 From: dev_agent Date: Fri, 9 Oct 2026 21:15:12 -0400 Subject: [PATCH] Reject malformed Home minutes and Supply run labels (review H1) parseHome now fails fast into the query error instead of rendering bad values: tonight's minutes_total must be a positive whole number or null, and a ready shoppingList must carry a non-empty label that is the label of its weekStart. Task: 5170be5d-4787-4ea8-8530-c19e98e591bc Co-Authored-By: Claude Opus 5.5 --- web/src/home/HomePage.test.tsx | 39 ++++++++++++++++++++++++++++++++++ web/src/home/api.ts | 36 ++++++++++++++++++++++--------- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/web/src/home/HomePage.test.tsx b/web/src/home/HomePage.test.tsx index 9416039..59fe8ef 100644 --- a/web/src/home/HomePage.test.tsx +++ b/web/src/home/HomePage.test.tsx @@ -215,4 +215,43 @@ describe('HomePage', () => { renderHome({ ...HOME, tonight: { ...HOME.tonight, origin: 'leftovers' } }) expect(await screen.findByRole('alert')).toHaveTextContent('origin "leftovers" is not one of regular, side_quest') }) + + it.each([0, -1, 2.5])('reports tonight with minutes_total %s rather than showing it', async (minutes) => { + renderHome({ ...HOME, tonight: { ...HOME.tonight, minutes_total: minutes } }) + + expect(await screen.findByRole('alert')).toHaveTextContent( + `tonight's minutes_total ${minutes} is not a positive whole number`, + ) + expect(screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(screen.queryByRole('region', { name: 'Tonight' })).not.toBeInTheDocument() + expect(screen.queryByText(/min$/)).not.toBeInTheDocument() + }) + + it.each([ + ['a missing', undefined], + ['an empty', ''], + ])('reports a ready Supply run with %s label rather than showing it', async (_, label) => { + renderHome({ ...HOME, shoppingList: { ...HOME.shoppingList, label } }) + + expect(await screen.findByRole('alert')).toHaveTextContent('the ready shoppingList has no label') + expect(screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(screen.queryByRole('link', { name: /Shopping list ready/ })).not.toBeInTheDocument() + expect(screen.queryByRole('list', { name: 'Status' })).not.toBeInTheDocument() + }) + + it('reports a ready Supply run whose label is not its week rather than showing it', async () => { + renderHome({ ...HOME, shoppingList: { ...HOME.shoppingList, label: 'Week 40 · Sep 28 – Oct 4' } }) + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'shoppingList label "Week 40 · Sep 28 – Oct 4" is not the week of 2026-10-05', + ) + expect(screen.queryByRole('link', { name: /Shopping list ready/ })).not.toBeInTheDocument() + }) + + it('reports a Supply run whose weekStart is not a date rather than showing it', async () => { + renderHome({ ...HOME, shoppingList: { ...HOME.shoppingList, weekStart: 'Oct 5' } }) + + expect(await screen.findByRole('alert')).toHaveTextContent('not a YYYY-MM-DD date') + expect(screen.queryByRole('list', { name: 'Status' })).not.toBeInTheDocument() + }) }) diff --git a/web/src/home/api.ts b/web/src/home/api.ts index ce597a5..ef5b7cd 100644 --- a/web/src/home/api.ts +++ b/web/src/home/api.ts @@ -11,8 +11,8 @@ export const ORIGINS = ['regular', 'side_quest'] as const export type Origin = (typeof ORIGINS)[number] // Tonight is the dinner planned for today: photo_id is its stored photo's -// id, from photo, and minutes_total its minutes from prep to table, null -// when the recipe does not say. +// id, from photo, and minutes_total its minutes from prep to table, a +// positive whole number, or null when the recipe does not say. export type Tonight = { id: string; name: string; photo_id: string | undefined; origin: Origin; minutes_total: number | null } // ShoppingListStatus is the Supply run Home points at: next week's once it @@ -39,35 +39,51 @@ function parseTonight(where: string, body: unknown): Tonight | null { if (!isRecord(body) || typeof body.id !== 'string' || typeof body.name !== 'string') { throw new Error(`${where}: tonight is not a dinner`) } - if (body.minutes_total !== null && !Number.isInteger(body.minutes_total)) { - throw new Error(`${where}: tonight's minutes_total is not a whole number`) + const minutes = body.minutes_total ?? null + if (minutes !== null && (!Number.isInteger(minutes) || (minutes as number) <= 0)) { + throw new Error(`${where}: tonight's minutes_total ${String(minutes)} is not a positive whole number`) } return { id: body.id, name: body.name, photo_id: photoId(where, body.photo), origin: oneOf(where, 'origin', ORIGINS, body.origin), - minutes_total: body.minutes_total as number | null, + minutes_total: minutes as number | null, } } +// parseShoppingList checks the shoppingList of a GET /api/v1/home body. A +// ready list must carry the API's label for its week, which must be the +// label weekStart has, since the status row shows dates from weekStart. +function parseShoppingList(where: string, list: unknown): ShoppingListStatus { + if (!isRecord(list) || typeof list.weekStart !== 'string' || typeof list.ready !== 'boolean') { + throw new Error(`${where}: shoppingList is not a week's Supply run status`) + } + const week = weekLabel(list.weekStart) + if (list.ready) { + if (typeof list.label !== 'string' || list.label === '') { + throw new Error(`${where}: the ready shoppingList has no label`) + } + if (list.label !== `${week.week} · ${week.dates}`) { + throw new Error(`${where}: shoppingList label "${list.label}" is not the week of ${list.weekStart}`) + } + } + return { weekStart: list.weekStart, dates: week.dates.replace(' – ', '–'), ready: list.ready } +} + // parseHome checks a GET /api/v1/home body. export function parseHome(where: string, body: unknown): Home { if (!isRecord(body) || typeof body.dateLabel !== 'string') throw new Error(`${where}: response is not the home`) if (!Number.isInteger(body.sideQuestCount) || (body.sideQuestCount as number) < 0) { throw new Error(`${where}: sideQuestCount is not a count`) } - const list = body.shoppingList - if (!isRecord(list) || typeof list.weekStart !== 'string' || typeof list.ready !== 'boolean') { - throw new Error(`${where}: shoppingList is not a week's Supply run status`) - } const updates = body.recentUpdates if (!isRecord(updates) || typeof updates.unread !== 'boolean') throw new Error(`${where}: recentUpdates has no unread flag`) return { dateLabel: body.dateLabel, tonight: parseTonight(where, body.tonight), sideQuestCount: body.sideQuestCount as number, - shoppingList: { weekStart: list.weekStart, dates: weekLabel(list.weekStart).dates.replace(' – ', '–'), ready: list.ready }, + shoppingList: parseShoppingList(where, body.shoppingList), unread: updates.unread, } }