diff --git a/web/src/components/Tasks/TaskDetailBody.test.tsx b/web/src/components/Tasks/TaskDetailBody.test.tsx index 3bc9aa1e..636fe5a9 100644 --- a/web/src/components/Tasks/TaskDetailBody.test.tsx +++ b/web/src/components/Tasks/TaskDetailBody.test.tsx @@ -1,8 +1,9 @@ -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { MockInstance } from 'vitest'; -import type { Task } from '../../api/client'; +import { api, type Task } from '../../api/client'; import { TaskDetailBody } from './TaskDetailBody'; vi.mock('../../api/client', () => ({ @@ -27,6 +28,7 @@ function task(content: string): Task & { content: string } { } const editor = () => screen.getByPlaceholderText('Task content...'); +const saveButton = () => screen.queryByRole('button', { name: /save/i }); async function startEditing() { await userEvent.click(screen.getByTitle('Edit')); @@ -66,3 +68,88 @@ describe('TaskDetailBody content sync', () => { expect(screen.getByRole('button', { name: /save/i })).toBeInTheDocument(); }); }); + +/** + * A save that fails has to look different from a save that worked. The old + * code cleared `dirty` either way, which hid the Save button and left the + * editor looking settled while the only copy of the text was still in the + * browser — the user found out on the next reload. + */ +describe('TaskDetailBody failed saves', () => { + const updateTask = vi.mocked(api.updateTask); + let consoleError: MockInstance; + + const savedResponse = (content: string) => ({ + task: task(content), task_id: 't1', updated: true, + }); + + beforeEach(() => { + updateTask.mockReset(); + // The store logs the rejection deliberately. Swallow it here so a genuine + // React warning still stands out in the run output. + consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleError.mockRestore(); + }); + + async function typeAndFailToSave(addition = ' plus my notes') { + render(); + await startEditing(); + await userEvent.type(editor(), addition); + await userEvent.click(screen.getByRole('button', { name: /save/i })); + return screen.findByRole('alert'); + } + + it('says the save failed and keeps offering the retry', async () => { + updateTask.mockRejectedValue(new Error('network down')); + + expect(await typeAndFailToSave()).toHaveTextContent(/save failed/i); + + // The edit is still in the box, and still offered for saving. Before the + // fix the button vanished, which read as "saved". + expect(editor()).toHaveValue('# original plus my notes'); + expect(saveButton()).toBeInTheDocument(); + }); + + it('keeps the failed edit safe from an update arriving underneath', async () => { + updateTask.mockRejectedValue(new Error('network down')); + const { rerender } = render(); + await startEditing(); + await userEvent.type(editor(), ' plus my notes'); + await userEvent.click(screen.getByRole('button', { name: /save/i })); + await screen.findByRole('alert'); + + rerender(); + + // `dirty` staying set is what keeps the content-sync guard armed. Clearing + // it on a failure would hand the only copy of the text to the next + // re-render, which is how the edit actually got lost. + expect(editor()).toHaveValue('# original plus my notes'); + }); + + it('clears the failure once a retry succeeds', async () => { + updateTask + .mockRejectedValueOnce(new Error('network down')) + .mockResolvedValueOnce(savedResponse('# original!')); + + await typeAndFailToSave('!'); + await userEvent.click(screen.getByRole('button', { name: /save/i })); + + await waitFor(() => expect(screen.queryByRole('alert')).not.toBeInTheDocument()); + expect(saveButton()).not.toBeInTheDocument(); + }); + + it('clears the failure as soon as the user edits again', async () => { + updateTask.mockRejectedValue(new Error('network down')); + + await typeAndFailToSave('!'); + await userEvent.type(editor(), '?'); + + // Stale complaint about text the user has already moved past, but the + // edit is still unsaved so Save has to stay. + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + expect(saveButton()).toBeInTheDocument(); + }); +}); diff --git a/web/src/components/Tasks/TaskDetailBody.tsx b/web/src/components/Tasks/TaskDetailBody.tsx index 1bbc16d8..13f9b982 100644 --- a/web/src/components/Tasks/TaskDetailBody.tsx +++ b/web/src/components/Tasks/TaskDetailBody.tsx @@ -23,6 +23,7 @@ export function TaskDetailBody({ task }: { task: Task }) { const [showHistory, setShowHistory] = useState(false); const [localContent, setLocalContent] = useState(''); const [dirty, setDirty] = useState(false); + const [saveError, setSaveError] = useState(false); // Adopt the loaded markdown, but never clobber unsaved edits. // @@ -40,8 +41,12 @@ export function TaskDetailBody({ task }: { task: Task }) { const handleSave = useCallback(async () => { if (!dirty) return; - await saveTaskContent(task.id, localContent); - setDirty(false); + // Keep `dirty` set when the save fails. It is what keeps the Save button + // on screen, so the retry stays available and the editor cannot look + // settled while the content is still only in the browser. + const saved = await saveTaskContent(task.id, localContent); + setSaveError(!saved); + if (saved) setDirty(false); }, [task.id, dirty, localContent, saveTaskContent]); const handleKeyDown = useCallback((e: React.KeyboardEvent) => { @@ -89,13 +94,20 @@ export function TaskDetailBody({ task }: { task: Task }) { {dirty && ( - + <> + {saveError && ( + + Save failed — not saved yet. + + )} + + )}