From 7d9b8c770db4510571ab5307d126e548888a5fde Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 20 Oct 2026 10:16:58 -0700 Subject: [PATCH] Cleanup doc detail run zone ocr --- .../document-detail.component.spec.ts | 47 ++++++++++--- .../document-detail.component.ts | 69 +++++++++---------- 2 files changed, 72 insertions(+), 44 deletions(-) diff --git a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts index b6fbb19ff..034af9df8 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts @@ -1900,7 +1900,7 @@ describe('DocumentDetailComponent', () => { expect(toastSpy).toHaveBeenCalledWith('Document reloaded.') }) - it('should run zone OCR, report results and force-reload the document', () => { + it('should run zone OCR, report results and reload the document', () => { component.documentId.set(doc.id) component.document.set(doc) jest.spyOn(documentService, 'runZoneOcr').mockReturnValue( @@ -1920,7 +1920,7 @@ describe('DocumentDetailComponent', () => { component.runZoneOcr() expect(toastSpy).toHaveBeenCalledWith( - 'Filled 1 of 2 fields. Failed to match zones: Due' + 'Filled 1 of 2 fields. No value found for: Due' ) expect(notifySpy).toHaveBeenCalledWith(doc.id) expect(loadSpy).toHaveBeenCalledWith(doc.id, true, expect.any(Function)) @@ -1930,7 +1930,33 @@ describe('DocumentDetailComponent', () => { expect(component.networkActive()).toBe(false) }) - it('asks before running zone OCR on a dirty document and leaves edits intact on cancel', () => { + it('should summarize zone OCR results', () => { + component.documentId.set(doc.id) + component.document.set(doc) + jest.spyOn(component as any, 'loadDocument').mockImplementation(() => {}) + const runSpy = jest.spyOn(documentService, 'runZoneOcr') + const toastSpy = jest.spyOn(toastService, 'showInfo') + + runSpy.mockReturnValueOnce(of({ status: 'completed', results: [] })) + component.runZoneOcr() + expect(toastSpy).toHaveBeenLastCalledWith( + 'Zone OCR found no fields to fill.' + ) + + component.networkActive.set(false) + runSpy.mockReturnValueOnce( + of({ + status: 'completed', + results: [ + { template: 'T', zone: 'Total', custom_field: 'Amount', value: 0 }, + ], + }) + ) + component.runZoneOcr() + expect(toastSpy).toHaveBeenLastCalledWith('Filled 1 of 1 fields.') + }) + + it('should confirm before running zone OCR over unsaved changes', () => { component.documentId.set(doc.id) component.document.set({ ...doc, __changedFields: ['title'] }) component.documentForm.patchValue({ title: 'Unsaved title' }) @@ -1950,7 +1976,7 @@ describe('DocumentDetailComponent', () => { expect(component.zoneOcrRunning()).toBe(false) }) - it('runs zone OCR only after confirmation and prevents duplicate requests', () => { + it('should not allow saving or a second zone OCR run while one is running', () => { component.documentId.set(doc.id) component.document.set({ ...doc, __changedFields: ['title'] }) const pending = new Subject() @@ -1976,7 +2002,7 @@ describe('DocumentDetailComponent', () => { expect(component.document().__changedFields).toEqual(['title']) }) - it('does not reload a different document when zone OCR finishes', () => { + it('should not reload after zone OCR if another document is open', () => { component.documentId.set(doc.id) component.document.set({ ...doc, __changedFields: [] }) const pending = new Subject() @@ -1992,7 +2018,7 @@ describe('DocumentDetailComponent', () => { pending.complete() }) - it('does not reload or discard local edits when zone OCR is skipped', () => { + it('should keep unsaved changes when zone OCR is skipped', () => { component.documentId.set(doc.id) component.document.set({ ...doc, __changedFields: ['title'] }) component.documentForm.patchValue({ title: 'Unsaved title' }) @@ -2010,7 +2036,9 @@ describe('DocumentDetailComponent', () => { expect(component.documentForm.get('title').value).toBe('Unsaved title') expect(component.zoneOcrRunning()).toBe(false) expect(component.networkActive()).toBe(false) - expect(infoSpy).toHaveBeenCalledWith('Zone OCR skipped.') + expect(infoSpy).toHaveBeenCalledWith( + 'No OCR template applies to this document.' + ) }) it('should show an error when zone OCR fails', () => { @@ -2023,7 +2051,10 @@ describe('DocumentDetailComponent', () => { component.runZoneOcr() - expect(errorSpy).toHaveBeenCalledWith('Zone OCR failed', expect.any(Error)) + expect(errorSpy).toHaveBeenCalledWith( + 'Error running zone OCR.', + expect.any(Error) + ) expect(loadSpy).not.toHaveBeenCalled() expect(component.zoneOcrRunning()).toBe(false) expect(component.networkActive()).toBe(false) diff --git a/src-ui/src/app/components/document-detail/document-detail.component.ts b/src-ui/src/app/components/document-detail/document-detail.component.ts index c713484ce..518d517a6 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.ts @@ -64,6 +64,7 @@ import { FILTER_STORAGE_PATH, } from 'src/app/data/filter-rule-type' import { ObjectWithId } from 'src/app/data/object-with-id' +import { OcrZoneRunResult } from 'src/app/data/ocr-template' import { StoragePath } from 'src/app/data/storage-path' import { Tag } from 'src/app/data/tag' import { SETTINGS_KEYS, SuggestionSource } from 'src/app/data/ui-settings' @@ -1590,55 +1591,51 @@ export class DocumentDetailComponent private executeZoneOcr(documentId: number) { this.zoneOcrRunning.set(true) this.networkActive.set(true) - const finish = () => { - this.zoneOcrRunning.set(false) - this.networkActive.set(false) - this.flushPendingIncomingUpdate() - } this.documentsService .runZoneOcr(documentId) .pipe(takeUntil(this.unsubscribeNotifier)) .subscribe({ - next: (res) => { - if (this.documentId() !== documentId) { - finish() - return - } - if (res.status === 'skipped') { - finish() - this.toastService.showInfo($localize`Zone OCR skipped.`) - return - } - const results = res.results ?? [] - if (results.length) { - const failed = results.filter( - (r) => - r.value === null || - r.value === undefined || - `${r.value}`.trim() === '' - ) - const filled = results.length - failed.length - let msg = $localize`Filled ${filled} of ${results.length} fields` - if (failed.length) { - const names = failed.map((r) => r.zone).join(', ') - msg = `${msg}. ${$localize`Failed to match zones: ${names}`}` + next: (response) => { + if ( + this.documentId() !== documentId || + response.status === 'skipped' + ) { + this.finishZoneOcr() + if (response.status === 'skipped') { + this.toastService.showInfo( + $localize`No OCR template applies to this document.` + ) } - this.toastService.showInfo(msg) - } else { - this.toastService.showInfo( - $localize`Zone OCR ran but no results extracted.` - ) + return } + this.toastService.showInfo(this.zoneOcrSummary(response.results)) this.docChangeNotifier.next(documentId) - this.loadDocument(documentId, true, finish) + this.loadDocument(documentId, true, () => this.finishZoneOcr()) }, error: (error) => { - finish() - this.toastService.showError($localize`Zone OCR failed`, error) + this.finishZoneOcr() + this.toastService.showError($localize`Error running zone OCR.`, error) }, }) } + private zoneOcrSummary(results: OcrZoneRunResult[]): string { + if (!results.length) return $localize`Zone OCR found no fields to fill.` + const missed = results + .filter((result) => !`${result.value ?? ''}`.trim()) + .map((result) => result.zone) + const filled = results.length - missed.length + return missed.length + ? $localize`Filled ${filled} of ${results.length} fields. No value found for: ${missed.join(', ')}` + : $localize`Filled ${filled} of ${results.length} fields.` + } + + private finishZoneOcr() { + this.zoneOcrRunning.set(false) + this.networkActive.set(false) + this.flushPendingIncomingUpdate() + } + private getSelectedNonLatestVersionId(): number | null { const versions = this.document()?.versions ?? [] if (!versions.length || !this.selectedVersionId()) {