From 6d72ee795f2cacb72cc8fee9cc6f15acd34cbd67 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Mon, 11 Aug 2025 10:29:48 -0700 Subject: [PATCH] Feature: PDF editor (#10318) --- docs/api.md | 12 + docs/usage.md | 10 +- src-ui/setup-jest.ts | 20 ++ ...delete-pages-confirm-dialog.component.html | 54 ----- ...delete-pages-confirm-dialog.component.scss | 28 --- ...ete-pages-confirm-dialog.component.spec.ts | 60 ----- .../delete-pages-confirm-dialog.component.ts | 69 ------ .../split-confirm-dialog.component.html | 59 ----- .../split-confirm-dialog.component.scss | 9 - .../split-confirm-dialog.component.spec.ts | 107 --------- .../split-confirm-dialog.component.ts | 98 -------- .../pdf-editor/pdf-editor.component.html | 103 +++++++++ .../pdf-editor/pdf-editor.component.scss | 70 ++++++ .../pdf-editor/pdf-editor.component.spec.ts | 142 ++++++++++++ .../common/pdf-editor/pdf-editor.component.ts | 133 +++++++++++ .../document-detail.component.html | 12 +- .../document-detail.component.spec.ts | 74 ++---- .../document-detail.component.ts | 112 ++------- src/documents/bulk_edit.py | 97 ++++++++ src/documents/serialisers.py | 53 ++++- src/documents/tests/test_api_bulk_edit.py | 213 ++++++++++++++++++ src/documents/tests/test_bulk_edit.py | 153 +++++++++++++ src/documents/views.py | 40 ++-- 23 files changed, 1066 insertions(+), 662 deletions(-) delete mode 100644 src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.html delete mode 100644 src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.scss delete mode 100644 src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.spec.ts delete mode 100644 src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.ts delete mode 100644 src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.html delete mode 100644 src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.scss delete mode 100644 src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.spec.ts delete mode 100644 src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.ts create mode 100644 src-ui/src/app/components/common/pdf-editor/pdf-editor.component.html create mode 100644 src-ui/src/app/components/common/pdf-editor/pdf-editor.component.scss create mode 100644 src-ui/src/app/components/common/pdf-editor/pdf-editor.component.spec.ts create mode 100644 src-ui/src/app/components/common/pdf-editor/pdf-editor.component.ts diff --git a/docs/api.md b/docs/api.md index 7b525efaa..cd3e462da 100644 --- a/docs/api.md +++ b/docs/api.md @@ -282,6 +282,18 @@ The following methods are supported: - `"merge": true or false` (defaults to false) - The `merge` flag determines if the supplied permissions will overwrite all existing permissions (including removing them) or be merged with existing permissions. +- `edit_pdf` + - Requires `parameters`: + - `"doc_ids": [DOCUMENT_ID]` A list of a single document ID to edit. + - `"operations": [OPERATION, ...]` A list of operations to perform on the documents. Each operation is a dictionary + with the following keys: + - `"page": PAGE_NUMBER` The page number to edit (1-based). + - `"rotate": DEGREES` Optional rotation in degrees (90, 180, 270). + - `"doc": OUTPUT_DOCUMENT_INDEX` Optional index of the output document for split operations. + - Optional `parameters`: + - `"delete_original": true` to delete the original documents after editing. + - `"update_document": true` to update the existing document with the edited PDF. + - `"include_metadata": true` to copy metadata from the original document to the edited document. - `merge` - No additional `parameters` required. - The ordering of the merged document is determined by the list of IDs. diff --git a/docs/usage.md b/docs/usage.md index 73d3336ce..9310d9a2f 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -580,12 +580,14 @@ The following custom field types are supported: ## PDF Actions -Paperless-ngx supports four basic editing operations for PDFs (these operations currently cannot be performed on non-PDF files): +Paperless-ngx supports basic editing operations for PDFs (these operations currently cannot be performed on non-PDF files). When viewing an individual document you can +open the 'PDF Editor' to use a simple UI for re-arranging, rotating, deleting pages and splitting documents. - Merging documents: available when selecting multiple documents for 'bulk editing'. -- Rotating documents: available when selecting multiple documents for 'bulk editing' and from an individual document's details page. -- Splitting documents: available from an individual document's details page. -- Deleting pages: available from an individual document's details page. +- Rotating documents: available when selecting multiple documents for 'bulk editing' and via the pdf editor on an individual document's details page. +- Splitting documents: via the pdf editor on an individual document's details page. +- Deleting pages: via the pdf editor on an individual document's details page. +- Re-arranging pages: via the pdf editor on an individual document's details page. !!! important diff --git a/src-ui/setup-jest.ts b/src-ui/setup-jest.ts index c24762313..c52c00647 100644 --- a/src-ui/setup-jest.ts +++ b/src-ui/setup-jest.ts @@ -121,6 +121,26 @@ if (!URL.revokeObjectURL) { } Object.defineProperty(window, 'ResizeObserver', { value: mock() }) +if (typeof IntersectionObserver === 'undefined') { + class MockIntersectionObserver { + constructor( + public callback: IntersectionObserverCallback, + public options?: IntersectionObserverInit + ) {} + + observe = jest.fn() + unobserve = jest.fn() + disconnect = jest.fn() + takeRecords = jest.fn() + } + + Object.defineProperty(window, 'IntersectionObserver', { + writable: true, + configurable: true, + value: MockIntersectionObserver, + }) +} + HTMLCanvasElement.prototype.getContext = < typeof HTMLCanvasElement.prototype.getContext >jest.fn() diff --git a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.html b/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.html deleted file mode 100644 index 01bf5d3fd..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.html +++ /dev/null @@ -1,54 +0,0 @@ -
- - -
-
-
-
-
-
-
Page
- -
of {{totalPages}}
-
-
- Pages to remove - -
-
-
- - -
-
-
-
-
-
- @if (message) { -

- } - @if (messageBold) { -

- } -
- - -
- - -
- -
- diff --git a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.scss b/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.scss deleted file mode 100644 index 4ddd79bfa..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.scss +++ /dev/null @@ -1,28 +0,0 @@ -.pdf-viewer-container { - background-color: gray; - height: 550px; - - pdf-viewer { - width: 100%; - height: 100%; - } -} - -.mw-60 { - max-width: 60px; -} - -div.position-absolute:has(.form-check-input:checked) { - background-color: rgba(var(--bs-dark-rgb), 0.4); -} - -.form-check-input { - &:checked { - background-color: var(--bs-danger); - border-color: var(--bs-danger); - } - &:focus { - box-shadow: 0 0 0 0.25rem rgba(var(--bs-danger-rgb), var(--pngx-focus-alpha)); - border-color: var(--bs-danger); - } -} diff --git a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.spec.ts b/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.spec.ts deleted file mode 100644 index 964cc05a7..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.spec.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http' -import { provideHttpClientTesting } from '@angular/common/http/testing' -import { ComponentFixture, TestBed } from '@angular/core/testing' -import { FormsModule, ReactiveFormsModule } from '@angular/forms' -import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap' -import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' -import { SafeHtmlPipe } from 'src/app/pipes/safehtml.pipe' -import { DeletePagesConfirmDialogComponent } from './delete-pages-confirm-dialog.component' - -describe('DeletePagesConfirmDialogComponent', () => { - let component: DeletePagesConfirmDialogComponent - let fixture: ComponentFixture - - beforeEach(async () => { - await TestBed.configureTestingModule({ - declarations: [], - imports: [ - NgxBootstrapIconsModule.pick(allIcons), - FormsModule, - ReactiveFormsModule, - DeletePagesConfirmDialogComponent, - ], - providers: [ - NgbActiveModal, - SafeHtmlPipe, - provideHttpClient(withInterceptorsFromDi()), - provideHttpClientTesting(), - ], - }).compileComponents() - fixture = TestBed.createComponent(DeletePagesConfirmDialogComponent) - component = fixture.componentInstance - fixture.detectChanges() - }) - - it('should return a string with comma-separated pages', () => { - component.pages = [1, 2, 3, 4] - expect(component.pagesString).toEqual('1, 2, 3, 4') - }) - - it('should update totalPages when pdf is loaded', () => { - component.pdfPreviewLoaded({ numPages: 5 } as any) - expect(component.totalPages).toEqual(5) - }) - - it('should update checks when page is rendered', () => { - const event = { - target: document.createElement('div'), - detail: { pageNumber: 1 }, - } as any - component.pageRendered(event) - expect(component['checks'].length).toEqual(1) - }) - - it('should update pages when page check is changed', () => { - component.pageCheckChanged(1) - expect(component.pages).toEqual([1]) - component.pageCheckChanged(1) - expect(component.pages).toEqual([]) - }) -}) diff --git a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.ts b/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.ts deleted file mode 100644 index 6d49a110e..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { Component, TemplateRef, ViewChild, inject } from '@angular/core' -import { FormsModule, ReactiveFormsModule } from '@angular/forms' -import { - PDFDocumentProxy, - PdfViewerComponent, - PdfViewerModule, -} from 'ng2-pdf-viewer' -import { SafeHtmlPipe } from 'src/app/pipes/safehtml.pipe' -import { DocumentService } from 'src/app/services/rest/document.service' -import { ConfirmDialogComponent } from '../confirm-dialog.component' - -@Component({ - selector: 'pngx-delete-pages-confirm-dialog', - templateUrl: './delete-pages-confirm-dialog.component.html', - styleUrl: './delete-pages-confirm-dialog.component.scss', - imports: [PdfViewerModule, FormsModule, ReactiveFormsModule, SafeHtmlPipe], -}) -export class DeletePagesConfirmDialogComponent extends ConfirmDialogComponent { - private documentService = inject(DocumentService) - - public documentID: number - public pages: number[] = [] - public currentPage: number = 1 - public totalPages: number - - @ViewChild('pdfViewer') pdfViewer: PdfViewerComponent - @ViewChild('pageCheckOverlay') pageCheckOverlay!: TemplateRef - private checks: HTMLElement[] = [] - - public get pagesString(): string { - return this.pages.join(', ') - } - - public get pdfSrc(): string { - return this.documentService.getPreviewUrl(this.documentID) - } - - constructor() { - super() - } - - public pdfPreviewLoaded(pdf: PDFDocumentProxy) { - this.totalPages = pdf.numPages - } - - pageRendered(event: CustomEvent) { - const pageDiv = event.target as HTMLDivElement - const check = this.pageCheckOverlay.createEmbeddedView({ - page: event.detail.pageNumber, - }) - this.checks[event.detail.pageNumber - 1] = check.rootNodes[0] - pageDiv?.insertBefore(check.rootNodes[0], pageDiv.firstChild) - this.updateChecks() - } - - pageCheckChanged(pageNumber: number) { - if (!this.pages.includes(pageNumber)) this.pages.push(pageNumber) - else if (this.pages.includes(pageNumber)) - this.pages.splice(this.pages.indexOf(pageNumber), 1) - this.updateChecks() - } - - private updateChecks() { - this.checks.forEach((check, i) => { - const input = check.getElementsByTagName('input')[0] - input.checked = this.pages.includes(i + 1) - }) - } -} diff --git a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.html b/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.html deleted file mode 100644 index 47e4c137c..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.html +++ /dev/null @@ -1,59 +0,0 @@ -
- - -
-
-

{{message}}

-
-
-
-
Page
- -
of {{totalPages}}
-
-
- - -
-
-
-
- -
- -
    - @for (pageStr of pagesString.split(','); track pageStr; let i = $index) { -
  • - {{pageStr}} - @if (pagesString.split(',').length > 1) { -   - - } -
    • - } -
-
-
-
-
-
- - -
- - -
diff --git a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.scss b/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.scss deleted file mode 100644 index 9b8df66c5..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.scss +++ /dev/null @@ -1,9 +0,0 @@ -.pdf-viewer-container { - background-color: gray; - height: 500px; - - pdf-viewer { - width: 100%; - height: 100%; - } - } diff --git a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.spec.ts b/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.spec.ts deleted file mode 100644 index b47101f6e..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.spec.ts +++ /dev/null @@ -1,107 +0,0 @@ -import { ComponentFixture, TestBed } from '@angular/core/testing' - -import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http' -import { provideHttpClientTesting } from '@angular/common/http/testing' -import { FormsModule, ReactiveFormsModule } from '@angular/forms' -import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap' -import { PdfViewerModule } from 'ng2-pdf-viewer' -import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' -import { of } from 'rxjs' -import { DocumentService } from 'src/app/services/rest/document.service' -import { SplitConfirmDialogComponent } from './split-confirm-dialog.component' - -describe('SplitConfirmDialogComponent', () => { - let component: SplitConfirmDialogComponent - let fixture: ComponentFixture - let documentService: DocumentService - - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [ - NgxBootstrapIconsModule.pick(allIcons), - ReactiveFormsModule, - FormsModule, - PdfViewerModule, - SplitConfirmDialogComponent, - ], - providers: [ - NgbActiveModal, - provideHttpClient(withInterceptorsFromDi()), - provideHttpClientTesting(), - ], - }).compileComponents() - - fixture = TestBed.createComponent(SplitConfirmDialogComponent) - documentService = TestBed.inject(DocumentService) - component = fixture.componentInstance - fixture.detectChanges() - }) - - it('should load document on init', () => { - const getSpy = jest.spyOn(documentService, 'get') - component.documentID = 1 - getSpy.mockReturnValue(of({ id: 1 } as any)) - component.ngOnInit() - expect(documentService.get).toHaveBeenCalledWith(1) - }) - - it('should update pagesString when pages are added', () => { - component.totalPages = 5 - component.page = 2 - component.addSplit() - expect(component.pagesString).toEqual('1-2,3-5') - component.page = 4 - component.addSplit() - expect(component.pagesString).toEqual('1-2,3-4,5') - }) - - it('should update pagesString when pages are removed', () => { - component.totalPages = 5 - component.page = 2 - component.addSplit() - component.page = 4 - component.addSplit() - expect(component.pagesString).toEqual('1-2,3-4,5') - component.removeSplit(0) - expect(component.pagesString).toEqual('1-4,5') - }) - - it('should enable confirm button when pages are added', () => { - component.totalPages = 5 - component.page = 2 - component.addSplit() - expect(component.confirmButtonEnabled).toBeTruthy() - }) - - it('should disable confirm button when all pages are removed', () => { - component.totalPages = 5 - component.page = 2 - component.addSplit() - component.removeSplit(0) - expect(component.confirmButtonEnabled).toBeFalsy() - }) - - it('should not add split if page is the last page', () => { - component.totalPages = 5 - component.page = 5 - component.addSplit() - expect(component.pagesString).toEqual('1-5') - }) - - it('should update totalPages when pdf is loaded', () => { - component.pdfPreviewLoaded({ numPages: 5 } as any) - expect(component.totalPages).toEqual(5) - }) - - it('should correctly disable split button', () => { - component.totalPages = 5 - component.page = 1 - expect(component.canSplit).toBeTruthy() - component.page = 5 - expect(component.canSplit).toBeFalsy() - component.page = 4 - expect(component.canSplit).toBeTruthy() - component['pages'] = new Set([1, 2, 3, 4]) - expect(component.canSplit).toBeFalsy() - }) -}) diff --git a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.ts b/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.ts deleted file mode 100644 index 656666be6..000000000 --- a/src-ui/src/app/components/common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component.ts +++ /dev/null @@ -1,98 +0,0 @@ -import { Component, OnInit, inject } from '@angular/core' -import { FormsModule, ReactiveFormsModule } from '@angular/forms' -import { PDFDocumentProxy, PdfViewerModule } from 'ng2-pdf-viewer' -import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons' -import { Document } from 'src/app/data/document' -import { PermissionsService } from 'src/app/services/permissions.service' -import { DocumentService } from 'src/app/services/rest/document.service' -import { ConfirmDialogComponent } from '../confirm-dialog.component' - -@Component({ - selector: 'pngx-split-confirm-dialog', - templateUrl: './split-confirm-dialog.component.html', - styleUrl: './split-confirm-dialog.component.scss', - imports: [ - FormsModule, - ReactiveFormsModule, - NgxBootstrapIconsModule, - PdfViewerModule, - ], -}) -export class SplitConfirmDialogComponent - extends ConfirmDialogComponent - implements OnInit -{ - private documentService = inject(DocumentService) - private permissionService = inject(PermissionsService) - - public get pagesString(): string { - let pagesStr = '' - - let lastPage = 1 - for (let i = 1; i <= this.totalPages; i++) { - if (this.pages.has(i) || i === this.totalPages) { - if (lastPage === i) { - pagesStr += `${i},` - lastPage = Math.min(i + 1, this.totalPages) - } else { - pagesStr += `${lastPage}-${i},` - lastPage = Math.min(i + 1, this.totalPages) - } - } - } - - return pagesStr.replace(/,$/, '') - } - - private pages: Set = new Set() - - public documentID: number - private document: Document - public page: number = 1 - public totalPages: number - public deleteOriginal: boolean = false - - public get canSplit(): boolean { - return ( - this.page < this.totalPages && - this.pages.size < this.totalPages - 1 && - !this.pages.has(this.page) - ) - } - - public get pdfSrc(): string { - return this.documentService.getPreviewUrl(this.documentID) - } - - constructor() { - super() - this.confirmButtonEnabled = this.pages.size > 0 - } - - ngOnInit(): void { - this.documentService.get(this.documentID).subscribe((r) => { - this.document = r - }) - } - - pdfPreviewLoaded(pdf: PDFDocumentProxy) { - this.totalPages = pdf.numPages - } - - addSplit() { - if (this.page === this.totalPages) return - this.pages.add(this.page) - this.pages = new Set(Array.from(this.pages).sort((a, b) => a - b)) - this.confirmButtonEnabled = this.pages.size > 0 - } - - removeSplit(i: number) { - let page = Array.from(this.pages)[Math.min(i, this.pages.size - 1)] - this.pages.delete(page) - this.confirmButtonEnabled = this.pages.size > 0 - } - - get userOwnsDocument(): boolean { - return this.permissionService.currentUserOwnsObject(this.document) - } -} diff --git a/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.html b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.html new file mode 100644 index 000000000..49a43141c --- /dev/null +++ b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.html @@ -0,0 +1,103 @@ + +
+

{{ title }}

+ +
+
+
+
+ + +
+
+ + + +
+
+
+ @for (p of pages; track p.page; let i = $index) { +
+
+
+ + +
+
+ + +
+
+
+
+ + +
+
+
+ @defer (on viewport) { + @if (!p.loaded) { +
+ +
+ } + + } @placeholder { +
+ +
+ } +
+ @if (p.splitAfter) { +
— Split here —
+ } +
+ } +
+
+
+
+
+ + + + +
+ @if (editMode === PdfEditorEditMode.Create) { +
+ + +
+
+ + +
+ } + + +
+
diff --git a/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.scss b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.scss new file mode 100644 index 000000000..c2e29463b --- /dev/null +++ b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.scss @@ -0,0 +1,70 @@ + + +.page-item { + position: relative; + cursor: pointer; + border: 1px solid transparent; + background-origin: border-box; + + &.selected { + background-color: var(--pngx-primary-darken-5); + } +} + +.pdf-viewer-container { + background-color: gray; + height: 240px; + + pdf-viewer { + width: 100%; + height: 100%; + } +} + +::ng-deep .ng2-pdf-viewer-container { + overflow: hidden; +} + +.hover-actions { + position: absolute; + top: 0; + right: 0; + display: none; +} + +.page-item:hover .hover-actions { + display: block; +} + +.document-check { + display: none; + position: absolute; + top: 0; + left: 0; + padding: 0.5rem; + border-top-left-radius: 0.25rem; + border-bottom-right-radius: 0.25rem; + pointer-events: none; + + .form-check { + padding: 0; + min-height: 0; + margin-bottom: 0; + + .form-check-input { + margin-left: 0; + } + } +} + +.page-item:hover .document-check, .selected .document-check { + display: block; +} + +.z-10 { + z-index: 10; +} + +.split-after { + writing-mode: vertical-rl; +} diff --git a/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.spec.ts b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.spec.ts new file mode 100644 index 000000000..434deb11d --- /dev/null +++ b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.spec.ts @@ -0,0 +1,142 @@ +import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http' +import { provideHttpClientTesting } from '@angular/common/http/testing' +import { ComponentFixture, TestBed } from '@angular/core/testing' +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap' +import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' +import { PDFEditorComponent } from './pdf-editor.component' + +describe('PDFEditorComponent', () => { + let component: PDFEditorComponent + let fixture: ComponentFixture + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [PDFEditorComponent, NgxBootstrapIconsModule.pick(allIcons)], + providers: [ + provideHttpClient(withInterceptorsFromDi()), + provideHttpClientTesting(), + { provide: NgbActiveModal, useValue: {} }, + ], + }).compileComponents() + fixture = TestBed.createComponent(PDFEditorComponent) + component = fixture.componentInstance + fixture.detectChanges() + }) + + it('should return correct operations with no changes', () => { + component.pages = [ + { page: 1, rotate: 0, splitAfter: false }, + { page: 2, rotate: 0, splitAfter: false }, + { page: 3, rotate: 0, splitAfter: false }, + ] + const ops = component.getOperations() + expect(ops).toEqual([ + { page: 1, rotate: 0, doc: 0 }, + { page: 2, rotate: 0, doc: 0 }, + { page: 3, rotate: 0, doc: 0 }, + ]) + }) + + it('should rotate, delete and reorder pages', () => { + component.pages = [ + { page: 1, rotate: 0, splitAfter: false, selected: false }, + { page: 2, rotate: 0, splitAfter: false, selected: false }, + ] + component.toggleSelection(0) + component.rotateSelected(90) + expect(component.pages[0].rotate).toBe(90) + component.toggleSelection(0) // deselect + component.toggleSelection(1) + component.deleteSelected() + expect(component.pages.length).toBe(1) + component.pages.push({ page: 2, rotate: 0, splitAfter: false }) + component.drop({ previousIndex: 0, currentIndex: 1 } as any) + expect(component.pages[0].page).toBe(2) + component.rotate(0) + expect(component.pages[0].rotate).toBe(90) + }) + + it('should handle empty pages array', () => { + component.pages = [] + expect(component.getOperations()).toEqual([]) + }) + + it('should increment doc index after splitAfter', () => { + component.pages = [ + { page: 1, rotate: 0, splitAfter: true }, + { page: 2, rotate: 0, splitAfter: false }, + { page: 3, rotate: 0, splitAfter: true }, + { page: 4, rotate: 0, splitAfter: false }, + ] + const ops = component.getOperations() + expect(ops).toEqual([ + { page: 1, rotate: 0, doc: 0 }, + { page: 2, rotate: 0, doc: 1 }, + { page: 3, rotate: 0, doc: 1 }, + { page: 4, rotate: 0, doc: 2 }, + ]) + }) + + it('should include rotations in operations', () => { + component.pages = [ + { page: 1, rotate: 90, splitAfter: false }, + { page: 2, rotate: 180, splitAfter: true }, + { page: 3, rotate: 270, splitAfter: false }, + ] + const ops = component.getOperations() + expect(ops).toEqual([ + { page: 1, rotate: 90, doc: 0 }, + { page: 2, rotate: 180, doc: 0 }, + { page: 3, rotate: 270, doc: 1 }, + ]) + }) + + it('should handle remove operation', () => { + component.pages = [ + { page: 1, rotate: 0, splitAfter: false, selected: false }, + { page: 2, rotate: 0, splitAfter: false, selected: true }, + { page: 3, rotate: 0, splitAfter: false, selected: false }, + ] + component.remove(1) // remove page 2 + expect(component.pages.length).toBe(2) + expect(component.pages[0].page).toBe(1) + expect(component.pages[1].page).toBe(3) + }) + + it('should toggle splitAfter correctly', () => { + component.pages = [ + { page: 1, rotate: 0, splitAfter: false }, + { page: 2, rotate: 0, splitAfter: false }, + ] + component.toggleSplit(0) + expect(component.pages[0].splitAfter).toBeTruthy() + component.toggleSplit(1) + expect(component.pages[1].splitAfter).toBeTruthy() + }) + + it('should select and deselect all pages', () => { + component.pages = [ + { page: 1, rotate: 0, splitAfter: false, selected: false }, + { page: 2, rotate: 0, splitAfter: false, selected: false }, + ] + component.selectAll() + expect(component.pages.every((p) => p.selected)).toBeTruthy() + expect(component.hasSelection()).toBeTruthy() + component.deselectAll() + expect(component.pages.every((p) => !p.selected)).toBeTruthy() + expect(component.hasSelection()).toBeFalsy() + }) + + it('should handle pdf loading and page generation', () => { + const mockPdf = { + numPages: 3, + getPage: (pageNum: number) => Promise.resolve({ pageNumber: pageNum }), + } + component.pdfLoaded(mockPdf as any) + expect(component.totalPages).toBe(3) + expect(component.pages.length).toBe(3) + expect(component.pages[0].page).toBe(1) + expect(component.pages[1].page).toBe(2) + expect(component.pages[2].page).toBe(3) + }) +}) diff --git a/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.ts b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.ts new file mode 100644 index 000000000..f417f5199 --- /dev/null +++ b/src-ui/src/app/components/common/pdf-editor/pdf-editor.component.ts @@ -0,0 +1,133 @@ +import { + CdkDragDrop, + DragDropModule, + moveItemInArray, +} from '@angular/cdk/drag-drop' +import { Component, inject } from '@angular/core' +import { FormsModule } from '@angular/forms' +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap' +import { PDFDocumentProxy, PdfViewerModule } from 'ng2-pdf-viewer' +import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons' +import { DocumentService } from 'src/app/services/rest/document.service' +import { ConfirmDialogComponent } from '../confirm-dialog/confirm-dialog.component' + +interface PageOperation { + page: number + rotate: number + splitAfter: boolean + selected?: boolean + loaded?: boolean +} + +export enum PdfEditorEditMode { + Update = 'update', + Create = 'create', +} + +@Component({ + selector: 'pngx-pdf-editor', + templateUrl: './pdf-editor.component.html', + styleUrl: './pdf-editor.component.scss', + imports: [ + DragDropModule, + FormsModule, + PdfViewerModule, + NgxBootstrapIconsModule, + ], +}) +export class PDFEditorComponent extends ConfirmDialogComponent { + public PdfEditorEditMode = PdfEditorEditMode + + private documentService = inject(DocumentService) + activeModal: NgbActiveModal = inject(NgbActiveModal) + + documentID: number + pages: PageOperation[] = [] + totalPages = 0 + editMode: PdfEditorEditMode = PdfEditorEditMode.Create + deleteOriginal: boolean = false + includeMetadata: boolean = true + + get pdfSrc(): string { + return this.documentService.getPreviewUrl(this.documentID) + } + + pdfLoaded(pdf: PDFDocumentProxy) { + this.totalPages = pdf.numPages + this.pages = Array.from({ length: this.totalPages }, (_, i) => ({ + page: i + 1, + rotate: 0, + splitAfter: false, + selected: false, + loaded: false, + })) + } + + toggleSelection(i: number) { + this.pages[i].selected = !this.pages[i].selected + } + + rotate(i: number) { + this.pages[i].rotate = (this.pages[i].rotate + 90) % 360 + } + + rotateSelected(dir: number) { + for (let p of this.pages) { + if (p.selected) { + p.rotate = (p.rotate + dir + 360) % 360 + } + } + } + + remove(i: number) { + this.pages.splice(i, 1) + } + + toggleSplit(i: number) { + this.pages[i].splitAfter = !this.pages[i].splitAfter + if (this.pages[i].splitAfter) { + // force create mode + this.editMode = PdfEditorEditMode.Create + } + } + + selectAll() { + this.pages.forEach((p) => (p.selected = true)) + } + + deselectAll() { + this.pages.forEach((p) => (p.selected = false)) + } + + deleteSelected() { + this.pages = this.pages.filter((p) => !p.selected) + } + + hasSelection(): boolean { + return this.pages.some((p) => p.selected) + } + + hasSplit(): boolean { + return this.pages.some((p) => p.splitAfter) + } + + drop(event: CdkDragDrop) { + moveItemInArray(this.pages, event.previousIndex, event.currentIndex) + } + + getOperations() { + return this.pages.map((p, idx) => ({ + page: p.page, + rotate: p.rotate, + doc: this.computeDocIndex(idx), + })) + } + + private computeDocIndex(index: number): number { + let docIndex = 0 + for (let i = 0; i <= index; i++) { + if (this.pages[i].splitAfter && i < index) docIndex++ + } + return docIndex + } +} diff --git a/src-ui/src/app/components/document-detail/document-detail.component.html b/src-ui/src/app/components/document-detail/document-detail.component.html index 067246335..c926c82d9 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.html +++ b/src-ui/src/app/components/document-detail/document-detail.component.html @@ -58,16 +58,8 @@  More like this - - - - - 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 80b160171..748150959 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 @@ -1158,81 +1158,43 @@ describe('DocumentDetailComponent', () => { ).not.toBeUndefined() }) - it('should support split', () => { + it('should support pdf editor, handle error', () => { let modal: NgbModalRef modalService.activeInstances.subscribe((m) => (modal = m[0])) + const closeSpy = jest.spyOn(openDocumentsService, 'closeDocument') + const errorSpy = jest.spyOn(toastService, 'showError') initNormally() - component.splitDocument() + component.editPdf() expect(modal).not.toBeUndefined() modal.componentInstance.documentID = doc.id - modal.componentInstance.totalPages = 5 - modal.componentInstance.page = 2 - modal.componentInstance.addSplit() + modal.componentInstance.pages = [{ page: 1, rotate: 0, splitAfter: false }] modal.componentInstance.confirm() let req = httpTestingController.expectOne( `${environment.apiBaseUrl}documents/bulk_edit/` ) expect(req.request.body).toEqual({ documents: [doc.id], - method: 'split', - parameters: { pages: '1-2,3-5', delete_originals: false }, + method: 'edit_pdf', + parameters: { + operations: [{ page: 1, rotate: 0, doc: 0 }], + delete_original: false, + update_document: false, + include_metadata: true, + }, }) - req.error(new ProgressEvent('failed')) - modal.componentInstance.confirm() - req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/bulk_edit/` - ) - req.flush(true) - }) + req.error(new ErrorEvent('failed')) + expect(errorSpy).toHaveBeenCalled() - it('should support rotate', () => { - let modal: NgbModalRef - modalService.activeInstances.subscribe((m) => (modal = m[0])) - initNormally() - component.rotateDocument() - expect(modal).not.toBeUndefined() + component.editPdf() modal.componentInstance.documentID = doc.id - modal.componentInstance.rotate() - modal.componentInstance.confirm() - let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/bulk_edit/` - ) - expect(req.request.body).toEqual({ - documents: [doc.id], - method: 'rotate', - parameters: { degrees: 90 }, - }) - req.error(new ProgressEvent('failed')) - modal.componentInstance.confirm() - req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/bulk_edit/` - ) - req.flush(true) - }) - - it('should support delete pages', () => { - let modal: NgbModalRef - modalService.activeInstances.subscribe((m) => (modal = m[0])) - initNormally() - component.deletePages() - expect(modal).not.toBeUndefined() - modal.componentInstance.documentID = doc.id - modal.componentInstance.pages = [1, 2] - modal.componentInstance.confirm() - let req = httpTestingController.expectOne( - `${environment.apiBaseUrl}documents/bulk_edit/` - ) - expect(req.request.body).toEqual({ - documents: [doc.id], - method: 'delete_pages', - parameters: { pages: [1, 2] }, - }) - req.error(new ProgressEvent('failed')) + modal.componentInstance.pages = [{ page: 1, rotate: 0, splitAfter: true }] + modal.componentInstance.deleteOriginal = true modal.componentInstance.confirm() req = httpTestingController.expectOne( `${environment.apiBaseUrl}documents/bulk_edit/` ) req.flush(true) + expect(closeSpy).toHaveBeenCalled() }) it('should support keyboard shortcuts', () => { 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 55b8ade39..45fb9fc83 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 @@ -82,9 +82,6 @@ import { getFilenameFromContentDisposition } from 'src/app/utils/http' import { ISODateAdapter } from 'src/app/utils/ngb-iso-date-adapter' import * as UTIF from 'utif' import { ConfirmDialogComponent } from '../common/confirm-dialog/confirm-dialog.component' -import { DeletePagesConfirmDialogComponent } from '../common/confirm-dialog/delete-pages-confirm-dialog/delete-pages-confirm-dialog.component' -import { RotateConfirmDialogComponent } from '../common/confirm-dialog/rotate-confirm-dialog/rotate-confirm-dialog.component' -import { SplitConfirmDialogComponent } from '../common/confirm-dialog/split-confirm-dialog/split-confirm-dialog.component' import { CustomFieldsDropdownComponent } from '../common/custom-fields-dropdown/custom-fields-dropdown.component' import { CorrespondentEditDialogComponent } from '../common/edit-dialog/correspondent-edit-dialog/correspondent-edit-dialog.component' import { DocumentTypeEditDialogComponent } from '../common/edit-dialog/document-type-edit-dialog/document-type-edit-dialog.component' @@ -102,6 +99,10 @@ import { TagsComponent } from '../common/input/tags/tags.component' import { TextComponent } from '../common/input/text/text.component' import { UrlComponent } from '../common/input/url/url.component' import { PageHeaderComponent } from '../common/page-header/page-header.component' +import { + PDFEditorComponent, + PdfEditorEditMode, +} from '../common/pdf-editor/pdf-editor.component' import { ShareLinksDialogComponent } from '../common/share-links-dialog/share-links-dialog.component' import { DocumentHistoryComponent } from '../document-history/document-history.component' import { DocumentNotesComponent } from '../document-notes/document-notes.component' @@ -1349,13 +1350,13 @@ export class DocumentDetailComponent this.documentForm.updateValueAndValidity() } - splitDocument() { - let modal = this.modalService.open(SplitConfirmDialogComponent, { + editPdf() { + let modal = this.modalService.open(PDFEditorComponent, { backdrop: 'static', - size: 'lg', + size: 'xl', + scrollable: true, }) - modal.componentInstance.title = $localize`Split confirm` - modal.componentInstance.messageBold = $localize`This operation will split the selected document(s) into new documents.` + modal.componentInstance.title = $localize`PDF Editor` modal.componentInstance.btnCaption = $localize`Proceed` modal.componentInstance.documentID = this.document.id modal.componentInstance.confirmClicked @@ -1363,103 +1364,30 @@ export class DocumentDetailComponent .subscribe(() => { modal.componentInstance.buttonsEnabled = false this.documentsService - .bulkEdit([this.document.id], 'split', { - pages: modal.componentInstance.pagesString, - delete_originals: modal.componentInstance.deleteOriginal, + .bulkEdit([this.document.id], 'edit_pdf', { + operations: modal.componentInstance.getOperations(), + delete_original: modal.componentInstance.deleteOriginal, + update_document: + modal.componentInstance.editMode == PdfEditorEditMode.Update, + include_metadata: modal.componentInstance.includeMetadata, }) .pipe(first(), takeUntil(this.unsubscribeNotifier)) .subscribe({ next: () => { this.toastService.showInfo( - $localize`Split operation for "${this.document.title}" will begin in the background.` + $localize`PDF edit operation for "${this.document.title}" will begin in the background.` ) modal.close() + if (modal.componentInstance.deleteOriginal) { + this.openDocumentService.closeDocument(this.document) + } }, error: (error) => { if (modal) { modal.componentInstance.buttonsEnabled = true } this.toastService.showError( - $localize`Error executing split operation`, - error - ) - }, - }) - }) - } - - rotateDocument() { - let modal = this.modalService.open(RotateConfirmDialogComponent, { - backdrop: 'static', - size: 'lg', - }) - modal.componentInstance.title = $localize`Rotate confirm` - modal.componentInstance.messageBold = $localize`This operation will permanently rotate the original version of the current document.` - modal.componentInstance.btnCaption = $localize`Proceed` - modal.componentInstance.documentID = this.document.id - modal.componentInstance.showPDFNote = false - modal.componentInstance.confirmClicked - .pipe(takeUntil(this.unsubscribeNotifier)) - .subscribe(() => { - modal.componentInstance.buttonsEnabled = false - this.documentsService - .bulkEdit([this.document.id], 'rotate', { - degrees: modal.componentInstance.degrees, - }) - .pipe(first(), takeUntil(this.unsubscribeNotifier)) - .subscribe({ - next: () => { - this.toastService.show({ - content: $localize`Rotation of "${this.document.title}" will begin in the background. Close and re-open the document after the operation has completed to see the changes.`, - delay: 8000, - action: this.close.bind(this), - actionName: $localize`Close`, - }) - modal.close() - }, - error: (error) => { - if (modal) { - modal.componentInstance.buttonsEnabled = true - } - this.toastService.showError( - $localize`Error executing rotate operation`, - error - ) - }, - }) - }) - } - - deletePages() { - let modal = this.modalService.open(DeletePagesConfirmDialogComponent, { - backdrop: 'static', - }) - modal.componentInstance.title = $localize`Delete pages confirm` - modal.componentInstance.messageBold = $localize`This operation will permanently delete the selected pages from the original document.` - modal.componentInstance.btnCaption = $localize`Proceed` - modal.componentInstance.documentID = this.document.id - modal.componentInstance.confirmClicked - .pipe(takeUntil(this.unsubscribeNotifier)) - .subscribe(() => { - modal.componentInstance.buttonsEnabled = false - this.documentsService - .bulkEdit([this.document.id], 'delete_pages', { - pages: modal.componentInstance.pages, - }) - .pipe(first(), takeUntil(this.unsubscribeNotifier)) - .subscribe({ - next: () => { - this.toastService.showInfo( - $localize`Delete pages operation for "${this.document.title}" will begin in the background. Close and re-open or reload this document after the operation has completed to see the changes.` - ) - modal.close() - }, - error: (error) => { - if (modal) { - modal.componentInstance.buttonsEnabled = true - } - this.toastService.showError( - $localize`Error executing delete pages operation`, + $localize`Error executing PDF edit operation`, error ) }, diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index 13773fe87..13c95ce6c 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -497,6 +497,103 @@ def delete_pages(doc_ids: list[int], pages: list[int]) -> Literal["OK"]: return "OK" +def edit_pdf( + doc_ids: list[int], + operations: list[dict], + *, + delete_original: bool = False, + update_document: bool = False, + include_metadata: bool = True, + user: User | None = None, +) -> Literal["OK"]: + """ + Operations is a list of dictionaries describing the final PDF pages. + Each entry must contain the original page number in `page` and may + specify `rotate` in degrees and `doc` indicating the output + document index (for splitting). Pages omitted from the list are + discarded. + """ + + logger.info( + f"Editing PDF of document {doc_ids[0]} with {len(operations)} operations", + ) + doc = Document.objects.get(id=doc_ids[0]) + import pikepdf + + pdf_docs: list[pikepdf.Pdf] = [] + + try: + with pikepdf.open(doc.source_path) as src: + # prepare output documents + max_idx = max(op.get("doc", 0) for op in operations) + pdf_docs = [pikepdf.new() for _ in range(max_idx + 1)] + + if update_document and len(pdf_docs) > 1: + logger.error( + "Update requested but multiple output documents specified", + ) + raise ValueError("Multiple output documents specified") + + for op in operations: + dst = pdf_docs[op.get("doc", 0)] + page = src.pages[op["page"] - 1] + dst.pages.append(page) + if op.get("rotate"): + dst.pages[-1].rotate(op["rotate"], relative=True) + + if update_document: + temp_path = doc.source_path.with_suffix(".tmp.pdf") + pdf = pdf_docs[0] + pdf.remove_unreferenced_resources() + # save the edited PDF to a temporary file in case of errors + pdf.save(temp_path) + # replace the original document with the edited one + temp_path.replace(doc.source_path) + doc.checksum = hashlib.md5(doc.source_path.read_bytes()).hexdigest() + doc.page_count = len(pdf.pages) + doc.save() + update_document_content_maybe_archive_file.delay(document_id=doc.id) + else: + consume_tasks = [] + overrides = ( + DocumentMetadataOverrides().from_document(doc) + if include_metadata + else DocumentMetadataOverrides() + ) + if user is not None: + overrides.owner_id = user.id + + for idx, pdf in enumerate(pdf_docs, start=1): + filepath: Path = ( + Path(tempfile.mkdtemp(dir=settings.SCRATCH_DIR)) + / f"{doc.id}_edit_{idx}.pdf" + ) + pdf.remove_unreferenced_resources() + pdf.save(filepath) + consume_tasks.append( + consume_file.s( + ConsumableDocument( + source=DocumentSource.ConsumeFolder, + original_file=filepath, + ), + overrides, + ), + ) + + if delete_original: + chord(header=consume_tasks, body=delete.si([doc.id])).delay() + else: + group(consume_tasks).delay() + + except Exception as e: + logger.exception(f"Error editing document {doc.id}: {e}") + raise ValueError( + f"An error occurred while editing the document: {e}", + ) from e + + return "OK" + + def reflect_doclinks( document: Document, field: CustomField, diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 5a1a6c685..5a9c089f3 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1293,6 +1293,7 @@ class BulkEditSerializer( "merge", "split", "delete_pages", + "edit_pdf", ], label="Method", write_only=True, @@ -1366,7 +1367,10 @@ class BulkEditSerializer( return bulk_edit.split elif method == "delete_pages": return bulk_edit.delete_pages - else: + elif method == "edit_pdf": + return bulk_edit.edit_pdf + else: # pragma: no cover + # This will never happen as it is handled by the ChoiceField raise serializers.ValidationError("Unsupported method.") def _validate_parameters_tags(self, parameters): @@ -1520,6 +1524,47 @@ class BulkEditSerializer( else: parameters["archive_fallback"] = False + def _validate_parameters_edit_pdf(self, parameters, document_id): + if "operations" not in parameters: + raise serializers.ValidationError("operations not specified") + if not isinstance(parameters["operations"], list): + raise serializers.ValidationError("operations must be a list") + for op in parameters["operations"]: + if not isinstance(op, dict): + raise serializers.ValidationError("invalid operation entry") + if "page" not in op or not isinstance(op["page"], int): + raise serializers.ValidationError("page must be an integer") + if "rotate" in op and not isinstance(op["rotate"], int): + raise serializers.ValidationError("rotate must be an integer") + if "doc" in op and not isinstance(op["doc"], int): + raise serializers.ValidationError("doc must be an integer") + if "update_document" in parameters: + if not isinstance(parameters["update_document"], bool): + raise serializers.ValidationError("update_document must be a boolean") + else: + parameters["update_document"] = False + if "include_metadata" in parameters: + if not isinstance(parameters["include_metadata"], bool): + raise serializers.ValidationError("include_metadata must be a boolean") + else: + parameters["include_metadata"] = True + + if parameters["update_document"]: + max_idx = max(op.get("doc", 0) for op in parameters["operations"]) + if max_idx > 0: + raise serializers.ValidationError( + "update_document only allowed with a single output document", + ) + + doc = Document.objects.get(id=document_id) + # doc existence is already validated + if doc.page_count: + for op in parameters["operations"]: + if op["page"] < 1 or op["page"] > doc.page_count: + raise serializers.ValidationError( + f"Page {op['page']} is out of bounds for document with {doc.page_count} pages.", + ) + def validate(self, attrs): method = attrs["method"] parameters = attrs["parameters"] @@ -1554,6 +1599,12 @@ class BulkEditSerializer( self._validate_parameters_delete_pages(parameters) elif method == bulk_edit.merge: self._validate_parameters_merge(parameters) + elif method == bulk_edit.edit_pdf: + if len(attrs["documents"]) > 1: + raise serializers.ValidationError( + "Edit PDF method only supports one document", + ) + self._validate_parameters_edit_pdf(parameters, attrs["documents"][0]) return attrs diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index bcbe5922d..31aaff946 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/src/documents/tests/test_api_bulk_edit.py @@ -41,6 +41,7 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): title="B", correspondent=self.c1, document_type=self.dt1, + page_count=5, ) self.doc3 = Document.objects.create( checksum="C", @@ -1369,6 +1370,218 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertIn(b"pages must be a list of integers", response.content) + @mock.patch("documents.serialisers.bulk_edit.edit_pdf") + def test_edit_pdf(self, m): + self.setup_mock(m, "edit_pdf") + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {"operations": [{"page": 1}]}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + m.assert_called_once() + args, kwargs = m.call_args + self.assertCountEqual(args[0], [self.doc2.id]) + self.assertEqual(kwargs["operations"], [{"page": 1}]) + self.assertEqual(kwargs["user"], self.user) + + def test_edit_pdf_invalid_params(self): + # multiple documents + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id, self.doc3.id], + "method": "edit_pdf", + "parameters": {"operations": [{"page": 1}]}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"Edit PDF method only supports one document", response.content) + + # no operations specified + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"operations not specified", response.content) + + # operations not a list + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {"operations": "not_a_list"}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"operations must be a list", response.content) + + # invalid operation + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {"operations": ["invalid_operation"]}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"invalid operation entry", response.content) + + # page not an int + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {"operations": [{"page": "not_an_int"}]}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"page must be an integer", response.content) + + # rotate not an int + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {"operations": [{"page": 1, "rotate": "not_an_int"}]}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"rotate must be an integer", response.content) + + # doc not an int + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {"operations": [{"page": 1, "doc": "not_an_int"}]}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"doc must be an integer", response.content) + + # update_document not a boolean + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": { + "update_document": "not_a_bool", + "operations": [{"page": 1}], + }, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"update_document must be a boolean", response.content) + + # include_metadata not a boolean + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": { + "include_metadata": "not_a_bool", + "operations": [{"page": 1}], + }, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"include_metadata must be a boolean", response.content) + + # update_document True but output would be multiple documents + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": { + "update_document": True, + "operations": [{"page": 1, "doc": 1}, {"page": 2, "doc": 2}], + }, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn( + b"update_document only allowed with a single output document", + response.content, + ) + + @mock.patch("documents.serialisers.bulk_edit.edit_pdf") + def test_edit_pdf_page_out_of_bounds(self, m): + """ + GIVEN: + - API data for editing PDF is called + - The page number is out of bounds + WHEN: + - API is called + THEN: + - The API fails with a correct error code + """ + self.setup_mock(m, "edit_pdf") + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id], + "method": "edit_pdf", + "parameters": {"operations": [{"page": 99}]}, + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn(b"out of bounds", response.content) + @override_settings(AUDIT_LOG_ENABLED=True) def test_bulk_edit_audit_log_enabled_simple_field(self): """ diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py index 245b56ad3..4e6200719 100644 --- a/src/documents/tests/test_bulk_edit.py +++ b/src/documents/tests/test_bulk_edit.py @@ -909,3 +909,156 @@ class TestPDFActions(DirectoriesMixin, TestCase): expected_str = "Error deleting pages from document" self.assertIn(expected_str, error_str) mock_update_archive_file.assert_not_called() + + @mock.patch("documents.bulk_edit.group") + @mock.patch("documents.tasks.consume_file.s") + def test_edit_pdf_basic_operations(self, mock_consume_file, mock_group): + """ + GIVEN: + - Existing document + WHEN: + - edit_pdf is called with two operations to split the doc and rotate pages + THEN: + - A grouped task is generated and delay() is called + """ + mock_group.return_value.delay.return_value = None + doc_ids = [self.doc2.id] + operations = [{"page": 1, "doc": 0}, {"page": 2, "doc": 1, "rotate": 90}] + + result = bulk_edit.edit_pdf(doc_ids, operations) + self.assertEqual(result, "OK") + mock_group.return_value.delay.assert_called_once() + + @mock.patch("documents.bulk_edit.group") + @mock.patch("documents.tasks.consume_file.s") + def test_edit_pdf_with_user_override(self, mock_consume_file, mock_group): + """ + GIVEN: + - Existing document + WHEN: + - edit_pdf is called with user override + THEN: + - Task is created with user context + """ + mock_group.return_value.delay.return_value = None + doc_ids = [self.doc2.id] + operations = [{"page": 1, "doc": 0}, {"page": 2, "doc": 1}] + user = User.objects.create(username="editor") + + result = bulk_edit.edit_pdf(doc_ids, operations, user=user) + self.assertEqual(result, "OK") + mock_group.return_value.delay.assert_called_once() + + @mock.patch("documents.bulk_edit.chord") + @mock.patch("documents.tasks.consume_file.s") + def test_edit_pdf_with_delete_original(self, mock_consume_file, mock_chord): + """ + GIVEN: + - Existing document + WHEN: + - edit_pdf is called with delete_original=True + THEN: + - Task group is triggered + """ + mock_chord.return_value.delay.return_value = None + doc_ids = [self.doc2.id] + operations = [{"page": 1}, {"page": 2}] + + result = bulk_edit.edit_pdf(doc_ids, operations, delete_original=True) + self.assertEqual(result, "OK") + mock_chord.assert_called_once() + + @mock.patch("documents.tasks.update_document_content_maybe_archive_file.delay") + def test_edit_pdf_with_update_document(self, mock_update_document): + """ + GIVEN: + - A single existing PDF document + WHEN: + - edit_pdf is called with update_document=True and a single output + THEN: + - The original document is updated in-place + - The update_document_content_maybe_archive_file task is triggered + """ + doc_ids = [self.doc2.id] + operations = [{"page": 1}, {"page": 2}] + original_checksum = self.doc2.checksum + original_page_count = self.doc2.page_count + + result = bulk_edit.edit_pdf( + doc_ids, + operations=operations, + update_document=True, + delete_original=False, + ) + + self.assertEqual(result, "OK") + self.doc2.refresh_from_db() + self.assertNotEqual(self.doc2.checksum, original_checksum) + self.assertNotEqual(self.doc2.page_count, original_page_count) + mock_update_document.assert_called_once_with(document_id=self.doc2.id) + + @mock.patch("documents.bulk_edit.group") + @mock.patch("documents.tasks.consume_file.s") + def test_edit_pdf_without_metadata(self, mock_consume_file, mock_group): + """ + GIVEN: + - Existing document + WHEN: + - edit_pdf is called with include_metadata=False + THEN: + - Tasks are created with empty metadata + """ + mock_group.return_value.delay.return_value = None + doc_ids = [self.doc2.id] + operations = [{"page": 1}] + + result = bulk_edit.edit_pdf(doc_ids, operations, include_metadata=False) + self.assertEqual(result, "OK") + mock_group.return_value.delay.assert_called_once() + + @mock.patch("documents.bulk_edit.group") + @mock.patch("documents.tasks.consume_file.s") + def test_edit_pdf_open_failure(self, mock_consume_file, mock_group): + """ + GIVEN: + - Existing document + WHEN: + - edit_pdf fails to open PDF + THEN: + - Task group is not called + """ + doc_ids = [self.doc2.id] + operations = [ + {"page": 9999}, # invalid page, forces error during PDF load + ] + with self.assertLogs("paperless.bulk_edit", level="ERROR"): + with self.assertRaises(Exception): + bulk_edit.edit_pdf(doc_ids, operations) + mock_group.assert_not_called() + mock_consume_file.assert_not_called() + + @mock.patch("documents.bulk_edit.group") + @mock.patch("documents.tasks.consume_file.s") + def test_edit_pdf_multiple_outputs_with_update_flag_errors( + self, + mock_consume_file, + mock_group, + ): + """ + GIVEN: + - Existing document + WHEN: + - edit_pdf is called with multiple outputs and update_document=True + THEN: + - An error is logged and task group is not called + """ + doc_ids = [self.doc2.id] + operations = [ + {"page": 1, "doc": 0}, + {"page": 2, "doc": 1}, + ] + with self.assertLogs("paperless.bulk_edit", level="ERROR"): + with self.assertRaises(ValueError): + bulk_edit.edit_pdf(doc_ids, operations, update_document=True) + mock_group.assert_not_called() + mock_consume_file.assert_not_called() diff --git a/src/documents/views.py b/src/documents/views.py index b84267d75..c98df6da4 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1321,6 +1321,7 @@ class BulkEditView(PassUserMixin): "delete_pages": "checksum", "split": None, "merge": None, + "edit_pdf": "checksum", "reprocess": "checksum", } @@ -1339,6 +1340,7 @@ class BulkEditView(PassUserMixin): if method in [ bulk_edit.split, bulk_edit.merge, + bulk_edit.edit_pdf, ]: parameters["user"] = user @@ -1358,27 +1360,36 @@ class BulkEditView(PassUserMixin): # check ownership for methods that change original document if ( - has_perms - and method - in [ - bulk_edit.set_permissions, - bulk_edit.delete, - bulk_edit.rotate, - bulk_edit.delete_pages, - ] - ) or ( - method in [bulk_edit.merge, bulk_edit.split] - and parameters["delete_originals"] + ( + has_perms + and method + in [ + bulk_edit.set_permissions, + bulk_edit.delete, + bulk_edit.rotate, + bulk_edit.delete_pages, + bulk_edit.edit_pdf, + ] + ) + or ( + method in [bulk_edit.merge, bulk_edit.split] + and parameters["delete_originals"] + ) + or (method == bulk_edit.edit_pdf and parameters["update_document"]) ): has_perms = user_is_owner_of_all_documents # check global add permissions for methods that create documents if ( has_perms - and method in [bulk_edit.split, bulk_edit.merge] - and not user.has_perm( - "documents.add_document", + and ( + method in [bulk_edit.split, bulk_edit.merge] + or ( + method == bulk_edit.edit_pdf + and not parameters["update_document"] + ) ) + and not user.has_perm("documents.add_document") ): has_perms = False @@ -1416,7 +1427,6 @@ class BulkEditView(PassUserMixin): ) } - # TODO: parameter validation result = method(documents, **parameters) if settings.AUDIT_LOG_ENABLED and modified_field: