From e0ea4a462565dad644a3563123d30bff407af889 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Thu, 30 Jan 2025 10:55:05 -0800 Subject: [PATCH] Fix: reflect doc links in bulk modify custom fields (#8962) --- src/documents/bulk_edit.py | 91 +++++++++++++++++++++++++++ src/documents/serialisers.py | 88 +------------------------- src/documents/tests/test_bulk_edit.py | 19 ++++-- 3 files changed, 108 insertions(+), 90 deletions(-) diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index de43aed87..7e2d2088e 100644 --- a/src/documents/bulk_edit.py +++ b/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()) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 0732fd242..4adadbcb2 100644 --- a/src/documents/serialisers.py +++ b/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, diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py index 03c177343..2d8af025b 100644 --- a/src/documents/tests/test_bulk_edit.py +++ b/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()