From bf316cba8a418d3e95db00a45716724028469d34 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Tue, 20 Oct 2026 08:57:24 -0700 Subject: [PATCH] Dont fail if one bulk fails, collect results separately --- .../document-detail.component.spec.ts | 21 ++++ .../document-detail.component.ts | 5 + .../bulk-editor/bulk-editor.component.spec.ts | 87 +++++++++++++++ .../bulk-editor/bulk-editor.component.ts | 36 +++++- src-ui/src/app/data/ocr-template.ts | 6 + .../src/app/services/rest/document.service.ts | 6 +- src/documents/tests/test_api_ocr_templates.py | 104 ++++++++++++++++++ src/documents/views.py | 44 +++++--- 8 files changed, 289 insertions(+), 20 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 55a3b2aec..b6fbb19ff 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 @@ -1992,6 +1992,27 @@ describe('DocumentDetailComponent', () => { pending.complete() }) + it('does not reload or discard local edits when zone OCR is skipped', () => { + component.documentId.set(doc.id) + component.document.set({ ...doc, __changedFields: ['title'] }) + component.documentForm.patchValue({ title: 'Unsaved title' }) + const modalSpy = jest.spyOn(modalService, 'open') + jest + .spyOn(documentService, 'runZoneOcr') + .mockReturnValue(of({ status: 'skipped', results: [] })) + const loadSpy = jest.spyOn(component as any, 'loadDocument') + const infoSpy = jest.spyOn(toastService, 'showInfo') + + component.runZoneOcr() + modalSpy.mock.results[0].value.componentInstance.confirm() + + expect(loadSpy).not.toHaveBeenCalled() + 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.') + }) + it('should show an error when zone OCR fails', () => { component.document.set(doc) jest 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 e55bb7994..c713484ce 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 @@ -1604,6 +1604,11 @@ export class DocumentDetailComponent 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( diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts index b80c886f9..7ec590631 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts @@ -1286,6 +1286,93 @@ describe('BulkEditorComponent', () => { ) // listAllFilteredIds }) + it('finishes bulk zone OCR despite skipped and failed documents and refreshes the list', () => { + jest + .spyOn(documentListViewService, 'selected', 'get') + .mockReturnValue(new Set([3, 4, 5, 6])) + const modalSpy = jest.spyOn(modalService, 'open') + const refreshSpy = jest + .spyOn(component as any, 'handleOperationSuccess') + .mockImplementation(() => {}) + const errorSpy = jest.spyOn(toastService, 'showError') + + component.runZoneOcrSelected() + const modal = modalSpy.mock.results[0].value + modal.componentInstance.confirm() + const requests = [3, 4, 5, 6].map((id) => + httpTestingController.expectOne( + `${environment.apiBaseUrl}documents/${id}/run-zone-ocr/` + ) + ) + requests[1].flush({ + status: 'skipped', + reason: 'no_document_type', + results: [], + }) + requests[2].flush({ status: 'skipped', reason: 'no_template', results: [] }) + requests[3].flush( + { error: 'OCR failed' }, + { status: 500, statusText: 'Server Error' } + ) + + expect(requests[0].cancelled).toBe(false) + expect(refreshSpy).not.toHaveBeenCalled() + requests[0].flush({ status: 'completed', results: [] }) + + expect(refreshSpy).toHaveBeenCalledWith(modal, false) + expect(errorSpy).toHaveBeenCalledWith( + 'Zone OCR: 1 completed, 2 skipped, 1 failed. Failed documents: 6', + [{ document: 6, error: { error: 'OCR failed' } }] + ) + }) + + it.each([ + [403, 'Permission denied'], + [404, 'Document file not found'], + [500, 'OCR failed'], + ])('reports a %s OCR error as a failure, not a skip', (status, message) => { + jest + .spyOn(documentListViewService, 'selected', 'get') + .mockReturnValue(new Set([3])) + const modalSpy = jest.spyOn(modalService, 'open') + jest + .spyOn(component as any, 'handleOperationSuccess') + .mockImplementation(() => {}) + const errorSpy = jest.spyOn(toastService, 'showError') + + component.runZoneOcrSelected() + modalSpy.mock.results[0].value.componentInstance.confirm() + httpTestingController + .expectOne(`${environment.apiBaseUrl}documents/3/run-zone-ocr/`) + .flush({ error: message }, { status, statusText: 'Error' }) + + expect(errorSpy).toHaveBeenCalledWith( + 'Zone OCR: 0 completed, 0 skipped, 1 failed. Failed documents: 3', + [{ document: 3, error: { error: message } }] + ) + }) + + it('reports a successful bulk zone OCR run', () => { + jest + .spyOn(documentListViewService, 'selected', 'get') + .mockReturnValue(new Set([3, 4])) + jest + .spyOn(documentService, 'runZoneOcr') + .mockReturnValue(of({ status: 'completed', results: [] })) + const modalSpy = jest.spyOn(modalService, 'open') + jest + .spyOn(component as any, 'handleOperationSuccess') + .mockImplementation(() => {}) + const infoSpy = jest.spyOn(toastService, 'showInfo') + + component.runZoneOcrSelected() + modalSpy.mock.results[0].value.componentInstance.confirm() + + expect(infoSpy).toHaveBeenCalledWith( + 'Zone OCR: 2 completed, 0 skipped, 0 failed.' + ) + }) + it('should support rotate', () => { let modal: NgbModalRef modalService.activeInstances.subscribe((m) => (modal = m[0])) diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts index 5089b8ff5..2cf6d3c17 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts @@ -21,10 +21,12 @@ import { import { saveAs } from 'file-saver' import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons' import { + catchError, first, forkJoin, map, Observable, + of, Subject, switchMap, takeUntil, @@ -983,7 +985,39 @@ export class BulkEditorComponent modal.componentInstance.buttonsEnabled.set(false) this.executeDocumentAction( modal, - forkJoin(ids.map((id) => this.documentService.runZoneOcr(id))) + forkJoin( + ids.map((id) => + this.documentService.runZoneOcr(id).pipe( + map(({ status }) => ({ id, status })), + catchError((error) => + of({ id, status: 'failed' as const, error }) + ) + ) + ) + ).pipe( + tap((results) => { + const completed = results.filter( + (r) => r.status === 'completed' + ).length + const skipped = results.filter( + (r) => r.status === 'skipped' + ).length + const failed = results.filter((r) => r.status === 'failed') + const summary = $localize`Zone OCR: ${completed} completed, ${skipped} skipped, ${failed.length} failed.` + if (failed.length) { + const failedIds = failed.map((r) => r.id).join(', ') + this.toastService.showError( + `${summary} ${$localize`Failed documents: ${failedIds}`}`, + failed.map((r) => ({ + document: r.id, + error: r.error.error ?? r.error.message, + })) + ) + } else { + this.toastService.showInfo(summary) + } + }) + ) ) }) } diff --git a/src-ui/src/app/data/ocr-template.ts b/src-ui/src/app/data/ocr-template.ts index 5bc9cb8bb..778755e07 100644 --- a/src-ui/src/app/data/ocr-template.ts +++ b/src-ui/src/app/data/ocr-template.ts @@ -162,3 +162,9 @@ export interface OcrZoneRunResult { custom_field: string value: string | number | null } + +export interface OcrZoneRunResponse { + status: 'completed' | 'skipped' + reason?: 'no_document_type' | 'no_template' + results: OcrZoneRunResult[] +} diff --git a/src-ui/src/app/services/rest/document.service.ts b/src-ui/src/app/services/rest/document.service.ts index b439f57e3..611df90c8 100644 --- a/src-ui/src/app/services/rest/document.service.ts +++ b/src-ui/src/app/services/rest/document.service.ts @@ -12,7 +12,7 @@ import { import { DocumentMetadata } from 'src/app/data/document-metadata' import { DocumentSuggestions } from 'src/app/data/document-suggestions' import { FilterRule } from 'src/app/data/filter-rule' -import { OcrZoneRunResult } from 'src/app/data/ocr-template' +import { OcrZoneRunResponse } from 'src/app/data/ocr-template' import { Results, SelectionData } from 'src/app/data/results' import { SETTINGS_KEYS } from 'src/app/data/ui-settings' import { queryParamsFromFilterRules } from '../../utils/query-params' @@ -361,8 +361,8 @@ export class DocumentService extends AbstractPaperlessService { }) } - runZoneOcr(id: number): Observable<{ results: OcrZoneRunResult[] }> { - return this.http.post<{ results: OcrZoneRunResult[] }>( + runZoneOcr(id: number): Observable { + return this.http.post( this.getResourceUrl(id, 'run-zone-ocr'), {} ) diff --git a/src/documents/tests/test_api_ocr_templates.py b/src/documents/tests/test_api_ocr_templates.py index d1dc30d8f..c474571bb 100644 --- a/src/documents/tests/test_api_ocr_templates.py +++ b/src/documents/tests/test_api_ocr_templates.py @@ -1,6 +1,7 @@ """Tests for the OCR Template API.""" import json +from unittest.mock import patch from django.contrib.auth.models import User from rest_framework import status @@ -11,6 +12,7 @@ from documents.models import DocumentType from documents.models import OcrTemplate from documents.models import OcrTemplateZone from paperless_testing.dirs import DirectoriesMixin +from paperless_testing.factories import DocumentFactory class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): @@ -395,3 +397,105 @@ class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): resp.status_code, (status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN), ) + + @patch("documents.zone_ocr.run_zone_extraction") + def test_run_zone_ocr_without_document_type_is_skipped(self, extract): + document = DocumentFactory() + + response = self.client.post(f"/api/documents/{document.pk}/run-zone-ocr/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + response.json(), + {"status": "skipped", "reason": "no_document_type", "results": []}, + ) + extract.assert_not_called() + + @patch("documents.zone_ocr.run_zone_extraction") + def test_run_zone_ocr_without_enabled_template_is_skipped(self, extract): + document = DocumentFactory(document_type=self.doc_type) + + for enabled in (None, False): + with self.subTest(enabled=enabled): + if enabled is False: + OcrTemplate.objects.create( + name="Disabled template", + document_type=self.doc_type, + source_width=1000, + source_height=1000, + enabled=False, + ) + + response = self.client.post( + f"/api/documents/{document.pk}/run-zone-ocr/", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + response.json(), + {"status": "skipped", "reason": "no_template", "results": []}, + ) + extract.assert_not_called() + + def test_run_zone_ocr_missing_document_is_an_error(self): + response = self.client.post("/api/documents/99999/run-zone-ocr/") + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + @patch("documents.zone_ocr.run_zone_extraction") + def test_run_zone_ocr_missing_file_is_an_error(self, extract): + document = DocumentFactory(document_type=self.doc_type) + OcrTemplate.objects.create( + name="Template", + document_type=self.doc_type, + source_width=1000, + source_height=1000, + ) + + response = self.client.post(f"/api/documents/{document.pk}/run-zone-ocr/") + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json(), {"error": "Document file not found"}) + extract.assert_not_called() + + @patch("documents.zone_ocr.run_zone_extraction") + def test_run_zone_ocr_reports_completed_results(self, extract): + document = DocumentFactory( + document_type=self.doc_type, + mime_type="application/pdf", + ) + document.source_path.write_bytes(b"test PDF") + template = OcrTemplate.objects.create( + name="Template", + document_type=self.doc_type, + source_width=1000, + source_height=1000, + ) + OcrTemplateZone.objects.create( + template=template, + name="Title zone", + target="title", + x=0, + y=0, + width=100, + height=100, + ) + + response = self.client.post(f"/api/documents/{document.pk}/run-zone-ocr/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + response.json(), + { + "status": "completed", + "results": [ + { + "template": template.name, + "zone": "Title zone", + "custom_field": "Title", + "value": document.title, + }, + ], + }, + ) + extract.assert_called_once() diff --git a/src/documents/views.py b/src/documents/views.py index dde0032ac..622706ed7 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -2278,19 +2278,16 @@ class DocumentViewSet( return version_doc @extend_schema( - operation_id="documents_delete_version", - parameters=[ - OpenApiParameter( - name="version_id", - type=OpenApiTypes.INT, - location=OpenApiParameter.PATH, - ), - ], + operation_id="documents_run_zone_ocr", responses=inline_serializer( - name="DeleteDocumentVersionResult", + name="RunZoneOcrResult", fields={ - "result": serializers.CharField(), - "current_version_id": serializers.IntegerField(), + "status": serializers.ChoiceField(choices=["completed", "skipped"]), + "reason": serializers.ChoiceField( + choices=["no_document_type", "no_template"], + required=False, + ), + "results": serializers.ListField(child=serializers.DictField()), }, ), ) @@ -2304,8 +2301,7 @@ class DocumentViewSet( if not document.document_type_id: return Response( - {"error": "Document has no type assigned"}, - status=status.HTTP_400_BAD_REQUEST, + {"status": "skipped", "reason": "no_document_type", "results": []}, ) templates = OcrTemplate.objects.filter( @@ -2314,8 +2310,7 @@ class DocumentViewSet( ) if not templates.exists(): return Response( - {"error": "No OCR templates found for this document type"}, - status=status.HTTP_404_NOT_FOUND, + {"status": "skipped", "reason": "no_template", "results": []}, ) doc_path = document.archive_path or document.source_path @@ -2359,8 +2354,25 @@ class DocumentViewSet( }, ) - return Response({"results": results}) + return Response({"status": "completed", "results": results}) + @extend_schema( + operation_id="documents_delete_version", + parameters=[ + OpenApiParameter( + name="version_id", + type=OpenApiTypes.INT, + location=OpenApiParameter.PATH, + ), + ], + responses=inline_serializer( + name="DeleteDocumentVersionResult", + fields={ + "result": serializers.CharField(), + "current_version_id": serializers.IntegerField(), + }, + ), + ) @action( methods=["delete"], detail=True,