Enhancement: add switch to allow merging non-PDFs with archive version (#9305)

This commit is contained in:
shamoon 2025-03-05 12:46:51 -08:00 committed by GitHub
parent edc7181843
commit 1e489a0666
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 96 additions and 8 deletions

View File

@ -3246,18 +3246,25 @@
<context context-type="linenumber">24</context>
</context-group>
</trans-unit>
<trans-unit id="2710430925353472741" datatype="html">
<source>Try to include archive version in merge for non-PDF files</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.html</context>
<context context-type="linenumber">32</context>
</context-group>
</trans-unit>
<trans-unit id="5612366187076076264" datatype="html">
<source>Delete original documents after successful merge</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.html</context>
<context context-type="linenumber">32</context>
<context context-type="linenumber">36</context>
</context-group>
</trans-unit>
<trans-unit id="5138283234724909648" datatype="html">
<source>Note that only PDFs will be included.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.html</context>
<context context-type="linenumber">34</context>
<context context-type="linenumber">39</context>
</context-group>
</trans-unit>
<trans-unit id="8157388568390631653" datatype="html">
@ -7431,21 +7438,21 @@
<source>Merged document will be queued for consumption.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/bulk-editor/bulk-editor.component.ts</context>
<context context-type="linenumber">863</context>
<context context-type="linenumber">866</context>
</context-group>
</trans-unit>
<trans-unit id="476913782630693351" datatype="html">
<source>Custom fields updated.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/bulk-editor/bulk-editor.component.ts</context>
<context context-type="linenumber">885</context>
<context context-type="linenumber">888</context>
</context-group>
</trans-unit>
<trans-unit id="3873496751167944011" datatype="html">
<source>Error updating custom fields.</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/document-list/bulk-editor/bulk-editor.component.ts</context>
<context context-type="linenumber">894</context>
<context context-type="linenumber">897</context>
</context-group>
</trans-unit>
<trans-unit id="6307402210351946694" datatype="html">

View File

@ -28,10 +28,16 @@
</select>
</div>
<div class="form-check form-switch mt-4">
<input class="form-check-input" type="checkbox" role="switch" id="archiveFallbackSwitch" [(ngModel)]="archiveFallback">
<label class="form-check-label" for="archiveFallbackSwitch" i18n>Try to include archive version in merge for non-PDF files</label>
</div>
<div class="form-check form-switch mt-2">
<input class="form-check-input" type="checkbox" role="switch" id="deleteOriginalsSwitch" [(ngModel)]="deleteOriginals" [disabled]="!userOwnsAllDocuments">
<label class="form-check-label" for="deleteOriginalsSwitch" i18n>Delete original documents after successful merge</label>
</div>
<p class="small text-muted fst-italic mt-4" i18n>Note that only PDFs will be included.</p>
@if (!archiveFallback) {
<p class="small text-muted fst-italic mt-4" i18n>Note that only PDFs will be included.</p>
}
</div>
<div class="modal-footer">
<button type="button" class="btn" [class]="cancelBtnClass" (click)="cancel()" [disabled]="!buttonsEnabled">

View File

@ -29,6 +29,7 @@ export class MergeConfirmDialogComponent
implements OnInit
{
public documentIDs: number[] = []
public archiveFallback: boolean = false
public deleteOriginals: boolean = false
private _documents: Document[] = []
get documents(): Document[] {

View File

@ -1040,6 +1040,27 @@ describe('BulkEditorComponent', () => {
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
) // listAllFilteredIds
expect(documentListViewService.selected.size).toEqual(0)
// Test with archiveFallback enabled
modal.componentInstance.deleteOriginals = false
modal.componentInstance.archiveFallback = true
modal.componentInstance.confirm()
req = httpTestingController.expectOne(
`${environment.apiBaseUrl}documents/bulk_edit/`
)
req.flush(true)
expect(req.request.body).toEqual({
documents: [3, 4],
method: 'merge',
parameters: { metadata_document_id: 3, archive_fallback: true },
})
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true`
) // list reload
httpTestingController.match(
`${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id`
) // listAllFilteredIds
expect(documentListViewService.selected.size).toEqual(0)
})
it('should support bulk download with archive, originals or both and file formatting', () => {

View File

@ -857,6 +857,9 @@ export class BulkEditorComponent
if (mergeDialog.deleteOriginals) {
args['delete_originals'] = true
}
if (mergeDialog.archiveFallback) {
args['archive_fallback'] = true
}
mergeDialog.buttonsEnabled = false
this.executeBulkOperation(modal, 'merge', args, mergeDialog.documentIDs)
this.toastService.showInfo(

View File

@ -318,6 +318,7 @@ def merge(
*,
metadata_document_id: int | None = None,
delete_originals: bool = False,
archive_fallback: bool = False,
user: User | None = None,
) -> Literal["OK"]:
logger.info(
@ -333,7 +334,14 @@ def merge(
for doc_id in doc_ids:
doc = qs.get(id=doc_id)
try:
with pikepdf.open(str(doc.source_path)) as pdf:
doc_path = (
doc.archive_path
if archive_fallback
and doc.mime_type != "application/pdf"
and doc.has_archive_version
else doc.source_path
)
with pikepdf.open(str(doc_path)) as pdf:
version = max(version, pdf.pdf_version)
merged_pdf.pages.extend(pdf.pages)
affected_docs.append(doc.id)
@ -349,7 +357,7 @@ def merge(
Path(
tempfile.mkdtemp(dir=settings.SCRATCH_DIR),
)
/ f"{'_'.join([str(doc_id) for doc_id in doc_ids])[:100]}_merged.pdf"
/ f"{'_'.join([str(doc_id) for doc_id in affected_docs])[:100]}_merged.pdf"
)
merged_pdf.remove_unreferenced_resources()
merged_pdf.save(filepath, min_version=version)

View File

@ -1446,6 +1446,11 @@ class BulkEditSerializer(
raise serializers.ValidationError("delete_originals must be a boolean")
else:
parameters["delete_originals"] = False
if "archive_fallback" in parameters:
if not isinstance(parameters["archive_fallback"], bool):
raise serializers.ValidationError("archive_fallback must be a boolean")
else:
parameters["archive_fallback"] = False
def validate(self, attrs):
method = attrs["method"]

View File

@ -514,12 +514,23 @@ class TestPDFActions(DirectoriesMixin, TestCase):
Path(__file__).parent / "samples" / "simple.jpg",
img_doc,
)
img_doc_archive = self.dirs.archive_dir / "sample_image.pdf"
shutil.copy(
Path(__file__).parent
/ "samples"
/ "documents"
/ "originals"
/ "0000001.pdf",
img_doc_archive,
)
self.img_doc = Document.objects.create(
checksum="D",
title="D",
filename=img_doc,
mime_type="image/jpeg",
)
self.img_doc.archive_filename = img_doc_archive
self.img_doc.save()
@mock.patch("documents.tasks.consume_file.s")
def test_merge(self, mock_consume_file):
@ -605,6 +616,32 @@ class TestPDFActions(DirectoriesMixin, TestCase):
doc_ids,
)
@mock.patch("documents.tasks.consume_file.s")
def test_merge_with_archive_fallback(self, mock_consume_file):
"""
GIVEN:
- Existing documents
WHEN:
- Merge action is called with 2 documents, one of which is an image and archive_fallback is set to True
THEN:
- Image document should be included
"""
doc_ids = [self.doc2.id, self.img_doc.id]
result = bulk_edit.merge(doc_ids, archive_fallback=True)
self.assertEqual(result, "OK")
expected_filename = (
f"{'_'.join([str(doc_id) for doc_id in doc_ids])[:100]}_merged.pdf"
)
mock_consume_file.assert_called()
consume_file_args, _ = mock_consume_file.call_args
self.assertEqual(
Path(consume_file_args[0].original_file).name,
expected_filename,
)
@mock.patch("documents.tasks.consume_file.delay")
@mock.patch("pikepdf.open")
def test_merge_with_errors(self, mock_open_pdf, mock_consume_file):