diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index c9bdb84..15390e7 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -225,4 +225,14 @@ describe('network status', () => { await expect(api.apiFetch('/api/v1/recipes/01J9ZR')).resolves.toEqual({ id: '01J9ZR' }) expect(api.network.offline()).toBe(false) }) + + it('says whether a read is the service worker\'s cached copy', async () => { + const cached = jsonResponse(200, { id: '01J9ZR' }) + cached.headers.set(cacheFallbackHeader, '1') + stubFetch(cached, jsonResponse(200, { id: '01J9ZR' })) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + + await expect(api.apiRead('/api/v1/recipes/01J9ZR')).resolves.toEqual({ body: { id: '01J9ZR' }, cached: true }) + await expect(api.apiRead('/api/v1/recipes/01J9ZR')).resolves.toEqual({ body: { id: '01J9ZR' }, cached: false }) + }) }) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index cbfc0f0..1fc884d 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -43,11 +43,18 @@ export type ApiRequest = { body?: unknown } +// ApiRead is a GET's decoded response body, and whether the service worker +// answered it from its cache because the server could not be reached in time. +export type ApiRead = { body: T; cached: boolean } + export type ApiClient = { // apiFetch sends a same-origin JSON request to path, which must be under // /api/v1/, and resolves with the decoded response body, or rejects with an // ApiError. apiFetch(path: string, request?: ApiRequest): Promise + // apiRead is apiFetch's GET that also says whether the body is the service + // worker's cached copy rather than the server's answer. + apiRead(path: string): Promise> // loadMe fetches GET /api/v1/me and caches its CSRF token. loadMe(): Promise // forbidden reports whether a request was refused because the signed-in @@ -114,6 +121,16 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s } async function apiFetch(path: string, request: ApiRequest = {}): Promise { + return (await exchange(path, request)).body + } + + function apiRead(path: string): Promise> { + return exchange(path, {}) + } + + // exchange sends request to path and decodes the response, noting whether + // it is the service worker's cached copy. + async function exchange(path: string, request: ApiRequest): Promise> { const method = request.method ?? 'GET' const url = apiURL(location.origin, method, path) // fetch refuses a GET with a body by rejecting with a TypeError, the same @@ -131,7 +148,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s response = await send(method, url, request.body) if (response.status === 403) throw await parseForbidden(method, path, response) } - return decode(method, path, response) + return { body: await decode(method, path, response), cached: response.headers.has(cacheFallbackHeader) } } // parseForbidden reads a 403. Any API error other than csrf means the @@ -179,6 +196,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus, s return { apiFetch, + apiRead, loadMe, forbidden: () => forbidden, subscribe(listener) { diff --git a/web/src/api/index.ts b/web/src/api/index.ts index cae3e1f..2f59f3a 100644 --- a/web/src/api/index.ts +++ b/web/src/api/index.ts @@ -1,5 +1,5 @@ export { ApiProvider, useApi } from './ApiContext.tsx' -export { createApiClient, CSRF_HEADER, ME_PATH, type ApiClient, type ApiLocation, type ApiRequest, type Me } from './client.ts' +export { createApiClient, CSRF_HEADER, ME_PATH, type ApiClient, type ApiRead, type ApiLocation, type ApiRequest, type Me } from './client.ts' export { ApiError } from './errors.ts' export { createQueryClient, shouldRetry } from './queryClient.ts' export { createVerifiedSession, VERIFIED_SESSION_KEY, type VerifiedSession } from './verifiedSession.ts' diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index 2e53e0a..9f4e916 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -1,8 +1,9 @@ import { QueryClientProvider } from '@tanstack/react-query' -import { fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts' import { createNetworkStatus } from '../pwa/networkStatus.ts' +import { cacheFallbackHeader } from '../sw/routes.ts' import { HOUSEHOLD_SETTINGS } from '../test/ApiProviders.tsx' import { declaredStyle } from '../test/cssRules.ts' import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' @@ -323,6 +324,45 @@ describe('PlanPage', () => { expect(screen.queryByRole('alert')).not.toBeInTheDocument() }) + it("doesn't land a succeeded draft on the plan cached before it, and lands it once the server is read", async () => { + let plan = planBody('draft') + let cached = false + let job = draftJob('running') + stubServer({ + [`GET ${THIS_WEEK}`]: () => { + const response = jsonResponse(200, plan) + if (cached) response.headers.set(cacheFallbackHeader, '1') + return response + }, + [`POST ${DRAFT}`]: () => jsonResponse(202, { jobId: 'j1' }), + [`GET ${JOB}`]: () => jsonResponse(200, job), + }) + await renderPage() + + fireEvent.click(await screen.findByRole('button', { name: 'Rebuild week' })) + expect(await screen.findByRole('region', { name: 'Drafting this week' })).toBeInTheDocument() + // The draft succeeds, but the server can't be reached: the service + // worker answers with the plan cached before the rebuild, same id. + cached = true + job = draftJob('succeeded') + const alert = await screen.findByRole('alert', {}, { timeout: 3000 }) + expect(alert).toHaveTextContent("Couldn't read the draft. The plan came from this device's cache, not the server") + expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument() + expect(screen.queryByText('Beef tacos')).not.toBeInTheDocument() + expect(screen.queryByTestId('skeleton-tile')).not.toBeInTheDocument() + + // The server is back: the next read of the plan is the draft. + plan = planBody('draft', ['Lasagna', ...NAMES.slice(1)]) + cached = false + vi.setSystemTime(new Date('2026-10-07T16:01:00Z')) + act(() => { + window.dispatchEvent(new Event('visibilitychange')) + }) + expect(await screen.findByText('Lasagna')).toBeInTheDocument() + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + expect(screen.queryByTestId('skeleton-tile')).not.toBeInTheDocument() + }) + it('moves between weeks with the previous and next week buttons', async () => { stubServer({ [`GET ${THIS_WEEK}`]: () => jsonResponse(200, planBody('draft')), diff --git a/web/src/plan/PlanPage.tsx b/web/src/plan/PlanPage.tsx index 8af6e34..0f1bcd5 100644 --- a/web/src/plan/PlanPage.tsx +++ b/web/src/plan/PlanPage.tsx @@ -115,15 +115,23 @@ function WeekBoard({ weekStart, timezone, onWeek }: { weekStart: string; timezon } }, [queryClient, ended, jobId, weekStart]) - // The plan read shows once it is the one drafted; no plan, or another - // one, is an error rather than an empty or stale board. + // The plan read shows once it is the one drafted, read from the server; no + // plan, another one, or a copy cached before the draft (a redraft keeps the + // plan's id) is an error rather than an empty or stale board. A later read + // from the server that is the draft still lands it. const { draft } = result const read = draft !== null && readJobId === jobId && plan.isSuccess - const landed = read && plan.data?.id === draft.plan_id + const landed = read && plan.data?.id === draft.plan_id && !plan.data.cached const resultError = result.error ?? (read && !landed - ? new Error(plan.data === null ? `The week has no plan, not draft ${draft.plan_id}.` : `The week's plan is not draft ${draft.plan_id}.`) + ? new Error( + plan.data === null + ? `The week has no plan, not draft ${draft.plan_id}.` + : plan.data.id !== draft.plan_id + ? `The week's plan is not draft ${draft.plan_id}.` + : `The plan came from this device's cache, not the server, so it may not be draft ${draft.plan_id} yet.`, + ) : null) useEffect(() => { if (landed) setJobId(null) @@ -168,6 +176,9 @@ function WeekBoard({ weekStart, timezone, onWeek }: { weekStart: string; timezon body = void plan.refetch()}>Couldn't load this week's plan. {plan.error.message} } else if (drafting) { body = Drafting your week… + } else if (resultError !== null) { + // An unread draft shows no board: the plan read is not the draft. + body = null } else if (plan.isPending) { body = } else if (plan.data === null) { diff --git a/web/src/plan/api.ts b/web/src/plan/api.ts index a726ea3..17b4539 100644 --- a/web/src/plan/api.ts +++ b/web/src/plan/api.ts @@ -1,5 +1,5 @@ import { useMutation, useQuery } from '@tanstack/react-query' -import { ApiError, useApi } from '../api/index.ts' +import { ApiError, useApi, type ApiRead } from '../api/index.ts' import { isRecord } from '../api/errors.ts' import { oneOf } from '../dinners/api.ts' @@ -32,13 +32,15 @@ export type PlanDay = { } // Plan is GET /api/v1/plans/{weekStart}: the week's seven days, Monday to -// Sunday, and the AI's note on the draft, if any. +// Sunday, and the AI's note on the draft, if any. cached says the service +// worker answered from its cache: the server may have changed the plan since. export type Plan = { id: string week_start: string status: PlanStatus ai_note: string | null days: PlanDay[] + cached: boolean } // DRAFT_JOB_KIND is the kind of the job drafting a week's plan. @@ -62,7 +64,7 @@ function parseDay(where: string, body: unknown): PlanDay { return { date: body.date, dinner: { id: body.dinner.id, name: body.dinner.name }, is_new: body.is_new, start_by: body.start_by } } -function parsePlan(where: string, weekStart: string, body: unknown): Plan { +function parsePlan(where: string, weekStart: string, body: unknown, cached: boolean): Plan { if ( !isRecord(body) || typeof body.id !== 'string' || @@ -79,6 +81,7 @@ function parsePlan(where: string, weekStart: string, body: unknown): Plan { status: oneOf(where, 'status', PLAN_STATUSES, body.status), ai_note: body.ai_note, days: body.days.map((day) => parseDay(where, day)), + cached, } } @@ -94,19 +97,19 @@ export function parseDraftResult(where: string, weekStart: string, body: unknown // usePlan is the plan of the week starting on Monday weekStart (GET // /api/v1/plans/{weekStart}), or null when the week has none (404). export function usePlan(weekStart: string) { - const { apiFetch } = useApi() + const { apiRead } = useApi() return useQuery({ queryKey: planKey(weekStart), queryFn: async (): Promise => { const path = planPath(weekStart) - let body: unknown + let read: ApiRead try { - body = await apiFetch(path) + read = await apiRead(path) } catch (error) { if (error instanceof ApiError && error.status === 404) return null throw error } - return parsePlan(`GET ${path}`, weekStart, body) + return parsePlan(`GET ${path}`, weekStart, read.body, read.cached) }, }) }