From b640c5fe385c4eec6226bc7281fb5fdcc8a95928 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Jan 2026 00:21:13 +0000 Subject: [PATCH 1/3] Initial plan From bcb9a57e94603f982194b23095d1928a2255fb68 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Jan 2026 00:25:03 +0000 Subject: [PATCH 2/3] Optimize delete_note to fix N+1 query pattern - Batch fetch all attachments for all notes in single query - Bulk delete notifications using 'in' filter - Bulk delete files using SQL delete with 'in' filter - Reduces queries from ~30 to ~7 for notes with 5 children and 10 attachments Co-authored-by: mrrobot47 <25586785+mrrobot47@users.noreply.github.com> --- next_crm/api/crm_note.py | 53 ++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/next_crm/api/crm_note.py b/next_crm/api/crm_note.py index d2e6842e01..6fe7649f87 100644 --- a/next_crm/api/crm_note.py +++ b/next_crm/api/crm_note.py @@ -44,10 +44,7 @@ def create_note( if attachments: new_note.set( "custom_note_attachments", - [ - {"filename": file} if isinstance(file, str) else file - for file in attachments - ], + [{"filename": file} if isinstance(file, str) else file for file in attachments], ) new_note.insert() notify_mentions_ncrm(note, new_note.name, docname, doctype) @@ -129,9 +126,9 @@ def notify_mentions_ncrm(note, note_name, docname, doctype): } ) - email_notification_message = _( - """[Next CRM] {0} mentioned you in a Note in {1} {2}""" - ).format(frappe.bold(owner), frappe.bold(doctype), get_title_html(title)) + email_notification_message = _("""[Next CRM] {0} mentioned you in a Note in {1} {2}""").format( + frappe.bold(owner), frappe.bold(doctype), get_title_html(title) + ) recipients = [ frappe.db.get_value( @@ -168,7 +165,7 @@ def delete_note(note_name): if not note: raise frappe.ValidationError(_("Note not found.")) - filenames_to_delete = [row.filename for row in note.custom_note_attachments] + note_names_to_delete = [note_name] parent_note = note.custom_parent_note if not parent_note: @@ -178,22 +175,26 @@ def delete_note(note_name): fields=["name"], pluck="name", ) - for child_note in child_notes: - child_note_doc = frappe.get_doc("CRM Note", child_note) - child_filenames = [ - row.filename for row in child_note_doc.custom_note_attachments - ] - filenames_to_delete.extend(child_filenames) - frappe.db.delete("CRM Notification", {"notification_type_doc": child_note}) - frappe.delete_doc("CRM Note", child_note) - - frappe.db.delete("CRM Notification", {"notification_type_doc": note_name}) - note.delete() - for filename in filenames_to_delete: - try: - frappe.delete_doc("File", filename) - except frappe.DoesNotExistError: - pass + note_names_to_delete.extend(child_notes) + + # Batch fetch all attachments for all notes + all_attachments = frappe.get_all( + "NCRM Attachments", + filters={"parent": ["in", note_names_to_delete], "parenttype": "CRM Note"}, + fields=["filename"], + pluck="filename", + ) + + # Bulk delete notifications + frappe.db.delete("CRM Notification", {"notification_type_doc": ["in", note_names_to_delete]}) + + # Delete notes (using delete_doc to ensure proper hooks are called) + for note_name_to_del in note_names_to_delete: + frappe.delete_doc("CRM Note", note_name_to_del, ignore_permissions=True) + + # Bulk delete files + if all_attachments: + frappe.db.delete("File", {"name": ["in", all_attachments]}) return True @@ -302,9 +303,7 @@ def copy_crm_notes_to_opportunity(lead, opportunity): frappe.db.commit() -def duplicate_file( - original_file_name, new_attached_to_doctype=None, new_attached_to_name=None -): +def duplicate_file(original_file_name, new_attached_to_doctype=None, new_attached_to_name=None): """ Create a duplicate of a file with new attachment references. Returns the name of the new file document. From b04a92ead7a0e8ae78ded0bd46cf3c85d1903436 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 14 Jan 2026 00:27:14 +0000 Subject: [PATCH 3/3] Address code review feedback: maintain proper permissions and file cleanup - Remove ignore_permissions flag to maintain proper permission checks - Use note.delete() instead of delete_doc for parent note - Keep file deletion loop to ensure proper disk cleanup - Maintain try/except for missing files Co-authored-by: mrrobot47 <25586785+mrrobot47@users.noreply.github.com> --- next_crm/api/crm_note.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/next_crm/api/crm_note.py b/next_crm/api/crm_note.py index 6fe7649f87..e009c23fe6 100644 --- a/next_crm/api/crm_note.py +++ b/next_crm/api/crm_note.py @@ -166,6 +166,7 @@ def delete_note(note_name): raise frappe.ValidationError(_("Note not found.")) note_names_to_delete = [note_name] + child_notes = [] parent_note = note.custom_parent_note if not parent_note: @@ -188,13 +189,19 @@ def delete_note(note_name): # Bulk delete notifications frappe.db.delete("CRM Notification", {"notification_type_doc": ["in", note_names_to_delete]}) - # Delete notes (using delete_doc to ensure proper hooks are called) - for note_name_to_del in note_names_to_delete: - frappe.delete_doc("CRM Note", note_name_to_del, ignore_permissions=True) + # Delete child notes first (using delete_doc to ensure proper hooks are called) + for child_note_name in child_notes: + frappe.delete_doc("CRM Note", child_note_name) - # Bulk delete files - if all_attachments: - frappe.db.delete("File", {"name": ["in", all_attachments]}) + # Delete the parent note + note.delete() + + # Delete files (loop needed to properly handle file cleanup and missing files) + for filename in all_attachments: + try: + frappe.delete_doc("File", filename) + except frappe.DoesNotExistError: + pass return True