diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index bb43dd8cb..9c7ba3db8 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/src/documents/tests/test_api_bulk_edit.py @@ -9,6 +9,7 @@ from rest_framework.test import APITestCase from documents.models import Correspondent from documents.models import CustomField +from documents.models import CustomFieldInstance from documents.models import Document from documents.models import DocumentType from documents.models import StoragePath @@ -2525,7 +2526,7 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): WHEN: - API to bulk edit documents is called THEN: - - Audit log is created + - Audit log is created with the old and new correspondent """ LogEntry.objects.all().delete() response = self.client.post( @@ -2541,7 +2542,8 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): ) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(LogEntry.objects.filter(object_pk=self.doc1.id).count(), 1) + entry = LogEntry.objects.get_for_object(self.doc1).get() + self.assertEqual(entry.changes, {"correspondent": [None, self.c2.id]}) @override_settings(AUDIT_LOG_ENABLED=True) def test_bulk_edit_audit_log_enabled_tags(self) -> None: @@ -2549,16 +2551,18 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): GIVEN: - Audit log is enabled WHEN: - - API to bulk edit tags is called + - API to bulk edit tags is called on an untagged document and a + document with several tags THEN: - - Audit log is created + - Audit log is created for each document with its full tag list + before and after the edit """ LogEntry.objects.all().delete() response = self.client.post( "/api/documents/bulk_edit/", json.dumps( { - "documents": [self.doc1.id], + "documents": [self.doc1.id, self.doc4.id], "method": "modify_tags", "parameters": { "add_tags": [self.t1.id], @@ -2570,18 +2574,32 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): ) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(LogEntry.objects.filter(object_pk=self.doc1.id).count(), 1) + entry = LogEntry.objects.get_for_object(self.doc1).get() + self.assertEqual(entry.changes, {"tags": [[], [self.t1.id]]}) + entry = LogEntry.objects.get_for_object(self.doc4).get() + self.assertEqual( + entry.changes, + {"tags": [[self.t1.id, self.t2.id], [self.t1.id]]}, + ) @override_settings(AUDIT_LOG_ENABLED=True) def test_bulk_edit_audit_log_enabled_custom_fields(self) -> None: """ GIVEN: - Audit log is enabled + - A document with two custom fields WHEN: - - API to bulk edit custom fields is called + - API to bulk edit custom fields is called to add a third THEN: - - Audit log is created + - Audit log is created with every custom field instance before and + after the edit + - Audit log is created for the new custom field instance """ + cf3 = CustomField.objects.create(name="cf3", data_type="string") + existing = [ + CustomFieldInstance.objects.create(document=self.doc1, field=field) + for field in (self.cf2, cf3) + ] LogEntry.objects.all().delete() response = self.client.post( "/api/documents/bulk_edit/", @@ -2599,7 +2617,14 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): ) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(LogEntry.objects.filter(object_pk=self.doc1.id).count(), 2) + added = CustomFieldInstance.objects.get(document=self.doc1, field=self.cf1) + existing_ids = [instance.id for instance in existing] + entry = LogEntry.objects.get_for_object(self.doc1).get() + self.assertEqual( + entry.changes, + {"custom_fields": [existing_ids, [*existing_ids, added.id]]}, + ) + self.assertEqual(LogEntry.objects.get_for_object(added).count(), 1) def test_api_bulk_edit_with_bad_search_query_returns_400(self) -> None: """ diff --git a/src/documents/views.py b/src/documents/views.py index cc09709a4..e36f181a5 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -49,7 +49,6 @@ from django.db.models import Sum from django.db.models import When from django.db.models.functions import Coalesce from django.db.models.functions import Lower -from django.db.models.manager import Manager from django.http import FileResponse from django.http import Http404 from django.http import HttpRequest @@ -3141,6 +3140,38 @@ class BulkEditView(DocumentOperationPermissionMixin): serializer_class = BulkEditSerializer + @staticmethod + def _snapshot_field(doc_ids: list[int], field: str) -> dict[int, Any]: + """ + Returns each document's current value of field, for the audit log. + + Tags and custom fields are one row per value, so they are gathered + into a sorted list of pks per document (empty when there are none). + Reading them through Document.values() instead would join those rows + and return one arbitrary value per document. + """ + if field == "tags": + rows = ( + Document.tags.through.objects.filter(document_id__in=doc_ids) + .order_by("tag_id") + .values_list("document_id", "tag_id") + ) + elif field == "custom_fields": + rows = ( + CustomFieldInstance.objects.filter(document_id__in=doc_ids) + .order_by("pk") + .values_list("document_id", "pk") + ) + else: + return dict( + Document.objects.filter(pk__in=doc_ids).values_list("pk", field), + ) + + values: dict[int, list[int]] = {doc_id: [] for doc_id in doc_ids} + for doc_id, pk in rows: + values[doc_id].append(pk) + return values + def post(self, request, *args, **kwargs): request_method = request.data.get("method") api_version = int(request.version or settings.REST_FRAMEWORK["DEFAULT_VERSION"]) @@ -3187,41 +3218,19 @@ class BulkEditView(DocumentOperationPermissionMixin): try: modified_field = self.MODIFIED_FIELD_BY_METHOD.get(method.__name__, None) if settings.AUDIT_LOG_ENABLED and modified_field: - old_documents = { - obj["pk"]: obj - for obj in Document.objects.filter(pk__in=documents).values( - "pk", - "correspondent", - "document_type", - "storage_path", - "tags", - "custom_fields", - "deleted_at", - "checksum", - ) - } + old_values = self._snapshot_field(documents, modified_field) result = method(documents, **parameters) if settings.AUDIT_LOG_ENABLED and modified_field: - new_documents = Document.objects.filter(pk__in=documents) - for doc in new_documents: - old_value = old_documents[doc.pk][modified_field] - new_value = getattr(doc, modified_field) - - if isinstance(new_value, Model): - # correspondent, document type, etc. - new_value = new_value.pk - elif isinstance(new_value, Manager): - # tags, custom fields - new_value = list(new_value.values_list("pk", flat=True)) - + new_values = self._snapshot_field(documents, modified_field) + for doc in Document.objects.filter(pk__in=documents): LogEntry.objects.log_create( instance=doc, changes={ modified_field: [ - old_value, - new_value, + old_values[doc.pk], + new_values[doc.pk], ], }, action=LogEntry.Action.UPDATE,