From 10915593011aafbda3f0708da882345274bd42d8 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:41:49 -0700 Subject: [PATCH] Sheesh, now we can bring back ordering --- ...-as-versions-confirm-dialog.component.html | 9 ++++- ...-as-versions-confirm-dialog.component.scss | 3 ++ ...-versions-confirm-dialog.component.spec.ts | 16 +++++++- ...ge-as-versions-confirm-dialog.component.ts | 31 +++++++++++++- src/documents/bulk_edit.py | 2 +- .../tests/test_merge_documents_as_versions.py | 40 ++++++++++++++++--- 6 files changed, 90 insertions(+), 11 deletions(-) diff --git a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.html b/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.html index 6fc4cef58..aa4ac08ab 100644 --- a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.html +++ b/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.html @@ -6,11 +6,15 @@

{{message}}

Versions: -
diff --git a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.scss b/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.scss index e69de29bb..c780e5a35 100644 --- a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.scss +++ b/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; +} diff --git a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.spec.ts b/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.spec.ts index 73c98ca08..939eaa31d 100644 --- a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.spec.ts +++ b/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]) + }) }) diff --git a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.ts b/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.ts index db87090fd..5fb69fc92 100644 --- a/src-ui/src/app/components/common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component.ts +++ b/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) { + 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) } diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index 3c8125f58..850758693 100644 --- a/src/documents/bulk_edit.py +++ b/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.", diff --git a/src/documents/tests/test_merge_documents_as_versions.py b/src/documents/tests/test_merge_documents_as_versions.py index 3f06c512f..f91ee7378 100644 --- a/src/documents/tests/test_merge_documents_as_versions.py +++ b/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], + )