Move the merge as version heirarchy checks to the bulk edit method

src/documents/bulk_edit.py

@@ -702,7 +702,15 @@ def merge_as_versions(
         )
         documents_by_id = {document.id: document for document in documents}
 
+        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]
+        if Document.global_objects.filter(root_document_id__in=source_ids).exists():
+            raise ValueError(
+                "Documents with existing versions cannot be merged into another document.",
+            )
+
         root_document = documents_by_id[root_document_id]
         next_version_index = (
             Document.global_objects.filter(

src/documents/serialisers.py

@@ -1717,19 +1717,8 @@ class MergeDocumentsAsVersionsSerializer(DocumentListSerializer):
                 "root_document_id must be one of the selected documents.",
             )
 
-        selected_documents = Document.objects.filter(id__in=documents)
-        if selected_documents.filter(root_document__isnull=False).exists():
-            raise serializers.ValidationError(
-                "Only top-level documents can be merged as versions.",
-            )
-
-        source_document_ids = set(documents) - {attrs["root_document_id"]}
-        if Document.global_objects.filter(
-            root_document_id__in=source_document_ids,
-        ).exists():
-            raise serializers.ValidationError(
-                "Documents with existing versions cannot be merged into another document.",
-            )
+        # Version state checks (e.g., whether a document is already part of a version chain)
+        # moved to merge_as_versions() under row locks, since it could change between queueing and execution
         return attrs
 
 

src/documents/tests/test_merge_documents_as_versions.py

@@ -105,67 +105,6 @@ class TestMergeDocumentsAsVersionsSerializer(TestCase):
         self.assertFalse(serializer.is_valid())
         self.assertIn("documents", serializer.errors)
 
-    def test_rejects_selected_version(self) -> None:
-        version = Document.objects.create(
-            checksum="D",
-            title="D",
-            root_document=self.doc1,
-            version_index=1,
-        )
-        serializer = MergeDocumentsAsVersionsSerializer(
-            data={
-                "documents": [version.id, self.doc2.id],
-                "root_document_id": self.doc2.id,
-            },
-        )
-
-        self.assertFalse(serializer.is_valid())
-        self.assertEqual(
-            serializer.errors["non_field_errors"][0],
-            "Only top-level documents can be merged as versions.",
-        )
-
-    def test_rejects_source_document_with_versions(self) -> None:
-        Document.objects.create(
-            checksum="D",
-            title="D",
-            root_document=self.doc1,
-            version_index=1,
-        )
-        serializer = MergeDocumentsAsVersionsSerializer(
-            data={
-                "documents": [self.doc1.id, self.doc2.id],
-                "root_document_id": self.doc2.id,
-            },
-        )
-
-        self.assertFalse(serializer.is_valid())
-        self.assertEqual(
-            serializer.errors["non_field_errors"][0],
-            "Documents with existing versions cannot be merged into another document.",
-        )
-
-    def test_rejects_source_document_with_trashed_versions(self) -> None:
-        version = Document.objects.create(
-            checksum="D",
-            title="D",
-            root_document=self.doc1,
-            version_index=1,
-        )
-        version.delete()  # trashed, but still points at doc1
-        serializer = MergeDocumentsAsVersionsSerializer(
-            data={
-                "documents": [self.doc1.id, self.doc2.id],
-                "root_document_id": self.doc2.id,
-            },
-        )
-
-        self.assertFalse(serializer.is_valid())
-        self.assertEqual(
-            serializer.errors["non_field_errors"][0],
-            "Documents with existing versions cannot be merged into another document.",
-        )
-
     def test_allows_root_document_with_versions(self) -> None:
         Document.objects.create(
             checksum="D",
@@ -184,6 +123,95 @@ class TestMergeDocumentsAsVersionsSerializer(TestCase):
 
 
 class TestMergeDocumentsAsVersions(TestCase):
+    def setUp(self) -> None:
+        self.doc1 = Document.objects.create(checksum="test-A", title="A")
+        self.doc2 = Document.objects.create(checksum="test-B", title="B")
+
+    def test_rejects_selected_version(self) -> None:
+        version = Document.objects.create(
+            checksum="D",
+            title="D",
+            root_document=self.doc1,
+            version_index=1,
+        )
+        with self.assertRaisesMessage(
+            ValueError,
+            "Only top-level documents can be merged as versions.",
+        ):
+            merge_as_versions(
+                [version.id, self.doc2.id],
+                root_document_id=self.doc2.id,
+            )
+
+    def test_rejects_source_document_with_versions(self) -> None:
+        Document.objects.create(
+            checksum="D",
+            title="D",
+            root_document=self.doc1,
+            version_index=1,
+        )
+        with self.assertRaisesMessage(
+            ValueError,
+            "Documents with existing versions cannot be merged into another document.",
+        ):
+            merge_as_versions(
+                [self.doc1.id, self.doc2.id],
+                root_document_id=self.doc2.id,
+            )
+
+    def test_rejects_source_document_with_trashed_versions(self) -> None:
+        version = Document.objects.create(
+            checksum="D",
+            title="D",
+            root_document=self.doc1,
+            version_index=1,
+        )
+        version.delete()  # trashed, but still points at doc1
+        with self.assertRaisesMessage(
+            ValueError,
+            "Documents with existing versions cannot be merged into another document.",
+        ):
+            merge_as_versions(
+                [self.doc1.id, self.doc2.id],
+                root_document_id=self.doc2.id,
+            )
+
+    @mock.patch("documents.bulk_edit.DocumentsStatusManager")
+    @mock.patch("documents.bulk_edit.bulk_update_documents.apply_async")
+    @mock.patch("documents.search.get_backend")
+    def test_rejects_source_that_gained_versions_after_validation(
+        self,
+        _get_backend_mock,
+        _bulk_update_mock,
+        _status_manager_mock,
+    ) -> None:
+        destination = Document.objects.create(checksum="destination")
+        serializer = MergeDocumentsAsVersionsSerializer(
+            data={
+                "documents": [self.doc1.id, destination.id],
+                "root_document_id": destination.id,
+            },
+        )
+        self.assertTrue(serializer.is_valid(), serializer.errors)
+
+        merge_as_versions(
+            [self.doc1.id, self.doc2.id],
+            root_document_id=self.doc1.id,
+        )
+        with self.assertRaisesMessage(
+            ValueError,
+            "Documents with existing versions cannot be merged into another document.",
+        ):
+            merge_as_versions(
+                serializer.validated_data["documents"],
+                root_document_id=serializer.validated_data["root_document_id"],
+            )
+
+        self.doc1.refresh_from_db()
+        self.doc2.refresh_from_db()
+        self.assertIsNone(self.doc1.root_document_id)
+        self.assertEqual(self.doc2.root_document_id, self.doc1.id)
+
     @mock.patch("documents.bulk_edit.DocumentsStatusManager")
     @mock.patch("documents.bulk_edit.bulk_update_documents.apply_async")
     @mock.patch("documents.search.get_backend")
@@ -456,8 +484,9 @@ class TestMergeDocumentsAsVersionsAPI(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         merge_mock.assert_not_called()
 
-    @mock.patch("documents.views.bulk_edit.merge_as_versions")
-    def test_rejects_source_document_with_versions(self, merge_mock) -> None:
+    @mock.patch("documents.views.execute_bulk_operation.apply_async")
+    def test_queues_source_document_with_versions(self, enqueue_mock) -> None:
+        enqueue_mock.return_value.id = "queued-merge"
         Document.objects.create(
             checksum="C",
             title="C",
@@ -475,8 +504,9 @@ class TestMergeDocumentsAsVersionsAPI(APITestCase):
             format="json",
         )
 
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        merge_mock.assert_not_called()
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        enqueue_mock.assert_called_once()
+        self.assertEqual(response.data["task_id"], "queued-merge")
         self.doc1.refresh_from_db()
         self.assertIsNone(self.doc1.root_document_id)