From 61485b0f1d86280ab3c72c5949a843295edab66b Mon Sep 17 00:00:00 2001
From: Trenton H <797416+stumpylog@users.noreply.github.com>
Date: Wed, 12 Jun 2024 16:23:47 -0700
Subject: [PATCH] Fix: Document history could include extra fields (#6989)

* Fixes creation of a custom field being included in a document's history even if not attached

* Show custom field creation in UI

---------

Co-authored-by: shamoon <4887959+shamoon@users.noreply.github.com>
---
 .../document-history.component.html           | 50 ++++++++++---------
 src/documents/tests/test_api_documents.py     | 21 ++++++--
 src/documents/views.py                        |  9 ++--
 3 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/src-ui/src/app/components/document-history/document-history.component.html b/src-ui/src/app/components/document-history/document-history.component.html
index ea4a3c9bb..c63a1223c 100644
--- a/src-ui/src/app/components/document-history/document-history.component.html
+++ b/src-ui/src/app/components/document-history/document-history.component.html
@@ -27,31 +27,33 @@
                         }
                         <span class="badge bg-secondary ms-auto" [class.bg-primary]="entry.action === AuditLogAction.Create">{{ entry.action | titlecase }}</span>
                     </div>
-                    @if (entry.action === AuditLogAction.Update) {
-                        <ul class="mt-2">
-                            @for (change of entry.changes | keyvalue; track change.key) {
-                                @if (change.value["type"] === 'm2m') {
-                                    <li>
-                                        <span class="fst-italic">{{ change.value["operation"] | titlecase }}</span>&nbsp;
-                                        <span>{{ change.key | titlecase }}</span>:&nbsp;
-                                        <code class="text-primary">{{ change.value["objects"].join(', ') }}</code>
-                                    </li>
-                                }
-                                @else if (change.value["type"] === 'custom_field') {
-                                    <li>
-                                        <span>{{ change.value["field"] }}</span>:&nbsp;
-                                        <code class="text-primary">{{ change.value["value"] }}</code>
-                                    </li>
-                                }
-                                @else {
-                                    <li>
-                                        <span>{{ change.key | titlecase }}</span>:&nbsp;
-                                        <code class="text-primary">{{ change.value[1] }}</code>
-                                    </li>
-                                }
+                    <ul class="mt-2">
+                        @for (change of entry.changes | keyvalue; track change.key) {
+                            @if (change.value["type"] === 'm2m') {
+                                <li>
+                                    <span class="fst-italic">{{ change.value["operation"] | titlecase }}</span>&nbsp;
+                                    <span>{{ change.key | titlecase }}</span>:&nbsp;
+                                    <code class="text-primary">{{ change.value["objects"].join(', ') }}</code>
+                                </li>
                             }
-                        </ul>
-                    }
+                            @else if (change.value["type"] === 'custom_field') {
+                                <li>
+                                    <span>{{ change.value["field"] }}</span>:&nbsp;
+                                    <code class="text-primary">{{ change.value["value"] }}</code>
+                                </li>
+                            }
+                            @else {
+                                <li>
+                                    <span>{{ change.key | titlecase }}</span>:&nbsp;
+                                    @if (change.key === 'content') {
+                                        <code class="text-primary">{{ change.value[1]?.substring(0,100) }}...</code>
+                                    } @else {
+                                        <code class="text-primary">{{ change.value[1] }}</code>
+                                    }
+                                </li>
+                            }
+                        }
+                    </ul>
                 </li>
             }
         }
diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py
index 7e69cb024..4ad6cb828 100644
--- a/src/documents/tests/test_api_documents.py
+++ b/src/documents/tests/test_api_documents.py
@@ -366,6 +366,16 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
             data_type=CustomField.FieldDataType.STRING,
         )
         self.client.force_login(user=self.user)
+
+        # Initial response should include only document's creation
+        response = self.client.get(f"/api/documents/{doc.pk}/history/")
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(len(response.data), 1)
+
+        self.assertIsNone(response.data[0]["actor"])
+        self.assertEqual(response.data[0]["action"], "create")
+
         self.client.patch(
             f"/api/documents/{doc.pk}/",
             data={
@@ -379,12 +389,15 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
             format="json",
         )
 
+        # Second response should include custom field addition
         response = self.client.get(f"/api/documents/{doc.pk}/history/")
+
         self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data[1]["actor"]["id"], self.user.id)
-        self.assertEqual(response.data[1]["action"], "create")
+        self.assertEqual(len(response.data), 2)
+        self.assertEqual(response.data[0]["actor"]["id"], self.user.id)
+        self.assertEqual(response.data[0]["action"], "create")
         self.assertEqual(
-            response.data[1]["changes"],
+            response.data[0]["changes"],
             {
                 "custom_fields": {
                     "type": "custom_field",
@@ -393,6 +406,8 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
                 },
             },
         )
+        self.assertIsNone(response.data[1]["actor"])
+        self.assertEqual(response.data[1]["action"], "create")
 
     @override_settings(AUDIT_LOG_ENABLED=False)
     def test_document_history_action_disabled(self):
diff --git a/src/documents/views.py b/src/documents/views.py
index 68addd0f4..9e8e7301d 100644
--- a/src/documents/views.py
+++ b/src/documents/views.py
@@ -19,7 +19,6 @@ from django.apps import apps
 from django.conf import settings
 from django.contrib.auth.models import Group
 from django.contrib.auth.models import User
-from django.contrib.contenttypes.models import ContentType
 from django.db import connections
 from django.db.migrations.loader import MigrationLoader
 from django.db.migrations.recorder import MigrationRecorder
@@ -106,7 +105,6 @@ from documents.matching import match_storage_paths
 from documents.matching import match_tags
 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 Note
@@ -799,15 +797,14 @@ class DocumentViewSet(
                     else None
                 ),
             }
-            for entry in LogEntry.objects.filter(object_pk=doc.pk).select_related(
+            for entry in LogEntry.objects.get_for_object(doc).select_related(
                 "actor",
             )
         ]
 
         # custom fields
-        for entry in LogEntry.objects.filter(
-            object_pk__in=list(doc.custom_fields.values_list("id", flat=True)),
-            content_type=ContentType.objects.get_for_model(CustomFieldInstance),
+        for entry in LogEntry.objects.get_for_objects(
+            doc.custom_fields.all(),
         ).select_related("actor"):
             entries.append(
                 {