From 6c7505586ccab949c8014a57b9c47722bd63d3ea Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:33:59 -0700 Subject: [PATCH] Audit log stuff: need a user, avoid post_save signals etc --- src/documents/bulk_edit.py | 47 ++++++++++++------- .../tests/test_merge_documents_as_versions.py | 24 ++++++++++ src/documents/views.py | 6 ++- 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index 09596fb74..3c8125f58 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -41,6 +41,9 @@ if TYPE_CHECKING: from django.contrib.auth.models import User +if settings.AUDIT_LOG_ENABLED: + from auditlog.models import LogEntry + logger: logging.Logger = logging.getLogger("paperless.bulk_edit") SourceMode = Literal["latest_version", "explicit_selection"] @@ -619,6 +622,7 @@ def merge_as_versions( *, root_document_id: int, version_label: str | None = None, + user: User | None = None, ) -> Literal["OK"]: with transaction.atomic(): documents = list( @@ -659,29 +663,24 @@ def merge_as_versions( ] for source_id in source_ids: - source_document = documents_by_id[source_id] next_version_index += 1 - source_document.root_document = root_document - source_document.version_index = next_version_index - source_document.archive_serial_number = None - update_fields = [ - "root_document", - "version_index", - "archive_serial_number", - ] + updates = { + "root_document": root_document, + "version_index": next_version_index, + "archive_serial_number": None, + } if version_label is not None: - source_document.version_label = version_label - update_fields.append("version_label") - source_document.save(update_fields=update_fields) + updates["version_label"] = version_label + # update() and not save to avoid post_save now + Document.objects.filter(pk=source_id).update(**updates) - root_update_fields = ["modified"] + root_updates = {"modified": timezone.now()} if source_asns and root_document.archive_serial_number is None: # If a version had one, hand the ASN over, the same as merge() does - root_document.archive_serial_number = source_asns.pop(0) - root_update_fields.append("archive_serial_number") + root_updates["archive_serial_number"] = source_asns.pop(0) logger.info( f"Document {root_document.id} took archive serial number " - f"{root_document.archive_serial_number} from a document merged into it", + f"{root_updates['archive_serial_number']} from a document merged into it", ) if source_asns: logger.warning( @@ -689,8 +688,20 @@ def merge_as_versions( f"those documents as versions of document {root_document.id}", ) - root_document.modified = timezone.now() - root_document.save(update_fields=root_update_fields) + Document.objects.filter(pk=root_document.pk).update(**root_updates) + + if settings.AUDIT_LOG_ENABLED: + # update() doesn't fire auditlog signals, so manual + LogEntry.objects.log_create( + instance=root_document, + changes={"Merged As Versions": ["None", source_ids]}, + action=LogEntry.Action.UPDATE, + actor=user, + additional_data={ + "reason": "Merged as versions", + "version_ids": source_ids, + }, + ) for source_id in source_ids: remove_document_from_index.apply_async(args=[source_id]) diff --git a/src/documents/tests/test_merge_documents_as_versions.py b/src/documents/tests/test_merge_documents_as_versions.py index 57e11c87e..3f06c512f 100644 --- a/src/documents/tests/test_merge_documents_as_versions.py +++ b/src/documents/tests/test_merge_documents_as_versions.py @@ -1,8 +1,10 @@ import json from unittest import mock +from auditlog.models import LogEntry from django.contrib.auth.models import Permission from django.contrib.auth.models import User +from django.contrib.contenttypes.models import ContentType from django.test import TestCase from rest_framework import status from rest_framework.test import APITestCase @@ -286,6 +288,27 @@ class TestMergeDocumentsAsVersions(TestCase): self.assertEqual(root.archive_serial_number, 7) self.assertIsNone(source.archive_serial_number) + @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_writes_audit_log_entry(self, *_mocks) -> None: + user = User.objects.create_user(username="merger") + root = Document.objects.create(checksum="A", title="Root") + source = Document.objects.create(checksum="B", title="Source") + LogEntry.objects.all().delete() + + merge_as_versions([root.id, source.id], root_document_id=root.id, user=user) + + entry = LogEntry.objects.filter( + content_type=ContentType.objects.get_for_model(Document), + object_id=root.id, + ).first() + self.assertIsNotNone(entry) + self.assertEqual(entry.actor, user) + self.assertEqual(entry.action, LogEntry.Action.UPDATE) + self.assertEqual(entry.changes, {"Merged As Versions": ["None", [source.id]]}) + self.assertEqual(entry.additional_data["version_ids"], [source.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") @@ -416,6 +439,7 @@ class TestMergeDocumentsAsVersionsAPI(APITestCase): [self.doc1.id, self.doc2.id], root_document_id=self.doc2.id, version_label="Imported", + user=self.user, ) @mock.patch("documents.views.bulk_edit.merge_as_versions") diff --git a/src/documents/views.py b/src/documents/views.py index 73752043b..84568b38b 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -2766,8 +2766,12 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin): "delete_pages", "edit_pdf", "remove_password", + "merge_as_versions", + } + # merge_as_versions doesn't queue any consume tasks + METHOD_NAMES_REQUIRING_TRIGGER_SOURCE = METHOD_NAMES_REQUIRING_USER - { + "merge_as_versions", } - METHOD_NAMES_REQUIRING_TRIGGER_SOURCE = METHOD_NAMES_REQUIRING_USER def _has_document_permissions( self,