diff --git a/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.test.tsx b/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.test.tsx index cf0028c4..8f09ff81 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.test.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.test.tsx @@ -130,6 +130,18 @@ jest.mock('./PassageDetailPlayer', () => { // }), // })); jest.mock('../../utils/logErrorService', () => jest.fn()); + +const mockMediaFile = { + id: 'm1', + type: 'mediafile', + attributes: { segments: '{}' }, +} as MediaFileD; + +jest.mock('../../crud/tryFindRecord', () => ({ + findRecord: jest.fn((_memory: unknown, table: string, id: string) => + table === 'mediafile' && id === 'm1' ? mockMediaFile : undefined + ), +})); jest.mock('../../hoc/SnackBar', () => { const actual = jest.requireActual('../../hoc/SnackBar'); return { @@ -493,7 +505,32 @@ test('should not add rows including refs already in table', async () => { expect(tbody.children.length).toBe(4); // 3 limits (with 3 verse refs) + 1 header }); -test('shows warning snackbar and blocks autosave when checkRefs has issues', async () => { +test('autosaves segment markup after debounce when only soft validation warnings', async () => { + runTest({ width: 1000 }); + await waitFor(() => + expect(screen.getByTestId('verse-sheet')).toBeInTheDocument() + ); + + act(() => { + if (mockPlayerAction) { + mockPlayerAction( + '{"regions":"[{\\"start\\":0,\\"end\\":5},{\\"start\\":5,\\"end\\":9}]"}', + false + ); + } + }); + + await waitFor( + () => expect(mockProjectSegmentSave).toHaveBeenCalled(), + { timeout: 3000 } + ); + expect(mockShowMessage).not.toHaveBeenCalledWith( + expect.anything(), + AlertSeverity.Warning + ); +}); + +test('blocks autosave when markup has hard reference errors', async () => { jest.useFakeTimers(); runTest({ width: 1000 }); const tbody = screen.getByTestId('verse-sheet')?.firstChild?.firstChild @@ -503,7 +540,7 @@ test('shows warning snackbar and blocks autosave when checkRefs has issues', asy act(() => { if (mockPlayerAction) { mockPlayerAction( - '{"regions":"[{\\"start\\":0,\\"end\\":5},{\\"start\\":5,\\"end\\":9}]"}', + '{"regions":"[{\\"start\\":0,\\"end\\":5,\\"label\\":\\"9:9\\"}]"}', false ); } @@ -516,12 +553,6 @@ test('shows warning snackbar and blocks autosave when checkRefs has issues', asy ); }); - const messageArg = mockShowMessage.mock.calls[0][0] as { - props: { children: unknown[] }; - }; - const messageText = JSON.stringify(messageArg); - expect(messageText).toMatch(/Autosave skipped: \d+ issue\(s\) found\./); - act(() => { jest.advanceTimersByTime(2000); }); diff --git a/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.tsx b/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.tsx index 51a50a57..d5dab1e8 100644 --- a/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.tsx +++ b/src/renderer/src/components/PassageDetail/PassageDetailMarkVerses.tsx @@ -63,9 +63,13 @@ import { isMarkVersesTableRowCompleted, isMarkVersesTableTailIncomplete, } from '../../utils/markVersesSegmentColors'; +import { + getMarkVersesAutosaveBlockers, + getMarkVersesValidationIssues, +} from '../../utils/markVersesValidation'; +import { verseToolId } from './markVersesTool'; const NotTable = 490; -const verseToolId = 'VerseTool'; /** Nudge past a join when seeking so the playhead lands in the right-hand segment. */ const SEGMENT_BOUNDARY_TOLERANCE_SEC = 0.1; /** Table limits use one decimal; waveform uses float seconds — allow rounding drift. */ @@ -334,49 +338,77 @@ export function PassageDetailMarkVerses({ width }: MarkVersesProps) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [passage, engVrs]); + const setStepCompleteRef = useRef(setStepComplete); + setStepCompleteRef.current = setStepComplete; + const gotoNextStepRef = useRef(gotoNextStep); + gotoNextStepRef.current = gotoNextStep; + const currentstepRef = useRef(currentstep); + currentstepRef.current = currentstep; + const handleComplete = (complete: boolean) => { waitForSave(undefined, 200).finally(async () => { - await setStepComplete(currentstep, complete); - if (complete) gotoNextStep(); + await setStepCompleteRef.current(currentstepRef.current, complete); + if (complete) gotoNextStepRef.current(); }); }; + const syncSegmentsRefFromTable = () => { + const regions: IRegion[] = []; + dataRef.current.forEach((r, i) => { + if (i > 0) { + const limits = `${r[ColName.Limits]?.value ?? ''}`.split('-'); + if (limits.length === 2) { + regions.push({ + start: parseFloat(limits[0]), + end: parseFloat(limits[1]), + label: `${r[ColName.Ref]?.value ?? ''}`, + }); + } + } + }); + segmentsRef.current = JSON.stringify({ regions }); + }; + const writeResources = async () => { if (!savingRef.current) { savingRef.current = true; - if (media) { - // update all three segment types: verse, transcription, backtranslation - let segments = updateSegments( - NamedRegions.Transcription, - updateSegments( - NamedRegions.Verse, - media.attributes?.segments, - segmentsRef.current - ), + syncSegmentsRefFromTable(); + if (!media) { + savingRef.current = false; + saveCompleted(verseToolId); + return; + } + // update all three segment types: verse, transcription, backtranslation + let segments = updateSegments( + NamedRegions.Transcription, + updateSegments( + NamedRegions.Verse, + media.attributes?.segments, + segmentsRef.current + ), + segmentsRef.current + ); + if (!hasBtRecordings) { + segments = updateSegments( + NamedRegions.BackTranslation, + segments, segmentsRef.current ); - if (!hasBtRecordings) { - segments = updateSegments( - NamedRegions.BackTranslation, - segments, - segmentsRef.current - ); - } - // remove TRTask segments that handle AI transcription - segments = updateSegments(NamedRegions.TRTask, segments, ''); - projectSegmentSave({ media, segments }) - .then(() => { - saveCompleted(verseToolId); - }) - .catch((err) => { - saveCompleted(verseToolId, err.message); - }) - .finally(() => { - savingRef.current = false; - canceling.current = false; - setComplete(0); - }); } + // remove TRTask segments that handle AI transcription + segments = updateSegments(NamedRegions.TRTask, segments, ''); + projectSegmentSave({ media, segments }) + .then(() => { + saveCompleted(verseToolId); + }) + .catch((err) => { + saveCompleted(verseToolId, err.message); + }) + .finally(() => { + savingRef.current = false; + canceling.current = false; + setComplete(0); + }); } }; @@ -756,51 +788,23 @@ export function PassageDetailMarkVerses({ width }: MarkVersesProps) { else showMessage(tt.noData.replace('{0}', t.markVerses)); }; - useEffect(() => { - if (saveRequested(verseToolId) && !savingRef.current) writeResources(); - else if (clearRequested(verseToolId)) clearCompleted(verseToolId); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [toolsChanged]); + const validationInput = () => ({ + rows: dataRef.current + .filter((_, i) => i > 0) + .map((row) => ({ + limits: `${(row[ColName.Limits] as ICell).value ?? ''}`, + ref: `${(row[ColName.Ref] as ICell).value ?? ''}`, + })), + expandedRefs: collectRefs(dataRef.current), + passageRefs: passageRefs.current, + hasBtRecordings, + strings: t, + }); - const checkRefs = () => { - const refs: string[] = collectRefs(dataRef.current); - const noSegRefs = dataRef.current - .filter((v, i) => i > 0) - .filter( - (v) => - (v[ColName.Ref] as ICell).value && !(v[ColName.Limits] as ICell).value - ) - .map((v) => (v[ColName.Ref] as ICell).value); - const noRefSegs = dataRef.current - .filter((v, i) => i > 0) - .some( - (v) => - !(v[ColName.Ref] as ICell).value && (v[ColName.Limits] as ICell).value - ); - const matchAll = refs.every((r) => refMatch(r)); - const refSet = new Set(passageRefs.current); - const outsideRefs = new Set(); - refs.forEach((r) => { - if (refSet.has(r)) refSet.delete(r); - else if (refMatch(r)) outsideRefs.add(r); - }); - const issues: string[] = []; - if (!matchAll) issues.push(t.badReferences); - if (noSegRefs.length > 0) - issues.push(t.noSegments.replace('{0}', noSegRefs.join(', '))); - if (refSet.size > 0) - issues.push( - t.missingReferences.replace('{0}', Array.from(refSet).sort().join(', ')) - ); - if (outsideRefs.size > 0) { - issues.push( - t.outsideReferences.replace('{0}', Array.from(outsideRefs).join(', ')) - ); - } - if (noRefSegs) issues.push(t.noReferences); - if (hasBtRecordings) issues.push(t.btNotUpdated); - return issues; - }; + const checkRefs = () => getMarkVersesValidationIssues(validationInput()); + + const checkAutosaveBlockers = () => + getMarkVersesAutosaveBlockers(validationInput()); const handleCancel = () => { if (savingRef.current) { @@ -832,13 +836,23 @@ export function PassageDetailMarkVerses({ width }: MarkVersesProps) { [scheduleAutosave] ); + useEffect(() => { + if (saveRequested(verseToolId) && !savingRef.current) { + scheduleAutosave.clear(); + void writeResourcesRef.current(); + } else if (clearRequested(verseToolId)) clearCompleted(verseToolId); + // saveRequested/clearRequested read live UnsavedContext refs via toolsChanged + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [toolsChanged, scheduleAutosave]); + useEffect(() => { if (!isChanged(verseToolId) || !hasPermission || savingRef.current) return; - const issues = checkRefs(); - if (issues.length > 0) { + const allIssues = checkRefs(); + const blockers = checkAutosaveBlockers(); + setSaveIssues(allIssues); + if (blockers.length > 0) { scheduleAutosave.clear(); - setSaveIssues(issues); - const fingerprint = issues.join('\0'); + const fingerprint = blockers.join('\0'); if (fingerprint !== lastIssuesNotifyRef.current) { lastIssuesNotifyRef.current = fingerprint; showMessage( @@ -851,7 +865,7 @@ export function PassageDetailMarkVerses({ width }: MarkVersesProps) { }} > - {t.autosaveSkipped.replace('{0}', String(issues.length))} + {t.autosaveSkipped.replace('{0}', String(blockers.length))}