Fix: reflect doc links in bulk modify custom fields (#8962)

src/documents/bulk_edit.py

@@ -12,6 +12,7 @@ from celery import shared_task
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.db.models import Q
+from django.utils import timezone
 
 from documents.data_models import ConsumableDocument
 from documents.data_models import DocumentMetadataOverrides
@@ -177,6 +178,12 @@ def modify_custom_fields(
                 field_id=field_id,
                 defaults=defaults,
             )
+            if (
+                custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK
+                and value
+            ):
+                doc = Document.objects.get(id=doc_id)
+                reflect_doclinks(doc, custom_field, value)
     CustomFieldInstance.objects.filter(
         document_id__in=affected_docs,
         field_id__in=remove_custom_fields,
@@ -447,3 +454,87 @@ def delete_pages(doc_ids: list[int], pages: list[int]) -> Literal["OK"]:
         logger.exception(f"Error deleting pages from document {doc.id}: {e}")
 
     return "OK"
+
+
+def reflect_doclinks(
+    document: Document,
+    field: CustomField,
+    target_doc_ids: list[int],
+):
+    """
+    Add or remove 'symmetrical' links to `document` on all `target_doc_ids`
+    """
+
+    if target_doc_ids is None:
+        target_doc_ids = []
+
+    # Check if any documents are going to be removed from the current list of links and remove the symmetrical links
+    current_field_instance = CustomFieldInstance.objects.filter(
+        field=field,
+        document=document,
+    ).first()
+    if current_field_instance is not None and current_field_instance.value is not None:
+        for doc_id in current_field_instance.value:
+            if doc_id not in target_doc_ids:
+                remove_doclink(
+                    document=document,
+                    field=field,
+                    target_doc_id=doc_id,
+                )
+
+    # Create an instance if target doc doesn't have this field or append it to an existing one
+    existing_custom_field_instances = {
+        custom_field.document_id: custom_field
+        for custom_field in CustomFieldInstance.objects.filter(
+            field=field,
+            document_id__in=target_doc_ids,
+        )
+    }
+    custom_field_instances_to_create = []
+    custom_field_instances_to_update = []
+    for target_doc_id in target_doc_ids:
+        target_doc_field_instance = existing_custom_field_instances.get(
+            target_doc_id,
+        )
+        if target_doc_field_instance is None:
+            custom_field_instances_to_create.append(
+                CustomFieldInstance(
+                    document_id=target_doc_id,
+                    field=field,
+                    value_document_ids=[document.id],
+                ),
+            )
+        elif target_doc_field_instance.value is None:
+            target_doc_field_instance.value_document_ids = [document.id]
+            custom_field_instances_to_update.append(target_doc_field_instance)
+        elif document.id not in target_doc_field_instance.value:
+            target_doc_field_instance.value_document_ids.append(document.id)
+            custom_field_instances_to_update.append(target_doc_field_instance)
+
+    CustomFieldInstance.objects.bulk_create(custom_field_instances_to_create)
+    CustomFieldInstance.objects.bulk_update(
+        custom_field_instances_to_update,
+        ["value_document_ids"],
+    )
+    Document.objects.filter(id__in=target_doc_ids).update(modified=timezone.now())
+
+
+def remove_doclink(
+    document: Document,
+    field: CustomField,
+    target_doc_id: int,
+):
+    """
+    Removes a 'symmetrical' link to `document` from the target document's existing custom field instance
+    """
+    target_doc_field_instance = CustomFieldInstance.objects.filter(
+        document_id=target_doc_id,
+        field=field,
+    ).first()
+    if (
+        target_doc_field_instance is not None
+        and document.id in target_doc_field_instance.value
+    ):
+        target_doc_field_instance.value.remove(document.id)
+        target_doc_field_instance.save()
+    Document.objects.filter(id=target_doc_id).update(modified=timezone.now())

src/documents/serialisers.py

@@ -16,7 +16,6 @@ from django.core.validators import DecimalValidator
 from django.core.validators import MaxLengthValidator
 from django.core.validators import RegexValidator
 from django.core.validators import integer_validator
-from django.utils import timezone
 from django.utils.crypto import get_random_string
 from django.utils.text import slugify
 from django.utils.translation import gettext as _
@@ -647,7 +646,7 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer):
 
         if custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK:
             # prior to update so we can look for any docs that are going to be removed
-            self.reflect_doclinks(document, custom_field, validated_data["value"])
+            bulk_edit.reflect_doclinks(document, custom_field, validated_data["value"])
 
         # Actually update or create the instance, providing the value
         # to fill in the correct attribute based on the type
@@ -767,89 +766,6 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer):
 
         return ret
 
-    def reflect_doclinks(
-        self,
-        document: Document,
-        field: CustomField,
-        target_doc_ids: list[int],
-    ):
-        """
-        Add or remove 'symmetrical' links to `document` on all `target_doc_ids`
-        """
-
-        if target_doc_ids is None:
-            target_doc_ids = []
-
-        # Check if any documents are going to be removed from the current list of links and remove the symmetrical links
-        current_field_instance = CustomFieldInstance.objects.filter(
-            field=field,
-            document=document,
-        ).first()
-        if (
-            current_field_instance is not None
-            and current_field_instance.value is not None
-        ):
-            for doc_id in current_field_instance.value:
-                if doc_id not in target_doc_ids:
-                    self.remove_doclink(document, field, doc_id)
-
-        # Create an instance if target doc doesn't have this field or append it to an existing one
-        existing_custom_field_instances = {
-            custom_field.document_id: custom_field
-            for custom_field in CustomFieldInstance.objects.filter(
-                field=field,
-                document_id__in=target_doc_ids,
-            )
-        }
-        custom_field_instances_to_create = []
-        custom_field_instances_to_update = []
-        for target_doc_id in target_doc_ids:
-            target_doc_field_instance = existing_custom_field_instances.get(
-                target_doc_id,
-            )
-            if target_doc_field_instance is None:
-                custom_field_instances_to_create.append(
-                    CustomFieldInstance(
-                        document_id=target_doc_id,
-                        field=field,
-                        value_document_ids=[document.id],
-                    ),
-                )
-            elif target_doc_field_instance.value is None:
-                target_doc_field_instance.value_document_ids = [document.id]
-                custom_field_instances_to_update.append(target_doc_field_instance)
-            elif document.id not in target_doc_field_instance.value:
-                target_doc_field_instance.value_document_ids.append(document.id)
-                custom_field_instances_to_update.append(target_doc_field_instance)
-
-        CustomFieldInstance.objects.bulk_create(custom_field_instances_to_create)
-        CustomFieldInstance.objects.bulk_update(
-            custom_field_instances_to_update,
-            ["value_document_ids"],
-        )
-        Document.objects.filter(id__in=target_doc_ids).update(modified=timezone.now())
-
-    @staticmethod
-    def remove_doclink(
-        document: Document,
-        field: CustomField,
-        target_doc_id: int,
-    ):
-        """
-        Removes a 'symmetrical' link to `document` from the target document's existing custom field instance
-        """
-        target_doc_field_instance = CustomFieldInstance.objects.filter(
-            document_id=target_doc_id,
-            field=field,
-        ).first()
-        if (
-            target_doc_field_instance is not None
-            and document.id in target_doc_field_instance.value
-        ):
-            target_doc_field_instance.value.remove(document.id)
-            target_doc_field_instance.save()
-        Document.objects.filter(id=target_doc_id).update(modified=timezone.now())
-
     class Meta:
         model = CustomFieldInstance
         fields = [
@@ -951,7 +867,7 @@ class DocumentSerializer(
                 ):
                     # Doc link field is being removed entirely
                     for doc_id in custom_field_instance.value:
-                        CustomFieldInstanceSerializer.remove_doclink(
+                        bulk_edit.remove_doclink(
                             instance,
                             custom_field_instance.field,
                             doc_id,

src/documents/tests/test_bulk_edit.py

@@ -268,7 +268,7 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
         )
         cf3 = CustomField.objects.create(
             name="cf3",
-            data_type=CustomField.FieldDataType.STRING,
+            data_type=CustomField.FieldDataType.DOCUMENTLINK,
         )
         CustomFieldInstance.objects.create(
             document=self.doc2,
@@ -282,9 +282,14 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
             document=self.doc2,
             field=cf3,
         )
+        doc3: Document = Document.objects.create(
+            title="doc3",
+            content="content",
+            checksum="D3",
+        )
         bulk_edit.modify_custom_fields(
             [self.doc1.id, self.doc2.id],
-            add_custom_fields={cf2.id: None, cf3.id: "value"},
+            add_custom_fields={cf2.id: None, cf3.id: [doc3.id]},
             remove_custom_fields=[cf.id],
         )
 
@@ -301,7 +306,7 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
         )
         self.assertEqual(
             self.doc1.custom_fields.get(field=cf3).value,
-            "value",
+            [doc3.id],
         )
         self.assertEqual(
             self.doc2.custom_fields.count(),
@@ -309,7 +314,13 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
         )
         self.assertEqual(
             self.doc2.custom_fields.get(field=cf3).value,
-            "value",
+            [doc3.id],
+        )
+        # assert reflect document link
+        doc3.refresh_from_db()
+        self.assertEqual(
+            doc3.custom_fields.first().value,
+            [self.doc2.id, self.doc1.id],
         )
 
         self.async_task.assert_called_once()