Sheesh, now we can bring back ordering

src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.html

@@ -6,11 +6,15 @@
   <p>{{message}}</p>
   <div class="form-group">
     <span class="form-label d-inline-block" i18n>Versions:</span>
-    <ul class="list-group">
+    <ul class="list-group"
+        cdkDropList
+        [cdkDropListData]="versionDocumentIDs()"
+        (cdkDropListDropped)="onDrop($event)">
       @for (documentID of versionDocumentIDs(); track documentID) {
         @let document = getDocument(documentID);
         @if (document) {
-          <li class="list-group-item d-flex align-items-center">
+          <li class="list-group-item d-flex align-items-center" cdkDrag>
+            <i-bs name="grip-vertical" class="me-2"></i-bs>
             <div class="d-flex flex-column">
               <div>
                 @if (document.correspondent) {
@@ -28,6 +32,7 @@
         }
       }
     </ul>
+    <div class="form-text" i18n>Drag to reorder. The last document becomes the current version.</div>
   </div>
   <div class="form-group mt-4">
     <label class="form-label" for="rootDocumentID" i18n>Root document:</label>

src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.scss

@@ -0,0 +1,3 @@
+.list-group-item {
+    cursor: move;
+}

src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.spec.ts

@@ -2,6 +2,7 @@ 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 { of } from 'rxjs'
 import { DocumentService } from 'src/app/services/rest/document.service'
 import { MergeAsVersionsConfirmDialogComponent } from './merge-as-versions-confirm-dialog.component'
@@ -13,7 +14,10 @@ describe('MergeAsVersionsConfirmDialogComponent', () => {
 
   beforeEach(async () => {
     await TestBed.configureTestingModule({
-      imports: [MergeAsVersionsConfirmDialogComponent],
+      imports: [
+        NgxBootstrapIconsModule.pick(allIcons),
+        MergeAsVersionsConfirmDialogComponent,
+      ],
       providers: [
         NgbActiveModal,
         provideHttpClient(withInterceptorsFromDi()),
@@ -53,4 +57,14 @@ describe('MergeAsVersionsConfirmDialogComponent', () => {
 
     expect(component.versionDocumentIDs()).toEqual([1, 3])
   })
+
+  it('should move draggable documents while keeping the root fixed', () => {
+    component.documentIDs.set([1, 2, 3])
+    component.rootDocumentID.set(1)
+
+    component.onDrop({ previousIndex: 1, currentIndex: 0 } as any)
+
+    expect(component.documentIDs()).toEqual([1, 3, 2])
+    expect(component.versionDocumentIDs()).toEqual([3, 2])
+  })
 })

src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.ts

@@ -1,6 +1,12 @@
+import {
+  CdkDragDrop,
+  DragDropModule,
+  moveItemInArray,
+} from '@angular/cdk/drag-drop'
 import { AsyncPipe } from '@angular/common'
 import { Component, OnInit, computed, inject, signal } from '@angular/core'
 import { FormsModule } from '@angular/forms'
+import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons'
 import { takeUntil } from 'rxjs'
 import { Document } from 'src/app/data/document'
 import { CorrespondentNamePipe } from 'src/app/pipes/correspondent-name.pipe'
@@ -11,7 +17,15 @@ import { ConfirmDialogComponent } from '../confirm-dialog.component'
 @Component({
   selector: 'pngx-merge-as-versions-confirm-dialog',
   templateUrl: './merge-as-versions-confirm-dialog.component.html',
-  imports: [AsyncPipe, CorrespondentNamePipe, CustomDatePipe, FormsModule],
+  styleUrl: './merge-as-versions-confirm-dialog.component.scss',
+  imports: [
+    AsyncPipe,
+    CorrespondentNamePipe,
+    CustomDatePipe,
+    DragDropModule,
+    FormsModule,
+    NgxBootstrapIconsModule,
+  ],
 })
 export class MergeAsVersionsConfirmDialogComponent
   extends ConfirmDialogComponent
@@ -35,6 +49,21 @@ export class MergeAsVersionsConfirmDialogComponent
       .subscribe((response) => this.documents.set(response.results))
   }
 
+  onDrop(event: CdkDragDrop<number[]>) {
+    const versionDocumentIDs = this.versionDocumentIDs().concat()
+    moveItemInArray(versionDocumentIDs, event.previousIndex, event.currentIndex)
+
+    // The root keeps its place in the list, only the versions move around it
+    let versionIndex = 0
+    this.documentIDs.update((documentIDs) =>
+      documentIDs.map((documentID) =>
+        documentID === this.rootDocumentID()
+          ? documentID
+          : versionDocumentIDs[versionIndex++]
+      )
+    )
+  }
+
   getDocument(documentID: number): Document {
     return this.documents().find((document) => document.id === documentID)
   }

src/documents/bulk_edit.py

@@ -637,7 +637,7 @@ def merge_as_versions(
         if any(document.root_document_id is not None for document in documents):
             raise ValueError("Only top-level documents can be merged as versions.")
 
-        source_ids = sorted(doc_id for doc_id in doc_ids if doc_id != root_document_id)
+        source_ids = [doc_id for doc_id in doc_ids if doc_id != root_document_id]
         if version_label is not None and len(source_ids) != 1:
             raise ValueError(
                 "A version label can only be set when merging one source document.",

src/documents/tests/test_merge_documents_as_versions.py

@@ -185,7 +185,7 @@ class TestMergeDocumentsAsVersions(TestCase):
     @mock.patch("documents.bulk_edit.DocumentsStatusManager")
     @mock.patch("documents.bulk_edit.bulk_update_documents.apply_async")
     @mock.patch("documents.bulk_edit.remove_document_from_index.apply_async")
-    def test_merges_documents_in_creation_order(
+    def test_merges_documents_in_selection_order(
         self,
         remove_from_index_mock,
         bulk_update_mock,
@@ -219,27 +219,28 @@ class TestMergeDocumentsAsVersions(TestCase):
         source1.refresh_from_db()
         source2.refresh_from_db()
         root.refresh_from_db()
+        # source2 was selected first, so it becomes the older of the two versions
         self.assertEqual(source2.root_document_id, root.id)
-        self.assertEqual(source2.version_index, 5)
+        self.assertEqual(source2.version_index, 4)
         self.assertEqual(source1.root_document_id, root.id)
-        self.assertEqual(source1.version_index, 4)
+        self.assertEqual(source1.version_index, 5)
         self.assertIsNone(source1.archive_serial_number)
         self.assertIsNone(source2.archive_serial_number)
         # The root had no ASN of its own, so it takes the first one
-        self.assertEqual(root.archive_serial_number, 1)
+        self.assertEqual(root.archive_serial_number, 2)
         self.assertGreater(root.modified, original_modified)
         self.assertEqual(existing_version.root_document_id, root.id)
 
         self.assertEqual(
             [call.kwargs["args"] for call in remove_from_index_mock.call_args_list],
-            [[source1.id], [source2.id]],
+            [[source2.id], [source1.id]],
         )
         bulk_update_mock.assert_called_once_with(
             kwargs={"document_ids": [root.id]},
             headers={"trigger_source": "system"},
         )
         status_manager_mock.return_value.send_documents_deleted.assert_called_once_with(
-            [source1.id, source2.id],
+            [source2.id, source1.id],
         )
 
     @mock.patch("documents.bulk_edit.DocumentsStatusManager")
@@ -529,3 +530,30 @@ class TestMergeDocumentsAsVersionsAPI(APITestCase):
         status_manager_mock.return_value.send_documents_deleted.assert_called_once_with(
             [self.doc1.id],
         )
+
+    @mock.patch("documents.bulk_edit.DocumentsStatusManager")
+    @mock.patch("documents.bulk_edit.bulk_update_documents.apply_async")
+    @mock.patch("documents.bulk_edit.remove_document_from_index.apply_async")
+    def test_chosen_order_survives_to_the_versions_list(self, *_mocks) -> None:
+        doc3 = Document.objects.create(checksum="C", title="C", owner=self.user)
+        # Deliberately not in id order, as dragging the dialog rows produces
+        ordered = [doc3.id, self.doc1.id]
+
+        response = self.client.post(
+            "/api/documents/merge_as_versions/",
+            {
+                "documents": [*ordered, self.doc2.id],
+                "root_document_id": self.doc2.id,
+            },
+            format="json",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        detail_response = self.client.get(
+            f"/api/documents/{self.doc2.id}/?fields=id,versions",
+        )
+        # Newest first, so the reverse of the order they were merged in
+        self.assertEqual(
+            [version["id"] for version in detail_response.data["versions"]],
+            [self.doc1.id, doc3.id, self.doc2.id],
+        )