Derp, no re-ordering

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

@@ -6,15 +6,11 @@
   <p>{{message}}</p>
   <div class="form-group">
     <label class="form-label" i18n>Versions:</label>
-    <ul class="list-group"
-        cdkDropList
-        [cdkDropListData]="draggableDocumentIDs()"
-        (cdkDropListDropped)="onDrop($event)">
-      @for (documentID of draggableDocumentIDs(); track documentID) {
+    <ul class="list-group">
+      @for (documentID of versionDocumentIDs(); track documentID) {
         @let document = getDocument(documentID);
         @if (document) {
-          <li class="list-group-item d-flex align-items-center" cdkDrag>
-            <i-bs name="grip-vertical" class="me-2"></i-bs>
+          <li class="list-group-item d-flex align-items-center">
             <div class="d-flex flex-column">
               <div>
                 @if (document.correspondent) {

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

@@ -1,3 +0,0 @@
-.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,7 +2,6 @@ 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'
@@ -14,10 +13,7 @@ describe('MergeAsVersionsConfirmDialogComponent', () => {
 
   beforeEach(async () => {
     await TestBed.configureTestingModule({
-      imports: [
-        NgxBootstrapIconsModule.pick(allIcons),
-        MergeAsVersionsConfirmDialogComponent,
-      ],
+      imports: [MergeAsVersionsConfirmDialogComponent],
       providers: [
         NgbActiveModal,
         provideHttpClient(withInterceptorsFromDi()),
@@ -55,16 +51,6 @@ describe('MergeAsVersionsConfirmDialogComponent', () => {
     component.documentIDs.set([1, 2, 3])
     component.rootDocumentID.set(2)
 
-    expect(component.draggableDocumentIDs()).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.draggableDocumentIDs()).toEqual([3, 2])
+    expect(component.versionDocumentIDs()).toEqual([1, 3])
   })
 })

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

@@ -1,12 +1,6 @@
-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'
@@ -17,15 +11,7 @@ import { ConfirmDialogComponent } from '../confirm-dialog.component'
 @Component({
   selector: 'pngx-merge-as-versions-confirm-dialog',
   templateUrl: './merge-as-versions-confirm-dialog.component.html',
-  styleUrl: './merge-as-versions-confirm-dialog.component.scss',
-  imports: [
-    AsyncPipe,
-    CorrespondentNamePipe,
-    CustomDatePipe,
-    DragDropModule,
-    FormsModule,
-    NgxBootstrapIconsModule,
-  ],
+  imports: [AsyncPipe, CorrespondentNamePipe, CustomDatePipe, FormsModule],
 })
 export class MergeAsVersionsConfirmDialogComponent
   extends ConfirmDialogComponent
@@ -36,7 +22,7 @@ export class MergeAsVersionsConfirmDialogComponent
   readonly documentIDs = signal<number[]>([])
   readonly documents = signal<Document[]>([])
   readonly rootDocumentID = signal(-1)
-  readonly draggableDocumentIDs = computed(() =>
+  readonly versionDocumentIDs = computed(() =>
     this.documentIDs().filter(
       (documentID) => documentID !== this.rootDocumentID()
     )
@@ -49,24 +35,6 @@ export class MergeAsVersionsConfirmDialogComponent
       .subscribe((response) => this.documents.set(response.results))
   }
 
-  onDrop(event: CdkDragDrop<number[]>) {
-    const draggableDocumentIDs = this.draggableDocumentIDs().concat()
-    moveItemInArray(
-      draggableDocumentIDs,
-      event.previousIndex,
-      event.currentIndex
-    )
-
-    let draggableIndex = 0
-    this.documentIDs.update((documentIDs) =>
-      documentIDs.map((documentID) =>
-        documentID === this.rootDocumentID()
-          ? documentID
-          : draggableDocumentIDs[draggableIndex++]
-      )
-    )
-  }
-
   getDocument(documentID: number): Document {
     return this.documents().find((document) => document.id === documentID)
   }

src/documents/bulk_edit.py

@@ -631,7 +631,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 = [doc_id for doc_id in doc_ids if doc_id != root_document_id]
+        source_ids = sorted(doc_id for doc_id in doc_ids if doc_id != root_document_id)
         if Document.objects.filter(root_document_id__in=source_ids).exists():
             raise ValueError(
                 "Documents with existing versions cannot be merged into another document.",

src/documents/tests/test_merge_documents_as_versions.py

@@ -135,7 +135,7 @@ class TestMergeDocumentsAsVersions(TestCase):
     @mock.patch("documents.bulk_edit.DocumentsStatusManager")
     @mock.patch("documents.bulk_edit.bulk_update_documents.apply_async")
     @mock.patch("documents.search.get_backend")
-    def test_merges_documents_in_selection_order(
+    def test_merges_documents_in_creation_order(
         self,
         get_backend_mock,
         bulk_update_mock,
@@ -170,9 +170,9 @@ class TestMergeDocumentsAsVersions(TestCase):
         source2.refresh_from_db()
         root.refresh_from_db()
         self.assertEqual(source2.root_document_id, root.id)
-        self.assertEqual(source2.version_index, 4)
+        self.assertEqual(source2.version_index, 5)
         self.assertEqual(source1.root_document_id, root.id)
-        self.assertEqual(source1.version_index, 5)
+        self.assertEqual(source1.version_index, 4)
         self.assertIsNone(source1.archive_serial_number)
         self.assertIsNone(source2.archive_serial_number)
         self.assertGreater(root.modified, original_modified)
@@ -181,14 +181,14 @@ class TestMergeDocumentsAsVersions(TestCase):
         batch = get_backend_mock.return_value.batch_update.return_value.__enter__.return_value
         self.assertEqual(
             [call.args[0] for call in batch.remove.call_args_list],
-            [source2.id, source1.id],
+            [source1.id, source2.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(
-            [source2.id, source1.id],
+            [source1.id, source2.id],
         )
 
     @mock.patch("documents.bulk_edit.DocumentsStatusManager")
@@ -227,6 +227,7 @@ class TestMergeDocumentsAsVersionsAPI(APITestCase):
         self.user = User.objects.create_user(username="user")
         self.user.user_permissions.add(
             Permission.objects.get(codename="change_document"),
+            Permission.objects.get(codename="view_document"),
         )
         self.doc1 = Document.objects.create(
             checksum="A",
@@ -304,3 +305,44 @@ class TestMergeDocumentsAsVersionsAPI(APITestCase):
 
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         merge_mock.assert_not_called()
+
+    @mock.patch("documents.bulk_edit.DocumentsStatusManager")
+    @mock.patch("documents.bulk_edit.bulk_update_documents.apply_async")
+    @mock.patch("documents.search.get_backend")
+    def test_merges_and_returns_documents_as_versions(
+        self,
+        get_backend_mock,
+        bulk_update_mock,
+        status_manager_mock,
+    ) -> None:
+        response = self.client.post(
+            "/api/documents/merge_as_versions/",
+            {
+                "documents": [self.doc1.id, self.doc2.id],
+                "root_document_id": self.doc2.id,
+            },
+            format="json",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.doc1.refresh_from_db()
+        self.assertEqual(self.doc1.root_document_id, self.doc2.id)
+
+        detail_response = self.client.get(
+            f"/api/documents/{self.doc2.id}/?fields=id,versions",
+        )
+        self.assertEqual(detail_response.status_code, status.HTTP_200_OK)
+        versions = detail_response.data["versions"]
+        self.assertEqual(
+            {version["id"] for version in versions},
+            {self.doc1.id, self.doc2.id},
+        )
+        self.assertEqual(
+            [version["id"] for version in versions if version["is_root"]],
+            [self.doc2.id],
+        )
+        get_backend_mock.assert_called_once()
+        bulk_update_mock.assert_called_once()
+        status_manager_mock.return_value.send_documents_deleted.assert_called_once_with(
+            [self.doc1.id],
+        )