From 87381fa4b9f52704bdbe1732baf2d34f40bf1f9b Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 11:38:54 -0400 Subject: [PATCH] Hold a succeeded draft until the plan read is the one drafted The Draft board let go of a succeeded draft_week job as soon as the plan was read again, whatever that read returned: a missing plan showed as "Nothing planned yet" and another plan as the new draft. Keep the job's plan_id and clear the job only once the plan read has that id; a missing or different plan is shown as "Couldn't read the draft" with Retry. Co-Authored-By: Claude Opus 5.5 --- web/src/plan/PlanPage.test.tsx | 34 +++++++++++++++++++++++++++++++-- web/src/plan/PlanPage.tsx | 35 +++++++++++++++++++++++----------- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/web/src/plan/PlanPage.test.tsx b/web/src/plan/PlanPage.test.tsx index ebb6c00..2e53e0a 100644 --- a/web/src/plan/PlanPage.test.tsx +++ b/web/src/plan/PlanPage.test.tsx @@ -46,8 +46,10 @@ function planBody(status: string, names = NAMES) { } } -function draftJob(status: string, error: string | null = null) { - const result = status === 'succeeded' ? { week_start: '2026-10-05', plan_id: 'plan-2' } : null +// draftJob is job j1 drafting the week of Oct 5 into plan planId: a +// redraft keeps the plan's id. +function draftJob(status: string, error: string | null = null, planId = 'plan-1') { + const result = status === 'succeeded' ? { week_start: '2026-10-05', plan_id: planId } : null return { id: 'j1', kind: 'draft_week', status, result, error } } @@ -293,6 +295,34 @@ describe('PlanPage', () => { expect(screen.queryByRole('alert')).not.toBeInTheDocument() }) + it.each([ + ['the week has no plan', null, 'The week has no plan, not draft plan-1.'], + ['the week has another plan', { ...planBody('draft'), id: 'plan-9' }, "The week's plan is not draft plan-1."], + ])('shows a succeeded draft with Retry when %s once read', async (_, landed, message) => { + let plan: unknown = null + let job = draftJob('running') + const fetchMock = stubServer({ + [`GET ${THIS_WEEK}`]: () => (plan === null ? errorResponse(404, 'not_found') : jsonResponse(200, plan)), + [`POST ${DRAFT}`]: () => jsonResponse(202, { jobId: 'j1' }), + [`GET ${JOB}`]: () => jsonResponse(200, job), + }) + await renderPage() + + fireEvent.click(await screen.findByRole('button', { name: 'Draft this week' })) + expect(await screen.findByRole('region', { name: 'Drafting this week' })).toBeInTheDocument() + plan = landed + job = draftJob('succeeded') + const alert = await screen.findByRole('alert', {}, { timeout: 3000 }) + expect(alert).toHaveTextContent(`Couldn't read the draft. ${message}`) + expect(screen.queryByTestId('skeleton-tile')).not.toBeInTheDocument() + + job = draftJob('running') + fireEvent.click(within(alert).getByRole('button', { name: 'Retry' })) + await waitFor(() => expect(posts(fetchMock)).toHaveLength(2)) + expect(await screen.findByRole('region', { name: 'Drafting this week' })).toBeInTheDocument() + expect(screen.queryByRole('alert')).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 670dcf2..8af6e34 100644 --- a/web/src/plan/PlanPage.tsx +++ b/web/src/plan/PlanPage.tsx @@ -11,7 +11,7 @@ import { Icon } from '../ui/Icon.tsx' import { IconButton } from '../ui/IconButton.tsx' import { Skeleton } from '../ui/Skeleton.tsx' import { Tag } from '../ui/Tag.tsx' -import { DRAFT_JOB_KIND, parseDraftResult, planKey, usePlan, useStartDraft, type PlanDay, type PlanStatus } from './api.ts' +import { DRAFT_JOB_KIND, parseDraftResult, planKey, usePlan, useStartDraft, type DraftResult, type PlanDay, type PlanStatus } from './api.ts' import { addDays, defaultWeek, localNow, weekdayKicker, weekLabel } from './week.ts' import styles from './PlanPage.module.css' @@ -90,31 +90,44 @@ function WeekBoard({ weekStart, timezone, onWeek }: { weekStart: string; timezon const label = weekLabel(weekStart) // The draft's result, once the job has succeeded, or why it cannot be read. - const resultError = useMemo(() => { - if (job.status !== 'succeeded') return null + const result = useMemo((): { draft: DraftResult | null; error: Error | null } => { + if (job.status !== 'succeeded') return { draft: null, error: null } try { - parseDraftResult(`GET /api/v1/jobs/${jobId}`, weekStart, job.result) - return null + return { draft: parseDraftResult(`GET /api/v1/jobs/${jobId}`, weekStart, job.result), error: null } } catch (error) { - return error as Error + return { draft: null, error: error as Error } } }, [job.status, job.result, jobId, weekStart]) // Once the job ends the plan is read again: a succeeded draft is followed - // until that read lands, which then shows it; a failed one stays shown, - // and may have met a plan changed meanwhile. + // until that read lands; a failed one stays shown, and may have met a + // plan changed meanwhile. + const [readJobId, setReadJobId] = useState(null) const ended = job.status === 'succeeded' || job.status === 'failed' - const drafted = job.status === 'succeeded' && resultError === null useEffect(() => { if (!ended) return let active = true void queryClient.invalidateQueries({ queryKey: planKey(weekStart) }).then(() => { - if (active && drafted) setJobId(null) + if (active) setReadJobId(jobId) }) return () => { active = false } - }, [queryClient, ended, drafted, weekStart]) + }, [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. + const { draft } = result + const read = draft !== null && readJobId === jobId && plan.isSuccess + const landed = read && plan.data?.id === draft.plan_id + 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}.`) + : null) + useEffect(() => { + if (landed) setJobId(null) + }, [landed]) const drafting = startDraft.isPending || (jobId !== null && job.status !== 'failed' && resultError === null)