From 1e489a0666c0a7dad38fe1edbb741157a6ffa094 Mon Sep 17 00:00:00 2001
From: shamoon <4887959+shamoon@users.noreply.github.com>
Date: Wed, 5 Mar 2025 12:46:51 -0800
Subject: [PATCH] Enhancement: add switch to allow merging non-PDFs with
 archive version (#9305)

---
 src-ui/messages.xlf                           | 17 ++++++---
 .../merge-confirm-dialog.component.html       |  8 +++-
 .../merge-confirm-dialog.component.ts         |  1 +
 .../bulk-editor/bulk-editor.component.spec.ts | 21 +++++++++++
 .../bulk-editor/bulk-editor.component.ts      |  3 ++
 src/documents/bulk_edit.py                    | 12 +++++-
 src/documents/serialisers.py                  |  5 +++
 src/documents/tests/test_bulk_edit.py         | 37 +++++++++++++++++++
 8 files changed, 96 insertions(+), 8 deletions(-)

diff --git a/src-ui/messages.xlf b/src-ui/messages.xlf
index cd1a97950..1649d6100 100644
--- a/src-ui/messages.xlf
+++ b/src-ui/messages.xlf
@@ -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">
diff --git a/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.html b/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.html
index 576861ff2..da57feca7 100644
--- a/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.html
+++ b/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.html
@@ -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">
diff --git a/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.ts b/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.ts
index 9359e4099..0438a33e7 100644
--- a/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.ts
+++ b/src-ui/src/app/components/common/confirm-dialog/merge-confirm-dialog/merge-confirm-dialog.component.ts
@@ -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[] {
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 aa4a07d12..8f89e541a 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
@@ -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', () => {
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 9864761fa..aa32497f3 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
@@ -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(
diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py
index be4608e36..b8e76f7c8 100644
--- a/src/documents/bulk_edit.py
+++ b/src/documents/bulk_edit.py
@@ -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)
diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py
index 38053baee..a67673efc 100644
--- a/src/documents/serialisers.py
+++ b/src/documents/serialisers.py
@@ -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"]
diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py
index 4a7145d34..dd59a6217 100644
--- a/src/documents/tests/test_bulk_edit.py
+++ b/src/documents/tests/test_bulk_edit.py
@@ -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):