diff --git a/.gitignore b/.gitignore index 76900ddd..a1dbba9e 100644 --- a/.gitignore +++ b/.gitignore @@ -33,6 +33,9 @@ temp-build # #endregion +# AI files +.claude + # Playwright test output e2e-tests/playwright-report e2e-tests/test-results diff --git a/cspell.json b/cspell.json index d754598f..3046472d 100644 --- a/cspell.json +++ b/cspell.json @@ -77,6 +77,7 @@ "verseref", "versification", "wordform", + "worktrees", "ZWNJ" ], "ignoreWords": ["Ελληνικά", "homme", "ʼelohim", "ʻokina"], diff --git a/jest.config.ts b/jest.config.ts index 992d3459..8ac28296 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -104,8 +104,11 @@ const config: Config = { '^(.+)\\.(scss|sass|css)\\?inline$': '/__mocks__/styleInlineMock.ts', }, - /** Exclude dist from module resolution to avoid Haste naming collision with root package.json. */ - modulePathIgnorePatterns: ['/dist'], + /** + * Exclude `.claude` from module resolution to avoid nested git worktrees. Exclude `dist` from + * module resolution to avoid Haste naming collision with root package.json. + */ + modulePathIgnorePatterns: ['/.claude', '/dist'], /** Load @testing-library/jest-dom matchers and browser API stubs for React component tests. */ setupFilesAfterEnv: [ @@ -129,9 +132,15 @@ const config: Config = { /** * Transform TS/TSX with ts-jest (webpack uses SWC; Jest does not run webpack). Explicitly list * ts-jest so other preprocessors can be added later without dropping TS support. + * + * `isolatedModules: true` transpiles each file individually without a full type-check pass. This + * is required when running from a git worktree whose `typeRoots` relative paths (e.g. + * `../paranext-core/lib`) do not resolve from the worktree subdirectory; those paths are correct + * for the repo root but would cause every test suite to fail with TS2307 otherwise. Type-safety + * is still enforced by `npm run lint:typecheck` (tsc --noEmit) from the repo root. */ transform: { - '\\.tsx?$': 'ts-jest', + '\\.tsx?$': ['ts-jest', { isolatedModules: true }], }, }; diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 8ee06100..55105cc5 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -188,7 +188,7 @@ jest.mock('../../components/modals/ProjectModals', () => ({ * Minimal ProjectModals stand-in that drives modal state and active-project state through the * same `useWebViewState` hook the real component uses, so tests can assert on state transitions * without mounting the full modal tree. Accepts (and ignores) the draft-related props the loader - * now passes (`dirty`, `getDraftSnapshot`, `loadFromProject`, `markSynced`, `resetDraft`). + * now passes (`dirty`, `getDraftSnapshot`, `loadFromProject`, `markSynced`). * * @param modal - Current modal identifier controlling which stub panel is rendered. * @param setModal - Callback to transition to a different modal state. @@ -214,7 +214,6 @@ jest.mock('../../components/modals/ProjectModals', () => ({ getDraftSnapshot: () => DraftProject | undefined; loadFromProject: (project: unknown) => void; markSynced: () => void; - resetDraft: (config: unknown) => void; useWebViewState: ( key: string, def: MockProject | undefined, @@ -958,9 +957,16 @@ describe('InterlinearizerLoader', () => { const edited = emptyAnalysis(); edited.tokenAnalyses.push({ id: 't1', surfaceText: 'In', gloss: { en: 'in' } }); + + // Switch to fake timers only for this test so we can advance past the 300ms debounce. + jest.useFakeTimers(); act(() => { capturedInterlinearizerProps?.onSaveAnalysis?.(edited); }); + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); const saveDraftCall = mockSendCommand.mock.calls.find( ([command]) => command === 'interlinearizer.saveDraft', diff --git a/src/__tests__/components/modals/CreateProjectModal.test.tsx b/src/__tests__/components/modals/CreateProjectModal.test.tsx index b48c343d..23615f88 100644 --- a/src/__tests__/components/modals/CreateProjectModal.test.tsx +++ b/src/__tests__/components/modals/CreateProjectModal.test.tsx @@ -97,6 +97,20 @@ describe('CreateProjectModal', () => { }); }); + it('disables both buttons when isSubmitting is true', () => { + render( {}} onCreateDraft={() => {}} />); + + expect(screen.getByRole('button', { name: /cancel/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: /^create$/i })).toBeDisabled(); + }); + + it('keeps both buttons enabled when isSubmitting is false', () => { + render( {}} onCreateDraft={() => {}} />); + + expect(screen.getByRole('button', { name: /cancel/i })).toBeEnabled(); + expect(screen.getByRole('button', { name: /^create$/i })).toBeEnabled(); + }); + it('defaults the language to ["und"] when the field contains only whitespace', async () => { const onCreateDraft = jest.fn(); render( {}} onCreateDraft={onCreateDraft} />); diff --git a/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx b/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx index 1b1a986b..b9889a5e 100644 --- a/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx +++ b/src/__tests__/components/modals/DiscardDraftConfirm.test.tsx @@ -40,6 +40,20 @@ describe('DiscardDraftConfirm', () => { expect(onConfirm).toHaveBeenCalledTimes(1); }); + it('disables both buttons when isSubmitting is true', () => { + render(); + + expect(screen.getByTestId('discard-draft-confirm')).toBeDisabled(); + expect(screen.getByRole('button', { name: /cancel/i })).toBeDisabled(); + }); + + it('keeps both buttons enabled when isSubmitting is false', () => { + render(); + + expect(screen.getByTestId('discard-draft-confirm')).toBeEnabled(); + expect(screen.getByRole('button', { name: /cancel/i })).toBeEnabled(); + }); + it('calls onCancel when the Cancel button is clicked', async () => { const onCancel = jest.fn(); render(); diff --git a/src/__tests__/components/modals/ProjectModals.test.tsx b/src/__tests__/components/modals/ProjectModals.test.tsx index e040e002..b6bcd925 100644 --- a/src/__tests__/components/modals/ProjectModals.test.tsx +++ b/src/__tests__/components/modals/ProjectModals.test.tsx @@ -93,10 +93,12 @@ jest.mock('../../../components/modals/CreateProjectModal', () => ({ __esModule: true, CreateProjectModal: ({ defaultAnalysisLanguage, + isSubmitting, onClose, onCreateDraft, }: { defaultAnalysisLanguage?: string; + isSubmitting?: boolean; onClose: () => void; onCreateDraft: (config: { analysisLanguages: string[]; @@ -105,12 +107,13 @@ jest.mock('../../../components/modals/CreateProjectModal', () => ({ }) => void; }) => (
- -
@@ -238,7 +248,6 @@ type ModalsOverrides = Partial<{ loadFromProject: jest.Mock; markSynced: jest.Mock; modal: ModalState; - resetDraft: jest.Mock; setModal: jest.Mock; useWebViewState: ReturnType; }>; @@ -260,7 +269,6 @@ function buildProps(overrides: ModalsOverrides = {}) { markSynced: overrides.markSynced ?? jest.fn(), modal: overrides.modal ?? 'none', projectId: 'source-proj', - resetDraft: overrides.resetDraft ?? jest.fn(), setModal: overrides.setModal ?? jest.fn(), useWebViewState: overrides.useWebViewState ?? makeWebViewState(), }; @@ -426,21 +434,84 @@ describe('ProjectModals', () => { }); describe('new (create) flow', () => { - it('resets the draft and closes when not dirty', async () => { - const resetDraft = jest.fn(); + it('creates a project via createProject and closes when not dirty', async () => { + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(MOCK_PROJECT)); const setModal = jest.fn(); - render(); + render(); await userEvent.click(screen.getByTestId('create-submit')); - expect(resetDraft).toHaveBeenCalledWith({ - analysisLanguages: ['en'], - name: 'New', - description: 'Desc', - }); + await waitFor(() => + expect(papi.commands.sendCommand).toHaveBeenCalledWith( + 'interlinearizer.createProject', + 'source-proj', + ['en'], + undefined, + 'New', + 'Desc', + ), + ); expect(setModal).toHaveBeenCalledWith('none'); }); + it('carries targetProjectId into the draft for a bilateral created project', async () => { + const bilateral = { ...MOCK_PROJECT, targetProjectId: 'tgt-new' }; + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(bilateral)); + const loadFromProject = jest.fn(); + render(); + + await userEvent.click(screen.getByTestId('create-submit')); + + await waitFor(() => + expect(loadFromProject).toHaveBeenCalledWith({ + analysisLanguages: ['en'], + targetProjectId: 'tgt-new', + analysis: emptyAnalysis(), + }), + ); + }); + + it('notifies and does not close when createProject returns a non-project shape', async () => { + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify({ bad: true })); + const setModal = jest.fn(); + render(); + + await userEvent.click(screen.getByTestId('create-submit')); + + await waitFor(() => expect(papi.notifications.send).toHaveBeenCalledTimes(1)); + expect(setModal).not.toHaveBeenCalledWith('none'); + }); + + it('does not crash when createProject throws', async () => { + jest.mocked(papi.commands.sendCommand).mockRejectedValueOnce(new Error('network')); + const setModal = jest.fn(); + render(); + + await userEvent.click(screen.getByTestId('create-submit')); + + await waitFor(() => expect(papi.commands.sendCommand).toHaveBeenCalled()); + expect(setModal).not.toHaveBeenCalledWith('none'); + }); + + it('disables the create-submit button while createProject is in flight', async () => { + let resolveCreate!: (value: string) => void; + jest.mocked(papi.commands.sendCommand).mockReturnValueOnce( + new Promise((resolve) => { + resolveCreate = resolve; + }), + ); + render(); + + expect(screen.getByTestId('create-submit')).toBeEnabled(); + + await userEvent.click(screen.getByTestId('create-submit')); + + expect(screen.getByTestId('create-submit')).toBeDisabled(); + + resolveCreate(JSON.stringify(MOCK_PROJECT)); + await waitFor(() => expect(screen.getByTestId('create-submit')).toBeEnabled()); + }); + it('calls setModal with none when the create modal closes without a select source', async () => { const setModal = jest.fn(); render(); @@ -494,19 +565,51 @@ describe('ProjectModals', () => { }); it('confirms before starting a new draft when the draft is dirty', async () => { - const resetDraft = jest.fn(); - render(); + jest.mocked(papi.commands.sendCommand).mockResolvedValueOnce(JSON.stringify(MOCK_PROJECT)); + render(); await userEvent.click(screen.getByTestId('create-submit')); expect(screen.getByTestId('discard-modal')).toBeInTheDocument(); - expect(resetDraft).not.toHaveBeenCalled(); + // createProject must not have been called yet. + expect(papi.commands.sendCommand).not.toHaveBeenCalledWith( + 'interlinearizer.createProject', + expect.anything(), + expect.anything(), + expect.anything(), + expect.anything(), + expect.anything(), + ); await userEvent.click(screen.getByTestId('discard-confirm')); - expect(resetDraft).toHaveBeenCalledWith({ - analysisLanguages: ['en'], - name: 'New', - description: 'Desc', - }); + await waitFor(() => + expect(papi.commands.sendCommand).toHaveBeenCalledWith( + 'interlinearizer.createProject', + 'source-proj', + ['en'], + undefined, + 'New', + 'Desc', + ), + ); + }); + + it('disables the discard-confirm button while createProject is in flight', async () => { + let resolveCreate!: (value: string) => void; + jest.mocked(papi.commands.sendCommand).mockReturnValueOnce( + new Promise((resolve) => { + resolveCreate = resolve; + }), + ); + render(); + + await userEvent.click(screen.getByTestId('create-submit')); + expect(screen.getByTestId('discard-confirm')).toBeEnabled(); + + await userEvent.click(screen.getByTestId('discard-confirm')); + expect(screen.getByTestId('discard-confirm')).toBeDisabled(); + + resolveCreate(JSON.stringify(MOCK_PROJECT)); + await waitFor(() => expect(screen.queryByTestId('discard-modal')).toBeNull()); }); }); diff --git a/src/__tests__/hooks/useDraftProject.test.ts b/src/__tests__/hooks/useDraftProject.test.ts index bbd615d0..602683c4 100644 --- a/src/__tests__/hooks/useDraftProject.test.ts +++ b/src/__tests__/hooks/useDraftProject.test.ts @@ -150,10 +150,16 @@ describe('useDraftProject', () => { it('stores the edited analysis, marks the draft dirty, and persists with dirty:true', async () => { const { result } = await renderLoaded(); + jest.useFakeTimers(); const edited = analysisWithToken('tok-autosave'); act(() => { result.current.autosaveAnalysis(edited); }); + // Advance past the debounce window so the scheduled persist fires. + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); expect(result.current.dirty).toBe(true); expect(result.current.draft?.analysis.tokenAnalyses[0].id).toBe('tok-autosave'); @@ -165,12 +171,18 @@ describe('useDraftProject', () => { it('does not error or re-render when called again while already dirty', async () => { const { result } = await renderLoaded(); + jest.useFakeTimers(); act(() => { result.current.autosaveAnalysis(analysisWithToken('first')); }); act(() => { result.current.autosaveAnalysis(analysisWithToken('second')); }); + // The second call replaces the first timer; advance past the debounce to flush the write. + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); expect(result.current.dirty).toBe(true); // The second autosave does not bump draftVersion and dirty was already true, so it does not @@ -182,55 +194,6 @@ describe('useDraftProject', () => { }); }); - describe('resetDraft', () => { - it('resets to an empty analysis, clears dirty, bumps the version, and persists', async () => { - const { result } = await renderLoaded(); - const versionBefore = result.current.draftVersion; - - act(() => { - result.current.resetDraft({ analysisLanguages: ['es'] }); - }); - - expect(result.current.draft?.analysis.tokenAnalyses).toEqual([]); - expect(result.current.draft?.analysisLanguages).toEqual(['es']); - expect(result.current.dirty).toBe(false); - expect(result.current.draftVersion).toBe(versionBefore + 1); - const saved = lastSavedDraft(); - expect(saved.dirty).toBe(false); - expect(saved.analysisLanguages).toEqual(['es']); - }); - - it('carries name, description, and target project id onto the reset draft', async () => { - const { result } = await renderLoaded(); - - act(() => { - result.current.resetDraft({ - analysisLanguages: ['es'], - targetProjectId: 'target-1', - name: 'My Draft', - description: 'A description', - }); - }); - - expect(result.current.draft?.targetProjectId).toBe('target-1'); - expect(result.current.draft?.suggestedName).toBe('My Draft'); - expect(result.current.draft?.suggestedDescription).toBe('A description'); - }); - - it('seeds the platform language when the New config supplies no analysis languages', async () => { - const { result } = await renderLoaded(); - - act(() => { - result.current.resetDraft({ analysisLanguages: [] }); - }); - - expect(result.current.draft?.analysisLanguages).toEqual([PLATFORM_LANGUAGE]); - expect(result.current.draft?.targetProjectId).toBeUndefined(); - expect(result.current.draft?.suggestedName).toBeUndefined(); - expect(result.current.draft?.suggestedDescription).toBeUndefined(); - }); - }); - describe('loadFromProject', () => { it('copies analysis, analysis languages, and target, clears dirty, and bumps the version', async () => { const { result } = await renderLoaded(); @@ -265,6 +228,35 @@ describe('useDraftProject', () => { expect(result.current.draft?.targetProjectId).toBeUndefined(); }); + + it('cancels a pending autosave so the stale edit is not written after the replacement', async () => { + const { result } = await renderLoaded(); + const savesBefore = mockSendCommand.mock.calls.filter( + (c) => c[0] === 'interlinearizer.saveDraft', + ).length; + + jest.useFakeTimers(); + act(() => { + result.current.autosaveAnalysis(analysisWithToken('pending')); + }); + act(() => { + result.current.loadFromProject({ + analysis: analysisWithToken('replaced'), + analysisLanguages: ['de'], + }); + }); + act(() => { + jest.advanceTimersByTime(300); + }); + jest.useRealTimers(); + + // loadFromProject itself persists once; the cancelled autosave must not add a second save. + const savesAfter = mockSendCommand.mock.calls.filter( + (c) => c[0] === 'interlinearizer.saveDraft', + ).length; + expect(savesAfter - savesBefore).toBe(1); + expect(lastSavedDraft().analysis.tokenAnalyses[0].id).toBe('replaced'); + }); }); describe('wipeBook', () => { diff --git a/src/__tests__/services/projectStorage.test.ts b/src/__tests__/services/projectStorage.test.ts index becbebf5..713a611c 100644 --- a/src/__tests__/services/projectStorage.test.ts +++ b/src/__tests__/services/projectStorage.test.ts @@ -599,6 +599,17 @@ describe('projectStorage', () => { await expect(getDraft(token, 'src-proj')).rejects.toThrow(SyntaxError); }); + + it('returns an empty draft and warns when the stored value does not match DraftProject shape', async () => { + __mockReadUserData.mockResolvedValue(JSON.stringify({ not: 'a draft' })); + + const result = await getDraft(token, 'src-proj'); + + expect(result).toEqual(emptyDraft('src-proj')); + expect(__mockLogger.warn).toHaveBeenCalledWith( + 'Interlinearizer: stored draft failed validation; resetting to empty draft', + ); + }); }); describe('saveDraft', () => { diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 637cfa68..f8e3d945 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -8,7 +8,7 @@ import { useData, useSetting } from '@papi/frontend/react'; import { TabToolbar } from 'platform-bible-react'; import type { SelectMenuItemHandler } from 'platform-bible-react'; import { isPlatformError } from 'platform-bible-utils'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import useDraftProject from '../hooks/useDraftProject'; import useInterlinearizerBookData from '../hooks/useInterlinearizerBookData'; import useOptimisticBooleanSetting from '../hooks/useOptimisticBooleanSetting'; @@ -142,7 +142,6 @@ function InterlinearizerLoaderInner({ draftVersion, dirty, autosaveAnalysis, - resetDraft, loadFromProject, getDraftSnapshot, markSynced, @@ -253,6 +252,8 @@ function InterlinearizerLoaderInner({ setPhraseMode({ kind: 'view' }); }, [draftVersion]); + const isSavingRef = useRef(false); + /** * Saves the current draft to the active project. When no project is active there is nothing to * save to yet, so it opens Save As instead. Errors are logged; the backend surfaces the @@ -265,9 +266,15 @@ function InterlinearizerLoaderInner({ setModal('saveAs'); return; } + /* v8 ignore next -- re-entry guard; handles simultaneous saves during async round-trip */ + if (isSavingRef.current) return; + isSavingRef.current = true; const snapshot = getDraftSnapshot(); - /* v8 ignore next -- save is only reachable once the editor (and draft) have loaded */ - if (!snapshot) return; + /* v8 ignore next 4 -- save is only reachable once the editor (and draft) have loaded */ + if (!snapshot) { + isSavingRef.current = false; + return; + } try { await papi.commands.sendCommand( 'interlinearizer.saveAnalysis', @@ -277,6 +284,8 @@ function InterlinearizerLoaderInner({ markSynced(snapshot.analysis); } catch (e) { logger.error('Interlinearizer: failed to save draft to project', e); + } finally { + isSavingRef.current = false; } }, [activeProject, getDraftSnapshot, markSynced, setModal]); @@ -456,7 +465,6 @@ function InterlinearizerLoaderInner({ markSynced={markSynced} modal={modal} projectId={projectId} - resetDraft={resetDraft} setModal={setModal} useWebViewState={useWebViewState} /> diff --git a/src/components/modals/CreateProjectModal.tsx b/src/components/modals/CreateProjectModal.tsx index 3e33d864..35781efe 100644 --- a/src/components/modals/CreateProjectModal.tsx +++ b/src/components/modals/CreateProjectModal.tsx @@ -30,14 +30,14 @@ export type CreateDraftConfig = { /** * Modal dialog that collects the configuration for a new draft — name, description, and analysis - * language(s) — then hands it back via {@link onCreateDraft}. No project is persisted here: "New" - * resets the working draft to an empty baseline, and a project is only materialized later via Save - * As. The typed name/description are retained on the draft to prefill that Save As dialog. + * language(s) — then hands it back via {@link onCreateDraft}. * * @param props - Component props * @param props.defaultAnalysisLanguage - BCP 47 tag pre-populated in the analysis language field; * caller should pass the platform UI language so the user sees a sensible starting value. * Defaults to `'und'` when absent. + * @param props.isSubmitting - When `true`, both buttons are disabled to prevent interaction while + * the caller is processing the submitted configuration. * @param props.onClose - Callback invoked when the modal should be dismissed (cancel). * @param props.onCreateDraft - Callback invoked with the collected configuration on submit. * @returns The modal overlay with name, description, and language inputs, or nothing while @@ -45,11 +45,14 @@ export type CreateDraftConfig = { */ export function CreateProjectModal({ defaultAnalysisLanguage, + isSubmitting = false, onClose, onCreateDraft, }: Readonly<{ /** BCP 47 tag pre-populated in the analysis language field; defaults to `'und'` when absent. */ defaultAnalysisLanguage?: string; + /** When `true`, both buttons are disabled while the caller processes the submitted config. */ + isSubmitting?: boolean; onClose: () => void; onCreateDraft: (config: CreateDraftConfig) => void; }>) { @@ -122,10 +125,10 @@ export function CreateProjectModal({ placeholder={localizedStrings['%interlinearizer_modal_create_language_placeholder%']} />
- -
diff --git a/src/components/modals/DiscardDraftConfirm.tsx b/src/components/modals/DiscardDraftConfirm.tsx index 7d03b50b..1f6a8e80 100644 --- a/src/components/modals/DiscardDraftConfirm.tsx +++ b/src/components/modals/DiscardDraftConfirm.tsx @@ -15,16 +15,20 @@ const DISCARD_DRAFT_CONFIRM_STRING_KEYS: `%${string}%`[] = [ * returns to the previous dialog. * * @param props - Component props - * @param props.onConfirm - Called when the user accepts discarding the draft's unsaved changes. + * @param props.isSubmitting - When `true`, both buttons are disabled to prevent interaction while + * the caller is processing the confirmed action. * @param props.onCancel - Called when the user backs out, leaving the draft untouched. + * @param props.onConfirm - Called when the user accepts discarding the draft's unsaved changes. * @returns The confirmation overlay, or nothing while localized strings are loading. */ export function DiscardDraftConfirm({ + isSubmitting = false, onConfirm, onCancel, }: Readonly<{ - onConfirm: () => void; + isSubmitting?: boolean; onCancel: () => void; + onConfirm: () => void; }>) { const [localizedStrings, stringsLoading] = useLocalizedStrings(DISCARD_DRAFT_CONFIRM_STRING_KEYS); @@ -45,10 +49,15 @@ export function DiscardDraftConfirm({ {localizedStrings['%interlinearizer_confirm_discard_body%']}

- -
diff --git a/src/components/modals/ProjectMetadataModal.tsx b/src/components/modals/ProjectMetadataModal.tsx index d4e8087e..dc22ce38 100644 --- a/src/components/modals/ProjectMetadataModal.tsx +++ b/src/components/modals/ProjectMetadataModal.tsx @@ -2,7 +2,7 @@ import papi, { logger } from '@papi/frontend'; import { useLocalizedStrings } from '@papi/frontend/react'; import { Trash2 } from 'lucide-react'; import { Button } from 'platform-bible-react'; -import { useCallback, useMemo, useRef, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; /** Localized string keys used by {@link ProjectMetadataModal}. */ const PROJECT_METADATA_MODAL_STRING_KEYS: `%${string}%`[] = [ @@ -82,7 +82,6 @@ export function ProjectMetadataModal({ const [editLanguages, setEditLanguages] = useState(analysisLanguages.join(', ')); const [confirmingDelete, setConfirmingDelete] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false); - const isSubmittingRef = useRef(false); const formattedDate = useMemo(() => new Date(createdAt).toLocaleString(), [createdAt]); @@ -98,9 +97,8 @@ export function ProjectMetadataModal({ * @returns A promise that resolves when the command completes or the error is logged. */ const handleSave = useCallback(async () => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); const newName = editName.trim() || undefined; const newDescription = editDescription.trim() || undefined; @@ -127,10 +125,10 @@ export function ProjectMetadataModal({ } catch (e) { logger.error('Interlinearizer: failed to save project metadata', e); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } }, [ + isSubmitting, editName, editDescription, editLanguages, @@ -148,9 +146,8 @@ export function ProjectMetadataModal({ * @returns A promise that resolves when the command completes or the error is logged. */ const handleDelete = useCallback(async () => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); try { await papi.commands.sendCommand('interlinearizer.deleteProject', interlinearProjectId); @@ -159,10 +156,9 @@ export function ProjectMetadataModal({ } catch (e) { logger.error('Interlinearizer: failed to delete project', e); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } - }, [interlinearProjectId, onProjectDeleted, onClose]); + }, [interlinearProjectId, isSubmitting, onClose, onProjectDeleted]); /* v8 ignore next */ if (stringsLoading) return undefined; diff --git a/src/components/modals/ProjectModals.tsx b/src/components/modals/ProjectModals.tsx index e0cd3e7f..ffcf086f 100644 --- a/src/components/modals/ProjectModals.tsx +++ b/src/components/modals/ProjectModals.tsx @@ -2,7 +2,8 @@ import type { UseWebViewStateHook } from '@papi/core'; import papi, { logger } from '@papi/frontend'; import type { DraftProject, TextAnalysis } from 'interlinearizer'; import { useCallback, useState } from 'react'; -import type { NewDraftConfig, OpenableProject } from '../../hooks/useDraftProject'; +import type { OpenableProject } from '../../hooks/useDraftProject'; +import { emptyAnalysis } from '../../types/empty-factories'; import type { InterlinearProjectSummary } from '../../types/interlinear-project-summary'; import { isInterlinearProjectSummary, isTextAnalysis } from '../../types/type-guards'; import { CreateProjectModal, type CreateDraftConfig } from './CreateProjectModal'; @@ -19,7 +20,7 @@ export type ModalState = 'none' | 'select' | 'create' | 'metadata' | 'saveAs'; * empty draft or opening an existing project into the draft. */ type PendingReplace = - | { kind: 'new'; config: NewDraftConfig } + | { kind: 'new'; config: CreateDraftConfig } | { kind: 'open'; project: InterlinearProjectSummary }; /** @@ -45,8 +46,6 @@ type PendingReplace = * given the analysis that was persisted; a no-op if an edit landed during the save. * @param props.modal - Which modal is currently open. * @param props.projectId - PAPI source project ID passed from the host. - * @param props.resetDraft - Resets the draft to an empty baseline with the given config (the "New" - * flow). * @param props.setModal - Setter for which modal is open. * @param props.useWebViewState - Hook for reading and writing values persisted in the WebView's * saved state (survives tab restores). @@ -61,7 +60,6 @@ export default function ProjectModals({ markSynced, modal, projectId, - resetDraft, setModal, useWebViewState, }: Readonly<{ @@ -73,7 +71,6 @@ export default function ProjectModals({ markSynced: (savedAnalysis: TextAnalysis) => void; modal: ModalState; projectId: string; - resetDraft: (config: NewDraftConfig) => void; setModal: (modal: ModalState) => void; useWebViewState: UseWebViewStateHook; }>) { @@ -108,6 +105,18 @@ export default function ProjectModals({ /** A draft-replacing action awaiting confirmation because the draft has unsaved changes. */ const [pendingReplace, setPendingReplace] = useState(undefined); + /** + * Whether a backend project-creation round-trip is in progress; drives `isSubmitting` on the + * Create modal. + */ + const [isCreating, setIsCreating] = useState(false); + + /** + * Whether the discard-and-replace confirmation flow is in flight (either opening an existing + * project or creating a new draft); disables DiscardDraftConfirm buttons to prevent races. + */ + const [isReplacing, setIsReplacing] = useState(false); + const resolvedMetadataProject = metadataProject ?? activeProject; /** @@ -175,6 +184,7 @@ export default function ProjectModals({ .catch(() => {}); return; } + loadFromProject({ analysisLanguages: parsed.analysisLanguages, ...(parsed.targetProjectId !== undefined && { targetProjectId: parsed.targetProjectId }), @@ -193,19 +203,58 @@ export default function ProjectModals({ ); /** - * Resets the draft to a fresh empty baseline with the chosen config and clears the Save target so - * the next Save behaves as Save As. + * Creates a new interlinear project with the given config, loads it into the draft as the active + * working copy, and dismisses the modal. Logs and notifies on failure, leaving the draft + * untouched. * * @param config - The configuration collected by the New dialog. + * @returns A promise that resolves once the project is created and loaded, or the failure is + * handled. */ const startNewDraft = useCallback( - (config: CreateDraftConfig) => { - resetDraft(config); - resetActiveProject(); + async (config: CreateDraftConfig) => { + try { + const createdJson = await papi.commands.sendCommand( + 'interlinearizer.createProject', + projectId, + config.analysisLanguages, + undefined, + config.name, + config.description, + ); + const created: unknown = JSON.parse(createdJson); + if (!isInterlinearProjectSummary(created)) { + await papi.notifications + .send({ + message: '%interlinearizer_error_create_project_failed%', + severity: 'error', + }) + .catch(() => {}); + return; + } + + loadFromProject({ + analysisLanguages: created.analysisLanguages, + ...(created.targetProjectId !== undefined && { + targetProjectId: created.targetProjectId, + }), + analysis: emptyAnalysis(), + }); + setActiveProject(created); + } catch (e) { + logger.error('Interlinearizer: failed to create interlinear project', e); + await papi.notifications + .send({ + message: '%interlinearizer_error_create_project_failed%', + severity: 'error', + }) + .catch(() => {}); + return; + } setCreateSourceIsSelect(false); setModal('none'); }, - [resetDraft, resetActiveProject, setModal], + [projectId, loadFromProject, setActiveProject, setModal], ); /** @@ -224,26 +273,49 @@ export default function ProjectModals({ /** * Called when the New dialog is submitted. Starts the new draft immediately, or defers behind the - * unsaved-changes confirmation when the draft is dirty. + * unsaved-changes confirmation when the draft is dirty. A re-entry guard prevents a second + * submission while the first creation round-trip is in flight. * * @param config - The configuration collected by the New dialog. */ const handleCreateDraft = useCallback( - (config: CreateDraftConfig) => { - if (dirty) setPendingReplace({ kind: 'new', config }); - else startNewDraft(config); + async (config: CreateDraftConfig) => { + if (dirty) { + setPendingReplace({ kind: 'new', config }); + return; + } + + /* v8 ignore next -- button is disabled while creating; guards against programmatic double-invocation */ + if (isCreating) return; + + setIsCreating(true); + await startNewDraft(config).finally(() => { + setIsCreating(false); + }); }, - [dirty, startNewDraft], + [dirty, isCreating, startNewDraft], ); /** Confirms the deferred draft-replacing action after the user accepts losing unsaved changes. */ - const handleConfirmReplace = useCallback(() => { + const handleConfirmReplace = useCallback(async () => { /* v8 ignore next -- the confirm only renders while a pending action exists */ if (!pendingReplace) return; - if (pendingReplace.kind === 'open') openProject(pendingReplace.project); - else startNewDraft(pendingReplace.config); + + /* v8 ignore next -- buttons are disabled while replacing; guards against programmatic double-invocation */ + if (isReplacing) return; + + setIsReplacing(true); + try { + if (pendingReplace.kind === 'open') { + await openProject(pendingReplace.project); + } else { + await startNewDraft(pendingReplace.config); + } + } finally { + setIsReplacing(false); + } setPendingReplace(undefined); - }, [pendingReplace, openProject, startNewDraft]); + }, [isReplacing, openProject, pendingReplace, startNewDraft]); /** Cancels the deferred action, returning to the underlying modal with the draft untouched. */ const handleCancelReplace = useCallback(() => setPendingReplace(undefined), []); @@ -262,6 +334,7 @@ export default function ProjectModals({ const snapshot = getDraftSnapshot(); /* v8 ignore next -- the Save As modal is only open once the draft has loaded */ if (!snapshot) return; + try { const createdJson = await papi.commands.sendCommand( 'interlinearizer.createProject', @@ -278,6 +351,7 @@ export default function ProjectModals({ .catch(() => {}); return; } + await papi.commands.sendCommand( 'interlinearizer.saveAnalysis', created.id, @@ -308,6 +382,7 @@ export default function ProjectModals({ const snapshot = getDraftSnapshot(); /* v8 ignore next -- the Save As modal is only open once the draft has loaded */ if (!snapshot) return; + try { await papi.commands.sendCommand( 'interlinearizer.saveAnalysis', @@ -394,6 +469,7 @@ export default function ProjectModals({ {modal === 'create' && ( @@ -429,7 +505,11 @@ export default function ProjectModals({ returns to that modal with its in-progress input intact, and confirming an Open does not unmount (and re-fetch) the still-open select modal underneath. */} {pendingReplace && ( - + )} ); diff --git a/src/components/modals/SaveAsProjectModal.tsx b/src/components/modals/SaveAsProjectModal.tsx index 98ecad36..deb5b9cf 100644 --- a/src/components/modals/SaveAsProjectModal.tsx +++ b/src/components/modals/SaveAsProjectModal.tsx @@ -70,9 +70,6 @@ export function SaveAsProjectModal({ * re-submits. */ const [isSubmitting, setIsSubmitting] = useState(false); - // Ref mirror of `isSubmitting` so a submit handler can short-circuit a second invocation - // synchronously, before the re-render that disables the button lands (guards programmatic races). - const isSubmittingRef = useRef(false); /** The existing project pending an overwrite confirmation, or `undefined`. */ const [confirmOverwrite, setConfirmOverwrite] = useState( @@ -128,17 +125,15 @@ export function SaveAsProjectModal({ * projects. */ const handleSaveNew = useCallback(async () => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); try { await onSaveNew(name.trim() || undefined, description.trim() || undefined); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } - }, [name, description, onSaveNew]); + }, [description, isSubmitting, name, onSaveNew]); /** * Overwrites the chosen existing project with the draft, blocking re-entry while the save is in @@ -148,18 +143,16 @@ export function SaveAsProjectModal({ */ const handleConfirmOverwrite = useCallback( async (project: InterlinearProjectSummary) => { - /* v8 ignore next -- button is disabled while submitting; ref guards against programmatic races */ - if (isSubmittingRef.current) return; - isSubmittingRef.current = true; + /* v8 ignore next -- button is disabled while submitting; guards against programmatic double-invocation */ + if (isSubmitting) return; setIsSubmitting(true); try { await onOverwrite(project); } finally { - isSubmittingRef.current = false; setIsSubmitting(false); } }, - [onOverwrite], + [isSubmitting, onOverwrite], ); /* v8 ignore next */ if (stringsLoading) return undefined; diff --git a/src/hooks/useDraftProject.ts b/src/hooks/useDraftProject.ts index bbeb5b44..4cc834ff 100644 --- a/src/hooks/useDraftProject.ts +++ b/src/hooks/useDraftProject.ts @@ -5,17 +5,8 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; import { removeBookFromAnalysis } from '../utils/analysis-book'; -/** Configuration captured by the "New" flow when resetting the draft to an empty baseline. */ -export type NewDraftConfig = { - /** BCP 47 gloss / annotation language tags for the new draft. */ - analysisLanguages: string[]; - /** Optional alignment target-text project ID. */ - targetProjectId?: string; - /** Optional name typed in the New dialog, retained on the draft to prefill Save As. */ - name?: string; - /** Optional description typed in the New dialog, retained on the draft to prefill Save As. */ - description?: string; -}; +/** Milliseconds to wait after the last keystroke before flushing an autosave write. */ +const AUTOSAVE_DEBOUNCE_MS = 300; /** The subset of an {@link InterlinearProject} needed to open it into the draft as a working copy. */ export type OpenableProject = Pick< @@ -58,12 +49,6 @@ export type UseDraftProjectResult = { * @param analysis - The updated analysis from the store. */ autosaveAnalysis: (analysis: TextAnalysis) => void; - /** - * Resets the draft to an empty analysis with the given config — the "New" flow. - * - * @param config - Gloss languages and optional alignment target for the fresh draft. - */ - resetDraft: (config: NewDraftConfig) => void; /** * Replaces the draft with a working copy of an existing project's analysis and config — the * "Open" flow. @@ -117,11 +102,16 @@ export default function useDraftProject( const [draftVersion, setDraftVersion] = useState(0); const [dirty, setDirty] = useState(false); - // Read the latest platform language via a ref so the load effect (keyed only on sourceProjectId) + // Read the latest platform language via a ref so the load effect (keyed on sourceProjectId) // does not re-run when the UI language changes after the draft has loaded. const platformLanguageRef = useRef(platformLanguage); platformLanguageRef.current = platformLanguage; + // Pending debounced-autosave timer. Flushed on unmount/source change (so the last edit is not + // lost), and cancelled on any wholesale replacement (applyReplacement) so stale keystroke data + // is never written after a New / Open / Wipe. + const autosaveTimeoutRef = useRef | undefined>(undefined); + /** * Persists `draft` to storage, fire-and-forget. The backend surfaces an error notification on * failure; here we only log so a rejected write never throws into a render or event handler. @@ -157,6 +147,7 @@ export default function useDraftProject( draft = emptyDraft(sourceProjectId); } if (canceled) return; + // Seed a gloss language in memory when the stored draft has none (a brand-new source). Not // persisted here — the first auto-save / New / Open carries it to storage. if (draft.analysisLanguages.length === 0) @@ -169,8 +160,14 @@ export default function useDraftProject( load(); return () => { canceled = true; + if (autosaveTimeoutRef.current !== undefined) { + clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = undefined; + // Flush the pending write so the last edit before unmount/source-change is not lost. + if (draftRef.current) persist(draftRef.current); + } }; - }, [sourceProjectId]); + }, [persist, sourceProjectId]); const getDraftSnapshot = useCallback(() => draftRef.current, []); @@ -182,6 +179,12 @@ export default function useDraftProject( */ const applyReplacement = useCallback( (next: DraftProject) => { + // Cancel any pending debounced autosave so stale keystroke data is not written after a + // wholesale replacement (New / Open / Wipe). + if (autosaveTimeoutRef.current !== undefined) { + clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = undefined; + } draftRef.current = next; persist(next); setDirty(next.dirty); @@ -195,34 +198,21 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- auto-save only fires from the mounted editor, which exists only post-load */ if (!current) return; + const next: DraftProject = { ...current, analysis, dirty: true }; draftRef.current = next; - persist(next); + // Debounce writes so rapid keystrokes don't queue unbounded commands to the backend. + if (autosaveTimeoutRef.current !== undefined) clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = setTimeout(() => { + autosaveTimeoutRef.current = undefined; + persist(next); + }, AUTOSAVE_DEBOUNCE_MS); // No version bump (no remount) and a no-op when already dirty, so editing does not re-render. setDirty(true); }, [persist], ); - const resetDraft = useCallback( - (config: NewDraftConfig) => { - const analysisLanguages = - config.analysisLanguages.length > 0 - ? config.analysisLanguages - : [platformLanguageRef.current]; - applyReplacement({ - sourceProjectId, - analysisLanguages, - ...(config.targetProjectId !== undefined && { targetProjectId: config.targetProjectId }), - ...(config.name !== undefined && { suggestedName: config.name }), - ...(config.description !== undefined && { suggestedDescription: config.description }), - analysis: emptyAnalysis(), - dirty: false, - }); - }, - [applyReplacement, sourceProjectId], - ); - const loadFromProject = useCallback( (project: OpenableProject) => { applyReplacement({ @@ -241,6 +231,7 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- wipe is only reachable from the mounted editor */ if (!current) return; + applyReplacement({ ...current, analysis: removeBookFromAnalysis(current.analysis, bookCode), @@ -254,6 +245,7 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- wipe is only reachable from the mounted editor */ if (!current) return; + // Wiping the whole draft is treated as a clean baseline rather than an unsaved edit: it clears // the unsaved-changes indicator (dirty: false) so the user is not nagged to save an empty // draft. The active project is intentionally left untouched, so a subsequent Save still targets @@ -266,11 +258,19 @@ export default function useDraftProject( const { current } = draftRef; /* v8 ignore next -- save is only reachable from the mounted editor */ if (!current) return; + // If an edit landed during the save round-trip, autosaveAnalysis has already swapped a newer // analysis (a fresh object) into the ref and marked the draft dirty. Leave it dirty so the // unsaved indicator and the next Save reflect that un-persisted edit, rather than clearing it // against the now-stale snapshot we just wrote. if (current.analysis !== savedAnalysis) return; + + // Cancel any pending debounced autosave before persisting the clean state so a stale + // {dirty: true} timer cannot fire after this and overwrite the {dirty: false} record. + if (autosaveTimeoutRef.current !== undefined) { + clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = undefined; + } const next: DraftProject = { ...current, dirty: false }; draftRef.current = next; persist(next); @@ -286,7 +286,6 @@ export default function useDraftProject( dirty, getDraftSnapshot, autosaveAnalysis, - resetDraft, loadFromProject, wipeBook, wipeAll, diff --git a/src/services/projectStorage.ts b/src/services/projectStorage.ts index 49cf9d8d..42b242a3 100644 --- a/src/services/projectStorage.ts +++ b/src/services/projectStorage.ts @@ -2,6 +2,7 @@ import papi, { logger } from '@papi/backend'; import type { ExecutionToken } from '@papi/core'; import type { DraftProject, InterlinearProject, TextAnalysis } from 'interlinearizer'; import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; +import { isDraftProject } from '../types/type-guards'; const PROJECT_IDS_KEY = 'projectIds'; @@ -372,7 +373,14 @@ export async function getDraft( sourceProjectId: string, ): Promise { try { - return JSON.parse(await papi.storage.readUserData(token, draftKey(sourceProjectId))); + const parsed: unknown = JSON.parse( + await papi.storage.readUserData(token, draftKey(sourceProjectId)), + ); + if (!isDraftProject(parsed) || parsed.sourceProjectId !== sourceProjectId) { + logger.warn('Interlinearizer: stored draft failed validation; resetting to empty draft'); + return emptyDraft(sourceProjectId); + } + return parsed; } catch (e) { if (isNotFound(e)) return emptyDraft(sourceProjectId); throw e;