diff --git a/src/App.tsx b/src/App.tsx index cc6b0869..15fcae2b 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -67,7 +67,7 @@ function App() { useEffect(() => { setApiKey(settings.anthropic_key ?? '') }, [settings.anthropic_key]) - const notes = useNoteActions({ addEntry: vault.addEntry, updateContent: vault.updateContent, entries: vault.entries, setToastMessage, updateEntry: vault.updateEntry }) + const notes = useNoteActions({ addEntry: vault.addEntry, removeEntry: vault.removeEntry, updateContent: vault.updateContent, entries: vault.entries, setToastMessage, updateEntry: vault.updateEntry }) const { handleSave, handleContentChange, savePendingForPath, savePending } = useEditorSave({ updateVaultContent: vault.updateContent, diff --git a/src/hooks/useNoteActions.test.ts b/src/hooks/useNoteActions.test.ts index 0a83e39d..c6ce27b5 100644 --- a/src/hooks/useNoteActions.test.ts +++ b/src/hooks/useNoteActions.test.ts @@ -1,5 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { renderHook, act } from '@testing-library/react' +import { invoke } from '@tauri-apps/api/core' +import { isTauri } from '../mock-tauri' import type { VaultEntry } from '../types' import { slugify, @@ -17,7 +19,7 @@ import type { NoteActionsConfig } from './useNoteActions' // Mock dependencies vi.mock('@tauri-apps/api/core', () => ({ invoke: vi.fn() })) vi.mock('../mock-tauri', () => ({ - isTauri: () => false, + isTauri: vi.fn(() => false), addMockEntry: vi.fn(), updateMockContent: vi.fn(), mockInvoke: vi.fn().mockResolvedValue(''), @@ -280,16 +282,18 @@ describe('frontmatterToEntryPatch', () => { describe('useNoteActions hook', () => { const addEntry = vi.fn() + const removeEntry = vi.fn() const updateContent = vi.fn() const updateEntry = vi.fn() const setToastMessage = vi.fn() const makeConfig = (entries: VaultEntry[] = []): NoteActionsConfig => ({ - addEntry, updateContent, entries, setToastMessage, updateEntry, + addEntry, removeEntry, updateContent, entries, setToastMessage, updateEntry, }) beforeEach(() => { vi.clearAllMocks() + vi.mocked(isTauri).mockReturnValue(false) }) it('handleCreateNote calls addEntry and creates correct entry', () => { @@ -436,4 +440,61 @@ describe('useNoteActions hook', () => { expect(updateEntry).not.toHaveBeenCalled() expect(setToastMessage).toHaveBeenCalledWith('Property updated') }) + + describe('optimistic error recovery (Tauri mode)', () => { + beforeEach(() => { + vi.mocked(isTauri).mockReturnValue(true) + }) + + it.each([ + ['handleCreateNote', 'Failing Note', 'Note', 'note/failing-note.md'], + ['handleCreateType', 'Recipe', 'Type', 'type/recipe.md'], + ])('reverts optimistic creation via %s when disk write fails', async (method, title, type, pathFragment) => { + vi.mocked(invoke).mockRejectedValueOnce(new Error('disk full')) + const { result } = renderHook(() => useNoteActions(makeConfig())) + + await act(async () => { + if (method === 'handleCreateNote') result.current.handleCreateNote(title, type) + else result.current.handleCreateType(title) + await new Promise((r) => setTimeout(r, 0)) + }) + + expect(addEntry).toHaveBeenCalledTimes(1) + expect(removeEntry).toHaveBeenCalledWith(expect.stringContaining(pathFragment)) + expect(setToastMessage).toHaveBeenCalledWith('Failed to create note — disk write error') + }) + + it('does not revert when disk write succeeds', async () => { + vi.mocked(invoke).mockResolvedValueOnce(undefined) + const { result } = renderHook(() => useNoteActions(makeConfig())) + + await act(async () => { + result.current.handleCreateNote('Good Note', 'Note') + await new Promise((r) => setTimeout(r, 0)) + }) + + expect(removeEntry).not.toHaveBeenCalled() + expect(setToastMessage).not.toHaveBeenCalled() + }) + + it('handles rapid creation with one failure independently', async () => { + vi.mocked(invoke) + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('disk full')) + .mockResolvedValueOnce(undefined) + + const { result } = renderHook(() => useNoteActions(makeConfig())) + + await act(async () => { + result.current.handleCreateNoteImmediate() + result.current.handleCreateNoteImmediate() + result.current.handleCreateNoteImmediate() + await new Promise((r) => setTimeout(r, 0)) + }) + + expect(addEntry).toHaveBeenCalledTimes(3) + expect(removeEntry).toHaveBeenCalledTimes(1) + expect(removeEntry).toHaveBeenCalledWith(expect.stringContaining('untitled-note-2.md')) + }) + }) }) diff --git a/src/hooks/useNoteActions.ts b/src/hooks/useNoteActions.ts index 7dd2aa58..08d005db 100644 --- a/src/hooks/useNoteActions.ts +++ b/src/hooks/useNoteActions.ts @@ -21,6 +21,7 @@ interface RenameResult { export interface NoteActionsConfig { addEntry: (entry: VaultEntry, content: string) => void + removeEntry: (path: string) => void updateContent: (path: string, content: string) => void entries: VaultEntry[] setToastMessage: (msg: string | null) => void @@ -49,13 +50,10 @@ async function loadNoteContent(path: string): Promise { : mockInvoke('get_note_content', { path }) } -/** Fire-and-forget: persist a newly created note to disk. */ -function persistNewNote(path: string, content: string): void { - if (isTauri()) { - invoke('save_note_content', { path, content }).catch((err) => - console.error('Failed to persist new note:', err), - ) - } +/** Persist a newly created note to disk. Returns a Promise for error handling. */ +function persistNewNote(path: string, content: string): Promise { + if (!isTauri()) return Promise.resolve() + return invoke('save_note_content', { path, content }).then(() => {}) } export function buildNewEntry({ path, slug, title, type, status }: NewEntryParams): VaultEntry { @@ -178,6 +176,30 @@ function findWikilinkTarget(entries: VaultEntry[], target: string): VaultEntry | return entries.find((e) => entryMatchesTarget(e, targetLower, targetAsWords)) } +/** Navigate to a wikilink target, logging a warning if not found. */ +function navigateWikilink(entries: VaultEntry[], target: string, selectNote: (e: VaultEntry) => void): void { + const found = findWikilinkTarget(entries, target) + if (found) selectNote(found) + else console.warn(`Navigation target not found: ${target}`) +} + +/** Persist to disk; on failure, call the revert handler. */ +function persistOptimistic(path: string, content: string, onFail: (p: string) => void): void { + persistNewNote(path, content).catch(() => onFail(path)) +} + +/** Optimistically add entry to UI, open tab, and persist to disk. */ +function createAndPersist( + resolved: { entry: VaultEntry; content: string }, + addFn: (e: VaultEntry, c: string) => void, + openTab: (e: VaultEntry, c: string) => void, + onFail: (p: string) => void, +): void { + addEntryWithMock(resolved.entry, resolved.content, addFn) + openTab(resolved.entry, resolved.content) + persistOptimistic(resolved.entry.path, resolved.content, onFail) +} + async function executeFrontmatterOp(op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue): Promise { if (op === 'update') { return isTauri() ? invokeFrontmatter('update_frontmatter', { path, key, value }) : applyMockFrontmatterUpdate(path, key, value!) @@ -202,10 +224,26 @@ async function reloadTabsAfterRename( } } +/** Run a frontmatter update/delete and apply the result to state. */ +async function runFrontmatterAndApply( + op: 'update' | 'delete', path: string, key: string, value: FrontmatterValue | undefined, + callbacks: { updateTab: (p: string, c: string) => void; updateEntry: (p: string, patch: Partial) => void; toast: (m: string | null) => void }, +): Promise { + try { + callbacks.updateTab(path, await executeFrontmatterOp(op, path, key, value)) + const patch = frontmatterToEntryPatch(op, key, value) + if (Object.keys(patch).length > 0) callbacks.updateEntry(path, patch) + callbacks.toast(op === 'update' ? 'Property updated' : 'Property deleted') + } catch (err) { + console.error(`Failed to ${op} frontmatter:`, err) + callbacks.toast(`Failed to ${op} property`) + } +} + export function useNoteActions(config: NoteActionsConfig) { - const { addEntry, updateContent, entries, setToastMessage, updateEntry } = config + const { addEntry, removeEntry, updateContent, entries, setToastMessage, updateEntry } = config const tabMgmt = useTabManagement() - const { setTabs, handleSelectNote, openTabWithContent, activeTabPathRef, handleSwitchTab } = tabMgmt + const { setTabs, handleSelectNote, openTabWithContent, handleCloseTab, activeTabPathRef, handleSwitchTab } = tabMgmt const tabsRef = useRef(tabMgmt.tabs) // eslint-disable-next-line react-hooks/refs tabsRef.current = tabMgmt.tabs @@ -215,22 +253,23 @@ export function useNoteActions(config: NoteActionsConfig) { updateContent(path, newContent) }, [setTabs, updateContent]) - const handleNavigateWikilink = useCallback((target: string) => { - const found = findWikilinkTarget(entries, target) - if (found) handleSelectNote(found) - else console.warn(`Navigation target not found: ${target}`) - }, [entries, handleSelectNote]) + const handleNavigateWikilink = useCallback( + (target: string) => navigateWikilink(entries, target, handleSelectNote), + [entries, handleSelectNote], + ) + + const revertOptimisticNote = useCallback((path: string) => { + handleCloseTab(path) + removeEntry(path) + setToastMessage('Failed to create note — disk write error') + }, [handleCloseTab, removeEntry, setToastMessage]) const pendingNamesRef = useRef>(new Set()) const handleCreateNote = useCallback((title: string, type: string) => { - const { entry, content } = resolveNewNote(title, type) - addEntryWithMock(entry, content, addEntry) - openTabWithContent(entry, content) - persistNewNote(entry.path, content) - }, [openTabWithContent, addEntry]) + createAndPersist(resolveNewNote(title, type), addEntry, openTabWithContent, revertOptimisticNote) + }, [openTabWithContent, addEntry, revertOptimisticNote]) - /** Create a note immediately with an auto-generated unique title. Dedup-safe for rapid calls. */ const handleCreateNoteImmediate = useCallback((type?: string) => { const noteType = type || 'Note' const title = generateUntitledName(entries, noteType, pendingNamesRef.current) @@ -241,28 +280,19 @@ export function useNoteActions(config: NoteActionsConfig) { }, [entries, handleCreateNote]) const handleCreateType = useCallback((typeName: string) => { - const { entry, content } = resolveNewType(typeName) - addEntryWithMock(entry, content, addEntry) - openTabWithContent(entry, content) - persistNewNote(entry.path, content) - }, [openTabWithContent, addEntry]) + createAndPersist(resolveNewType(typeName), addEntry, openTabWithContent, revertOptimisticNote) + }, [openTabWithContent, addEntry, revertOptimisticNote]) - const runFrontmatterOp = useCallback(async (op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue) => { - try { - updateTabContent(path, await executeFrontmatterOp(op, path, key, value)) - const patch = frontmatterToEntryPatch(op, key, value) - if (Object.keys(patch).length > 0) updateEntry(path, patch) - setToastMessage(op === 'update' ? 'Property updated' : 'Property deleted') - } catch (err) { - console.error(`Failed to ${op} frontmatter:`, err) - setToastMessage(`Failed to ${op} property`) - } - }, [updateTabContent, updateEntry, setToastMessage]) + const fmCallbacks = { updateTab: updateTabContent, updateEntry, toast: setToastMessage } + + const runFrontmatterOp = useCallback( + (op: 'update' | 'delete', path: string, key: string, value?: FrontmatterValue) => + runFrontmatterAndApply(op, path, key, value, fmCallbacks), + [updateTabContent, updateEntry, setToastMessage], // eslint-disable-line react-hooks/exhaustive-deps -- fmCallbacks is stable when deps are + ) const handleRenameNote = useCallback(async ( - path: string, - newTitle: string, - vaultPath: string, + path: string, newTitle: string, vaultPath: string, onEntryRenamed: (oldPath: string, newEntry: Partial & { path: string }, newContent: string) => void, ) => { try { @@ -270,15 +300,10 @@ export function useNoteActions(config: NoteActionsConfig) { const newContent = await loadNoteContent(result.new_path) const entry = entries.find((e) => e.path === path) const newEntry = buildRenamedEntry(entry ?? {} as VaultEntry, newTitle, result.new_path) - - const otherTabPaths = tabsRef.current - .filter(t => t.entry.path !== path) - .map(t => t.entry.path) - + const otherTabPaths = tabsRef.current.filter(t => t.entry.path !== path).map(t => t.entry.path) setTabs((prev) => prev.map((t) => t.entry.path === path ? { entry: newEntry, content: newContent } : t)) if (activeTabPathRef.current === path) handleSwitchTab(result.new_path) onEntryRenamed(path, newEntry, newContent) - await reloadTabsAfterRename(otherTabPaths, updateTabContent) setToastMessage(renameToastMessage(result.updated_files)) } catch (err) { diff --git a/src/hooks/useVaultLoader.test.ts b/src/hooks/useVaultLoader.test.ts index 55404b81..bc0a6a87 100644 --- a/src/hooks/useVaultLoader.test.ts +++ b/src/hooks/useVaultLoader.test.ts @@ -49,17 +49,20 @@ vi.mock('../mock-tauri', () => ({ mockInvoke: (cmd: string, args?: Record) => mockInvokeFn(cmd, args), })) +/** Render the vault loader hook and wait for initial data to load. */ +async function renderVaultLoader() { + const hook = renderHook(() => useVaultLoader('/vault')) + await waitFor(() => { expect(hook.result.current.entries).toHaveLength(1) }) + return hook +} + describe('useVaultLoader', () => { beforeEach(() => { mockInvokeFn.mockImplementation(defaultMockInvoke) }) it('loads entries and content on mount', async () => { - const { result } = renderHook(() => useVaultLoader('/vault')) - - await waitFor(() => { - expect(result.current.entries).toHaveLength(1) - }) + const { result } = await renderVaultLoader() expect(result.current.entries[0].title).toBe('Hello') expect(result.current.allContent['/vault/note/hello.md']).toContain('# Hello') @@ -77,22 +80,10 @@ describe('useVaultLoader', () => { describe('addEntry', () => { it('prepends new entry and adds content', async () => { - const { result } = renderHook(() => useVaultLoader('/vault')) + const { result } = await renderVaultLoader() + const newEntry: VaultEntry = { ...mockEntries[0], path: '/vault/note/new.md', filename: 'new.md', title: 'New Note' } - await waitFor(() => { - expect(result.current.entries).toHaveLength(1) - }) - - const newEntry: VaultEntry = { - ...mockEntries[0], - path: '/vault/note/new.md', - filename: 'new.md', - title: 'New Note', - } - - act(() => { - result.current.addEntry(newEntry, '# New Note') - }) + act(() => { result.current.addEntry(newEntry, '# New Note') }) expect(result.current.entries).toHaveLength(2) expect(result.current.entries[0].title).toBe('New Note') @@ -100,18 +91,8 @@ describe('useVaultLoader', () => { }) it('ignores duplicate entry with same path', async () => { - const { result } = renderHook(() => useVaultLoader('/vault')) - - await waitFor(() => { - expect(result.current.entries).toHaveLength(1) - }) - - const newEntry: VaultEntry = { - ...mockEntries[0], - path: '/vault/note/new.md', - filename: 'new.md', - title: 'New Note', - } + const { result } = await renderVaultLoader() + const newEntry: VaultEntry = { ...mockEntries[0], path: '/vault/note/new.md', filename: 'new.md', title: 'New Note' } act(() => { result.current.addEntry(newEntry, '# New Note') @@ -124,31 +105,38 @@ describe('useVaultLoader', () => { describe('updateContent', () => { it('updates content for an existing path', async () => { - const { result } = renderHook(() => useVaultLoader('/vault')) + const { result } = await renderVaultLoader() - await waitFor(() => { - expect(result.current.allContent['/vault/note/hello.md']).toBeDefined() - }) - - act(() => { - result.current.updateContent('/vault/note/hello.md', '# Updated') - }) + act(() => { result.current.updateContent('/vault/note/hello.md', '# Updated') }) expect(result.current.allContent['/vault/note/hello.md']).toBe('# Updated') }) }) + describe('removeEntry', () => { + it('removes entry and content by path', async () => { + const { result } = await renderVaultLoader() + + act(() => { result.current.removeEntry('/vault/note/hello.md') }) + + expect(result.current.entries).toHaveLength(0) + expect(result.current.allContent['/vault/note/hello.md']).toBeUndefined() + }) + + it('is a no-op for non-existent paths', async () => { + const { result } = await renderVaultLoader() + + act(() => { result.current.removeEntry('/vault/note/nonexistent.md') }) + + expect(result.current.entries).toHaveLength(1) + }) + }) + describe('updateEntry', () => { it('patches an existing entry by path', async () => { - const { result } = renderHook(() => useVaultLoader('/vault')) + const { result } = await renderVaultLoader() - await waitFor(() => { - expect(result.current.entries).toHaveLength(1) - }) - - act(() => { - result.current.updateEntry('/vault/note/hello.md', { archived: true, status: 'Done' }) - }) + act(() => { result.current.updateEntry('/vault/note/hello.md', { archived: true, status: 'Done' }) }) expect(result.current.entries[0].archived).toBe(true) expect(result.current.entries[0].status).toBe('Done') @@ -168,22 +156,10 @@ describe('useVaultLoader', () => { }) it('returns new for freshly added entries', async () => { - const { result } = renderHook(() => useVaultLoader('/vault')) + const { result } = await renderVaultLoader() + const newEntry: VaultEntry = { ...mockEntries[0], path: '/vault/note/brand-new.md', filename: 'brand-new.md', title: 'Brand New' } - await waitFor(() => { - expect(result.current.entries).toHaveLength(1) - }) - - const newEntry: VaultEntry = { - ...mockEntries[0], - path: '/vault/note/brand-new.md', - filename: 'brand-new.md', - title: 'Brand New', - } - - act(() => { - result.current.addEntry(newEntry, '# Brand New') - }) + act(() => { result.current.addEntry(newEntry, '# Brand New') }) expect(result.current.getNoteStatus('/vault/note/brand-new.md')).toBe('new') }) diff --git a/src/hooks/useVaultLoader.ts b/src/hooks/useVaultLoader.ts index 6807247a..b3233703 100644 --- a/src/hooks/useVaultLoader.ts +++ b/src/hooks/useVaultLoader.ts @@ -91,6 +91,11 @@ export function useVaultLoader(vaultPath: string) { const updateEntry = useCallback((path: string, patch: Partial) => setEntries((prev) => prev.map((e) => e.path === path ? { ...e, ...patch } : e)), []) + const removeEntry = useCallback((path: string) => { + setEntries((prev) => prev.filter((e) => e.path !== path)) + setAllContent((prev) => { const next = { ...prev }; delete next[path]; return next }) + }, []) + const replaceEntry = useCallback((oldPath: string, patch: Partial & { path: string }, newContent: string) => { setEntries((prev) => prev.map((e) => e.path === oldPath ? { ...e, ...patch } : e)) setAllContent((prev) => { const next = { ...prev }; delete next[oldPath]; next[patch.path] = newContent; return next }) @@ -115,7 +120,7 @@ export function useVaultLoader(vaultPath: string) { return { entries, allContent, modifiedFiles, - addEntry, updateEntry, replaceEntry, updateContent, + addEntry, updateEntry, removeEntry, replaceEntry, updateContent, loadModifiedFiles, loadGitHistory, loadDiff, loadDiffAtCommit, getNoteStatus, commitAndPush, }