diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index 4418fb14b..696f15397 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -638,7 +638,7 @@ def merge_as_versions( raise ValueError( "A version label can only be set when merging one source document.", ) - if Document.objects.filter(root_document_id__in=source_ids).exists(): + if Document.global_objects.filter(root_document_id__in=source_ids).exists(): raise ValueError( "Documents with existing versions cannot be merged into another document.", ) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 6ec8ba7f4..0d3cc24c3 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1717,7 +1717,7 @@ class MergeDocumentsAsVersionsSerializer(DocumentListSerializer): ) source_document_ids = set(documents) - {attrs["root_document_id"]} - if Document.objects.filter( + if Document.global_objects.filter( root_document_id__in=source_document_ids, ).exists(): raise serializers.ValidationError( diff --git a/src/documents/tests/test_merge_documents_as_versions.py b/src/documents/tests/test_merge_documents_as_versions.py index ffeb851fa..bb9fa18dc 100644 --- a/src/documents/tests/test_merge_documents_as_versions.py +++ b/src/documents/tests/test_merge_documents_as_versions.py @@ -141,6 +141,27 @@ class TestMergeDocumentsAsVersionsSerializer(TestCase): "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", @@ -268,6 +289,42 @@ class TestMergeDocumentsAsVersions(TestCase): bulk_update_mock.assert_not_called() status_manager_mock.assert_not_called() + @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_rejects_source_document_with_trashed_versions( + self, + remove_from_index_mock, + bulk_update_mock, + status_manager_mock, + ) -> None: + source = Document.objects.create(checksum="A", title="Source") + version = Document.objects.create( + checksum="B", + title="Source version", + root_document=source, + version_index=1, + ) + version.delete() # trashed, but still points at source + root = Document.objects.create(checksum="C", title="Root") + + with self.assertRaisesRegex(ValueError, "existing versions"): + merge_as_versions( + [source.id, root.id], + root_document_id=root.id, + ) + + source.refresh_from_db() + self.assertIsNone(source.root_document_id) + # Restoring the version must not produce a version of a version + self.assertEqual( + Document.global_objects.get(pk=version.pk).root_document_id, + source.id, + ) + remove_from_index_mock.assert_not_called() + bulk_update_mock.assert_not_called() + status_manager_mock.assert_not_called() + class TestMergeDocumentsAsVersionsAPI(APITestCase): def setUp(self) -> None: