diff --git a/docs/usage.md b/docs/usage.md index cbda299857..cc0b52907c 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -863,8 +863,11 @@ details page: - The button next to it opens a dialog where you can choose one template to run instead. A template you choose runs even if it does not match the document or is disabled. -The bulk editor's **Run Zone OCR** action opens the same dialog for the selected documents. Bulk runs are -processed in the background. +Both actions use the file version selected in document details. The latest version is selected by default. +Extracted values update the document's metadata, which is shared across its versions. + +The bulk editor's **Run Zone OCR** action opens the same dialog for the selected documents. Bulk runs use +the latest version of each document and are processed in the background. !!! warning 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 79da1faf3b..69a6e24fef 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 @@ -1909,9 +1909,10 @@ describe('DocumentDetailComponent', () => { expect(toastSpy).toHaveBeenCalledWith('Document reloaded.') }) - it('should run zone OCR, report results and reload the document', () => { + it('should run a chosen zone OCR template on the selected version and reload', () => { component.documentId.set(doc.id) - component.document.set(doc) + component.document.set({ ...doc, versions: docVersions }) + component.selectedVersionId.set(10) jest.spyOn(documentService, 'runZoneOcr').mockReturnValue( of({ results: [ @@ -1935,7 +1936,7 @@ describe('DocumentDetailComponent', () => { dialog.template = 3 dialog.confirmClicked.emit() - expect(documentService.runZoneOcr).toHaveBeenCalledWith(doc.id, 3) + expect(documentService.runZoneOcr).toHaveBeenCalledWith(doc.id, 3, 10) expect(toastSpy).toHaveBeenCalledWith( 'Filled 1 of 2 fields. No value found for: Due' ) @@ -1949,7 +1950,8 @@ describe('DocumentDetailComponent', () => { it('should summarize zone OCR results', () => { component.documentId.set(doc.id) - component.document.set(doc) + component.document.set({ ...doc, versions: docVersions }) + component.selectedVersionId.set(12) jest.spyOn(component as any, 'loadDocument').mockImplementation(() => {}) const runSpy = jest.spyOn(documentService, 'runZoneOcr') const toastSpy = jest.spyOn(toastService, 'showInfo') @@ -1958,7 +1960,7 @@ describe('DocumentDetailComponent', () => { runSpy.mockReturnValueOnce(of({ status: 'completed', results: [] })) component.runZoneOcr() expect(modalSpy).not.toHaveBeenCalled() - expect(runSpy).toHaveBeenCalledWith(doc.id, null) + expect(runSpy).toHaveBeenCalledWith(doc.id, null, null) expect(toastSpy).toHaveBeenLastCalledWith( 'Zone OCR found no fields to fill.' ) @@ -2016,7 +2018,12 @@ describe('DocumentDetailComponent', () => { 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'] }) + component.document.set({ + ...doc, + versions: docVersions, + __changedFields: ['title'], + }) + component.selectedVersionId.set(doc.id) const pending = new Subject() const runSpy = jest .spyOn(documentService, 'runZoneOcr') @@ -2026,7 +2033,7 @@ describe('DocumentDetailComponent', () => { component.runZoneOcr() modalSpy.mock.results[0].value.componentInstance.confirmClicked.emit() - expect(runSpy).toHaveBeenCalledWith(doc.id, null) + expect(runSpy).toHaveBeenCalledWith(doc.id, null, doc.id) expect(component.zoneOcrRunning()).toBe(true) expect(component.networkActive()).toBe(true) const saveSpy = jest.spyOn(documentService, 'patch') 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 475eab52d8..698ddf2ee3 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 @@ -1572,8 +1572,9 @@ export class DocumentDetailComponent runZoneOcr() { if (this.networkActive()) return const documentId = this.documentId() + const versionId = this.getSelectedNonLatestVersionId() if (!this.hasLocalEdits(this.document())) { - this.executeZoneOcr(documentId, null) + this.executeZoneOcr(documentId, null, versionId) return } const modal = this.modalService.open(ConfirmDialogComponent, { @@ -1589,7 +1590,7 @@ export class DocumentDetailComponent .subscribe(() => { modal.close() if (this.documentId() === documentId && !this.networkActive()) { - this.executeZoneOcr(documentId, null) + this.executeZoneOcr(documentId, null, versionId) } }) } @@ -1598,6 +1599,7 @@ export class DocumentDetailComponent runZoneOcrWithOptions() { if (this.networkActive()) return const documentId = this.documentId() + const versionId = this.getSelectedNonLatestVersionId() const modal = this.modalService.open(ZoneOcrConfirmDialogComponent, { backdrop: 'static', }) @@ -1616,16 +1618,20 @@ export class DocumentDetailComponent .subscribe(() => { modal.close() if (this.documentId() === documentId && !this.networkActive()) { - this.executeZoneOcr(documentId, dialog.template) + this.executeZoneOcr(documentId, dialog.template, versionId) } }) } - private executeZoneOcr(documentId: number, templateId: number | null) { + private executeZoneOcr( + documentId: number, + templateId: number | null, + versionId: number | null + ) { this.zoneOcrRunning.set(true) this.networkActive.set(true) this.documentsService - .runZoneOcr(documentId, templateId) + .runZoneOcr(documentId, templateId, versionId) .pipe(takeUntil(this.unsubscribeNotifier)) .subscribe({ next: (response) => { diff --git a/src-ui/src/app/services/rest/document.service.spec.ts b/src-ui/src/app/services/rest/document.service.spec.ts index 2fa69e9b2b..b9f461a72e 100644 --- a/src-ui/src/app/services/rest/document.service.spec.ts +++ b/src-ui/src/app/services/rest/document.service.spec.ts @@ -572,8 +572,8 @@ describe(`DocumentService`, () => { expect(req.request.body).toEqual({}) req.flush({ status: 'completed', results: [] }) - subscription = service.runZoneOcr(documents[0].id, 3).subscribe() - req = httpTestingController.expectOne(url) + subscription = service.runZoneOcr(documents[0].id, 3, 10).subscribe() + req = httpTestingController.expectOne(`${url}?version=10`) expect(req.request.body).toEqual({ template: 3 }) req.flush({ status: 'completed', results: [] }) }) diff --git a/src-ui/src/app/services/rest/document.service.ts b/src-ui/src/app/services/rest/document.service.ts index f2740b32be..5357017d8d 100644 --- a/src-ui/src/app/services/rest/document.service.ts +++ b/src-ui/src/app/services/rest/document.service.ts @@ -373,11 +373,13 @@ export class DocumentService extends AbstractPaperlessService { runZoneOcr( id: number, - templateId: number | null = null + templateId: number | null = null, + versionId: number | null = null ): Observable { return this.http.post( this.getResourceUrl(id, 'run-zone-ocr'), - templateId ? { template: templateId } : {} + templateId ? { template: templateId } : {}, + { params: versionId ? { version: versionId.toString() } : {} } ) } diff --git a/src/documents/tasks.py b/src/documents/tasks.py index c7d1f4910d..b2768ef65e 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -69,6 +69,7 @@ from documents.utils import IterWrapper from documents.utils import compute_checksum from documents.utils import identity from documents.versioning import annotate_effective_content +from documents.versioning import latest_version from documents.workflows.utils import get_workflows_for_trigger from documents.zone_ocr import run_zone_extraction from paperless.config import AIConfig @@ -391,7 +392,7 @@ def extract_zones(document_id: int, template_id: int | None = None) -> None: return templates = [template] - run_zone_extraction(document, templates=templates, reindex=True) + run_zone_extraction(latest_version(document), templates=templates, reindex=True) @shared_task diff --git a/src/documents/tests/test_api_ocr_templates.py b/src/documents/tests/test_api_ocr_templates.py index 926ec0499c..379de9319c 100644 --- a/src/documents/tests/test_api_ocr_templates.py +++ b/src/documents/tests/test_api_ocr_templates.py @@ -9,6 +9,7 @@ import pytest from django.contrib.auth.models import Permission from django.contrib.auth.models import User from django.db import IntegrityError +from django.utils import timezone from PIL import Image from rest_framework import status from rest_framework.test import APITestCase @@ -31,8 +32,10 @@ CONTENT = "Northwind Power & Light statement" @pytest.mark.django_db @pytest.mark.parametrize("entry_point", ["ingestion", "single", "bulk"]) +@pytest.mark.parametrize("versioned", [False, True], ids=["original", "versioned"]) def test_zone_ocr_indexes_extracted_metadata( entry_point, + versioned, admin_client, paperless_dirs, django_capture_on_commit_callbacks, @@ -40,11 +43,22 @@ def test_zone_ocr_indexes_extracted_metadata( ): document = DocumentFactory( title="Before OCR", - content=CONTENT, + content="Previous layout" if versioned else CONTENT, mime_type="image/png", + page_count=2 if versioned else 1, ) + file_document = document + if versioned: + file_document = DocumentFactory( + root_document=document, + version_index=1, + title="Uploaded version", + content=CONTENT, + mime_type="image/png", + page_count=1, + ) with Image.new("RGB", (100, 100), "white") as image: - image.save(document.source_path, format="PNG") + image.save(file_document.source_path, format="PNG") template = OcrTemplate.objects.create( name="Invoice template", match="Northwind", @@ -57,6 +71,7 @@ def test_zone_ocr_indexes_extracted_metadata( template=template, name="Title", target="title", + page=-1, x=0, y=0, width=50, @@ -66,6 +81,7 @@ def test_zone_ocr_indexes_extracted_metadata( template=template, name="Invoice code", custom_field=field, + page=-1, x=0, y=50, width=50, @@ -84,7 +100,7 @@ def test_zone_ocr_indexes_extracted_metadata( with django_capture_on_commit_callbacks(execute=True): if entry_point == "ingestion": - document_consumption_finished.send(sender=None, document=document) + document_consumption_finished.send(sender=None, document=file_document) elif entry_point == "single": response = admin_client.post(f"/api/documents/{document.pk}/run-zone-ocr/") assert response.status_code == status.HTTP_200_OK @@ -93,6 +109,10 @@ def test_zone_ocr_indexes_extracted_metadata( document.refresh_from_db() assert document.title == "After OCR" + if versioned: + file_document.refresh_from_db() + assert file_document.title == "Uploaded version" + assert not file_document.custom_fields.exists() assert backend.search_ids("title:before", user=None) == [] assert backend.search_ids("title:after", user=None) == [document.pk] assert backend.search_ids("custom_fields.value:INV42", user=None) == [document.pk] @@ -862,6 +882,54 @@ class TestOcrTemplatesAPI(DirectoriesMixin, APITestCase): self.assertEqual(response.json()["status"], "completed") extract.assert_called_once_with(document, templates=[template], reindex=True) + @patch("documents.zone_ocr.run_zone_extraction", return_value=[]) + def test_run_zone_ocr_matches_the_requested_file_version(self, extract): + document = DocumentFactory(content=CONTENT) + older = DocumentFactory( + root_document=document, + version_index=1, + content=CONTENT, + ) + DocumentFactory(root_document=document, version_index=2, content="Contoso") + template = OcrTemplate.objects.create( + name="Northwind", + match="Northwind", + source_width=100, + source_height=100, + ) + url = f"/api/documents/{document.pk}/run-zone-ocr/" + + for file_document in (document, older): + with self.subTest(version=file_document.pk): + file_document.source_path.write_bytes(b"test PDF") + response = self.client.post(f"{url}?version={file_document.pk}") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json()["status"], "completed") + extract.assert_called_once_with( + file_document, + templates=[template], + reindex=True, + ) + extract.reset_mock() + + @patch("documents.zone_ocr.run_zone_extraction") + def test_run_zone_ocr_rejects_invalid_or_unavailable_versions(self, extract): + document = DocumentFactory() + unrelated = DocumentFactory() + deleted = DocumentFactory( + root_document=document, + version_index=1, + deleted_at=timezone.now(), + ) + + for version in ("invalid", unrelated.pk, deleted.pk): + with self.subTest(version=version): + response = self.client.post( + f"/api/documents/{document.pk}/run-zone-ocr/?version={version}", + ) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + extract.assert_not_called() + @patch("documents.zone_ocr.run_zone_extraction") def test_run_zone_ocr_rejects_a_template_the_user_cannot_view(self, extract): user = User.objects.create_user(username="editor") diff --git a/src/documents/tests/test_zone_ocr.py b/src/documents/tests/test_zone_ocr.py index 9e9a5e1356..988e99b70b 100644 --- a/src/documents/tests/test_zone_ocr.py +++ b/src/documents/tests/test_zone_ocr.py @@ -637,6 +637,16 @@ class TestTemplateVisibility(DirectoriesMixin, TestCase): [self.mine, self.unowned], ) + def test_matching_uses_the_selected_files_content(self): + version = DocumentFactory(root_document=self.document, content="Contoso") + contoso = self.template("Contoso", match="Contoso") + + self.assertCountEqual( + get_templates_for_document(self.document), + [self.mine, self.unowned], + ) + self.assertEqual(get_templates_for_document(version), [contoso]) + def test_superuser_owner_can_use_all_visible_templates(self): self.owner.is_superuser = True self.assertCountEqual( diff --git a/src/documents/views.py b/src/documents/views.py index aab8b6e011..ea436bcd1a 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -247,6 +247,7 @@ from documents.versioning import get_latest_version_for_root from documents.versioning import get_request_version_param from documents.versioning import get_root_document from documents.versioning import latest_version_content_prefetch +from documents.versioning import resolve_effective_document from documents.versioning import resolve_requested_version_for_root from documents.versioning import versions_newest_first from paperless import version @@ -2261,6 +2262,14 @@ class DocumentViewSet( @extend_schema( operation_id="documents_run_zone_ocr", + parameters=[ + OpenApiParameter( + name="version", + type=OpenApiTypes.INT, + location=OpenApiParameter.QUERY, + description="File version ID. Defaults to the latest version.", + ), + ], request=RunZoneOcrSerializer, responses=inline_serializer( name="RunZoneOcrResult", @@ -2308,22 +2317,31 @@ class DocumentViewSet( from documents.zone_ocr import get_templates_for_document from documents.zone_ocr import run_zone_extraction + resolution = resolve_effective_document(document, request) + if resolution.error == VersionResolutionError.INVALID: + raise NotFound("Invalid version parameter") + file_document = resolution.document + if file_document is None: + raise Http404 + templates = ( - [template] if template is not None else get_templates_for_document(document) + [template] + if template is not None + else get_templates_for_document(file_document) ) if not templates: return Response( {"status": "skipped", "reason": "no_template", "results": []}, ) - doc_path = document.archive_path or document.source_path + doc_path = file_document.archive_path or file_document.source_path if not doc_path or not Path(doc_path).is_file(): return Response( {"error": "Document file not found"}, status=status.HTTP_404_NOT_FOUND, ) - results = run_zone_extraction(document, templates=templates, reindex=True) + results = run_zone_extraction(file_document, templates=templates, reindex=True) return Response( {"status": "completed", "results": [asdict(result) for result in results]}, ) diff --git a/src/documents/zone_ocr.py b/src/documents/zone_ocr.py index d05561f0e9..d184becdf6 100644 --- a/src/documents/zone_ocr.py +++ b/src/documents/zone_ocr.py @@ -20,6 +20,7 @@ import zxingcpp from django.conf import settings from django.core.exceptions import ValidationError from django.db import transaction +from django.db.models import F from PIL import Image from documents.matching import matches @@ -74,9 +75,8 @@ def _zone_result( def get_templates_for_document(document: Document) -> list[OcrTemplate]: """ - The enabled templates that match the document's content and that its owner - can view, the same rule as matching. A version is owned through its root - document. + The enabled templates that match the selected file's content and that the + root document's owner can view. """ owner = get_root_document(document).owner templates = ( @@ -87,6 +87,15 @@ def get_templates_for_document(document: Document) -> list[OcrTemplate]: .order_by("name", "pk") .prefetch_related("zones", "zones__custom_field") ) + if document.root_document_id is None and templates: + # The original file can be selected even when newer versions exist. + document = ( + Document.objects.filter(pk=document.pk) + .annotate(effective_content=F("content")) + .first() + ) + if document is None: + return [] return [template for template in templates if matches(template, document)] @@ -98,8 +107,8 @@ def run_zone_extraction( reindex: bool = False, ) -> list[ZoneOcrResult]: """ - Run zone-based OCR extraction for the given templates, or for every - template that matches the document when none are given. + Extract zones from the selected file version and update its root document. + Run all matching templates when no templates are given. Manual runs can request a search refresh after successful writes commit. """ @@ -150,7 +159,8 @@ def run_zone_extraction( ) if reindex and any(result.reason is None for result in results): # queue a reindex of the document after the transaction commits - transaction.on_commit(partial(_reindex_document, document.pk)) + root_document = get_root_document(document) + transaction.on_commit(partial(_reindex_document, root_document.pk)) return results @@ -244,7 +254,7 @@ def _process_template( result.reason = ( "unassigned" if key == "custom_field" else "invalid_value" ) - _apply_field_updates(document, updates) + _apply_field_updates(get_root_document(document), updates) return results