From b41d032fdfdc5c41abe114c06fccae63ac868856 Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:59:57 -0700 Subject: [PATCH] Fix: select_related in remove_doclink() to avoid signal-triggered reload Same pattern as the update_or_create() fix: target_doc_field_instance was fetched without select_related, so its .document/.field weren't cached when .save() fired the post_save signal -- auditlog's receiver touching .document re-fetched it, once per (source, target) pair being unlinked with no batching across calls. Also benefits the single-document PATCH path in serialisers.py, which calls the same helper. Broadened the removal test's query assertion now that both sides are fixed. --- src/documents/bulk_edit.py | 11 +++++++---- src/documents/tests/test_bulk_edit.py | 14 ++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index 04bdf78c6..8d8d99f2e 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -1199,10 +1199,13 @@ def remove_doclink( """ 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() + # select_related: a signal receiver (auditlog) touches .document/.field + # on save() below -- without this, that's a per-call reload query. + target_doc_field_instance = ( + CustomFieldInstance.objects.filter(document_id=target_doc_id, field=field) + .select_related("document", "field") + .first() + ) if ( target_doc_field_instance is not None and document.id in target_doc_field_instance.value diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py index bf65f87c2..afdef161c 100644 --- a/src/documents/tests/test_bulk_edit.py +++ b/src/documents/tests/test_bulk_edit.py @@ -500,9 +500,7 @@ class TestBulkEdit(DirectoriesMixin, TestCase): - The field is removed from all of them in one call THEN: - The symmetrical links are removed from the target - - The source documents come from one batched query, not one - lookup per source document (the target side, fetched inside - remove_doclink(), is untouched by this PR and out of scope) + - No per-document lookup query is issued, on either side """ target = Document.objects.create(checksum="rm-target", title="rm-target") docs = [ @@ -530,17 +528,17 @@ class TestBulkEdit(DirectoriesMixin, TestCase): remove_custom_fields=[field.id], ) - source_doc_lookups = [ + single_document_lookups = [ q for q in ctx.captured_queries if 'FROM "documents_document"' in q["sql"] - and any(f'."id" = {doc.id} ' in q["sql"] for doc in docs) + and '"documents_document"."id" = ' in q["sql"] ] self.assertEqual( - source_doc_lookups, + single_document_lookups, [], - "Expected source documents to come from a batched query, not " - f"per-document lookups, got: {source_doc_lookups}", + "Expected batched document resolution, not per-document lookups, " + f"got: {single_document_lookups}", ) self.assertEqual(target.custom_fields.get(field=field).value, [])